[Ted Hart (TedHartMS)]

Root Cause

ClusterReplicationFuzzyRegionTxnTests.TransactionCommittedDuringPrimaryCheckpointReachesReplica failed intermittently in CI with:

no transactions were committed
Assert.That(arg1, Is.GreaterThan(arg2))
  Expected: greater than 0
  But was:  0

The test started four writer tasks and immediately entered its checkpoint loop. Instrumented locally, that loop finishes in ~4.2 s — it is mostly WaitUntilNextSecond waiting out the one second resolution of LASTSAVE, not checkpoint work — while each writer must first complete a blocking ConnectionMultiplexer.Connect (ConnectTimeout = 5000, four at once from the thread pool, sync over async).

A writer still connecting when stop.Cancel() fired then tested the token before its first commit and exited cleanly:

• the task completed normally, so Task.WaitAll reported nothing;
• AbortOnConnectFail = false meant a failed connect did not throw either.

So the only symptom was a zero increment count, surfacing as a misleading "no transactions were committed" well after the actual problem. Delaying writer startup past the loop reproduces the CI message exactly.

Description of Change

Test only. The checkpoints now wait for every writer to commit a transaction before the first BGSAVE.

Key changes:

• Each writer signals a TaskCompletionSource after its first successful commit, and the checkpoint loop waits on all of them before starting. This removes the race and also strengthens the test: previously the early checkpoints could run against an idle primary, which is the opposite of what the fixture exists to cover.
• A writer that cannot commit faults its own wait, so the real exception is reported instead of an absent increment. A simulated connect failure now fails in 8 s with the actual cause rather than running to a timeout.
• The wait has its own timeout and message (writers did not start committing transactions within ...), so a genuine startup stall is no longer misreported as a zero count.
• Writers are joined on every path, including failing ones. Each holds a multiplexer with AbortOnConnectFail = false, which once abandoned reconnects to whichever server next binds that port, and every cluster fixture in this assembly binds the same ports — the hazard already documented on ClusterLargeTransactionSlotTests.redis.
• Dropped the CancellationToken argument to Task.Run. It only added a cancel-before-start state that surfaces as a confusing TaskCanceledException; the delegate observes the token itself, and a writer that never runs would leave the new wait hanging.
• Raised SyncTimeout off the 5 s default: a transaction of several hundred commands issued while the primary is checkpointing can exceed it on a loaded machine.
• Raised the fixture's context timeout to 120 s so the startup wait can be spent in full and report its own failure, rather than being pre-empted by ClusterTestContext's deadline (which surfaces as BackOff → Assert.Fail("Cancellation Requested")).

Key Technical Details

Affected types:

• ClusterReplicationFuzzyRegionTxnTests — the only file changed.
• ClusterTestContext.Setup(monitorTests, testTimeoutSeconds) — existing parameter, now passed explicitly.

No product code is touched.

What NOT to Do (for future agents)

• ❌ Don't start a timed loop and assume background clients are already live. ConnectionMultiplexer.Connect blocks on async plumbing; several at once from the thread pool take seconds on a small CI runner, and this loop is over in about four.
• ❌ Don't rely on Task.WaitAll to surface a worker's problems when the worker's loop condition can be false on its first check. The task completes normally and the failure is silent.
• ❌ Don't pass a CancellationToken to Task.Run for a worker that already observes it. It only adds a cancel-before-start path that reports as a TaskCanceledException from an unrelated WaitAll.

Validation

• Reproduced the exact CI failure by delaying writer startup past the checkpoint loop, then confirmed the fix defeats the same delay.
• Both new failure paths verified to give precise diagnostics and to fail fast.
• 8/8 passes under 24 CPU-saturating background jobs; 5/5 clean on an idle machine.
• Builds on net8.0 and net10.0; dotnet format --verify-no-changes clean.
• Runtime unchanged at ~12-13 s.

[Copilot]

Copilot review overview

🟡 Changes recommended

A startup timeout can still hide the underlying writer exception during cleanup.

Review effort: Balanced
Findings: 1

Open (1)

• Preserve writer failure when startup timeout and join both fail · New

What changed in this PR

Stabilizes the cluster fuzzy-region replication test by waiting for every transaction writer to commit before starting checkpoints. No product code changes.

Changes:

• Adds per-writer readiness signals and a startup timeout.
• Joins writers on success and failure paths.
• Increases transaction and fixture timeouts for loaded runners.

File	Description
test/​cluster/​Garnet.test.cluster.replication/​ReplicationTests/​ClusterReplicationFuzzyRegionTxnTests.cs	Coordinates writer startup, cleanup, and timeout handling.

---

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