This is an automated email from the ASF dual-hosted git repository. kturner pushed a commit to branch elasticity in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/elasticity by this push: new 46d8c0b71d makes behavior of reading absent fate consistent (#4688) 46d8c0b71d is described below commit 46d8c0b71deb4e7287eff0158e7014247f929ae2 Author: Keith Turner <ktur...@apache.org> AuthorDate: Sat Jun 22 14:13:01 2024 -0400 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/accumulo/core/fate/MetaFateStore.java | 18 ++++++++--------- .../org/apache/accumulo/test/fate/FateStoreIT.java | 23 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 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 index f701f34dcc..726f5c614c 100644 --- 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 class MetaFateStore<T> extends AbstractFateStore<T> { 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 class MetaFateStore<T> extends AbstractFateStore<T> { } 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 index ac1930cf59..2ffdb59031 100644 --- 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.ReadOnlyFateStore.TStatus; 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 @@ public abstract class FateStoreIT extends SharedMiniClusterBase implements FateT } + @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);