cshannon commented on code in PR #4204:
URL: https://github.com/apache/accumulo/pull/4204#discussion_r1477429903
##########
test/src/main/java/org/apache/accumulo/test/fate/accumulo/FateStoreIT.java:
##########
@@ -216,7 +227,104 @@ 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.getTxid()).delete());
+ // All stores should already be unreserved
+ store.list()
+ .forEach(fateIdStatus ->
store.tryReserve(fateIdStatus.getTxid()).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"));
+ KeyExtent ke2 = new KeyExtent(TableId.of("tableId2"), new Text("zzz"), new
Text("aaa"));
+
+ byte[] key1 = serialize(ke1);
+ long tid1 = store.create(key1);
+
+ byte[] key2 = serialize(ke2);
+ long tid2 = store.create(key2);
+ assertNotEquals(tid1, tid2);
+
+ FateTxStore<TestEnv> txStore1 = store.reserve(tid1);
+ FateTxStore<TestEnv> txStore2 = store.reserve(tid2);
+ try {
+ assertTrue(txStore1.timeCreated() > 0);
+ assertEquals(TStatus.NEW, txStore1.getStatus());
+ assertArrayEquals(key1, txStore1.getKey().orElseThrow());
+
+ assertTrue(txStore2.timeCreated() > 0);
+ assertEquals(TStatus.NEW, txStore2.getStatus());
+ assertArrayEquals(key2, txStore2.getKey().orElseThrow());
+
+ assertEquals(2, store.list().count());
+ } finally {
+ txStore1.delete();
+ txStore2.delete();
+ }
+ }
+
+ @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
+ byte[] key = serialize(ke);
+ long tid1 = store.create(key);
+ long tid2 = store.create(key);
+ assertEquals(tid1, tid2);
+
+ FateTxStore<TestEnv> txStore = store.reserve(tid1);
+ try {
+ assertTrue(txStore.timeCreated() > 0);
+ assertEquals(TStatus.NEW, txStore.getStatus());
+ assertArrayEquals(key, txStore.getKey().orElseThrow());
+ assertEquals(1, store.list().count());
+ } finally {
+ txStore.delete();
+ }
+ }
+
+ @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"));
+
+ byte[] key = serialize(ke);
+ long tid1 = store.create(key);
+
+ FateTxStore<TestEnv> txStore = store.reserve(tid1);
+ 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(key));
+ } finally {
+ txStore.delete();
+ }
+ }
+
Review Comment:
In order to make this easier to test I updated the store to allow providing
a different hash code or FateId generated from the key and i replace it for the
test to see if the collision detection worked
--
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
