FateId to use UUID instead of long (#4388)

- FateId now uses a 128bit UUID instead of a (64bit) long - FateIds created by ZooStore and AccumuloStore now use a v4 UUID (random UUID) - FateIds created by FateIdGenerator now use a v3 UUID (name based) so the same FateKey gives the same UUID - Necessary updates to classes and tests which used the long id
apache · Mar 16, 2024 · eb71e3e · eb71e3e
1 parent d5d123a
commit eb71e3e
Show file tree

Hide file tree

Showing 33 changed files with 334 additions and 310 deletions.
diff --git a/core/src/main/java/org/apache/accumulo/core/fate/AbstractFateStore.java b/core/src/main/java/org/apache/accumulo/core/fate/AbstractFateStore.java
@@ -35,6 +35,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
@@ -48,8 +49,6 @@
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Preconditions;
-import com.google.common.hash.HashCode;
-import com.google.common.hash.Hashing;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
@@ -64,9 +63,8 @@ public abstract class AbstractFateStore<T> implements FateStore<T> {
   public static final FateIdGenerator DEFAULT_FATE_ID_GENERATOR = new FateIdGenerator() {
     @Override
     public FateId fromTypeAndKey(FateInstanceType instanceType, FateKey fateKey) {
-      HashCode hashCode = Hashing.murmur3_128().hashBytes(fateKey.getSerialized());
-      long tid = hashCode.asLong() & 0x7fffffffffffffffL;
-      return FateId.from(instanceType, tid);
+      UUID txUUID = UUID.nameUUIDFromBytes(fateKey.getSerialized());
+      return FateId.from(instanceType, txUUID);
     }
   };
 
@@ -271,9 +269,9 @@ private Optional<FateId> create(FateKey fateKey) {
       // mean a collision
       if (status == TStatus.NEW) {
         Preconditions.checkState(tFateKey.isPresent(), "Tx Key is missing from tid %s",
-            fateId.getTid());
+            fateId.getTxUUIDStr());
         Preconditions.checkState(fateKey.equals(tFateKey.orElseThrow()),
-            "Collision detected for tid %s", fateId.getTid());
+            "Collision detected for tid %s", fateId.getTxUUIDStr());
         // Case 2: Status is some other state which means already in progress
         // so we can just log and return empty optional
       } else {

diff --git a/core/src/main/java/org/apache/accumulo/core/fate/FateId.java b/core/src/main/java/org/apache/accumulo/core/fate/FateId.java
@@ -18,28 +18,29 @@
  */
 package org.apache.accumulo.core.fate;
 
+import java.util.UUID;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.apache.accumulo.core.data.AbstractId;
 import org.apache.accumulo.core.manager.thrift.TFateId;
 import org.apache.accumulo.core.manager.thrift.TFateInstanceType;
-import org.apache.accumulo.core.util.FastFormat;
 
 /**
  * A strongly typed FATE Transaction ID. This is used to uniquely identify a FATE transaction.
- * Consists of its {@link FateInstanceType} and its transaction id (long). The canonical string is
- * of the form "FATE:[FateInstanceType]:[hex long tid]" (without the brackets).
+ * Consists of its {@link FateInstanceType} and its transaction {@link UUID}. The canonical string
+ * is of the form "FATE:[FateInstanceType]:[UUID]" (without the brackets).
  */
 public class FateId extends AbstractId<FateId> {
 
   private static final long serialVersionUID = 1L;
   private static final String PREFIX = "FATE:";
-  private static final Pattern HEX_PATTERN = Pattern.compile("^[0-9a-fA-F]+$");
+  private static final String UUID_REGEX = "[0-9a-fA-F]{8}-([0-9a-fA-F]{4}-){3}[0-9a-fA-F]{12}";
+  private static final Pattern UUID_PATTERN = Pattern.compile("^" + UUID_REGEX + "$");
   private static final Pattern FATEID_PATTERN = Pattern.compile("^" + PREFIX + "("
-      + Stream.of(FateInstanceType.values()).map(Enum::name).collect(Collectors.joining("|"))
-      + "):[0-9a-fA-F]+$");
+      + Stream.of(FateInstanceType.values()).map(Enum::name).collect(Collectors.joining("|")) + "):"
+      + UUID_REGEX + "$");
 
   private FateId(String canonical) {
     super(canonical);
@@ -53,42 +54,42 @@ public FateInstanceType getType() {
   }
 
   /**
-   * @return the decimal value of the transaction id
+   * @return the transaction {@link UUID}
    */
-  public long getTid() {
-    return Long.parseLong(getHexTid(), 16);
+  public UUID getTxUUID() {
+    return UUID.fromString(getTxUUIDStr());
   }
 
   /**
-   * @return the hexadecimal value of the transaction id
+   * @return the transaction {@link UUID} as a String
    */
-  public String getHexTid() {
+  public String getTxUUIDStr() {
     return canonical().split(":")[2];
   }
 
   /**
    * Creates a new FateId object from the given parameters
    *
    * @param type the {@link FateInstanceType}
-   * @param tid the decimal transaction id
+   * @param txUUID the {@link UUID}
    * @return a new FateId object
    */
-  public static FateId from(FateInstanceType type, long tid) {
-    return new FateId(PREFIX + type + ":" + formatTid(tid));
+  public static FateId from(FateInstanceType type, UUID txUUID) {
+    return new FateId(PREFIX + type + ":" + txUUID);
   }
 
   /**
    * Creates a new FateId object from the given parameters
    *
    * @param type the {@link FateInstanceType}
-   * @param hexTid the hexadecimal transaction id
+   * @param txUUIDStr the transaction {@link UUID} as a String
    * @return a new FateId object
    */
-  public static FateId from(FateInstanceType type, String hexTid) {
-    if (HEX_PATTERN.matcher(hexTid).matches()) {
-      return new FateId(PREFIX + type + ":" + hexTid);
+  public static FateId from(FateInstanceType type, String txUUIDStr) {
+    if (UUID_PATTERN.matcher(txUUIDStr).matches()) {
+      return new FateId(PREFIX + type + ":" + txUUIDStr);
     } else {
-      throw new IllegalArgumentException("Invalid Hex Transaction ID: " + hexTid);
+      throw new IllegalArgumentException("Invalid Transaction UUID: " + txUUIDStr);
     }
   }
 
@@ -118,7 +119,7 @@ public static boolean isFateId(String fateIdStr) {
    */
   public static FateId fromThrift(TFateId tFateId) {
     FateInstanceType type;
-    long tid = tFateId.getTid();
+    String txUUIDStr = tFateId.getTxUUIDStr();
 
     switch (tFateId.getType()) {
       case USER:
@@ -131,7 +132,11 @@ public static FateId fromThrift(TFateId tFateId) {
         throw new IllegalArgumentException("Invalid TFateInstanceType: " + tFateId.getType());
     }
 
-    return new FateId(PREFIX + type + ":" + formatTid(tid));
+    if (UUID_PATTERN.matcher(txUUIDStr).matches()) {
+      return new FateId(PREFIX + type + ":" + txUUIDStr);
+    } else {
+      throw new IllegalArgumentException("Invalid Transaction UUID: " + txUUIDStr);
+    }
   }
 
   /**
@@ -151,13 +156,6 @@ public TFateId toThrift() {
       default:
         throw new IllegalArgumentException("Invalid FateInstanceType: " + type);
     }
-    return new TFateId(thriftType, getTid());
-  }
-
-  /**
-   * Returns the hex string equivalent of the tid
-   */
-  public static String formatTid(long tid) {
-    return FastFormat.toHexString(tid);
+    return new TFateId(thriftType, getTxUUIDStr());
   }
 }
diff --git a/core/src/main/java/org/apache/accumulo/core/fate/ZooStore.java b/core/src/main/java/org/apache/accumulo/core/fate/ZooStore.java
@@ -20,7 +20,6 @@
 
 import static com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
-import static org.apache.accumulo.core.util.LazySingletons.RANDOM;
 
 import java.io.ByteArrayOutputStream;
 import java.io.DataOutputStream;
@@ -32,6 +31,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.UUID;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
 
@@ -62,7 +62,7 @@ public class ZooStore<T> extends AbstractFateStore<T> {
   private ZooReaderWriter zk;
 
   private String getTXPath(FateId fateId) {
-    return path + "/tx_" + fateId.getHexTid();
+    return path + "/tx_" + fateId.getTxUUIDStr();
   }
 
   public ZooStore(String path, ZooReaderWriter zk) throws KeeperException, InterruptedException {
@@ -88,9 +88,7 @@ public ZooStore(String path, ZooReaderWriter zk, int maxDeferred, FateIdGenerato
   public FateId create() {
     while (true) {
       try {
-        // looking at the code for SecureRandom, it appears to be thread safe
-        long tid = RANDOM.get().nextLong() & 0x7fffffffffffffffL;
-        FateId fateId = FateId.from(fateInstanceType, tid);
+        FateId fateId = FateId.from(fateInstanceType, UUID.randomUUID());
         zk.putPersistentData(getTXPath(fateId), new NodeValue(TStatus.NEW).serialize(),
             NodeExistsPolicy.FAIL);
         return fateId;

diff --git a/core/src/main/java/org/apache/accumulo/core/fate/accumulo/AccumuloStore.java b/core/src/main/java/org/apache/accumulo/core/fate/accumulo/AccumuloStore.java
@@ -18,13 +18,12 @@
  */
 package org.apache.accumulo.core.fate.accumulo;
 
-import static org.apache.accumulo.core.util.LazySingletons.RANDOM;
-
 import java.io.Serializable;
 import java.util.List;
 import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.UUID;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -116,8 +115,7 @@ public FateId create() {
   }
 
   public FateId getFateId() {
-    long tid = RANDOM.get().nextLong() & 0x7fffffffffffffffL;
-    return FateId.from(fateInstanceType, tid);
+    return FateId.from(fateInstanceType, UUID.randomUUID());
   }
 
   @Override
@@ -250,7 +248,7 @@ static Range getRow(FateId fateId) {
   }
 
   public static String getRowId(FateId fateId) {
-    return "tx_" + fateId.getHexTid();
+    return "tx_" + fateId.getTxUUIDStr();
   }
 
   private FateMutatorImpl<T> newMutator(FateId fateId) {

diff --git a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooReservation.java b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooReservation.java
@@ -30,7 +30,7 @@
 
 public class ZooReservation {
 
-  private static final String DELIMITER = "-";
+  private static final String DELIMITER = "_";
 
   public static boolean attempt(ZooReaderWriter zk, String path, FateId fateId, String debugInfo)
       throws KeeperException, InterruptedException {