makes behavior of reading absent fate consistent (#4688)

The two different fate stores had slightly different behavior when reading an absent fateId. Changed the behavior to make it consistent and added a test to ensure they stay consistent.
apache · Jun 22, 2024 · 46d8c0b · 46d8c0b
1 parent 47a53ae
commit 46d8c0b
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 9 deletions.
diff --git a/core/src/main/java/org/apache/accumulo/core/fate/MetaFateStore.java b/core/src/main/java/org/apache/accumulo/core/fate/MetaFateStore.java
@@ -139,14 +139,9 @@ public Repo<T> top() {
       for (int i = 0; i < RETRIES; i++) {
         String txpath = getTXPath(fateId);
         try {
-          String top;
-          try {
-            top = findTop(txpath);
-            if (top == null) {
-              return null;
-            }
-          } catch (KeeperException.NoNodeException ex) {
-            throw new IllegalStateException(ex);
+          String top = findTop(txpath);
+          if (top == null) {
+            return null;
           }
 
           byte[] ser = zk.getData(txpath + "/" + top);
@@ -165,7 +160,12 @@ public Repo<T> top() {
     }
 
     private String findTop(String txpath) throws KeeperException, InterruptedException {
-      List<String> ops = zk.getChildren(txpath);
+      List<String> ops;
+      try {
+        ops = zk.getChildren(txpath);
+      } catch (NoNodeException e) {
+        return null;
+      }
 
       ops = new ArrayList<>(ops);
 

diff --git a/test/src/main/java/org/apache/accumulo/test/fate/FateStoreIT.java b/test/src/main/java/org/apache/accumulo/test/fate/FateStoreIT.java
@@ -55,6 +55,7 @@
 import org.apache.accumulo.core.fate.ReadOnlyRepo;
 import org.apache.accumulo.core.fate.StackOverflowException;
 import org.apache.accumulo.core.metadata.schema.ExternalCompactionId;
+import org.apache.accumulo.core.util.Pair;
 import org.apache.accumulo.harness.SharedMiniClusterBase;
 import org.apache.accumulo.server.ServerContext;
 import org.apache.accumulo.test.fate.FateIT.TestRepo;
@@ -460,6 +461,28 @@ protected void testCollisionWithRandomFateId(FateStore<TestEnv> store, ServerCon
 
   }
 
+  @Test
+  public void testAbsent() throws Exception {
+    executeTest(this::testAbsent);
+  }
+
+  protected void testAbsent(FateStore<TestEnv> store, ServerContext sctx) {
+    // Ensure both store implementations have consistent behavior when reading a fateId that does
+    // not exists.
+
+    var fateId = FateId.from(store.type(), UUID.randomUUID());
+    var txStore = store.read(fateId);
+
+    assertEquals(TStatus.UNKNOWN, txStore.getStatus());
+    assertNull(txStore.top());
+    assertNull(txStore.getTransactionInfo(TxInfo.TX_NAME));
+    assertEquals(0, txStore.timeCreated());
+    assertEquals(Optional.empty(), txStore.getKey());
+    assertEquals(fateId, txStore.getID());
+    assertEquals(List.of(), txStore.getStack());
+    assertEquals(new Pair<>(TStatus.UNKNOWN, Optional.empty()), txStore.getStatusAndKey());
+  }
+
   @Test
   public void testListFateKeys() throws Exception {
     executeTest(this::testListFateKeys);