Repository navigation
fix(replication): follower keeps reading the stream while it applies a large graph - #391
Merged
Merged
Conversation
…a large graph (#356) The follower read the socket and applied on one task, so applying one graph from a checkpoint stopped it reading; the primary's send timed out and the resync started over. Reading and applying now run apart. A reader task fills a buffer of at most 64 MiB; when it is full the reader waits and the primary's send blocks, so nothing is dropped. The follower pings every heartbeat_ms it has nothing to acknowledge, and the primary keeps a blocked send waiting while it hears from the follower: it drops one only when it has neither read nor been heard from for the send timeout. The primary also sends heartbeats while it captures a checkpoint.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Fix GitHub issue #356 in inputlayer/inputlayer (production readiness, replication): "Warm standby: follower must keep reading the stream while it reconciles a large graph".
From the EN-12 part 1 review (the review of PR #283, warm-standby WAL streaming to a read-only follower): the follower reconciles a whole graph in spawn_blocking without reading the socket; the primary's send times out after max(timeout_ms, 10 s) and drops the connection, and the follower restarts the resync from scratch. Decouple socket reads from reconcile (bounded buffer with backpressure), so large graphs never cause a resync loop. Acceptance: resync of a graph 10x the measured 1M-fact case under steady commits completes on the first attempt.
Deliver it as a small PR; done means merged with CI green.
What Changed
src/protocol/replication/buffer.rs, 64 MiB) that the session applies from, and a writer task sends acks plus a WebSocket ping wheneverreplication.heartbeat_mspasses with nothing to acknowledge. When the buffer is full the reader waits, so the primary's send blocks instead of anything being dropped; time spent waiting for room does not count towards the follower'stimeout_mssilence check.max(timeout_ms, 10 s)). It also sends heartbeats while it captures a checkpoint. A primary from before this change ignores the pings, so the replication guide now says to upgrade the primary first or both together, and its "Known limit" note is replaced by a description of the new behaviour.test-utiladded as a dev-dependency), and an integration test that a 60,000-fact resync under steady commits completes on the first attempt. The 10,000,000-fact acceptance run (large_resync_under_steady_commits) is added as an#[ignore]d lab test, so it does not run in CI.🤖 Generated with Claude Code
Risk Assessment
✅ Low: The change decouples reading from applying with a byte-bounded, order-preserving buffer and a ping-based liveness check that stays compatible with older peers, and tracing the reader, writer, batch and primary send paths found no reachable defect; the only open point is that the 10M-fact acceptance run is an ignored lab test whose result is not verifiable from source.
Testing
Drove real
inputlayer-serverprimary and follower processes through the repository's replication test harness, release build. On head, the committed acceptance run (10M facts, a commit every 5 ms) finished the resync in 31.2 s on the first attempt and converged, and the committed 60k-fact first-attempt test plus the retained-log-resync and partition-heal tests passed. To reproduce the reported failure I used a temporary harness knob that makes each steady commit carry 5,000 extra tuples: base failed (connection reset, resync restarted, never recovered within the run), head passed on the first attempt. An adversarial SIGSTOP of the follower for 25 s under bulk commits showed head recovering after thaw with one reconnect, no second resync and converged data. The temporary harness changes were reverted and saved as a patch in the evidence directory. No UI is involved, so there is no visual evidence.Evidence: Head: committed acceptance run (10M facts, commit every 5 ms) plus committed replication tests
resync of 10000000 facts under a commit every 5ms: 31.2 s, 3656 commits meanwhile follower status at the end: {"applied_lsn":4663,"lag_events":0,"primary_head_lsn":4663,"reconnects":0,"resync_failures":0,"resyncs":1,"state":"streaming"} test result: ok. 4 passed; 0 failedEvidence: Head: 10M resync under heavy commits (5,000 tuples per commit) completes on the first attempt
resync of 10000000 facts under a commit every 5ms: 42.4 s, 154 commits meanwhile follower status at the end: {"applied_lsn":1161,"lag_events":0,"primary_head_lsn":1161,"reconnects":0,"resync_failures":0,"resyncs":1,"state":"streaming"} test large_resync_under_steady_commits ... okEvidence: Base (527bacc): the same heavy run drops the connection and restarts the resync
follower status at the end: {"applied_lsn":0,"last_error":"IO error: Connection reset by peer (os error 104)","reconnects":1,"resync_failures":1,"resyncs":2,"state":"resyncing"} test large_resync_under_steady_commits ... FAILEDEvidence: Head: follower frozen 25 s with SIGSTOP, then thawed
still listed in primary status after 25 s: [{"acked_lsn":5,"lag_events":184,"lag_ms":25070,"sent_lsn":18}] after thaw (26.0 s, caught up: true), follower: {"applied_lsn":190,"lag_events":0,"reconnects":1,"resync_failures":0,"resyncs":1,"state":"streaming"}Evidence: Base: same frozen-follower run, for comparison
Evidence: Temporary test-harness patch used for the heavy-commit and frozen-follower runs (reverted, not part of the change)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
tests/replication_tests.rs:1029- The intent's acceptance run ("resync of a graph 10x the measured 1M-fact case under steady commits completes on the first attempt") exists only as the#[ignore]d lab testlarge_resync_under_steady_commits, so CI never runs it and the source holds no record of its result. The CI-run version (a_resync_under_steady_commits_completes_on_the_first_attempt, line 515) uses 60,000 facts, which applies well inside the old 10 s send timeout and would also pass without the fix. The mechanism itself is covered by the paused-time tests insrc/protocol/replication/follower.rs(a follower busy for 6x the send timeout keeps the stream and loses nothing). No code change needed; whoever merges should confirm the 10M lab run was done on the perf host.src/protocol/replication/primary.rs:387- Tradeoff, as designed: the primary now keeps a blocked send waiting for as long as the follower pings, and the follower's ping task (write, follower.rs:559) runs independently of apply. A follower whose apply is stuck forever is therefore never dropped by the primary, where before it was dropped after max(timeout_ms, 10 s). The cost is bounded: the resync pin is capped (pin_cap), sync-mode writes are still bounded bysync_timeout_ms, and a dead or partitioned follower stops pinging and is dropped after the send timeout. The follower-side ping cadence is the follower's ownheartbeat_ms, so a follower configured withheartbeat_msabove the primary's send timeout would still be dropped while busy; the default (500 ms against 10 s) is safe.🔧 **Test** - 2 issues found → no changes applied ✅
git -C ~/.no-mistakes/worktrees/9eaf1dfa27af/01M4768V684HF0KVV7GYF2AHAV statusandgit -C ~/.no-mistakes/worktrees/9eaf1dfa27af/01M4768V684HF0KVV7GYF2AHAV diff). Respond with fix to validate it, or abort.🔧 No changes applied.
✅ Re-checked - no issues remain.
cargo test --release --test replication_tests -- --include-ignored --nocapture --test-threads=1 a_resync_under_steady_commits_completes_on_the_first_attempt large_resync_under_steady_commits a_follower_behind_the_retained_log_resyncs a_partitioned_followeron head with the committed test file (4 passed)LAB_COMMIT_TUPLES=5000 cargo test --release --test replication_tests large_resync -- --ignored --nocaptureon head with the temporary knob (passed, first attempt)The sameLAB_COMMIT_TUPLES=5000run on a copy of base commit 527bacc3 (failed: connection reset, resync_failures 1, resyncs 2)INPUTLAYER_TRACE_LEVEL=info cargo test --release --test replication_tests nm_frozen -- --ignored --nocapture(temporary SIGSTOP/SIGCONT follower test) on head and on baseRevertedtests/replication_tests.rsto the committed version, removed.nm-base/, and confirmedgit status --shortis empty✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
Acceptance run (benchmark host)
Resync of a graph 10x the 1M-fact case under steady commits, on the benchmark host under its shared lock (32 cores, release build, primary and follower on the one host). Measured on d343eb7c, this change before the pipeline rebased it onto main; no before/after pair against the old code was run.
resyncs1,resync_failures0,reconnects0resync_pin_overflows0Perf gate on the same host and commit (
make perf-gate-remote): PASS (policy status provisional).