From 5a656ec6328ea0e8df18a26be11d5f2f2dbb4ace Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Mon, 5 Aug 2024 14:26:11 +0000 Subject: [PATCH 1/3] Modified UserFateStore.create to behave like MetaFateStore MetaFateStore.create will retry forever when a collision happens when trying to create a Fate transaction. The probabiliy of a collision is low due to a random UUID being used. Before this change the UserFateStore.create method would retry 5 times then throw an exception. This was removed in favor of an unlimited retry so that the behavior of the two Fate stores is the same. Closes #4246 --- .../core/fate/user/UserFateStore.java | 8 ++--- .../test/fate/user/UserFateStoreIT.java | 36 ------------------- 2 files changed, 4 insertions(+), 40 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java b/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java index 75fdeca989b..7fdcfd3aa0e 100644 --- a/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java +++ b/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java @@ -88,9 +88,10 @@ public UserFateStore(ClientContext context) { @Override public FateId create() { - final int maxAttempts = 5; - for (int attempt = 0; attempt < maxAttempts; attempt++) { + int attempt = 0; + while (true) { + FateId fateId = getFateId(); if (attempt >= 1) { @@ -106,13 +107,12 @@ public FateId create() { return fateId; case UNKNOWN: case REJECTED: + attempt++; continue; default: throw new IllegalStateException("Unknown status " + status); } } - - throw new IllegalStateException("Failed to create new id after " + maxAttempts + " attempts"); } public FateId getFateId() { diff --git a/test/src/main/java/org/apache/accumulo/test/fate/user/UserFateStoreIT.java b/test/src/main/java/org/apache/accumulo/test/fate/user/UserFateStoreIT.java index 337f9e4bdc4..760fa871491 100644 --- a/test/src/main/java/org/apache/accumulo/test/fate/user/UserFateStoreIT.java +++ b/test/src/main/java/org/apache/accumulo/test/fate/user/UserFateStoreIT.java @@ -24,11 +24,9 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Iterator; -import java.util.LinkedHashSet; import java.util.List; import java.util.Set; import java.util.UUID; -import java.util.stream.Collectors; import org.apache.accumulo.core.client.Accumulo; import org.apache.accumulo.core.client.BatchWriter; @@ -61,14 +59,11 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.google.common.collect.MoreCollectors; public class UserFateStoreIT extends SharedMiniClusterBase { - private static final Logger log = LoggerFactory.getLogger(UserFateStore.class); private static final FateInstanceType fateInstanceType = FateInstanceType.USER; @BeforeAll @@ -150,37 +145,6 @@ public void testFateInitialConfigCorrectness() throws Exception { } } - @Test - public void testCreateWithCollisionAndExceedRetryLimit() throws Exception { - String table = getUniqueNames(1)[0]; - try (ClientContext client = - (ClientContext) Accumulo.newClient().from(getClientProps()).build()) { - createFateTable(client, table); - - UUID[] uuids = new UUID[5]; - for (int i = 0; i < uuids.length; i++) { - uuids[i] = UUID.randomUUID(); - } - List txids = - List.of(uuids[0], uuids[0], uuids[0], uuids[1], uuids[2], uuids[2], uuids[2], uuids[2], - uuids[3], uuids[3], uuids[4], uuids[4], uuids[4], uuids[4], uuids[4], uuids[4]); - List fateIds = txids.stream().map(txid -> FateId.from(fateInstanceType, txid)) - .collect(Collectors.toList()); - Set expectedFateIds = new LinkedHashSet<>(fateIds); - TestUserFateStore store = new TestUserFateStore(client, table, fateIds); - - // call create and expect we get the unique txids - for (FateId expectedFateId : expectedFateIds) { - FateId fateId = store.create(); - log.info("Created fate id: " + fateId); - assertEquals(expectedFateId, fateId, "Expected " + expectedFateId + " but got " + fateId); - } - - // Calling create again on 5L should throw an exception since we've exceeded the max retries - assertThrows(IllegalStateException.class, store::create); - } - } - @Nested class TestStatusEnforcement { From 588221231f2f7b604a8c2ccbce9d25bc4cbf0929 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Mon, 5 Aug 2024 19:53:48 +0000 Subject: [PATCH 2/3] Throw exception on Unknown status return --- .../org/apache/accumulo/core/fate/user/UserFateStore.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java b/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java index 7fdcfd3aa0e..7e1fff5189a 100644 --- a/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java +++ b/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java @@ -106,7 +106,11 @@ public FateId create() { case ACCEPTED: return fateId; case UNKNOWN: + log.error("Fate creation returned UNKNOWN status, raising exception"); + throw new IllegalStateException("Fate creation returned UNKNOWN status"); case REJECTED: + log.debug( + "Fate creation returned REJECTED status (most likely FateId collision), retrying..."); attempt++; continue; default: From 99b1eced6cd4f6c1da35bf039fbff4c40e5e0171 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Mon, 5 Aug 2024 20:37:42 +0000 Subject: [PATCH 3/3] Revert "Throw exception on Unknown status return" This reverts commit 588221231f2f7b604a8c2ccbce9d25bc4cbf0929. --- .../org/apache/accumulo/core/fate/user/UserFateStore.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java b/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java index 7e1fff5189a..7fdcfd3aa0e 100644 --- a/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java +++ b/core/src/main/java/org/apache/accumulo/core/fate/user/UserFateStore.java @@ -106,11 +106,7 @@ public FateId create() { case ACCEPTED: return fateId; case UNKNOWN: - log.error("Fate creation returned UNKNOWN status, raising exception"); - throw new IllegalStateException("Fate creation returned UNKNOWN status"); case REJECTED: - log.debug( - "Fate creation returned REJECTED status (most likely FateId collision), retrying..."); attempt++; continue; default: