Skip to content

Commit

Permalink
Modified UserFateStore.create to behave like MetaFateStore (#4787)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dlmarion authored Aug 9, 2024
1 parent cc95159 commit 0d72500
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<UUID> 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<FateId> fateIds = txids.stream().map(txid -> FateId.from(fateInstanceType, txid))
.collect(Collectors.toList());
Set<FateId> 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 {

Expand Down

0 comments on commit 0d72500

Please sign in to comment.