cshannon commented on code in PR #4204:
URL: https://github.com/apache/accumulo/pull/4204#discussion_r1481371291
##########
test/src/main/java/org/apache/accumulo/test/fate/accumulo/FateStoreIT.java:
##########
@@ -217,10 +228,144 @@ protected void testDeferredOverflow(FateStore<TestEnv>
store, ServerContext sctx
} finally {
executor.shutdownNow();
// Cleanup so we don't interfere with other tests
- store.list().forEach(fateIdStatus ->
store.reserve(fateIdStatus.getFateId()).delete());
+ // All stores should already be unreserved
+ store.list().forEach(
+ fateIdStatus ->
store.tryReserve(fateIdStatus.getFateId()).orElseThrow().delete());
}
}
+ @Test
+ public void testCreateWithKey() throws Exception {
+ executeTest(this::testCreateWithKey);
+ }
+
+ protected void testCreateWithKey(FateStore<TestEnv> store, ServerContext
sctx) {
+ KeyExtent ke1 = new KeyExtent(TableId.of("tableId"), new Text("zzz"), new
Text("aaa"));
+
+ FateKey fateKey1 = FateKey.forSplit(ke1);
+ FateId fateId1 = store.create(fateKey1);
+
+ FateKey fateKey2 =
+
FateKey.forCompactionCommit(ExternalCompactionId.generate(UUID.randomUUID()));
+ FateId fateId2 = store.create(fateKey2);
+ assertNotEquals(fateId1, fateId2);
+
+ FateTxStore<TestEnv> txStore1 = store.reserve(fateId1);
+ FateTxStore<TestEnv> txStore2 = store.reserve(fateId2);
+ try {
+ assertTrue(txStore1.timeCreated() > 0);
+ assertEquals(TStatus.NEW, txStore1.getStatus());
+ assertEquals(fateKey1, txStore1.getKey().orElseThrow());
+
+ assertTrue(txStore2.timeCreated() > 0);
+ assertEquals(TStatus.NEW, txStore2.getStatus());
+ assertEquals(fateKey2, txStore2.getKey().orElseThrow());
+
+ assertEquals(2, store.list().count());
+ } finally {
+ txStore1.delete();
+ txStore2.delete();
+ txStore1.unreserve(0, TimeUnit.SECONDS);
+ txStore2.unreserve(0, TimeUnit.SECONDS);
+ }
+ }
+
+ @Test
+ public void testCreateWithKeyDuplicate() throws Exception {
+ executeTest(this::testCreateWithKeyDuplicate);
+ }
+
+ protected void testCreateWithKeyDuplicate(FateStore<TestEnv> store,
ServerContext sctx) {
+ KeyExtent ke = new KeyExtent(TableId.of("tableId"), new Text("zzz"), new
Text("aaa"));
+
+ // Creating with the same key should be fine if the status is NEW
+ // It should just return the same id and allow us to continue reserving
+ FateKey fateKey = FateKey.forSplit(ke);
+ FateId fateId1 = store.create(fateKey);
+ FateId fateId2 = store.create(fateKey);
+ assertEquals(fateId1, fateId2);
+
+ FateTxStore<TestEnv> txStore = store.reserve(fateId1);
+ try {
+ assertTrue(txStore.timeCreated() > 0);
+ assertEquals(TStatus.NEW, txStore.getStatus());
+ assertEquals(fateKey, txStore.getKey().orElseThrow());
+ assertEquals(1, store.list().count());
+ } finally {
+ txStore.delete();
+ txStore.unreserve(0, TimeUnit.SECONDS);
+ }
+ }
+
+ @Test
+ public void testCreateWithKeyInProgress() throws Exception {
+ executeTest(this::testCreateWithKeyInProgress);
+ }
+
+ protected void testCreateWithKeyInProgress(FateStore<TestEnv> store,
ServerContext sctx) {
+ KeyExtent ke = new KeyExtent(TableId.of("tableId"), new Text("zzz"), new
Text("aaa"));
+
+ FateKey fateKey = FateKey.forSplit(ke);
+ FateId fateId = store.create(fateKey);
+
+ FateTxStore<TestEnv> txStore = store.reserve(fateId);
+ try {
+ assertTrue(txStore.timeCreated() > 0);
+ txStore.setStatus(TStatus.IN_PROGRESS);
+
+ // We have an existing transaction with the same key in progress
+ // so should not be allowed
+ assertThrows(IllegalStateException.class, () -> store.create(fateKey));
+ } finally {
+ txStore.delete();
+ txStore.unreserve(0, TimeUnit.SECONDS);
+ }
+
+ try {
+ // After deletion, make sure we can create again with the same key
+ fateId = store.create(fateKey);
+ txStore = store.reserve(fateId);
+ assertTrue(txStore.timeCreated() > 0);
+ assertEquals(TStatus.NEW, txStore.getStatus());
+ } finally {
+ txStore.delete();
+ txStore.unreserve(0, TimeUnit.SECONDS);
+ }
+
+ }
+
+ @Test
+ public void testCreateWithKeyCollision() throws Exception {
+ // Replace the default hasing algorithm with one that always returns the
same tid so
+ // we can check duplicate detection with different keys
+ executeTest(this::testCreateWithKeyCollision,
AbstractFateStore.DEFAULT_MAX_DEFERRED,
+ (instanceType, fateKey) -> FateId.from(instanceType, 1000));
+ }
+
+ protected void testCreateWithKeyCollision(FateStore<TestEnv> store,
ServerContext sctx) {
+ KeyExtent ke1 = new KeyExtent(TableId.of("tableId1"), new Text("zzz"), new
Text("aaa"));
+ KeyExtent ke2 = new KeyExtent(TableId.of("tableId2"), new Text("ddd"), new
Text("bbb"));
+
+ FateKey fateKey1 = FateKey.forSplit(ke1);
+ FateKey fateKey2 = FateKey.forSplit(ke2);
+ FateId fateId1 = store.create(fateKey1);
+
+ FateTxStore<TestEnv> txStore = store.reserve(fateId1);
+ try {
+ try {
+ store.create(fateKey2);
+ fail("Expected IllegalStateException due to hashing collision");
+ } catch (Exception e) {
+ assertInstanceOf(IllegalStateException.class, e);
+ assertEquals("Collision detected for tid 1000", e.getMessage());
+ }
Review Comment:
Thanks for the suggestion, I was trying to use assertThrows() but I wanted
to check the message and didn't see a way to do that with assertThrows(). I
didn't realize that method also returns the exception so that makes it easy to
check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
