[Badrish Chandramouli (badrishc)]

Fixes defects in replication, checkpoint transfer, Vector Set recovery, gossip and TLS accept that surface as intermittent Garnet .NET CI failures. CI runners have two cores, and each defect reproduces under taskset -c 0-1. No timeout is raised, no retry added, and no assertion weakened.

Product changes

Replication and cluster

• AofSyncTask — the tail witness pulse is gated on the log being sharded across physical sublogs. It was gated on MultiLogEnabled, which is also true for a single sublog replayed by several tasks; there the pulse read a sequence number generator that is only constructed for a sharded log, so it dereferenced null and killed the AOF sync task, stranding every subsequent write on the primary.
• AofSyncDriver.Dispose — closes each task's connection and waits for the task to leave its monitor; each task disposes its own client session before leaving. GarnetClientSession expects mono-threaded access, so no session is disposed by a thread other than the one using it.
• GarnetClientSession — adds CloseConnection, which invalidates the socket without disposing the session, and a sticky close request that ThrowIfCloseRequested observes, so a close arriving while the connection is still being established is not lost.
• Gossip — a failed gossip round is contained and the loop continues. Nothing restarts that loop, so a round allowed to terminate it would leave the node silent for the rest of the process.
• SingleDatabaseManager — failing to recover the checkpoint received from a primary fails the replica sync, independently of FailOnRecoveryError, which governs standalone startup. A replica cannot advertise the primary's replication offset over a store it failed to recover.

Checkpoint transfer

• Recovery.GetLogFileSize — the object-log range end is hlogEndObjectLogTail, captured at the same checkpoint phase as the main-log range end it is paired with, and the address recovery installs as the object log tail. A transferred checkpoint therefore carries every object byte its main log references.

Vector Sets

• VectorManager.ReconcileRecoveredState — completes the recovered index set from the store, since a checkpoint snapshot reports only the records in its own range, and restores each context reservation from the index records. An index with nothing recovered keeps its in-memory metadata, which is what the second reconcile of an AOF-enabled cluster startup runs on.
• VectorManager.Replication — an incoming migrated record whose context is occupied locally is steered onto a context that is free locally. The mapping is remembered so every record of that migration resolves identically, and is injective so two migrations cannot share a context.
• VectorManager.ContextMetadata — migration remappings are discarded when contextMetadatas is rebuilt, since they name contexts in the array being replaced.

Networking

• GarnetServerTcp — the TLS handshake runs on the thread pool rather than the accept loop, so a peer that negotiates slowly, or never, does not stop the server accepting other connections.

Other

• HashObject — the copy-update constructor initializes the alternate lookup when the source carries field expirations.
• GarnetServer.Dispose, StoreWrapper, GarnetServerMonitor — dispose runs under an injection-parking suspension, and routine cancellation from dispose or a primary-to-replica transition is not logged at Error/Critical.

Test infrastructure

• ExceptionInjectionHelper — SuspendParking/ResumeParking, so a server shutting down releases threads parked at injection points and refuses later arrivals. A stranded waiter holds a pooled network buffer, which hangs the whole test process in LimitedFixedBufferPool.Dispose.
• ClusterTestContext.DisposeCluster — every node's dispose is started, each on a dedicated thread, before any is waited on, under one shared time budget, so a node that stalls cannot leave later nodes holding their ports.
• ClusterTestUtils.ClusterReplicate — the index-based overload waits for the replica to know the primary's node id. CLUSTER REPLICATE resolves that id against the replica's own configuration, and CLUSTER MEET only makes the node it is sent to aware of its target; the reverse direction arrives through the gossip handshake that follows, so the command could reach the replica first and fail with I don't know about node.
• ClusterNegativeTests.ClusterReplicateFails — connects through the shared harness configuration instead of a hand-built connection string, so the command map matches the pub/sub-disabled servers the test creates and a connect failure carries exception detail and the test's client name.
• ClusterTestUtils.WaitForSyncAsync — reports which node disagreed and how.
• ClusterTestUtils.WaitForPrimaryRole — also waits until the multiplexer has observed the promotion, forcing it to re-read the topology. StackExchange.Redis caches a per-endpoint replica flag and rejects a primary-only command before it leaves the client, so a promoted node is not writable by the test until that flag is refreshed. Used by the failover tests in place of their own role polls.
• TestUtils — the TLS client certificate is imported once and shared rather than per connection attempt, and the multiplexer's reconnect retry gate is shortened.
• MemberExpiry — absolute deadlines for collection member-expiration tests, so no assertion depends on an upper bound on elapsed wall-clock time.

Tests added

