Skip to content

Commit 0f09e4d

Browse files
LukaszRozmejclaude
andcommitted
fix(db): cross-column snapshot atomicity in SnapshotableMemColumnsDb
SnapshotableMemColumnsDb.CreateSnapshot iterates columns in a loop and captures each column's snapshot independently. InMemoryColumnWriteBatch .Dispose iterates the per-column batches and disposes each independently. Without a global guard, a CreateSnapshot call concurrent with an in-flight writeBatch.Dispose can capture some columns AFTER the new writes and others BEFORE — producing a cross-column-inconsistent reader view. This race only matters in the test backend (RocksDB has atomic cross-CF snapshots), but it manifested concretely in E2ESyncTests.SnapSync on Windows: post-snap-sync block processing leases a fresh persistenceReader while the persistence pipeline is committing, gets a snapshot whose Accounts column is updated but whose StateNodes column is not, then walks the trie at the new state root and throws TrieNodeException for a node that "should" be there. Add a ReaderWriterLockSlim around CreateSnapshot (read) and writeBatch.Dispose (write) so multi-column commits are atomic w.r.t. snapshot creation. Multiple snapshots can still proceed concurrently — the only contention is the rare overlap of a snapshot creation with a multi-column commit. This eliminates the TrieNodeException failure mode in the stress reproducer (`SnapSync_StressRepro`). The "missing in flat" mode is a separate, deterministic bug (the same ~9 addresses miss reliably) that appears unaffected by this fix and needs follow-up investigation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent edc0d3d commit 0f09e4d

1 file changed

Lines changed: 54 additions & 5 deletions

File tree

src/Nethermind/Nethermind.Db/SnapshotableMemColumnsDb.cs

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Threading;
67
using Nethermind.Core;
78

89
namespace Nethermind.Db
@@ -16,6 +17,17 @@ public class SnapshotableMemColumnsDb<TKey> : IColumnsDb<TKey> where TKey : stru
1617
private readonly Dictionary<TKey, SnapshotableMemDb> _columnDbs = new();
1718
private readonly bool _neverPrune;
1819

20+
// Cross-column atomicity guard. Each per-column SnapshotableMemDb has its own version
21+
// counter and lock, so per-column reads/writes are individually consistent. But a
22+
// multi-column writeBatch dispose applies columns one-by-one, and CreateSnapshot
23+
// captures column snapshots one-by-one. Without this lock a snapshot taken concurrently
24+
// with an in-flight writeBatch dispose can capture some columns AFTER the new writes
25+
// and others BEFORE, producing a cross-column-inconsistent reader view. RocksDB does
26+
// not have this problem (its snapshots are atomic across CFs); this lock makes the
27+
// in-memory test backend match. The lock is only contended when a snapshot is taken
28+
// simultaneously with a multi-column writeBatch.Dispose, which is rare.
29+
private readonly ReaderWriterLockSlim _atomicityLock = new();
30+
1931
private SnapshotableMemColumnsDb(TKey[] keys, bool neverPrune)
2032
{
2133
_neverPrune = neverPrune;
@@ -55,16 +67,52 @@ public IDb GetColumnDb(TKey key)
5567

5668
public IReadOnlyColumnDb<TKey> CreateReadOnly(bool createInMemWriteStore) => new ReadOnlyColumnsDb<TKey>(this, createInMemWriteStore);
5769

58-
public IColumnsWriteBatch<TKey> StartWriteBatch() => new InMemoryColumnWriteBatch<TKey>(this);
70+
public IColumnsWriteBatch<TKey> StartWriteBatch() => new AtomicColumnsWriteBatch(this);
5971

6072
public IColumnDbSnapshot<TKey> CreateSnapshot()
6173
{
62-
Dictionary<TKey, IKeyValueStoreSnapshot> snapshots = new();
63-
foreach (KeyValuePair<TKey, SnapshotableMemDb> kvp in _columnDbs)
74+
// Read lock: many concurrent snapshots are fine; writeBatch dispose blocks them.
75+
_atomicityLock.EnterReadLock();
76+
try
77+
{
78+
Dictionary<TKey, IKeyValueStoreSnapshot> snapshots = new();
79+
foreach (KeyValuePair<TKey, SnapshotableMemDb> kvp in _columnDbs)
80+
{
81+
snapshots[kvp.Key] = kvp.Value.CreateSnapshot();
82+
}
83+
return new ColumnSnapshot(snapshots);
84+
}
85+
finally
86+
{
87+
_atomicityLock.ExitReadLock();
88+
}
89+
}
90+
91+
/// <summary>
92+
/// Wraps <see cref="InMemoryColumnWriteBatch{TKey}"/> so the per-column commit phase
93+
/// happens under the columns DB's write lock, making the multi-column commit atomic
94+
/// w.r.t. <see cref="CreateSnapshot"/>.
95+
/// </summary>
96+
private sealed class AtomicColumnsWriteBatch(SnapshotableMemColumnsDb<TKey> db) : IColumnsWriteBatch<TKey>
97+
{
98+
private readonly InMemoryColumnWriteBatch<TKey> _inner = new(db);
99+
100+
public IWriteBatch GetColumnBatch(TKey key) => _inner.GetColumnBatch(key);
101+
102+
public void Clear() => _inner.Clear();
103+
104+
public void Dispose()
64105
{
65-
snapshots[kvp.Key] = kvp.Value.CreateSnapshot();
106+
db._atomicityLock.EnterWriteLock();
107+
try
108+
{
109+
_inner.Dispose();
110+
}
111+
finally
112+
{
113+
db._atomicityLock.ExitWriteLock();
114+
}
66115
}
67-
return new ColumnSnapshot(snapshots);
68116
}
69117

70118
public void Dispose()
@@ -73,6 +121,7 @@ public void Dispose()
73121
{
74122
db.Dispose();
75123
}
124+
_atomicityLock.Dispose();
76125
}
77126

78127
public void Flush(bool onlyWal = false)

0 commit comments

Comments
 (0)