From 0e467fbb9609d19d515cfd140f4d93b043fc5ada Mon Sep 17 00:00:00 2001 From: Akshay Bheda Date: Thu, 2 Jul 2026 09:40:40 -0400 Subject: [PATCH 1/2] fix: condition counter writes on the read-time ETag to prevent duplicate ids TryOptimisticWrite conditioned its upload on the blob ETag fetched at write time (GetProperties), not on the ETag of the value GetData returned. A competing writer that committed between the read and the write therefore went undetected: its commit changed the ETag, the late writer fetched that new ETag, and the If-Match condition passed. Both writers succeeded and both processes handed out the same id range. GetData(Async) now uses DownloadContent, which returns the content and its ETag from a single response, and records that read-time ETag per block. TryOptimisticWrite(Async) consumes it for the If-Match condition, turning the read->write cycle into a correct compare-and-swap: an intervening commit fails the write with 412 and UniqueIdGenerator re-reads and retries. Writers without a preceding read keep the previous current-ETag behaviour. Adds an Azurite integration test with four independent generators allocating concurrently from one scope (BatchSize 1), which reproduces the duplicate ids on the previous implementation. --- AutoNumber/BlobOptimisticDataStore.cs | 44 +++++++++++++++++++-------- IntegrationTests/Azure.cs | 32 +++++++++++++++++++ 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/AutoNumber/BlobOptimisticDataStore.cs b/AutoNumber/BlobOptimisticDataStore.cs index f0e5f1e..0b7f9bf 100644 --- a/AutoNumber/BlobOptimisticDataStore.cs +++ b/AutoNumber/BlobOptimisticDataStore.cs @@ -21,6 +21,12 @@ public class BlobOptimisticDataStore : IOptimisticDataStore private readonly ConcurrentDictionary blobReferences; private readonly object blobReferencesLock = new object(); + // ETag observed by the most recent GetData per block, so TryOptimisticWrite can condition + // on the state that was actually read. Conditioning on the blob's current ETag (fetched at + // write time) does not detect writers that committed between our read and our write, which + // allows two processes to hand out the same id range. + private readonly ConcurrentDictionary readETags = new ConcurrentDictionary(); + public BlobOptimisticDataStore(BlobServiceClient blobServiceClient, string containerName) { blobContainer = blobServiceClient.GetBlobContainerClient(containerName.ToLower()); @@ -36,22 +42,20 @@ public string GetData(string blockName) { var blobReference = GetBlobReference(blockName); - using (var stream = new MemoryStream()) - { - blobReference.DownloadTo(stream); - return Encoding.UTF8.GetString(stream.ToArray()); - } + // DownloadContent returns the content and its ETag from a single response, so the + // value/ETag pair is consistent + var download = blobReference.DownloadContent(); + readETags[blockName] = download.Value.Details.ETag; + return download.Value.Content.ToString(); } public async Task GetDataAsync(string blockName) { var blobReference = GetBlobReference(blockName); - using (var stream = new MemoryStream()) - { - await blobReference.DownloadToAsync(stream).ConfigureAwait(false); - return Encoding.UTF8.GetString(stream.ToArray()); - } + var download = await blobReference.DownloadContentAsync().ConfigureAwait(false); + readETags[blockName] = download.Value.Details.ETag; + return download.Value.Content.ToString(); } public async Task InitAsync() @@ -69,11 +73,21 @@ public bool Init() public bool TryOptimisticWrite(string blockName, string data) { var blobReference = GetBlobReference(blockName); + + // Condition the write on the ETag captured by the preceding GetData, so any writer that + // committed after our read fails this write with 412 and the caller re-reads and retries. + // Each read ETag is consumed at most once; callers without a preceding read fall back to + // the blob's current ETag (previous behaviour). + if (!readETags.TryRemove(blockName, out var readETag)) + { + readETag = blobReference.GetProperties().Value.ETag; + } + try { var blobRequestCondition = new BlobRequestConditions { - IfMatch = (blobReference.GetProperties()).Value.ETag + IfMatch = readETag }; UploadText( blobReference, @@ -94,11 +108,17 @@ public bool TryOptimisticWrite(string blockName, string data) public async Task TryOptimisticWriteAsync(string blockName, string data) { var blobReference = GetBlobReference(blockName); + + if (!readETags.TryRemove(blockName, out var readETag)) + { + readETag = (await blobReference.GetPropertiesAsync().ConfigureAwait(false)).Value.ETag; + } + try { var blobRequestCondition = new BlobRequestConditions { - IfMatch = (await blobReference.GetPropertiesAsync()).Value.ETag + IfMatch = readETag }; await UploadTextAsync( blobReference, diff --git a/IntegrationTests/Azure.cs b/IntegrationTests/Azure.cs index f3755ab..f6fc3b0 100644 --- a/IntegrationTests/Azure.cs +++ b/IntegrationTests/Azure.cs @@ -159,6 +159,38 @@ public void ShouldReturnIdsAcrossMultipleGenerators() Assert.Equal(expected, generatedIds); } + [Fact] + public void ShouldNotGenerateDuplicateIdsAcrossGeneratorsUnderContention() + { + // Regression test: TryOptimisticWrite used to condition on the blob's CURRENT ETag + // (fetched at write time) instead of the ETag observed by the preceding GetData, so a + // competing generator that committed between our read and our write went undetected and + // both handed out the same ids. Independent stores/generators simulate separate + // processes; BatchSize = 1 maximises contention on the counter blob. + using var testScope = BuildTestScope(); + const int generatorCount = 4; + const int idsPerGenerator = 100; + + var generators = Enumerable.Range(0, generatorCount) + .Select(_ => new UniqueIdGenerator(BuildStore(testScope)) + { + BatchSize = 1, + MaxWriteAttempts = 100 + }) + .ToArray(); + + var generatedIds = new ConcurrentQueue(); + var scopeName = testScope.IdScopeName; + Parallel.ForEach(generators, generator => + { + for (var i = 0; i < idsPerGenerator; i++) + generatedIds.Enqueue(generator.NextId(scopeName)); + }); + + Assert.Equal(generatorCount * idsPerGenerator, generatedIds.Count); + Assert.Equal(generatedIds.Count, generatedIds.Distinct().Count()); + } + [Fact] public void ShouldSupportUsingOneGeneratorFromMultipleThreads() { From 43643aa300f44db080a6c87b49350168255daf03 Mon Sep 17 00:00:00 2001 From: Akshay Bheda Date: Wed, 8 Jul 2026 15:32:26 -0400 Subject: [PATCH 2/2] docs: document per-instance read->write serialization invariant on BlobOptimisticDataStore The read-time ETag captured by GetData is stored per block name and consumed by the next TryOptimisticWrite. That requires the read->write cycle for a given block to stay serialized per instance (as UniqueIdGenerator does via its per-scope lock). Add a class-level XML doc comment noting the invariant and the silent duplicate-id fallback that occurs if a shared instance interleaves concurrent read+write for the same block, so future direct users of the class stay within it. Doc-only; no behaviour change. Co-Authored-By: Claude Opus 4.8 --- AutoNumber/BlobOptimisticDataStore.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/AutoNumber/BlobOptimisticDataStore.cs b/AutoNumber/BlobOptimisticDataStore.cs index 0b7f9bf..25f0b57 100644 --- a/AutoNumber/BlobOptimisticDataStore.cs +++ b/AutoNumber/BlobOptimisticDataStore.cs @@ -14,6 +14,19 @@ namespace AutoNumber { + /// + /// Optimistic-concurrency counter store backed by Azure block blobs. + /// + /// + /// The read-time ETag captured by / is stored + /// per block name and consumed by the next / + /// for that block, turning the read->write cycle into a + /// compare-and-swap. Callers must therefore keep the read->write cycle for a given block + /// serialized per instance; UniqueIdGenerator does this via its per-scope lock. + /// Interleaving concurrent read+write for the same block on one instance can drop a read-time + /// ETag, causing a silent fallback to the blob's current ETag (fetched at write time), which can + /// hand out duplicate id ranges. + /// public class BlobOptimisticDataStore : IOptimisticDataStore { private const string SeedValue = "1";