ObjectLogCheckpointTransferTests, ExceptionInjectionShutdownTests, VectorSetRecoveredContextReservationTests and VectorSetMigratedContextRemapTests, plus cases in ClusterConfigTests, ClusterNegativeTests, ClusterReplicationShardedLog, RespVectorSetTests and RespTlsTests. Each fix is covered by a test that fails when that fix is reverted.

Dependency

StackExchange.Redis 3.2.0 → 3.2.1 (StackExchange/StackExchange.Redis#3220). IBufferWriter<byte>.GetSpan(sizeHint) guarantees one byte rather than sizeHint; 3.2.0 assumed otherwise and tore down the connection with an ArgumentOutOfRangeException under concurrent writes.

[Copilot]

🟡 Changes recommended

Recovery can index beyond rebuilt metadata, waiter release has a lost-release race, and late node disposals may remain unstarted.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes recurring low-core CI flakes across Vector Set recovery/migration, exception injection, and cluster teardown.

Changes:

• Reconciles and remaps Vector Set contexts.
• Hardens shutdown and cluster-test cleanup.
• Isolates affected StackExchange.Redis assertions and reduces expensive tests.

File summaries

File	Description
TestUtils.cs	Adds dedicated-thread Redis exception assertions.
VectorSetRecoveredContextReservationTests.cs	Tests context reservation after recovery.
VectorSetOverwriteTests.cs	Splits costly SET scenarios.
RespVectorSetTests.cs	Uses isolated exception assertions.
ExceptionInjectionShutdownTests.cs	Tests disposal with parked injection waiters.
LuaScriptTests.cs	Uses isolated exception assertions.
ClusterTestContext.cs	Adds bounded per-node disposal handling.
ClusterReplicationBaseTests.cs	Waits for slot ownership convergence.
ClusterMigrateTests.cs	Waits for bidirectional gossip convergence.
VectorManager.Replication.cs	Remaps migrated contexts locally.
VectorManager.cs	Restores recovered context reservations.
VectorManager.ContextMetadata.cs	Clears remaps during flush.
GarnetServer.cs	Releases parked waiters during shutdown.
ExceptionInjectionHelper.cs	Adds epoch-based waiter release.

Review details

• Files reviewed: 14/14 changed files
• Comments generated: 3
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Badrish Chandramouli (badrishc)]

Worked through all three from first principles. Two were legitimate and are fixed; one is a pre-existing property of main that this PR neither introduces nor widens.

---

1. ExceptionInjectionHelper lost-release race — legitimate, fixed

The diagnosis is right, and tracing it further showed the one-shot release had a second hole with the same fatal outcome. InternalDispose closes listeners before it drains handlers, so a request already in flight can reach a still-armed injection point after the release and park there. Releasing only the waiters that were already parked just moves the hang a moment later.

Both windows have the same shape — a waiter whose release edge has already passed — so rather than move the epoch sample, parking is now suspended for the duration of InternalDispose: it releases anyone already parked and stops anyone new from parking. That closes the read-then-sample window as a consequence, since the suspension is not a one-shot edge that can be missed.

