Fix NUID sequence (#488)

* fix NUID sequence * format
nats-io · Apr 25, 2024 · 65ef7c5 · 65ef7c5
1 parent dcfaac1
commit 65ef7c5
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 10 deletions.
diff --git a/src/NATS.Client.Core/Internal/NuidWriter.cs b/src/NATS.Client.Core/Internal/NuidWriter.cs
@@ -63,8 +63,10 @@ private static bool TryWriteNuidCore(Span<char> buffer, Span<char> prefix, ulong
         // NOTE: We must never write to digitsPtr!
         ref var digitsPtr = ref MemoryMarshal.GetReference(Digits);
 
-        for (nuint i = PrefixLength; i < NuidLength; i++)
+        // write backwards so the last two characters change the fastest
+        for (var i = NuidLength; i > PrefixLength;)
         {
+            i--;
             var digitIndex = (nuint)(sequential % Base);
             Unsafe.Add(ref buffer[0], i) = Unsafe.Add(ref digitsPtr, digitIndex);
             sequential /= Base;

diff --git a/tests/NATS.Client.Core.Tests/NuidWriterTests.cs b/tests/NATS.Client.Core.Tests/NuidWriterTests.cs
@@ -186,7 +186,6 @@ public void DifferentThreads_DifferentPrefixes()
         ConcurrentQueue<(char[] nuid, int threadId)> nuids = new();
 
         // Act
-        var count = 0;
         var threads = new List<Thread>();
 
         for (var i = 0; i < 10; i++)
@@ -196,11 +195,6 @@ public void DifferentThreads_DifferentPrefixes()
                 var buffer = new char[22];
                 NuidWriter.TryWriteNuid(buffer);
                 nuids.Enqueue((buffer, Environment.CurrentManagedThreadId));
-
-                Interlocked.Increment(ref count);
-
-                // Avoid exiting the thread so the ids won't clash.
-                SpinWait.SpinUntil(() => Volatile.Read(ref count) == 10);
             });
             t.Start();
             threads.Add(t);
@@ -223,7 +217,7 @@ public void DifferentThreads_DifferentPrefixes()
         Assert.Equal(10, uniqueThreadIds.Count);
     }
 
-    [Fact(Skip = "long running")]
+    [Fact]
     public void AllNuidsAreUnique()
     {
         const int count = 1_000 * 1_000 * 10;
@@ -249,7 +243,7 @@ public void AllNuidsAreUnique()
         }
     }
 
-    [Fact(Skip = "long running")]
+    [Fact]
     public void AllNuidsAreUnique_SmallSequentials()
     {
         var writeFailed = false;
@@ -292,7 +286,7 @@ public void AllNuidsAreUnique_SmallSequentials()
         Assert.Equal(string.Empty, duplicateFailure);
     }
 
-    [Fact(Skip = "long running")]
+    [Fact]
     public void AllNuidsAreUnique_ZeroSequential()
     {
         var writeFailed = false;
@@ -332,6 +326,22 @@ public void AllNuidsAreUnique_ZeroSequential()
         Assert.Equal(string.Empty, duplicateFailure);
     }
 
+    [Fact]
+    public void Only_last_two_digits_change()
+    {
+        var nuid1 = NuidWriter.NewNuid();
+        var head1 = nuid1.Substring(0, 20);
+        var tail1 = nuid1.Substring(20, 2);
+
+        var nuid2 = NuidWriter.NewNuid();
+        var head2 = nuid2.Substring(0, 20);
+        var tail2 = nuid2.Substring(20, 2);
+
+        Assert.NotEqual(nuid1, nuid2);
+        Assert.Equal(head1, head2);
+        Assert.NotEqual(tail1, tail2);
+    }
+
     // This messes with NuidWriter's internal state and must be used
     // on separate threads (distinct NuidWriter instances) only.
     private static void SetSequentialAndIncrement(ulong sequential, ulong increment)

diff --git a/tests/NATS.Client.Services.Tests/ServicesTests.cs b/tests/NATS.Client.Services.Tests/ServicesTests.cs
@@ -350,4 +350,37 @@ await s1.AddEndpointAsync<int>(
         var formatRegex = new Regex(@"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{7}Z$");
         Assert.Matches(formatRegex, stats.Started);
     }
+
+    [Fact]
+    public async Task Service_ids_unique()
+    {
+        await using var server = NatsServer.Start();
+        await using var nats = server.CreateClientConnection();
+        var svc = new NatsSvcContext(nats);
+
+        var cts = new CancellationTokenSource(TimeSpan.FromSeconds(60));
+        var cancellationToken = cts.Token;
+
+        List<string> ids = new();
+        for (var i = 0; i < 100; i++)
+        {
+            await using var s = await svc.AddServiceAsync($"s{i}", "1.0.0", cancellationToken: cancellationToken);
+            ids.Add(s.GetInfo().Id);
+        }
+
+        ids.Sort();
+
+        HashSet<string> uniqueIds = new();
+        foreach (var id in ids)
+        {
+            if (!uniqueIds.Add(id))
+            {
+                _output.WriteLine($"Duplicate ID: {id}");
+            }
+
+            _output.WriteLine($"{id.Substring(0, 12)} {id.Substring(12, 10)}");
+        }
+
+        Assert.Equal(ids.Count, uniqueIds.Count);
+    }
 }