From edd4647e8b1ffb0e5a7fbf5374b8de36668b8ba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Sat, 17 Oct 2026 15:58:05 +0200 Subject: [PATCH] fix(storage): drain only the flushed WAL image, atomically, after a checkpoint (#311) The checkpoint drained each flushed entry with TryGetValue + ReferenceEquals followed by a separate TryRemove(key). Commits do not wait for the drain (single-process mode takes no lock; multi-process mode has released _commitLock by then), so an image a commit published between the check and the remove was removed instead of the flushed one. The page file kept the older image and nothing held the newer one: a committed index entry vanished, or an insert read the stale data page back and reused the slot of a committed document, so that document's id returned the next one. This is the HighContention_ManySmallTransactions_ConsistentFinalState failure in CI run 38749678260 (line 244, the "d" assertion). The drain now removes the entry only if it still holds the flushed image, in one step: ICollection.Remove compares the byte[] value by reference under the bucket lock. The test's three field assertions keep their conditions and now name the document that came back, so a future failure says what was wrong. Co-Authored-By: Claude Opus 5.5 --- src/BLite.Core/Storage/StorageEngine.Recovery.cs | 10 +++++++--- tests/BLite.Tests/ConcurrentConsistencyTests.cs | 13 ++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/BLite.Core/Storage/StorageEngine.Recovery.cs b/src/BLite.Core/Storage/StorageEngine.Recovery.cs index e247b421..cecd226b 100644 --- a/src/BLite.Core/Storage/StorageEngine.Recovery.cs +++ b/src/BLite.Core/Storage/StorageEngine.Recovery.cs @@ -141,13 +141,17 @@ private async Task CheckpointCoreAsync(bool commitLockHeld, CancellationToken ct // Drain _walIndex (and _walOffsets) entries that we successfully flushed. // Track the highest checkpointed WAL offset in the same loop (avoids a // second O(n) pass over toCheckpoint). - // ReferenceEquals check ensures we only remove the exact version we flushed. + // Only the exact image we flushed is removed, in one atomic step: commits do not wait + // for this loop, and a check followed by a separate remove could drop an image a commit + // published in between, leaving the older flushed one on disk (the commit is lost, and + // an insert reading the page back reuses its slot) (#311). long maxCheckpointedOffset = 0; foreach (var kvp in toCheckpoint) { - if (_walIndex.TryGetValue(kvp.Key, out var current) && ReferenceEquals(current, kvp.Value)) + // byte[] values compare by reference, so this removes the entry only if it still + // holds the flushed image. + if (((ICollection>)_walIndex).Remove(kvp)) { - _walIndex.TryRemove(kvp.Key, out _); // Track max offset while removing from _walOffsets. if (_walOffsets != null && _walOffsets.TryRemove(kvp.Key, out long removedOffset) && removedOffset > maxCheckpointedOffset) diff --git a/tests/BLite.Tests/ConcurrentConsistencyTests.cs b/tests/BLite.Tests/ConcurrentConsistencyTests.cs index e13cdaf9..cf8d2c24 100644 --- a/tests/BLite.Tests/ConcurrentConsistencyTests.cs +++ b/tests/BLite.Tests/ConcurrentConsistencyTests.cs @@ -239,9 +239,16 @@ public async Task HighContention_ManySmallTransactions_ConsistentFinalState() { var found = await readCol.FindByIdAsync(id); Assert.NotNull(found); - Assert.True(found.TryGetInt32("t", out var ft) && ft == thread); - Assert.True(found.TryGetInt32("tx", out var ftx) && ftx == txn); - Assert.True(found.TryGetInt32("d", out var fd) && fd == doc); + // Name the document that came back, so a failure says what was wrong. + string Mismatch() => + $"contention_{thread}: id {id} written as t={thread} tx={txn} d={doc} read back as " + + string.Join(", ", found.EnumerateFields().Select(f => $"{f.Name}={f.Value}")); + var tOk = found.TryGetInt32("t", out var ft) && ft == thread; + Assert.True(tOk, tOk ? null : Mismatch()); + var txOk = found.TryGetInt32("tx", out var ftx) && ftx == txn; + Assert.True(txOk, txOk ? null : Mismatch()); + var dOk = found.TryGetInt32("d", out var fd) && fd == doc; + Assert.True(dOk, dOk ? null : Mismatch()); } } }