• Count, not a flag, so concurrent shutdowns are independent (DisposeCluster now starts them all at once — see #3).
• Unwound in a finally, so it can't leak into a later test sharing this process-global state. A leaked suspension would silently stop every subsequent injection point from pausing anything, which is why that has its own test.
• No timeout introduced. Both methods and the call site stay [Conditional("DEBUG")].

Ablation — restoring the one-shot semantics and re-running:

Failed ArrivingAtAnInjectionPointDuringShutdownDoesNotPark [30 s]
  a caller reaching an injection point after shutdown began parked instead of
  proceeding, so it would still be holding its pooled buffer when the drain runs
Failed! - Failed: 1, Passed: 2, Total: 3, Duration: 31 s

With the fix: Passed! - Failed: 0, Passed: 3, Total: 3, Duration: 1 s.

---

2. Recovery indexing beyond rebuilt metadata — pre-existing on main, identical exposure

contextIndex comes from ContextMetadata.DecomposeContext(context) inside foreach (var (context, hashSlot) in recoveredIndexes). main already indexes contextMetadatas with that same value, in that same loop iteration:

// origin/main, VectorManager.cs:332-336
foreach (var (context, _) in recoveredIndexes)
{
    var (contextIndex, contextValue) = ContextMetadata.DecomposeContext(context);

    if (contextMetadatas[contextIndex].IsCleaningUp(contextIndex != 0, contextValue))

For a given context, the new block and the pre-existing IsCleaningUp block either both run or neither does, on the identical index. So if this scenario is reachable, main throws IndexOutOfRangeException at line 336 today; the new block only changes which line reports it. The set of indices touched is unchanged.

Growing the array here would therefore not fix the reachable case — main's line would still be there — and it would re-introduce code I removed earlier in this PR after proving it never fires. ContextStep = 8 and DecomposeContext divides by 64 * ContextStep, so contextIndex == 1 requires context ≥ 512; the guard did not fire once across 487 tests, and the IndexOutOfRangeException that originally motivated it was traced to a store scan that has since been deleted. Adding it back would be wallpaper over a condition that is either unreachable or a main bug, and either way not caused by this change.

Worth a separate issue if it can be shown reachable; deliberately out of scope here.

---

3. Late node disposals may remain unstarted — legitimate, fixed

Correct: Task.Run was called inside the wait loop, so a node that consumed the 45 s budget left every later node with its dispose merely queued and unwaited.

All disposes are now started before any of them is waited on, and the budget is then spent waiting on them collectively. Following the thread-pool half of the point: each dispose gets a dedicated thread (TaskCreationOptions.LongRunning) rather than a pool thread, since on a two-core runner the pool injects threads slowly enough that a queued dispose could sit unstarted behind the ones already blocked — the same abandoned port by another route. Dispose closes listeners early, so every node now frees its port even when the budget is blown.

---

Verification

Rebased onto d20d639 first. All suites re-run on 2 cores (taskset -c 0-1), Debug, net8.0 — the injection helper is process-global and DisposeCluster is used by every cluster suite, so none of these were assumed:

Suite	Result
Garnet.test.vectorset	400 / 400
Garnet.test.scripting	619 passed, 30 skipped
Garnet.test.cluster	156 / 156
Garnet.test.cluster.replication	107 / 107
Garnet.test.cluster.vectorsets	84 / 84
Garnet.test.cluster.replication.vectorsets	7 / 7
Garnet.test.cluster.migrate	56 / 56

Build clean with 0 warnings; dotnet format reports no changes.

[Marc Gravell (mgravell)]

Looks like there's a SE.Redis client timing bug in here somewhere. If so: I'm assuming that's a me thing, that you shouldn't have to worry about. I'll look at that ASAP.

[Badrish Chandramouli (badrishc)]

Looks like there's a SE.Redis client timing bug in here somewhere. If so: I'm assuming that's a me thing, that you shouldn't have to worry about. I'll look at that ASAP.

Yes, that would be the better fix here. These are the relevant clues:

StackExchange.Redis recycles a result box while a pulse is still outstanding

SE.Redis 2.12.8 pools SimpleResultBox in a [ThreadStatic] field and sets the exception outside the lock that ActivateContinuations pulses. When a server error faults the box before the caller reaches Monitor.Wait, the caller skips the wait and recycles the box while the reader thread still has a PulseAll outstanding. The next synchronous call on that thread consumes the stale pulse and returns with neither result nor exception, shifting every subsequent reply on that thread by one — a permanent lag that survives new multiplexers and new servers, because the box is thread-static and NUnit runs a fixture on one thread.

The CI logs show this exactly: LuaScriptTests.Issue1079 loses its exception and Issue1235 later receives Issue1079's error; ScriptLoadErrors pairs with Struct the same way, with counts matching 1:1. RespVectorSetTests.VADDErrors fails identically.

Adds TestUtils.ThrowsRedisException, which runs the failing call on a dedicated thread so the poisoned box never lands on the shared test thread, applied to the two fixtures CI proves are affected. Garnet's wire output was confirmed byte-exact with a command/reply balance counter, so this corrects the client-side defect without hiding any server behaviour.

[Marc Gravell (mgravell)]

The SE.Redis side of this should be fixed in 3.2.0

[Marc Gravell (mgravell)]

Btw, SE.Redis 3.2.0 also includes a new API for executing ad-hoc raw commands / scripts: ExecuteResp[Async] and ScriptEvaluate[ReadOnly]Resp[Async], which (from a quick eyeball) may be useful for your scenarios:

• input takes ReadOnlyMemory<T> for pooling scenarios, with full eager snapshot so the buffer can be recycled / reused as soon as the synchronous part of the method has returned, even for fire-and-forget / async scenarios
• output is a leased (disposable) buffer exposed via RespReader, for efficient forwards-only tree walking, rather than building a DOM

[Badrish Chandramouli (badrishc)]

The SE.Redis side of this should be fixed in 3.2.0

We incorporated this, thank you Marc Gravell (@mgravell). My agent did find another SE.Redis/RESPite issue, though, here is the write-up:

https://gist.github.com/badrishc/9df83aacf238705cd2114bbbc3ff0a24

(I have not validated it manually.)

Thanks!

[Marc Gravell (mgravell)]

Will check the GetSpan problem, thanks. Leem 'em coming!

Update: 3.2.1 going out now Badrish Chandramouli (@badrishc)