Repository navigation
feat(replication): stream committed changes to a read-only warm-standby follower - #283
Merged
Merged
Conversation
Contributor
Author
|
Perf gate on sam-dev-benchmarks, candidate 3cb0b22 (the later commit is docs only) vs merge base f8a43f9, under the shared bench lock: Performance gate: INCONCLUSIVEPolicy provisional: cost budget p50 +5%, p99 +10%, throughput -5%; 95% interval of the median paired per-round ratio. Budgets are the plan's proposed ceilings, pending captain decision D1; tightening is always allowed.
Required metrics
Diagnostic metrics (not gated)
|
jsam
marked this pull request as ready for review
October 5, 2026 15:03
…wer that applies it (EN-12 part 1)
…ios between real servers; docs
…tch with one fsync; frames carry the primary head; http-only stream URL
…ey issue and revoke apply on the follower
…followers, idempotent index replay
This was referenced Oct 5, 2026
Merged
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
I want you to really hyper focus on making the symbolic engine, you know, crisp and really truly production ready.
(Context: plan ~/firstmate/data/nsdl-plan/report.md §6.1, GitHub milestone 7, issue #264. Item EN-12: warm standby: WAL streaming to a follower, promote on failover, fenced writer epoch — why: HA for the SLA; RPO set by synchronous or asynchronous shipping)
Issue #264 splits EN-12 into four PRs: WAL streaming, follower apply, promotion with writer epoch, and failover in the Kubernetes deployment; RPO and RTO measured. This change is EN-12 part 1: WAL streaming to a read-only follower that applies the stream and stays consistent.
What Changed
[replication]config section with three roles:standalone,primaryandfollower. A primary records every durable change (data, catalog, vector index, API keys) in an in-memory replication log, numbered by LSN. It streams that log overGET /v1/replication/stream, a WebSocket guarded by a shared token. A follower that has fallen out of the retained window gets a checkpoint resync first.GET /v1/replication/statusreports lag and follower state.apply_replicated,reconcile_graph). The follower applies each batch of received frames with one fsync, acks the primary, saves its position so it resumes after a restart, and refuses writes. It reconnects with backoff when the primary goes silent or the connection drops, and it refusesasyncdurability.tests/replication_tests.rssuite covering kill-primary, partition, restart, resync and concurrent multi-graph writers. Also adds a replication guide with measured lag, RPO and catch-up times, configuration docs, andconfig.example.tomlentries.Risk Assessment
✅ Low: The latest fix round is small and correct. The new
resumedsetter records the saved position without setting last_contact, andappliedstill sets it. The gitleaks entry is an exact-match placeholder, and the new test checks the primary-down restart case. The earlier fixes (replaying from every LSN, keeping the resync backoff until caught up) are present in the current code.Testing
I built the server and client and ran a real primary and follower in throwaway temp directories, driving them with the CLI client and the status endpoint. The follower matched the primary through inserts, deletes, rules, graph creates and drops, and relation drops, and it refused a write. A follower restart resumed from its saved position with lag 0 and no resync. When I SIGKILLed the primary during 80 acknowledged writes, the follower held all 80 as a contiguous prefix. After a follower restart while the primary was down,
last_contact_mswas absent and the follower kept its position. When the primary restarted, the follower resynced and converged. A follower with async durability or the wrong token was refused. For subscriptions, partitions and falling behindretain_bytes, I ran the end-to-end replication test file between real processes; all 8 tests passed. The targeted replica unit tests also passed. Everything passed, and I removed all temp servers and data.cargo test --test replication_tests, 8 passed)Evidence: Replicate facts/rules/graphs, follower reads and refuses writes
Evidence: Deletes, relation drop, graph drop replicate
Evidence: Follower restart resumes with zero lag on both status endpoints
Evidence: Kill primary under writer: no acknowledged write lost
Evidence: Follower restart with primary down: no last_contact_ms
Evidence: Primary restart triggers resync and convergence
Evidence: Async follower and wrong-token follower refused
Evidence: End-to-end replication tests between real servers
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (2) ✅
src/storage_engine/replica_tests.rs:263- The round 2 fix was never applied, although the user chose to fix it in that round.replaying_events_already_applied_changes_nothingstill replays only from[0, head / 2, head - 1]. Round 1 made CreateIndex/DropIndex a no-op on a missing graph (replica.rs:225-235) and added the trailing doomed index events towrite_everything(replica_tests.rs:215-223). Neither change is exercised by this test.Why no tested start reaches that path:
insert_tuples_into("doomed", ..)/register_schema_in("doomed", ..)commits. These re-create the graph throughensure_replica_graph, so the later CreateIndex/DropIndex find it.The crash sequence the round 1 finding describes needs a start inside the trailing create-index/drop-index/create-index run. With no such start, the test still passes if the old
self.kg_handle(&kg)?is put back. The fix is the one recorded in round 2: replay from every position (for from in 0..head), so a start at head-2 or head-3 runs CreateIndex/DropIndex against the dropped graph.src/protocol/replication/follower.rs:274- This is in round 1's resync-livelock fix. The follower stops backing off as soon as it applies its first tail batch after a checkpoint (self.resyncing = falseat follower.rs:274). The primary, however, holds the pin and can still end the resync withEnd::PinOverfloweduntil the follower is up to date (primary.rs:234-236, 252).Failing sequence:
read_afterreturns Unavailable, so the primary ends with PinOverflowed and sends Error.resync_failedis false andstreamedis true.resync_backoffresets to zero and the backoff falls back to MIN_BACKOFF (100 ms).capture_checkpoint_at_lsn.The primary's own message says 'the resync is retried' after a backoff (End::PinOverflowed doc), and the user asked for backoff between consecutive resync attempts, reset only once a resync completes and streaming starts. The pin lifetime defines completion as reaching the primary's head.
Fix: clear
resyncingonly once the applied LSN reaches the head carried in the frame (lsn >= head) or on a Heartbeat, both of which mean the primary is up to date. Do the same for thestreamed-driven reset at follower.rs:96-97. Update the module doc (follower.rs:9-11, 'before their checkpoint is applied') and replication.mdx ('resets once a resync finishes and streaming starts') to match.🔧 Fix applied.
✅ Re-checked - no issues remain.
src/protocol/replication/follower.rs:74- Commit ab0336f, a fixer commit from the previous run, makes the follower report its resumed position at startup by callingstatus.applied(loaded.stream_id, loaded.lsn, loaded.primary_revision).ReplicationStatus::applied(status.rs:201-212) also setslast_contact = Some(Instant::now()), so the follower records contact with the primary before it has connected to it.Failing sequence: the follower restarts while the primary is unreachable.
/v1/replication/statusthen reportslast_contact_msas the time since the follower process started. The field should be absent, because the primary has not been heard from in this run. The docs define the field as 'time since the primary was last heard', so a monitor reading it right after a restart sees a recent contact that never happened.Fix: at startup, set only stream_id, applied_lsn, primary_head and primary_revision. Either add a small status setter that leaves
last_contactalone, or letappliedtake a flag.🔧 Fix applied.
✅ Re-checked - no issues remain.
🔧 **Test** - 2 issues found → auto-fixed ✅
src/protocol/replication/follower.rs:52- After a follower restarts and resumes from its saved position (replication/follower.json, lsn 22 in the live run),/v1/replication/statusreports the wrong position on both servers. The follower showsstream_id 0000000000000000,applied_lsn 0,lag_events 22. The primary's entry for it showsacked_lsn 0,lag_events 22. This lasts as long as the primary stays idle, although the follower holds every change. The data was correct.Cause:
run()loads the Position but never callsreplication_status().applied(..); onlysave_positiondoes, and in Tail mode with nothing new to apply that never runs. On the primary,follower_ackedis set only from Ack messages, not seeded from the hello's LSN. As a result, lag alerts fire falsely after every follower restart, and tooling that waits forlag_events == 0waits forever on a quiet primary.Fix: publish the loaded position to the status right after
Position::load(or when the Start message arrives in Tail mode), and on the primary seed the follower's acked LSN from the hello's LSN when the stream ID matches. Add a test toa_restarted_follower_resumes_from_its_saved_positionthat checks lag 0 before any new commit.cargo test --test replication_testsa_partitioned_follower_notices_the_silence_and_catches_up_after_the_heal and a_follower_behind_the_retained_log_resyncs (both start real server processes through…cargo test --test replication_tests(8 tests that start real server processes: happy path, kill primary, partition, follower restart, behind the retained log, wrong token, concurrent writers, API keys)cargo test --lib replicaandcargo test --lib config(replay from every LSN, pin cap, refusal of an async follower)Live: builtinputlayer-serverandinputlayer-client, started a primary on :18101 and a follower on :18102 from TOML configs in a temp directory, seeded facts, rules, a second graph and an HNSW index, then compared reads on both serversLive: tried+edge(9, 9)and.kg create rogueon the follower; inserted, deleted, dropped an index and dropped a graph on the primary, then compared reads and checked status on both serversLive: created, dropped and re-created an index on a graph, dropped the graph, SIGKILLed the follower, restarted it, and checkedfollower.json, its status and its readsLive: SIGKILLed the primary while a client was committing+run(i)(rate limit off); checked the follower held exactly 1..k with k ≤ acknowledged; restarted the primary and checked the resync and row countsLive: started a follower withdurability_mode = "async"(exits with code 1), a primary with async (starts), a follower with the wrong token (gets 401 and retries), and a follower with a short token (refused at startup)🔧 Fix applied.
✅ Re-checked - no issues remain.
cargo build --bin inputlayer-server --bin inputlayer-client, then a primary on :18201 and a follower on :18202 started from /tmp config dirs (immediate durability, shared token)inputlayer-client on the primary: facts, recursive rules, a second graph and vector facts; the same reads on both servers; a+edge(9, 9)write on the followerPrimary:.kg create scratch, schema,.index create/.index drop/.index create, then.kg drop scratch; SIGKILL the follower and restart it; poll /v1/replication/status on both servers at 5 s and 10 s with no new commits; then one more commitA 300-commit writer on the primary, SIGKILL of the primary mid-run, a contiguity check on the follower, a primary restart and a primary/follower comparisonStarting a follower with durability_mode=async; starting a follower with a wrong tokencargo test --test replication_tests(8 e2e tests, including a_restarted_follower_resumes_from_its_saved_position)cargo test --lib replica(includes replaying_events_already_applied_changes_nothing, which replays from every LSN)✅ No issues found.
cargo test --test replication_tests, 8 passed)cargo build --bin inputlayer-server --bin inputlayer-client, then a disposable primary (:18611) and follower (:18612) started from temp configs under /tmpinputlayer-clientscripts on the primary (inserts, rules,.kg create/.kg drop,-edge,.rel drop), then the same queries on the follower using the primary's API keyinputlayer-clientwrite+edge(9, 9)sent to the follower (refused)curl /v1/replication/statuson both servers after each stepSIGKILL and restart of the follower while the primary is idle (resume from replication/follower.json, no resync)SIGKILL of the primary under a steady writer, then a diff of acknowledged writes against the follower's?w(X)rowsSIGKILL and restart of the follower while the primary is down (status:last_contact_msabsent, resumedapplied_lsn)Primary restart, then a check that the follower resyncs and converges with post-restart writesFollower start withdurability_mode = "async"(refused) and with a wrong token (401, never joins)cargo test --test replication_tests(8 end-to-end tests between real server processes: subscription deltas, partition via proxy, behind retain_bytes, concurrent multi-graph writers, API keys)cargo test --lib replica(includes replaying_events_already_applied_changes_nothing from every LSN)✅ **Document** - passed
✅ No issues found.
✅ No issues found.
🔧 **Lint** - 1 issue found ✅
src/ast/mod.rs:709-make doc-check(cargo doc with -D warnings) fails on a broken intra-doc link tocrate::params::bind_statements. No such function exists. This was already broken at the base commit, and this change does not touch it, so I left it alone. It needs a follow-up outside this change.✅ No issues found.
✅ **Push** - passed
✅ No issues found.
✅ No issues found.