diff --git a/libs/common/Testing/ExceptionInjectionHelper.cs b/libs/common/Testing/ExceptionInjectionHelper.cs index 0d215bfb035..734f9435464 100644 --- a/libs/common/Testing/ExceptionInjectionHelper.cs +++ b/libs/common/Testing/ExceptionInjectionHelper.cs @@ -149,6 +149,16 @@ public static void WaitOnClear(ExceptionInjectionType exceptionType) Thread.Yield(); } + /// + /// Blocks until is disabled. + /// + /// + public static void WaitOnClearWithThreadSleep(ExceptionInjectionType exceptionType) + { + while (IsEnabled(exceptionType)) + Thread.Sleep(1); + } + /// /// Wait on clear condition /// diff --git a/libs/common/Testing/ExceptionInjectionType.cs b/libs/common/Testing/ExceptionInjectionType.cs index fbc998efeb2..4dde85b290b 100644 --- a/libs/common/Testing/ExceptionInjectionType.cs +++ b/libs/common/Testing/ExceptionInjectionType.cs @@ -112,5 +112,11 @@ public enum ExceptionInjectionType /// in-flight ProcessRecord. /// RangeIndex_Migration_Receive_Pause_In_ProcessRecord, + /// + /// RangeIndex CPR snapshot: while enabled, the snapshot holds the per-tree snapshot lock + /// (blocking, no CPU) to emulate a slow cpr_snapshot, so a test can observe how + /// concurrent snapshots on the same tree (e.g. migration) behave under contention. + /// + RangeIndex_Snapshot_Inject_Latency, } } \ No newline at end of file diff --git a/libs/server/Resp/RangeIndex/RangeIndexManager.cs b/libs/server/Resp/RangeIndex/RangeIndexManager.cs index 2df78025e12..57cb82f4c1e 100644 --- a/libs/server/Resp/RangeIndex/RangeIndexManager.cs +++ b/libs/server/Resp/RangeIndex/RangeIndexManager.cs @@ -66,6 +66,14 @@ public sealed partial class RangeIndexManager : IDisposable /// Gets the number of live (registered) BfTree indexes (activated + pending). internal int LiveIndexCount => liveIndexes.Count; + /// + /// Test-only: returns the live for the given key, or null + /// if no live (activated) tree exists for it. Used by tests that need to drive the + /// per-tree snapshot claim directly. + /// + internal TreeEntry GetLiveTreeEntryForTest(ReadOnlySpan keyBytes) + => liveIndexes.TryGetValue(KeyId(keyBytes), out var entry) && entry?.Tree != null ? entry : null; + /// Gets the log-tied root directory for RI files. Used by the cluster replication layer /// to determine the target directory for received flush files on the replica side. public string RiLogRoot => riLogRoot; @@ -149,45 +157,31 @@ internal sealed class TreeEntry public int SnapshotPending; /// - /// Per-tree snapshot serialization atomic. 0 = idle; 1 = a snapshot is in flight. - /// Both and - /// claim this before calling - /// cpr_snapshot so the two callers do not race for bftree's internal - /// snapshot_in_progress flag (which would no-op one of them silently). + /// Serializes CPR snapshots of this tree; concurrent snapshots are not allowed. /// - public int SnapshotInProgress; + private readonly SemaphoreSlim snapshotLock = new(1, 1); - /// Try to claim the per-tree snapshot atomic. Returns true if claimed. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool TryClaimSnapshot() - => Interlocked.CompareExchange(ref SnapshotInProgress, 1, 0) == 0; - - /// Release the per-tree snapshot atomic. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void ReleaseSnapshot() - => Volatile.Write(ref SnapshotInProgress, 0); + /// + /// Test-only: true while a snapshot holds . Lets a test + /// detect that the holder has entered the critical section. + /// + internal bool IsSnapshotInProgressForTest => snapshotLock.CurrentCount == 0; /// - /// Spin to claim the per-tree snapshot atomic, take a CPR snapshot of the live tree - /// directly into , then release the claim. The - /// claim serializes against concurrent flush / checkpoint / migration snapshots on - /// the same tree — bftree silently no-ops a cpr_snapshot that races its - /// internal snapshot_in_progress flag, so each caller must hold the claim - /// while it snapshots. bftree takes the destination path as a cpr_snapshot - /// argument and writes a fresh self-contained file, so we snapshot straight to - /// . + /// Under the per-tree snapshot lock (which serializes concurrent snapshots), take a CPR + /// snapshot of the live tree straight into . /// public void SnapshotUnderClaim(string destinationPath) { - while (!TryClaimSnapshot()) - Thread.Yield(); + snapshotLock.Wait(); try { + ExceptionInjectionHelper.WaitOnClearWithThreadSleep(ExceptionInjectionType.RangeIndex_Snapshot_Inject_Latency); BfTreeService.CprSnapshotByPtr(Tree.NativePtr, destinationPath); } finally { - ReleaseSnapshot(); + snapshotLock.Release(); } } @@ -648,7 +642,7 @@ internal void LogOnFlushInvariantViolation(string hashPrefix, long logicalAddres /// /// Live case (stub.TreeHandle != 0): take a CPR snapshot via the /// native handle. CPR is concurrent-safe with workers (no per-key X-lock needed). - /// Per-tree atomic serializes against + /// Per-tree lock serializes against /// concurrent for the same tree (otherwise /// bftree's internal snapshot_in_progress would no-op one of them). /// @@ -784,7 +778,7 @@ internal void ClearCheckpointBarrier() /// to snapshot from; data.bftree was pre-staged by PreStage). /// /// - /// Uses the per-tree atomic to serialize + /// Uses the per-tree lock to serialize /// against concurrent for the same tree. Per-key /// X-lock is NOT taken here — that lock would deadlock if any deferred OnFlush fired /// on the checkpoint thread while it held S-locks on hot-path readers' shards. diff --git a/test/standalone/Garnet.test.rangeindex/RespRangeIndexTests.cs b/test/standalone/Garnet.test.rangeindex/RespRangeIndexTests.cs index da03a66d6af..dc3fd9ab5f2 100644 --- a/test/standalone/Garnet.test.rangeindex/RespRangeIndexTests.cs +++ b/test/standalone/Garnet.test.rangeindex/RespRangeIndexTests.cs @@ -6,6 +6,9 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +#if DEBUG +using Garnet.common; +#endif using Garnet.server; using NUnit.Framework; using NUnit.Framework.Legacy; @@ -2491,5 +2494,60 @@ public void RangeIndexManagerKeyExistsTest() ClassicAssert.IsTrue(rangeIndexManager.KeyExists(System.Text.Encoding.ASCII.GetBytes("str-key"), ref ctx)); ClassicAssert.IsFalse(rangeIndexManager.KeyExists(System.Text.Encoding.ASCII.GetBytes("nope"), ref ctx)); } + +#if DEBUG + /// + /// Concurrent CPR snapshots of the same tree serialize by blocking on the per-tree + /// SemaphoreSlim, not busy-spinning. A holder takes the snapshot lock with injected + /// latency; concurrent snapshots (the migration path: SnapshotForMigration → + /// TreeEntry.SnapshotUnderClaim) park instead of burning a core each. + /// + /// Deterministic: no waiter completes while the holder holds the lock; all complete + /// once the latency clears. + /// + [Test] + public void RIConcurrentSnapshotsTest() + { + const int waiterCount = 10; + + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + // DISK-backed RI mirrors migration and writes a real snapshot file to riLogRoot. + db.Execute("RI.CREATE", "snapidx", "DISK", "CACHESIZE", "65536", "MINRECORD", "8"); + db.Execute("RI.SET", "snapidx", "field1", "value1"); + + var manager = server.Provider.StoreWrapper.rangeIndexManager; + var entry = manager.GetLiveTreeEntryForTest("snapidx"u8); + ClassicAssert.IsNotNull(entry, "expected a live BfTree for 'snapidx'"); + + string SnapPath(string name) => Path.Combine(manager.RiLogRoot, $"{name}.snapshot.bftree"); + + // Arm the latency injection so the holder parks (negligible CPU) while holding the lock. + ExceptionInjectionHelper.EnableException(ExceptionInjectionType.RangeIndex_Snapshot_Inject_Latency); + try + { + var holder = Task.Run(() => entry.SnapshotUnderClaim(SnapPath("holder"))); + ClassicAssert.IsTrue(SpinWait.SpinUntil(() => entry.IsSnapshotInProgressForTest, TimeSpan.FromSeconds(10)), "holder failed to acquire the snapshot lock"); + + // Waiters contend on the SAME tree; they block on the semaphore (no busy-spin). + var waiters = Enumerable.Range(0, waiterCount).Select(i => Task.Run(() => entry.SnapshotUnderClaim(SnapPath($"waiter{i}")))).ToArray(); + + // #1: while the holder holds the lock, no waiter can complete. + Thread.Sleep(500); + ClassicAssert.IsFalse(waiters.Any(t => t.IsCompletedSuccessfully), "a waiter completed while the holder held the snapshot lock"); + + // Release the latency; holder + waiters each run a real (~3s) snapshot serially. + ExceptionInjectionHelper.DisableException(ExceptionInjectionType.RangeIndex_Snapshot_Inject_Latency); + + // #2: everything completes (and any fault surfaces) once the lock is released. + ClassicAssert.IsTrue(Task.WaitAll(waiters.Append(holder).ToArray(), TimeSpan.FromSeconds(90)), "snapshots did not all complete after the lock was released"); + } + finally + { + ExceptionInjectionHelper.DisableException(ExceptionInjectionType.RangeIndex_Snapshot_Inject_Latency); + } + } +#endif } } \ No newline at end of file diff --git a/website/docs/commands/range-index.md b/website/docs/commands/range-index.md index 2a811f12583..8b96ea96290 100644 --- a/website/docs/commands/range-index.md +++ b/website/docs/commands/range-index.md @@ -44,6 +44,41 @@ Range Index is fully integrated with Garnet's checkpoint and AOF mechanisms: - **Eviction and lazy restore:** When memory pressure causes the stub to be evicted from the log, the BfTree data file is preserved. On next access, the tree is lazily restored from its snapshot. +#### Snapshot performance + +A BfTree checkpoint takes a CPR (Concurrent Prefix Recovery) snapshot, which is synchronous and +non-blocking to concurrent readers/writers. It has a **fixed floor of ~3 seconds** (the snapshot +state machine advances through several phases, each with a short barrier) plus a component that +grows roughly **linearly with the tree's data size**. The snapshot is dominated by total data +**size**, not the number of keys. + +Measured on a single disk-backed tree (local NVMe, 1 KB values, cache large enough to keep the +tree resident; times are approximate and hardware-dependent): + +| Tree data | Keys | Snapshot file | Snapshot time | +|-----------|------|---------------|---------------| +| 32 MB | 32 K | 39 MB | ~3.0 s | +| 64 MB | 65 K | 78 MB | ~3.1 s | +| 128 MB | 129 K| 145 MB | ~3.2 s | +| 512 MB | 516 K| 594 MB | ~4.0 s | +| 1 GB | 1.0 M| 1.16 GB | ~5.1 s | +| 2 GB | 2.1 M| 2.44 GB | ~6.3 s | +| 4 GB | 4.1 M| 4.90 GB | ~8.6 s | + +Holding the data size roughly constant (~500 MB) while varying the value size — and therefore the +key count — shows snapshot time is **independent of key count**: + +| Value size | Keys | Snapshot time | +|------------|-------|---------------| +| 64 B | 6.7 M | ~4.2 s | +| 256 B | 2.0 M | ~4.0 s | +| 1 KB | 516 K | ~4.1 s | +| 4 KB | 131 K | ~4.0 s | + +> **Note:** Each live tree is snapshotted independently during a checkpoint, and snapshots of the +> same tree are serialized. A checkpoint of many large indexes therefore scales with the sum of +> their sizes. + --- ## Lifecycle Commands