Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EVA-3664 Reserve new block - create new transaction for every retry #90

Merged
merged 3 commits into from
Oct 9, 2024
Merged

EVA-3664 Reserve new block - create new transaction for every retry #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
29ae9e4
reserve new block - create new transaction for every retry
nitin-ebi Oct 7, 2024
53cff3c
update tests
nitin-ebi Oct 7, 2024
a8ec14c
update tests
nitin-ebi Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .../ampt2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package uk.ac.ebi.ampt2d.commons.accession.persistence.jpa.monotonic.service;

import org.springframework.transaction.annotation.Isolation;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;
import uk.ac.ebi.ampt2d.commons.accession.block.initialization.BlockParameters;
import uk.ac.ebi.ampt2d.commons.accession.persistence.jpa.monotonic.entities.ContiguousIdBlock;
Expand Down Expand Up @@ -91,7 +92,7 @@ public void save(ContiguousIdBlock block) {
entityManager.flush();
}

@Transactional(isolation = Isolation.SERIALIZABLE)
@Transactional(isolation = Isolation.SERIALIZABLE, propagation = Propagation.REQUIRES_NEW)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to read up on how this propagation works but I feel it would be good to get your explanation.

public ContiguousIdBlock reserveNewBlock(String categoryId, String instanceId) {
ContiguousIdBlock lastBlock = repository.findFirstByCategoryIdOrderByLastValueDesc(categoryId);
BlockParameters blockParameters = getBlockParameters(categoryId);
Expand Down
2 changes: 2 additions & 0 deletions .../ebi/ampt2d/commons/accession/core/BasicMonotonicAccessioningWithAlternateRangesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringRunner;
import uk.ac.ebi.ampt2d.commons.accession.block.initialization.BlockInitializationException;
Expand Down Expand Up @@ -53,6 +54,7 @@
@RunWith(SpringRunner.class)
@DataJpaTest
@ContextConfiguration(classes = {TestMonotonicDatabaseServiceTestConfiguration.class})
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)
public class BasicMonotonicAccessioningWithAlternateRangesTest {

private static final String INSTANCE_ID = "test-instance";
Expand Down
2 changes: 2 additions & 0 deletions ...ac/ebi/ampt2d/commons/accession/generators/monotonic/MonotonicAccessionGeneratorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.mockito.Mockito;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringRunner;
import uk.ac.ebi.ampt2d.commons.accession.core.exceptions.AccessionCouldNotBeGeneratedException;
Expand Down Expand Up @@ -59,6 +60,7 @@
@RunWith(SpringRunner.class)
@DataJpaTest
@ContextConfiguration(classes = {MonotonicAccessionGeneratorTestConfiguration.class, TestMonotonicDatabaseServiceTestConfiguration.class})
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)
public class MonotonicAccessionGeneratorTest {

private static final int BLOCK_SIZE = 1000;
Expand Down
34 changes: 21 additions & 13 deletions ...t2d/commons/accession/persistence/jpa/monotonic/service/ContiguousIdBlockServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
import org.springframework.boot.test.autoconfigure.orm.jpa.TestEntityManager;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringRunner;
import uk.ac.ebi.ampt2d.commons.accession.persistence.jpa.monotonic.entities.ContiguousIdBlock;
import uk.ac.ebi.ampt2d.commons.accession.persistence.jpa.monotonic.repositories.ContiguousIdBlockRepository;
import uk.ac.ebi.ampt2d.test.configuration.MonotonicAccessionGeneratorTestConfiguration;

import javax.persistence.EntityManager;
import javax.persistence.PersistenceContext;
import javax.persistence.PersistenceException;
import java.time.LocalDateTime;
import java.util.Arrays;
Expand All @@ -46,6 +46,7 @@
@RunWith(SpringRunner.class)
@DataJpaTest
@ContextConfiguration(classes = {MonotonicAccessionGeneratorTestConfiguration.class})
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)
public class ContiguousIdBlockServiceTest {

private static final String CATEGORY_ID = "cat-test";
Expand All @@ -59,8 +60,8 @@ public class ContiguousIdBlockServiceTest {
@Autowired
private ContiguousIdBlockService service;

@PersistenceContext
EntityManager entityManager;
@Autowired
private TestEntityManager testEntityManager;

@Test
public void testReserveNewBlocks() {
Expand All @@ -78,6 +79,9 @@ public void testReserveNewBlocks() {
public void testReserveWithExistingData() {
//Save a block
service.save(Arrays.asList(new ContiguousIdBlock(CATEGORY_ID, INSTANCE_ID, 0, 5)));
testEntityManager.flush();
testEntityManager.getEntityManager().getTransaction().commit();

ContiguousIdBlock block = service.reserveNewBlock(CATEGORY_ID, INSTANCE_ID);
assertEquals(5, block.getFirstValue());
assertEquals(1004, block.getLastValue());
Expand Down Expand Up @@ -166,10 +170,14 @@ public void testBlockSizeAndIntervalWithMultipleInstances() {
ContiguousIdBlock block2 = service.reserveNewBlock(CATEGORY_ID_2, INSTANCE_ID_2);
assertEquals(2000, block2.getFirstValue());
assertEquals(2999, block2.getLastValue());
assertEquals(block2, repository.findFirstByCategoryIdOrderByLastValueDesc(CATEGORY_ID_2));
ContiguousIdBlock block2InDB = repository.findFirstByCategoryIdOrderByLastValueDesc(CATEGORY_ID_2);
assertEquals(2000, block2InDB.getFirstValue());
assertEquals(2999, block2InDB.getLastValue());

//Manually save a block of size 500, so for the current range only a block size of 500 reserved
repository.save(new ContiguousIdBlock(CATEGORY_ID_2, INSTANCE_ID, 4000, 500));
testEntityManager.flush();
testEntityManager.getEntityManager().getTransaction().commit();
//Reserve a new block with size 1000
ContiguousIdBlock block3 = service.reserveNewBlock(CATEGORY_ID_2, INSTANCE_ID_2);

Expand Down Expand Up @@ -260,12 +268,12 @@ public void testNextBlockWithStartingPointOtherThanZero() {
public void testBlocksWithDuplicateCategoryAndFirstValue() {
ContiguousIdBlock block1 = new ContiguousIdBlock(CATEGORY_ID, INSTANCE_ID, 100, 1000);
repository.save(block1);
entityManager.flush();
testEntityManager.flush();

ContiguousIdBlock block2 = new ContiguousIdBlock(CATEGORY_ID, INSTANCE_ID_2, 100, 2000);
repository.save(block2);

Throwable exception = assertThrows(PersistenceException.class, () -> entityManager.flush());
Throwable exception = assertThrows(PersistenceException.class, () -> testEntityManager.flush());
assertTrue(exception.getCause() instanceof ConstraintViolationException);
}

Expand All @@ -274,7 +282,7 @@ public void testLastUpdateTimeStampAutoUpdate() {
// block saved with initial value
ContiguousIdBlock block = new ContiguousIdBlock(CATEGORY_ID, INSTANCE_ID, 100, 1000);
repository.save(block);
entityManager.flush();
testEntityManager.flush();

// assert block values
List<ContiguousIdBlock> blockInDBList = StreamSupport.stream(repository.findAll().spliterator(), false)
Expand All @@ -292,7 +300,7 @@ public void testLastUpdateTimeStampAutoUpdate() {
// block updated with last committed 100
block.setLastCommitted(100);
repository.save(block);
entityManager.flush();
testEntityManager.flush();
blockInDB = repository.findAll().iterator().next();
assertEquals(100, blockInDB.getLastCommitted());

Expand All @@ -301,7 +309,7 @@ public void testLastUpdateTimeStampAutoUpdate() {
// block updated with new instance id
block.setApplicationInstanceId(INSTANCE_ID_2);
repository.save(block);
entityManager.flush();
testEntityManager.flush();
blockInDB = repository.findAll().iterator().next();
assertEquals(INSTANCE_ID_2, blockInDB.getApplicationInstanceId());

Expand All @@ -310,7 +318,7 @@ public void testLastUpdateTimeStampAutoUpdate() {
// block updated - release reserved
block.releaseReserved();
repository.save(block);
entityManager.flush();
testEntityManager.flush();
blockInDB = repository.findAll().iterator().next();
assertTrue(blockInDB.isNotReserved());

Expand All @@ -319,7 +327,7 @@ public void testLastUpdateTimeStampAutoUpdate() {
// block update - mark as reserved
block.markAsReserved();
repository.save(block);
entityManager.flush();
testEntityManager.flush();
blockInDB = repository.findAll().iterator().next();
assertTrue(blockInDB.isReserved());

Expand Down Expand Up @@ -349,7 +357,7 @@ public void testGetBlocksWithLastUpdatedTimeStampLessThan() throws InterruptedEx
repository.save(block3);
repository.save(block4);
repository.save(block5);
entityManager.flush();
testEntityManager.flush();

LocalDateTime cutOffTimestamp = block4.getLastUpdatedTimestamp();
List<ContiguousIdBlock> blocksList = service.allBlocksForCategoryIdReservedBeforeTheGivenTimeFrame(CATEGORY_ID, cutOffTimestamp);
Expand Down
Loading