From 9d8298ed5df092cd37a24e233fd5e4554bef34e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 16:25:23 +0000 Subject: [PATCH 01/10] Initial plan From 14f20bbd3398e6d17efd306aeaebf3158c0438d0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 16:47:16 +0000 Subject: [PATCH 02/10] Optimize write throughput: release indexLock before 16KiB data copy BlockCache.WriteAsync used to hold the global indexLock semaphore for the entire write, including the 16KiB memcpy into the backing store. The read path already released the lock before its data copy; this brings writes in line. New locking sequence for writes: 1. Hold indexLock: eviction, UpdateIndex (dictionary only), MarkDirtyAsync, acquire block write lock (while still under indexLock to prevent races). 2. Release indexLock. 3. CommitWrite under block lock only: persisted index entry + 16KiB copy. Supporting changes: - BlockIndex.UpdateDictionary: updates in-memory positions dictionary only. - BlockIndex.CommitEntry: writes the persisted index entry only. - BlockStorage.UpdateIndex/CommitWrite: thin wrappers exposing the split. - BlockStorage.MarkDirtyAsync: promoted private -> internal. Co-authored-by: lostmsu <239520+lostmsu@users.noreply.github.com> --- src/BlockCache.cs | 44 ++++++++++++++++++++++++++++++++++---------- src/BlockIndex.cs | 27 +++++++++++++++++++++++++++ src/BlockStorage.cs | 18 +++++++++++++++++- 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/BlockCache.cs b/src/BlockCache.cs index fc3d819..4f5dfa6 100644 --- a/src/BlockCache.cs +++ b/src/BlockCache.cs @@ -46,8 +46,10 @@ public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemory WriteAsync(ContentHash hash, ReadOnlyMemoryindex dictionary under the index lock so + // concurrent readers and writers see a consistent view immediately. + this.storage.UpdateIndex(index, newHash: hash, oldHash: evicted); + await this.storage.MarkDirtyAsync().ConfigureAwait(false); + } catch { + await blockWriteLock.DisposeAsync().ConfigureAwait(false); + blockWriteLock = default; + throw; + } } else { #if DEBUG index = this.storage.BlockIndex(hash); @@ -84,12 +94,26 @@ await this.storage.WriteAsync(index, content, hash, CancellationToken.None) return TimeSpan.Zero; #endif } - - this.Available?.Invoke(this, hash, content.Span); - return start.Elapsed; } finally { + // Release the index lock before writing block data so other reads and writes + // can proceed concurrently while the (potentially large) copy is in progress. this.indexLock.Release(); } + + // Write the persisted index entry and block data under only the block write lock. + // The index lock has already been released, allowing other operations to proceed. + // index is always valid here: the early-return branches never reach this point. + Debug.Assert(index >= 0); + await using (blockWriteLock) { + this.storage.CommitWrite(index, content, hash); + } + + this.log.Log( + perfLevel, "Set content[{Length}] for {Hash} at {Index} in {Microseconds:F0}us", + content.Length, hash, index, start.Elapsed.TotalMicroseconds); + + this.Available?.Invoke(this, hash, content.Span); + return start.Elapsed; } public async ValueTask ReadAsync(ContentHash hash, long offset, Memory buffer, diff --git a/src/BlockIndex.cs b/src/BlockIndex.cs index f15894e..1db34fc 100644 --- a/src/BlockIndex.cs +++ b/src/BlockIndex.cs @@ -78,6 +78,33 @@ public bool TrySet(int index, Entry value) { return true; } + /// + /// Updates the in-memory hash→index dictionary without touching the persisted entry. + /// Must be called under the index lock. + /// + internal void UpdateDictionary(int blockIndex, ContentHash newHash, ContentHash oldHash) { + if (blockIndex < 0 || blockIndex >= this.BlockCount) + throw new ArgumentOutOfRangeException(nameof(blockIndex)); + + if (!this.positions.TryAdd(newHash, blockIndex)) + throw new InvalidOperationException( + $"Internal error: hash {newHash} is already present in the index"); + if (!this.positions.Remove(oldHash)) + throw new InvalidOperationException( + $"Internal error: evicted hash {oldHash} was not found in the index"); + } + + /// + /// Writes the persisted index entry without touching the in-memory dictionary. + /// Must be called under the block lock (not the index lock). + /// + internal void CommitEntry(int blockIndex, Entry value) { + if (blockIndex < 0 || blockIndex >= this.BlockCount) + throw new ArgumentOutOfRangeException(nameof(blockIndex)); + + this.SetUnchecked(blockIndex, value); + } + void SetUnchecked(int index, Entry value) { var span = MemoryMarshal.CreateSpan(ref value, 1); var bytes = MemoryMarshal.Cast(span); diff --git a/src/BlockStorage.cs b/src/BlockStorage.cs index a2d9e97..04b551a 100644 --- a/src/BlockStorage.cs +++ b/src/BlockStorage.cs @@ -116,7 +116,7 @@ public async ValueTask FlushAsync(CancellationToken cancel = default) { blocksTime, indexTime); } - async ValueTask MarkDirtyAsync() { + internal async ValueTask MarkDirtyAsync() { if (this.dirty) return; @@ -125,6 +125,22 @@ async ValueTask MarkDirtyAsync() { this.dirty = true; } + /// + /// Updates only the in-memory hash→index dictionary for the given block. + /// Must be called under the index lock, with the block write lock already held. + /// + internal void UpdateIndex(int blockIndex, ContentHash newHash, ContentHash oldHash) + => this.index.UpdateDictionary(blockIndex, newHash, oldHash); + + /// + /// Writes the persisted index entry and block data. + /// Must be called under the block write lock (not the index lock). + /// + internal void CommitWrite(int blockIndex, ReadOnlyMemory block, ContentHash hash) { + this.index.CommitEntry(blockIndex, new(hash, block.Length)); + this.writer.Write(block.Span, blockIndex, offset: 0); + } + public static async Task CreateAsync(string indexPath, string blocksPath, int blockSize, ILogger log, From 48e0a41640a8db1b401639d7937b74e289500ce4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 17:36:17 +0000 Subject: [PATCH 03/10] Add before/after write throughput comparison test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StressTest.MeasureWriteThroughputAsync drives N concurrent write-only tasks against any IContentCache for the given duration, returning bytes/second. ThroughputTests.WriteIsFasterWithIndexLockReleasedBeforeDataCopy runs two 10-second scenarios and asserts the optimized BlockCache is faster: Before (GlobalWriteSerializingCache — global SemaphoreSlim(1) for writes, simulating old indexLock-held-during-copy behaviour): ~725 MiB/s After (optimized BlockCache, indexLock released before data copy): ~1.36 GiB/s GlobalWriteSerializingCache implements IDisposable so the SemaphoreSlim is disposed properly. Co-authored-by: lostmsu <239520+lostmsu@users.noreply.github.com> Agent-Logs-Url: https://github.com/BorgGames/Hash/sessions/5ec2bd17-f616-46ca-a50d-5d637e6b2c39 --- perf/StressTest.cs | 33 ++++++++++++++++ test/ThroughputTests.cs | 84 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 test/ThroughputTests.cs diff --git a/perf/StressTest.cs b/perf/StressTest.cs index 627ad74..55ed1cb 100644 --- a/perf/StressTest.cs +++ b/perf/StressTest.cs @@ -46,6 +46,39 @@ public static async Task RunAsync(TimeSpan duration, ILogger log) { return bytesPerSecond; } + /// + /// Measures write-only throughput for the given cache over the specified + /// using * 2 + /// concurrent writer tasks. Useful for isolating write-path contention. + /// + public static async Task MeasureWriteThroughputAsync(IContentCache cache, + TimeSpan duration) { + var timeIsUp = duration.ToCancellation(); + var tasks = new List>(); + var stopwatch = StopwatchTimestamp.Now; + for (int i = 0; i < Environment.ProcessorCount * 2; i++) + tasks.Add(WriteLoopAsync(cache, timeIsUp)); + + long[] bytes = await Task.WhenAll(tasks); + return (ulong)(bytes.Sum() / stopwatch.Elapsed.TotalSeconds); + } + + static async Task WriteLoopAsync(IContentCache cache, CancellationToken cancel) { + byte[] data = new byte[cache.MaxBlockSize]; + var random = new Random(); + long transmitted = 0; + try { + while (!cancel.IsCancellationRequested) { + random.NextBytes(data); + var hash = ContentHash.Compute(data); + await cache.WriteAsync(hash, data, cancel).ConfigureAwait(false); + transmitted += data.Length; + } + } catch (OperationCanceledException) when (cancel.IsCancellationRequested) { } + + return transmitted; + } + static async Task AbuseAsync(IContentCache cache, CancellationToken cancel) { byte[] data = new byte[cache.MaxBlockSize]; var random = new Random(); diff --git a/test/ThroughputTests.cs b/test/ThroughputTests.cs new file mode 100644 index 0000000..f40339f --- /dev/null +++ b/test/ThroughputTests.cs @@ -0,0 +1,84 @@ +namespace Hash; + +using Borg; + +using Microsoft.Extensions.Logging.Abstractions; + +/// +/// Compares write throughput before and after the optimization that releases +/// indexLock before the 16 KiB data copy in . +/// +public class ThroughputTests(ITestOutputHelper output) { + // Small in-memory cache — fills quickly so every write causes an eviction, + // maximising write-path contention. + const int BlockCount = 256; + + [Fact] + public async Task WriteIsFasterWithIndexLockReleasedBeforeDataCopy() { + var duration = TimeSpan.FromSeconds(10); + var log = NullLogger.Instance; + + // "Before": simulate old behaviour where indexLock was held for the full + // write duration (including data copy) by wrapping with a global write gate. + var innerBefore = new BlockCache(BlockCache.DEFAULT_BLOCK_SIZE, BlockCount, log); + await using var _before = innerBefore; + using var serialized = new GlobalWriteSerializingCache(innerBefore); + ulong before = await StressTest.MeasureWriteThroughputAsync(serialized, duration); + + // "After": optimized BlockCache releases indexLock before the data copy, + // allowing concurrent writes to different blocks. + var afterCache = new BlockCache(BlockCache.DEFAULT_BLOCK_SIZE, BlockCount, log); + await using var _after = afterCache; + ulong after = await StressTest.MeasureWriteThroughputAsync(afterCache, duration); + + output.WriteLine( + $"Before (serialized writes): {HumanReadable.Bytes(before)}/s"); + output.WriteLine( + $"After (parallel writes): {HumanReadable.Bytes(after)}/s"); + + Assert.True(after > before, + $"Optimized BlockCache ({HumanReadable.Bytes(after)}/s) should be faster " + + $"than globally-serialized writes ({HumanReadable.Bytes(before)}/s)"); + } +} + +/// +/// Wraps an and holds a global +/// for the full duration of every write, including the data-copy phase. +/// This reproduces the pre-optimization behaviour where indexLock was not +/// released until the entire write (dictionary update + 16 KiB memcpy) completed. +/// +sealed class GlobalWriteSerializingCache(IBlockCache inner) : IBlockCache, IDisposable { + readonly SemaphoreSlim writeGate = new(1); + + public event Action? Evicted { + add => inner.Evicted += value; + remove => inner.Evicted -= value; + } + + public event IBlockCache.AvailableHandler? Available { + add => inner.Available += value; + remove => inner.Available -= value; + } + + public long MaxBlockSize => inner.MaxBlockSize; + + public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemory content, + CancellationToken cancel = default) { + await writeGate.WaitAsync(cancel).ConfigureAwait(false); + try { + return await inner.WriteAsync(hash, content, cancel).ConfigureAwait(false); + } finally { + writeGate.Release(); + } + } + + public ValueTask ReadAsync(ContentHash hash, long offset, Memory buffer, + CancellationToken cancel = default) + => inner.ReadAsync(hash, offset, buffer, cancel); + + public ValueTask FlushAsync(CancellationToken cancel = default) + => inner.FlushAsync(cancel); + + public void Dispose() => writeGate.Dispose(); +} From 0800bec4aef4203b6e8176c7f85c4cc18941ca06 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 17:45:19 +0000 Subject: [PATCH 04/10] Add direct BlockCache correctness stress test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StressTest.RunCorrectnessAsync drives ProcessorCount*2 concurrent read/write/ hash-verify tasks against any IContentCache for the given duration, throwing HashMismatchException immediately if any data corruption is detected. CorrectnessTests.ConcurrentReadWriteProducesNoDataCorruption exercises an in-memory BlockCache (64 blocks, forcing constant evictions) directly — no TCP layer. This directly answers "are there correctness stress-tests?" and validates the locking changes produce no data corruption. Also address code-review feedback from the previous commit: - BlockCache: remove redundant `blockWriteLock = default` after disposal in the catch path (exception is re-thrown immediately, variable unused) - BlockIndex.UpdateDictionary: include blockIndex in both error messages to aid debugging of concurrency faults Co-authored-by: lostmsu <239520+lostmsu@users.noreply.github.com> Agent-Logs-Url: https://github.com/BorgGames/Hash/sessions/0930c8ef-24d0-4fbb-8238-b8802f739a5b --- perf/StressTest.cs | 16 ++++++++++++++++ src/BlockCache.cs | 1 - src/BlockIndex.cs | 6 ++++-- test/CorrectnessTests.cs | 26 ++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 test/CorrectnessTests.cs diff --git a/perf/StressTest.cs b/perf/StressTest.cs index 55ed1cb..ba06bd9 100644 --- a/perf/StressTest.cs +++ b/perf/StressTest.cs @@ -46,6 +46,22 @@ public static async Task RunAsync(TimeSpan duration, ILogger log) { return bytesPerSecond; } + /// + /// Runs a correctness stress test against for the specified + /// using * 2 + /// concurrent read/write tasks. Each task writes random blocks and reads them back, + /// recomputing the hash to verify data integrity. Throws + /// on corruption. + /// + public static async Task RunCorrectnessAsync(IContentCache cache, TimeSpan duration) { + var timeIsUp = duration.ToCancellation(); + var tasks = new List(); + for (int i = 0; i < Environment.ProcessorCount * 2; i++) + tasks.Add(AbuseAsync(cache, timeIsUp)); + + await Task.WhenAll(tasks); + } + /// /// Measures write-only throughput for the given cache over the specified /// using * 2 diff --git a/src/BlockCache.cs b/src/BlockCache.cs index 4f5dfa6..aad3fac 100644 --- a/src/BlockCache.cs +++ b/src/BlockCache.cs @@ -75,7 +75,6 @@ public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemory diff --git a/test/CorrectnessTests.cs b/test/CorrectnessTests.cs new file mode 100644 index 0000000..add90b9 --- /dev/null +++ b/test/CorrectnessTests.cs @@ -0,0 +1,26 @@ +namespace Hash; + +using Microsoft.Extensions.Logging.Abstractions; + +/// +/// Correctness stress tests for . +/// Verifies that concurrent reads and writes never produce data corruption, +/// exercising the locking logic directly without a TCP layer. +/// +public class CorrectnessTests { + // Small block count so evictions happen frequently — maximises the chance of + // exposing races between eviction, index update, and the data copy. + const int BlockCount = 64; + + [Fact] + public async Task ConcurrentReadWriteProducesNoDataCorruption() { + var cache = new BlockCache(BlockCache.DEFAULT_BLOCK_SIZE, BlockCount, + NullLogger.Instance); + await using var _ = cache; + + // RunCorrectnessAsync drives ProcessorCount*2 concurrent tasks, each writing + // random blocks and reading them back, recomputing the hash to detect corruption. + // A HashMismatchException is thrown (and propagated) if any mismatch is found. + await StressTest.RunCorrectnessAsync(cache, TimeSpan.FromSeconds(15)); + } +} From e4b49046e97127ca4e163899b79febb3d5d08b71 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 18:03:35 +0000 Subject: [PATCH 05/10] Polish before review: fix unused exception var, add hoisting comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - StressTest.AbuseAsync: remove unused exception variable e from catch (OperationCanceledException e) — fixes CS0168 warning - BlockCache.WriteAsync: add comment explaining why index and blockWriteLock are hoisted before the try block (both are needed after the finally/indexLock.Release continuation) Co-authored-by: lostmsu <239520+lostmsu@users.noreply.github.com> Agent-Logs-Url: https://github.com/BorgGames/Hash/sessions/39ffc0ea-d8a6-48b7-b728-ec388d6592af --- perf/StressTest.cs | 2 +- src/BlockCache.cs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/perf/StressTest.cs b/perf/StressTest.cs index ba06bd9..1bb0530 100644 --- a/perf/StressTest.cs +++ b/perf/StressTest.cs @@ -133,7 +133,7 @@ static async Task AbuseAsync(IContentCache cache, CancellationToken cancel throw new HashMismatchException(); transmitted += read; } - } catch (OperationCanceledException e) { + } catch (OperationCanceledException) { var opTime = accessStart.Elapsed; if (!cancel.IsCancellationRequested) { await Console.Error.WriteLineAsync($"last access: {opTime.TotalMilliseconds:N0}ms"); diff --git a/src/BlockCache.cs b/src/BlockCache.cs index aad3fac..0220b36 100644 --- a/src/BlockCache.cs +++ b/src/BlockCache.cs @@ -47,6 +47,9 @@ public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemory Date: Fri, 20 Mar 2026 18:10:21 +0000 Subject: [PATCH 06/10] Remove throughput comparison test and old code path - Delete test/ThroughputTests.cs: removes the before/after perf comparison test (added for discussion only) and GlobalWriteSerializingCache, which simulated the old code path that held indexLock through the data copy - Remove StressTest.MeasureWriteThroughputAsync and WriteLoopAsync: both were only used by ThroughputTests - Simplify BlockIndex.UpdateDictionary error messages to single-line interpolated strings (code review feedback) CI already runs `dotnet test` with no filter, so CorrectnessTests runs on every push/PR without any workflow changes. Co-authored-by: lostmsu <239520+lostmsu@users.noreply.github.com> Agent-Logs-Url: https://github.com/BorgGames/Hash/sessions/4bdb24b4-cd77-463e-ad7f-7d1da349ec3b --- perf/StressTest.cs | 33 ---------------- src/BlockIndex.cs | 6 +-- test/ThroughputTests.cs | 84 ----------------------------------------- 3 files changed, 2 insertions(+), 121 deletions(-) delete mode 100644 test/ThroughputTests.cs diff --git a/perf/StressTest.cs b/perf/StressTest.cs index 1bb0530..1da38e8 100644 --- a/perf/StressTest.cs +++ b/perf/StressTest.cs @@ -62,39 +62,6 @@ public static async Task RunCorrectnessAsync(IContentCache cache, TimeSpan durat await Task.WhenAll(tasks); } - /// - /// Measures write-only throughput for the given cache over the specified - /// using * 2 - /// concurrent writer tasks. Useful for isolating write-path contention. - /// - public static async Task MeasureWriteThroughputAsync(IContentCache cache, - TimeSpan duration) { - var timeIsUp = duration.ToCancellation(); - var tasks = new List>(); - var stopwatch = StopwatchTimestamp.Now; - for (int i = 0; i < Environment.ProcessorCount * 2; i++) - tasks.Add(WriteLoopAsync(cache, timeIsUp)); - - long[] bytes = await Task.WhenAll(tasks); - return (ulong)(bytes.Sum() / stopwatch.Elapsed.TotalSeconds); - } - - static async Task WriteLoopAsync(IContentCache cache, CancellationToken cancel) { - byte[] data = new byte[cache.MaxBlockSize]; - var random = new Random(); - long transmitted = 0; - try { - while (!cancel.IsCancellationRequested) { - random.NextBytes(data); - var hash = ContentHash.Compute(data); - await cache.WriteAsync(hash, data, cancel).ConfigureAwait(false); - transmitted += data.Length; - } - } catch (OperationCanceledException) when (cancel.IsCancellationRequested) { } - - return transmitted; - } - static async Task AbuseAsync(IContentCache cache, CancellationToken cancel) { byte[] data = new byte[cache.MaxBlockSize]; var random = new Random(); diff --git a/src/BlockIndex.cs b/src/BlockIndex.cs index 8e75e7c..2b028ce 100644 --- a/src/BlockIndex.cs +++ b/src/BlockIndex.cs @@ -88,12 +88,10 @@ internal void UpdateDictionary(int blockIndex, ContentHash newHash, ContentHash if (!this.positions.TryAdd(newHash, blockIndex)) throw new InvalidOperationException( - $"Internal error: hash {newHash} is already mapped to a block " - + $"(attempted to map to block {blockIndex})"); + $"Internal error: hash {newHash} is already mapped to a block (attempted to map to block {blockIndex})"); if (!this.positions.Remove(oldHash)) throw new InvalidOperationException( - $"Internal error: evicted hash {oldHash} was not found in the index " - + $"(block {blockIndex})"); + $"Internal error: evicted hash {oldHash} was not found in the index (block {blockIndex})"); } /// diff --git a/test/ThroughputTests.cs b/test/ThroughputTests.cs deleted file mode 100644 index f40339f..0000000 --- a/test/ThroughputTests.cs +++ /dev/null @@ -1,84 +0,0 @@ -namespace Hash; - -using Borg; - -using Microsoft.Extensions.Logging.Abstractions; - -/// -/// Compares write throughput before and after the optimization that releases -/// indexLock before the 16 KiB data copy in . -/// -public class ThroughputTests(ITestOutputHelper output) { - // Small in-memory cache — fills quickly so every write causes an eviction, - // maximising write-path contention. - const int BlockCount = 256; - - [Fact] - public async Task WriteIsFasterWithIndexLockReleasedBeforeDataCopy() { - var duration = TimeSpan.FromSeconds(10); - var log = NullLogger.Instance; - - // "Before": simulate old behaviour where indexLock was held for the full - // write duration (including data copy) by wrapping with a global write gate. - var innerBefore = new BlockCache(BlockCache.DEFAULT_BLOCK_SIZE, BlockCount, log); - await using var _before = innerBefore; - using var serialized = new GlobalWriteSerializingCache(innerBefore); - ulong before = await StressTest.MeasureWriteThroughputAsync(serialized, duration); - - // "After": optimized BlockCache releases indexLock before the data copy, - // allowing concurrent writes to different blocks. - var afterCache = new BlockCache(BlockCache.DEFAULT_BLOCK_SIZE, BlockCount, log); - await using var _after = afterCache; - ulong after = await StressTest.MeasureWriteThroughputAsync(afterCache, duration); - - output.WriteLine( - $"Before (serialized writes): {HumanReadable.Bytes(before)}/s"); - output.WriteLine( - $"After (parallel writes): {HumanReadable.Bytes(after)}/s"); - - Assert.True(after > before, - $"Optimized BlockCache ({HumanReadable.Bytes(after)}/s) should be faster " - + $"than globally-serialized writes ({HumanReadable.Bytes(before)}/s)"); - } -} - -/// -/// Wraps an and holds a global -/// for the full duration of every write, including the data-copy phase. -/// This reproduces the pre-optimization behaviour where indexLock was not -/// released until the entire write (dictionary update + 16 KiB memcpy) completed. -/// -sealed class GlobalWriteSerializingCache(IBlockCache inner) : IBlockCache, IDisposable { - readonly SemaphoreSlim writeGate = new(1); - - public event Action? Evicted { - add => inner.Evicted += value; - remove => inner.Evicted -= value; - } - - public event IBlockCache.AvailableHandler? Available { - add => inner.Available += value; - remove => inner.Available -= value; - } - - public long MaxBlockSize => inner.MaxBlockSize; - - public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemory content, - CancellationToken cancel = default) { - await writeGate.WaitAsync(cancel).ConfigureAwait(false); - try { - return await inner.WriteAsync(hash, content, cancel).ConfigureAwait(false); - } finally { - writeGate.Release(); - } - } - - public ValueTask ReadAsync(ContentHash hash, long offset, Memory buffer, - CancellationToken cancel = default) - => inner.ReadAsync(hash, offset, buffer, cancel); - - public ValueTask FlushAsync(CancellationToken cancel = default) - => inner.FlushAsync(cancel); - - public void Dispose() => writeGate.Dispose(); -} From 4903cadc935efde1a0733f53c1e4ac77f44ae746 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 18:41:23 +0000 Subject: [PATCH 07/10] Fix write-before-index-update ordering in BlockCache.WriteAsync The optimization called UpdateIndex (making the new hash visible in the in-memory dictionary) before CommitWrite (writing the actual block data). This violated the invariant: the hash was observable in the index before the corresponding data was committed. Fix: swap the order so CommitWrite runs first, then UpdateIndex, both still under indexLock and blockWriteLock. The hash is now only visible in the dictionary after the data is already written. The blockWriteLock is still needed to prevent a concurrent reader that already holds readLock from reading a block while data is being written. Update docstrings in BlockStorage and BlockIndex to reflect the new calling contract for CommitWrite/UpdateIndex and CommitEntry/UpdateDictionary. Co-authored-by: lostmsu <239520+lostmsu@users.noreply.github.com> Agent-Logs-Url: https://github.com/BorgGames/Hash/sessions/6c77ca38-b6da-482e-9bf0-059168358e19 --- src/BlockCache.cs | 30 +++++++++++++++--------------- src/BlockIndex.cs | 6 ++++-- src/BlockStorage.cs | 7 +++++-- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/BlockCache.cs b/src/BlockCache.cs index 0220b36..ef6a9fb 100644 --- a/src/BlockCache.cs +++ b/src/BlockCache.cs @@ -47,9 +47,9 @@ public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemory WriteAsync(ContentHash hash, ReadOnlyMemoryindex dictionary under the index lock so - // concurrent readers and writers see a consistent view immediately. - this.storage.UpdateIndex(index, newHash: hash, oldHash: evicted); await this.storage.MarkDirtyAsync().ConfigureAwait(false); + // Write the block data and persisted index entry BEFORE updating the + // in-memory dictionary. Readers locate a block via the dictionary and + // then acquire a block-level read lock; keeping the hash out of the + // dictionary until CommitWrite completes means no reader can ever + // observe the hash without the corresponding data already committed. + this.storage.CommitWrite(index, content, hash); + this.storage.UpdateIndex(index, newHash: hash, oldHash: evicted); } catch { await blockWriteLock.DisposeAsync().ConfigureAwait(false); throw; @@ -97,18 +101,14 @@ public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemory= 0); - await using (blockWriteLock) { - this.storage.CommitWrite(index, content, hash); - } + // Release the block write lock after the index lock so that any reader that + // finds the new hash and blocks on readLock sees committed data once unblocked. + await using (blockWriteLock) { } this.log.Log( perfLevel, "Set content[{Length}] for {Hash} at {Index} in {Microseconds:F0}us", diff --git a/src/BlockIndex.cs b/src/BlockIndex.cs index 2b028ce..43fb506 100644 --- a/src/BlockIndex.cs +++ b/src/BlockIndex.cs @@ -80,7 +80,8 @@ public bool TrySet(int index, Entry value) { /// /// Updates the in-memory hash→index dictionary without touching the persisted entry. - /// Must be called under the index lock. + /// Must be called under the index lock and block write lock, after + /// has already written the persisted entry and block data. /// internal void UpdateDictionary(int blockIndex, ContentHash newHash, ContentHash oldHash) { if (blockIndex < 0 || blockIndex >= this.BlockCount) @@ -96,7 +97,8 @@ internal void UpdateDictionary(int blockIndex, ContentHash newHash, ContentHash /// /// Writes the persisted index entry without touching the in-memory dictionary. - /// Must be called under the block lock (not the index lock). + /// Must be called under both the index lock and the block write lock, before + /// so the hash is only visible after data is committed. /// internal void CommitEntry(int blockIndex, Entry value) { if (blockIndex < 0 || blockIndex >= this.BlockCount) diff --git a/src/BlockStorage.cs b/src/BlockStorage.cs index 04b551a..97b48f2 100644 --- a/src/BlockStorage.cs +++ b/src/BlockStorage.cs @@ -127,14 +127,17 @@ internal async ValueTask MarkDirtyAsync() { /// /// Updates only the in-memory hash→index dictionary for the given block. - /// Must be called under the index lock, with the block write lock already held. + /// Must be called under the index lock and block write lock, after + /// has already written the block data. /// internal void UpdateIndex(int blockIndex, ContentHash newHash, ContentHash oldHash) => this.index.UpdateDictionary(blockIndex, newHash, oldHash); /// /// Writes the persisted index entry and block data. - /// Must be called under the block write lock (not the index lock). + /// Must be called under both the index lock and the block write lock, and + /// before so that block data is committed before + /// the hash becomes visible to readers via the dictionary. /// internal void CommitWrite(int blockIndex, ReadOnlyMemory block, ContentHash hash) { this.index.CommitEntry(blockIndex, new(hash, block.Length)); From 668170469569db887ad9afe70a54ddeaaac75617 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 18:57:56 +0000 Subject: [PATCH 08/10] Three-phase write: placeholder persisted index, restore early indexLock release The previous fix incorrectly held indexLock for the entire data write, serialising all operations and defeating the throughput optimisation. The reviewer confirmed the runtime concern ("hash visible before data written") is not a problem because blockWriteLock already prevents any reader from observing incomplete data. The correct improvement is the "invalid hash" approach for the persisted index: first set the persisted index entry to a placeholder, then write block data, then set the persisted index entry to the real hash. Phase 1 (under indexLock + blockWriteLock): - MarkDirtyAsync - CommitPlaceholder: persisted index <- zero-length random-hash sentinel - UpdateIndex: in-memory dict <- remove evicted hash, add new hash Release indexLock Phase 2 (blockWriteLock only): WriteBlockData Phase 3 (blockWriteLock only): CommitFinalEntry (persisted index <- real hash+length) Release blockWriteLock This restores the early-indexLock-release optimisation while ensuring the persisted index never contains a "real" hash for a block whose data has not yet been written, making crash recovery unambiguous. BlockStorage: replace CommitWrite with CommitPlaceholder / WriteBlockData / CommitFinalEntry BlockIndex: update CommitEntry and UpdateDictionary docstrings accordingly Co-authored-by: lostmsu <239520+lostmsu@users.noreply.github.com> Agent-Logs-Url: https://github.com/BorgGames/Hash/sessions/0d9d79ca-c2b7-41f1-a9dc-ae373fa77a34 --- src/BlockCache.cs | 37 ++++++++++++++++++++++++------------- src/BlockIndex.cs | 8 ++++---- src/BlockStorage.cs | 32 +++++++++++++++++++++++--------- 3 files changed, 51 insertions(+), 26 deletions(-) diff --git a/src/BlockCache.cs b/src/BlockCache.cs index ef6a9fb..8b1a95b 100644 --- a/src/BlockCache.cs +++ b/src/BlockCache.cs @@ -48,8 +48,7 @@ public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemory WriteAsync(ContentHash hash, ReadOnlyMemory WriteAsync(ContentHash hash, ReadOnlyMemory= 0); - // Release the block write lock after the index lock so that any reader that - // finds the new hash and blocks on readLock sees committed data once unblocked. - await using (blockWriteLock) { } + + // Phase 2 (block write lock only): write the raw block bytes. + // Phase 3 (block write lock only): commit the real persisted index entry. + // blockWriteLock is released by the await-using; any reader that found the new + // hash in Phase 1 and is waiting on the block read lock will unblock only after + // both phases complete, guaranteeing it sees fully-written data. + await using (blockWriteLock) { + this.storage.WriteBlockData(index, content); + this.storage.CommitFinalEntry(index, content, hash); + } this.log.Log( perfLevel, "Set content[{Length}] for {Hash} at {Index} in {Microseconds:F0}us", diff --git a/src/BlockIndex.cs b/src/BlockIndex.cs index 43fb506..23051b1 100644 --- a/src/BlockIndex.cs +++ b/src/BlockIndex.cs @@ -81,7 +81,7 @@ public bool TrySet(int index, Entry value) { /// /// Updates the in-memory hash→index dictionary without touching the persisted entry. /// Must be called under the index lock and block write lock, after - /// has already written the persisted entry and block data. + /// has written the placeholder entry. /// internal void UpdateDictionary(int blockIndex, ContentHash newHash, ContentHash oldHash) { if (blockIndex < 0 || blockIndex >= this.BlockCount) @@ -96,9 +96,9 @@ internal void UpdateDictionary(int blockIndex, ContentHash newHash, ContentHash } /// - /// Writes the persisted index entry without touching the in-memory dictionary. - /// Must be called under both the index lock and the block write lock, before - /// so the hash is only visible after data is committed. + /// Writes a persisted index entry without touching the in-memory dictionary. + /// Used both for the placeholder entry (before data write) and the final entry + /// (after data write). Must be called under the block write lock. /// internal void CommitEntry(int blockIndex, Entry value) { if (blockIndex < 0 || blockIndex >= this.BlockCount) diff --git a/src/BlockStorage.cs b/src/BlockStorage.cs index 97b48f2..1a53d75 100644 --- a/src/BlockStorage.cs +++ b/src/BlockStorage.cs @@ -128,21 +128,35 @@ internal async ValueTask MarkDirtyAsync() { /// /// Updates only the in-memory hash→index dictionary for the given block. /// Must be called under the index lock and block write lock, after - /// has already written the block data. + /// and before releasing the index lock. /// internal void UpdateIndex(int blockIndex, ContentHash newHash, ContentHash oldHash) => this.index.UpdateDictionary(blockIndex, newHash, oldHash); /// - /// Writes the persisted index entry and block data. - /// Must be called under both the index lock and the block write lock, and - /// before so that block data is committed before - /// the hash becomes visible to readers via the dictionary. + /// Writes a placeholder entry (zero-length, random hash) to the persisted index, + /// marking the block as "write in progress" for crash recovery. + /// Must be called under both the index lock and the block write lock, + /// before releasing the index lock and before . /// - internal void CommitWrite(int blockIndex, ReadOnlyMemory block, ContentHash hash) { - this.index.CommitEntry(blockIndex, new(hash, block.Length)); - this.writer.Write(block.Span, blockIndex, offset: 0); - } + internal void CommitPlaceholder(int blockIndex) + => this.index.CommitEntry(blockIndex, new(ContentHash.Fake(Random.Shared), 0)); + + /// + /// Writes the raw block bytes. Must be called under the block write lock only, + /// after the index lock has been released, after , + /// and before . + /// + internal void WriteBlockData(int blockIndex, ReadOnlyMemory block) + => this.writer.Write(block.Span, blockIndex, offset: 0); + + /// + /// Writes the real persisted index entry (correct hash and byte count) after + /// the block data has been fully written. + /// Must be called under the block write lock only, after . + /// + internal void CommitFinalEntry(int blockIndex, ReadOnlyMemory block, ContentHash hash) + => this.index.CommitEntry(blockIndex, new(hash, block.Length)); public static async Task CreateAsync(string indexPath, string blocksPath, int blockSize, From 335c0646fd7ef7f666ce0781c0d14c57dbd25956 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 22:00:47 +0000 Subject: [PATCH 09/10] Revert to cfb1e15: indexLock intersection is sufficient; no special index ordering needed The two previous commits added unnecessary complexity to address the concern "hash is findable before data is committed". Re-reasoning from cfb1e15: The indexLock intersection with block lock acquisitions is sufficient: - Writer: acquires blockWriteLock WHILE holding indexLock, updates in-memory dict, releases indexLock, then runs CommitWrite under blockWriteLock only. - Reader: acquires indexLock, finds hash->blockIndex, acquires blockReadLock WHILE holding indexLock, then releases indexLock and reads data. - Any reader that finds the new hash must acquire blockReadLock, which blocks until blockWriteLock is released (after CommitWrite). Readers always see committed data. Crash safety: MarkDirtyAsync fires before CommitWrite, so the dirty bit is always set before any block data changes in persisted storage. A crash anywhere in CommitWrite always triggers a full index rebuild on restart. No special index-update ordering is needed. Changes vs cfb1e15: - Expand the comment on blockWriteLock acquisition to explain the invariant explicitly - Add a comment explaining CommitWrite safety at the await-using site - Add doc comments to BlockStorage.UpdateIndex, CommitWrite, MarkDirtyAsync - Tighten BlockIndex.UpdateDictionary doc comment Co-authored-by: lostmsu <239520+lostmsu@users.noreply.github.com> Agent-Logs-Url: https://github.com/BorgGames/Hash/sessions/75d86e09-4608-4350-a920-0933ee4ecd03 --- src/BlockCache.cs | 42 ++++++++++++++++++++---------------------- src/BlockIndex.cs | 8 +++----- src/BlockStorage.cs | 42 ++++++++++++++++++------------------------ 3 files changed, 41 insertions(+), 51 deletions(-) diff --git a/src/BlockCache.cs b/src/BlockCache.cs index 8b1a95b..ce74d8e 100644 --- a/src/BlockCache.cs +++ b/src/BlockCache.cs @@ -47,8 +47,9 @@ public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemory WriteAsync(ContentHash hash, ReadOnlyMemoryindex dictionary under the index lock so + // concurrent readers and writers see a consistent view immediately. this.storage.UpdateIndex(index, newHash: hash, oldHash: evicted); + // Mark dirty before CommitWrite so that a crash between here and the + // data write below always triggers a full index rebuild on restart. + await this.storage.MarkDirtyAsync().ConfigureAwait(false); } catch { await blockWriteLock.DisposeAsync().ConfigureAwait(false); throw; @@ -108,17 +108,15 @@ public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemory= 0); - - // Phase 2 (block write lock only): write the raw block bytes. - // Phase 3 (block write lock only): commit the real persisted index entry. - // blockWriteLock is released by the await-using; any reader that found the new - // hash in Phase 1 and is waiting on the block read lock will unblock only after - // both phases complete, guaranteeing it sees fully-written data. await using (blockWriteLock) { - this.storage.WriteBlockData(index, content); - this.storage.CommitFinalEntry(index, content, hash); + this.storage.CommitWrite(index, content, hash); } this.log.Log( diff --git a/src/BlockIndex.cs b/src/BlockIndex.cs index 23051b1..d074fad 100644 --- a/src/BlockIndex.cs +++ b/src/BlockIndex.cs @@ -80,8 +80,7 @@ public bool TrySet(int index, Entry value) { /// /// Updates the in-memory hash→index dictionary without touching the persisted entry. - /// Must be called under the index lock and block write lock, after - /// has written the placeholder entry. + /// Must be called under both the index lock and the block write lock. /// internal void UpdateDictionary(int blockIndex, ContentHash newHash, ContentHash oldHash) { if (blockIndex < 0 || blockIndex >= this.BlockCount) @@ -96,9 +95,8 @@ internal void UpdateDictionary(int blockIndex, ContentHash newHash, ContentHash } /// - /// Writes a persisted index entry without touching the in-memory dictionary. - /// Used both for the placeholder entry (before data write) and the final entry - /// (after data write). Must be called under the block write lock. + /// Writes the persisted index entry without touching the in-memory dictionary. + /// Must be called under the block lock (not the index lock). /// internal void CommitEntry(int blockIndex, Entry value) { if (blockIndex < 0 || blockIndex >= this.BlockCount) diff --git a/src/BlockStorage.cs b/src/BlockStorage.cs index 1a53d75..f548209 100644 --- a/src/BlockStorage.cs +++ b/src/BlockStorage.cs @@ -116,6 +116,13 @@ public async ValueTask FlushAsync(CancellationToken cancel = default) { blocksTime, indexTime); } + /// + /// Persists a "dirty" state tag so that any subsequent crash triggers a full index + /// rebuild on restart. Must be called before to guarantee + /// the invariant: if a crash occurs between the two writes inside + /// (persisted index entry vs block data), the dirty bit is already set and recovery will + /// re-hash all blocks from their actual data. + /// internal async ValueTask MarkDirtyAsync() { if (this.dirty) return; @@ -127,36 +134,23 @@ internal async ValueTask MarkDirtyAsync() { /// /// Updates only the in-memory hash→index dictionary for the given block. - /// Must be called under the index lock and block write lock, after - /// and before releasing the index lock. + /// Must be called under both the index lock and the block write lock so that any + /// concurrent reader that observes the new hash is guaranteed to block on the block + /// read lock until has finished writing the data. /// internal void UpdateIndex(int blockIndex, ContentHash newHash, ContentHash oldHash) => this.index.UpdateDictionary(blockIndex, newHash, oldHash); /// - /// Writes a placeholder entry (zero-length, random hash) to the persisted index, - /// marking the block as "write in progress" for crash recovery. - /// Must be called under both the index lock and the block write lock, - /// before releasing the index lock and before . - /// - internal void CommitPlaceholder(int blockIndex) - => this.index.CommitEntry(blockIndex, new(ContentHash.Fake(Random.Shared), 0)); - - /// - /// Writes the raw block bytes. Must be called under the block write lock only, - /// after the index lock has been released, after , - /// and before . + /// Writes the persisted index entry and block data. + /// Must be called under the block write lock (not the index lock). + /// must be called before this method so that a crash + /// between the two writes always triggers a full index rebuild on restart. /// - internal void WriteBlockData(int blockIndex, ReadOnlyMemory block) - => this.writer.Write(block.Span, blockIndex, offset: 0); - - /// - /// Writes the real persisted index entry (correct hash and byte count) after - /// the block data has been fully written. - /// Must be called under the block write lock only, after . - /// - internal void CommitFinalEntry(int blockIndex, ReadOnlyMemory block, ContentHash hash) - => this.index.CommitEntry(blockIndex, new(hash, block.Length)); + internal void CommitWrite(int blockIndex, ReadOnlyMemory block, ContentHash hash) { + this.index.CommitEntry(blockIndex, new(hash, block.Length)); + this.writer.Write(block.Span, blockIndex, offset: 0); + } public static async Task CreateAsync(string indexPath, string blocksPath, int blockSize, From a589ab6a4b09827c93ef04ddbe1a99287a15ffaa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Mar 2026 11:23:22 +0000 Subject: [PATCH 10/10] Remove single-use wrapper methods UpdateDictionary and CommitEntry from BlockIndex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BlockIndex.UpdateDictionary was only ever called from BlockStorage.UpdateIndex — a single-use wrapper with no added value. Removed; its TryAdd/Remove logic is now inlined directly into BlockStorage.UpdateIndex. BlockIndex.positions made internal to allow this. BlockIndex.CommitEntry was only ever called from BlockStorage.CommitWrite — another single-use wrapper. Removed; BlockStorage.CommitWrite now calls BlockIndex.SetUnchecked directly (made internal). Also trim the verbose doc comments added in the previous session back to their original concise form (matching cfb1e15). Net: -2 methods, -~25 lines, no behaviour change. Tests: 2/2 pass. CodeQL: 0 alerts. Co-authored-by: lostmsu <239520+lostmsu@users.noreply.github.com> Agent-Logs-Url: https://github.com/BorgGames/Hash/sessions/c7e89350-1e1b-4841-a92f-df3794515c95 --- src/BlockCache.cs | 15 +++------------ src/BlockIndex.cs | 31 ++----------------------------- src/BlockStorage.cs | 25 ++++++++++--------------- 3 files changed, 15 insertions(+), 56 deletions(-) diff --git a/src/BlockCache.cs b/src/BlockCache.cs index ce74d8e..0220b36 100644 --- a/src/BlockCache.cs +++ b/src/BlockCache.cs @@ -68,19 +68,13 @@ public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemoryindex dictionary under the index lock so // concurrent readers and writers see a consistent view immediately. this.storage.UpdateIndex(index, newHash: hash, oldHash: evicted); - // Mark dirty before CommitWrite so that a crash between here and the - // data write below always triggers a full index rebuild on restart. await this.storage.MarkDirtyAsync().ConfigureAwait(false); } catch { await blockWriteLock.DisposeAsync().ConfigureAwait(false); @@ -109,10 +103,7 @@ public async ValueTask WriteAsync(ContentHash hash, ReadOnlyMemory= 0); await using (blockWriteLock) { diff --git a/src/BlockIndex.cs b/src/BlockIndex.cs index d074fad..3587327 100644 --- a/src/BlockIndex.cs +++ b/src/BlockIndex.cs @@ -4,7 +4,7 @@ namespace Hash; using System.Runtime.InteropServices; sealed class BlockIndex: IAsyncDisposable { - readonly Dictionary positions = new(); + internal readonly Dictionary positions = new(); readonly IBlockReader reader; readonly IBlockWriter writer; @@ -78,34 +78,7 @@ public bool TrySet(int index, Entry value) { return true; } - /// - /// Updates the in-memory hash→index dictionary without touching the persisted entry. - /// Must be called under both the index lock and the block write lock. - /// - internal void UpdateDictionary(int blockIndex, ContentHash newHash, ContentHash oldHash) { - if (blockIndex < 0 || blockIndex >= this.BlockCount) - throw new ArgumentOutOfRangeException(nameof(blockIndex)); - - if (!this.positions.TryAdd(newHash, blockIndex)) - throw new InvalidOperationException( - $"Internal error: hash {newHash} is already mapped to a block (attempted to map to block {blockIndex})"); - if (!this.positions.Remove(oldHash)) - throw new InvalidOperationException( - $"Internal error: evicted hash {oldHash} was not found in the index (block {blockIndex})"); - } - - /// - /// Writes the persisted index entry without touching the in-memory dictionary. - /// Must be called under the block lock (not the index lock). - /// - internal void CommitEntry(int blockIndex, Entry value) { - if (blockIndex < 0 || blockIndex >= this.BlockCount) - throw new ArgumentOutOfRangeException(nameof(blockIndex)); - - this.SetUnchecked(blockIndex, value); - } - - void SetUnchecked(int index, Entry value) { + internal void SetUnchecked(int index, Entry value) { var span = MemoryMarshal.CreateSpan(ref value, 1); var bytes = MemoryMarshal.Cast(span); this.writer.Write(bytes, index, offset: 0); diff --git a/src/BlockStorage.cs b/src/BlockStorage.cs index f548209..996047e 100644 --- a/src/BlockStorage.cs +++ b/src/BlockStorage.cs @@ -116,13 +116,6 @@ public async ValueTask FlushAsync(CancellationToken cancel = default) { blocksTime, indexTime); } - /// - /// Persists a "dirty" state tag so that any subsequent crash triggers a full index - /// rebuild on restart. Must be called before to guarantee - /// the invariant: if a crash occurs between the two writes inside - /// (persisted index entry vs block data), the dirty bit is already set and recovery will - /// re-hash all blocks from their actual data. - /// internal async ValueTask MarkDirtyAsync() { if (this.dirty) return; @@ -134,21 +127,23 @@ internal async ValueTask MarkDirtyAsync() { /// /// Updates only the in-memory hash→index dictionary for the given block. - /// Must be called under both the index lock and the block write lock so that any - /// concurrent reader that observes the new hash is guaranteed to block on the block - /// read lock until has finished writing the data. + /// Must be called under the index lock, with the block write lock already held. /// - internal void UpdateIndex(int blockIndex, ContentHash newHash, ContentHash oldHash) - => this.index.UpdateDictionary(blockIndex, newHash, oldHash); + internal void UpdateIndex(int blockIndex, ContentHash newHash, ContentHash oldHash) { + if (!this.index.positions.TryAdd(newHash, blockIndex)) + throw new InvalidOperationException( + $"Internal error: hash {newHash} is already mapped to a block (attempted to map to block {blockIndex})"); + if (!this.index.positions.Remove(oldHash)) + throw new InvalidOperationException( + $"Internal error: evicted hash {oldHash} was not found in the index (block {blockIndex})"); + } /// /// Writes the persisted index entry and block data. /// Must be called under the block write lock (not the index lock). - /// must be called before this method so that a crash - /// between the two writes always triggers a full index rebuild on restart. /// internal void CommitWrite(int blockIndex, ReadOnlyMemory block, ContentHash hash) { - this.index.CommitEntry(blockIndex, new(hash, block.Length)); + this.index.SetUnchecked(blockIndex, new(hash, block.Length)); this.writer.Write(block.Span, blockIndex, offset: 0); }