From a672a19e8093ee45dc37e6f4a86e32dbd0ad6e15 Mon Sep 17 00:00:00 2001
From: "Colin P. McCabe" <cmccabe@apache.org>
Date: Mon, 30 Oct 2023 16:36:31 -0700
Subject: [PATCH] MINOR: small optimization for DirectoryId.random

DirectoryId.random doesn't need to instantiate the first 100 IDs to check if an ID is one of them.

Reviewers: Ismael Juma <ismael@juma.me.uk>, Justine Olshan <jolshan@confluent.io>, Proven Provenzano <93720617+pprovenzano@users.noreply.github.com>
---
 .../org/apache/kafka/common/DirectoryId.java  | 38 +++++++++----------
 .../apache/kafka/common/DirectoryIdTest.java  | 20 +++++-----
 2 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/server-common/src/main/java/org/apache/kafka/common/DirectoryId.java b/server-common/src/main/java/org/apache/kafka/common/DirectoryId.java
index 82cb64f4df4..9f1145a9def 100644
--- a/server-common/src/main/java/org/apache/kafka/common/DirectoryId.java
+++ b/server-common/src/main/java/org/apache/kafka/common/DirectoryId.java
@@ -18,12 +18,9 @@ package org.apache.kafka.common;
 
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 public class DirectoryId {
 
@@ -47,30 +44,31 @@ public class DirectoryId {
     public static final Uuid MIGRATING = new Uuid(0L, 2L);
 
     /**
-     * The set of reserved UUIDs that will never be returned by the random method.
+     * Static factory to generate a directory ID.
+     *
+     * This will not generate a reserved UUID (first 100), or one whose string representation
+     * starts with a dash ("-")
      */
-    public static final Set<Uuid> RESERVED;
-
-    static {
-        HashSet<Uuid> reserved = new HashSet<>();
-        // The first 100 Uuids are reserved for future use.
-        for (long i = 0L; i < 100L; i++) {
-            reserved.add(new Uuid(0L, i));
+    public static Uuid random() {
+        while (true) {
+            // Uuid.randomUuid does not generate Uuids whose string representation starts with a
+            // dash.
+            Uuid uuid = Uuid.randomUuid();
+            if (!DirectoryId.reserved(uuid)) {
+                return uuid;
+            }
         }
-        RESERVED = Collections.unmodifiableSet(reserved);
     }
 
     /**
-     * Static factory to generate a directory ID.
+     * Check if a directory ID is part of the first 100 reserved IDs.
      *
-     * This will not generate a reserved UUID (first 100), or one whose string representation starts with a dash ("-")
+     * @param uuid the directory ID to check.
+     * @return     true only if the directory ID is reserved.
      */
-    public static Uuid random() {
-        Uuid uuid = Uuid.randomUuid();
-        while (RESERVED.contains(uuid) || uuid.toString().startsWith("-")) {
-            uuid = Uuid.randomUuid();
-        }
-        return uuid;
+    public static boolean reserved(Uuid uuid) {
+        return (uuid.getMostSignificantBits() == 0 &&
+            uuid.getLeastSignificantBits() < 100);
     }
 
     /**
diff --git a/server-common/src/test/java/org/apache/kafka/common/DirectoryIdTest.java b/server-common/src/test/java/org/apache/kafka/common/DirectoryIdTest.java
index 5db21931077..a560a3dc7d1 100644
--- a/server-common/src/test/java/org/apache/kafka/common/DirectoryIdTest.java
+++ b/server-common/src/test/java/org/apache/kafka/common/DirectoryIdTest.java
@@ -28,17 +28,19 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class DirectoryIdTest {
+    @Test
+    void testUnassignedIsReserved() {
+        assertTrue(DirectoryId.reserved(DirectoryId.UNASSIGNED));
+    }
 
     @Test
-    void testReserved() {
-        Set<Long> seen = new HashSet<>(100);
-        for (Uuid reservedId : DirectoryId.RESERVED) {
-            assertEquals(0L, reservedId.getMostSignificantBits(), "Unexpected reserved msb value");
-            long lsb = reservedId.getLeastSignificantBits();
-            assertTrue(lsb >= 0 && lsb < 100L, "Unexpected reserved lsb value");
-            assertTrue(seen.add(lsb), "Duplicate reserved value");
-        }
-        assertEquals(100, DirectoryId.RESERVED.size());
+    void testLostIsReserved() {
+        assertTrue(DirectoryId.reserved(DirectoryId.LOST));
+    }
+
+    @Test
+    void testMigratingIsReserved() {
+        assertTrue(DirectoryId.reserved(DirectoryId.MIGRATING));
     }
 
     @Test