Pace replication retries on one jittered schedule and bound subscription setup

[kriszyp]

One backoff schedule for every replication retry site, and admission control on the one surface where a retry could amplify: subscription setup.

The defect (#327)

A transient boot-time DNS failure kept onNodeUpdate → onDatabase firing. Each qualifying event turned straight into a retained (not unref'd) 200 ms setTimeout, a subscribe-to-node message, worker-side WebSocket/TLS setup, and one Setting up subscription with leader … warn — no dedup, no cap, no jitter. Whatever re-drove the event amplified 1:1: ~1,400 log lines/s/node in the field, ending in an OOM kill. Pacing alone would not have fixed it — the amplification is per event, not per delay — so the storm surface gets admission control as well as a schedule.

The utility

createBackoff in replication/backoff.ts — {initialMs, maxMs, minMs?, factor?, jitter?, budgetMs?, maxAttempts?, random?, now?}: exponential ceiling, full jitter, optional fixed floor, optional wall-clock/attempt budget, injectable RNG and monotonic clock. Two properties worth knowing:

• an exhausted schedule returns undefined, never 0, so a missed exhaustion check cannot become a busy loop;
• budgetMs is a deadline read off the clock, not a sum of requested sleeps, so a resolver hang or an event-loop stall cannot stretch a grace period past what it advertises.

Before this there was no jitter anywhere in production code: every exponential in the repo was lockstep across a fleet.

Admission control

Subscription setup (createSubscribeSetupScheduler) holds at most one armed setup per (peer URL, database), carrying the newest payload, on a 200 ms floor / 400 ms → 30 s jittered ceiling that resets when the pair connects. It lives in its own map rather than on the connectionReplicationMap entry because onDatabase's stale-worker path deletes and recreates that entry — per-entry state would be wiped on exactly the path that most needs the dedup. Self-catchup stays separate one-shot state, attached on a fresh array immediately before dispatch and consumed only once the worker message is accepted, so cancellation, a stale dispatch, or a synchronous postMessage throw cannot lose it. The warn moved inside the "actually armed" branch: it now describes an attempt, not an event. Setups are cancelled on connect, unsubscribe, node deletion and same-name URL migration — all reachable once a pending timer can live 30 s instead of 200 ms.

The worker readiness boundary used to attach a continuation to the component-readiness promise per message (createWorkerSubscriptionAdmission). Before readiness, one insertion-ordered map now retains the latest action per connection key and database behind a single continuation, with exactly one armed re-attempt if that readiness rejects; after readiness the handlers run inline and allocate no queue state. Subscribe, unsubscribe, force-reconnect and this map all derive identity through one exported getSubscriptionConnectionKey.

Recovery timers are owned. One entry.reDriveTimer per entry, unref'd, disarmed on unsubscribe, node deletion, worker exit and entry replacement, with the sweep's jitter drawn once and used as a common base so the RECONNECT_STAGGER_MS spacing that bounds concurrent TLS setup (#446) survives the decorrelation. A connect report resets the escalated delay but cancels nothing: the main thread cannot attribute a report to the entry that armed the work (connectToNextWorker subscribes a failover peer on a worker that is not entry.worker), and both timers already re-check live state when they fire — the stall kick's half of that is shouldFireStallKick, which also hands back the re-detection stamp when a reconnect cancels a kick.

Sites adopting the schedule

scheduleReconnect (full jitter under the unchanged 500 ms → 30 s ceiling, keeping the hard 500 ms floor from #339; onFrameSent is still the only reset), the wedge and receive-stall re-drives, the hdb_nodes watcher restart, the send-auth reprobe (a 30 s wall-clock deadline in place of 60 fixed sleeps), the copy-cursor flush retry, the two clone probes, and the blob-repair per-record pause. DESIGN.md carries the per-site table and the list of what is deliberately excluded.

The hdb_nodes watcher reset bug: iteratedSuccessfully was set the instant subscribe() resolved, so a subscription that resolved and immediately threw counted as a success every pass and pinned the restart delay at 1 s forever. Progress is now an iteration that stayed live past NODE_WATCHER_HEALTHY_UPTIME_MS, measured from when the subscription actually came up.

For the human reviewer

Decisions worth disagreeing with:

• Full jitter at the reconnect site, resolved by the task owner in favour of decorrelation. It halves the expected dial interval the Harper OOM-killed (SIGKILL) during high-throughput bulk upsert writes (10 GB) #339 fix installed (mean ~15 s against a deterministic 30 s at the cap), so a node facing a permanently-dead peer dials ~4×/min instead of 2×/min — far below the incident rate, and the 500 ms floor keeps the hard minimum. Equal jitter (ceiling/2 + rand·ceiling/2) would restore the old mean and is a one-line change, cheapest before this reaches a fleet.
• Subscription setup escalates to a 30 s ceiling. This path is also the recovery path, so worst-case time-to-resubscribe for a never-connecting pair grows from 200 ms to 30 s in order to bound the storm. Two constants.
• The setup schedule outlives its entry, deliberately — but that is a second lifecycle every future entry path has to remember to cancel.
• The armed setup carries its own payload plus a refreshPending side channel, rather than fixing onDatabase's early-return path so entry.nodes is always the enriched array and reading it at fire time. The root-cause fix touches enrichment ordering in onDatabase; this does not.
• Connect resets the setup delay but cancels nothing. An earlier revision cancelled the armed setup on connect and gated that on the reporting worker's identity; the chaos-restart suite caught it — attribution fails on the failover path, so the delay stopped resetting and cycles converged in 5.3 s, 12.0 s, then not within the 25 s budget. Resetting unconditionally means a redundant subscribe-to-node can reach a live connection, which is what this path always did.
• retryTime is now a derived getter over the backoff ceiling. It has no production reader left — it exists for the existing tests — and its meaning shifts from "next delay" to "next ceiling".
• The blob-repair single-flight guard was removed rather than kept: getRecord() has no timeout, so one wedged-but-open peer would hold the flag for the life of the thread and take away the operator's repair_blob_data lever until restart. Concurrent sweeps stack again, as on main.
• A hopeless blob-repair sweep now runs unpaced after 60 s of consecutive failures instead of stopping: a cluster-wide-lost prefix must not make the repairable records behind it unreachable through the operation. The per-record warn is sampled once unpaced.

Open findings from the final review round (round 12, verdict: CHANGES, adjudicated severity major), not fixed here — this was the first full re-read since round 1, so these interactions had not been looked at since the branch changed underneath them:

• The Bound replication worker exit listeners (#357); membership-aware cluster_status.is_enabled (#217) #446 stagger does not survive decorrelation on the stale-worker reassignment path. The summary above is accurate but scoped: it says the sweep's jitter is drawn once, and the sweep does draw once. Nothing states the guarantee for the reassignment path, and it does not hold there. createSubscribeSetupScheduler.schedule() computes delay = backoffDelay + staggerMs, and each (url, database) owns an independent createBackoff with its own RNG, so the fixed count++ * RECONNECT_STAGGER_MS ladder is added on top of independent jittered draws. Adjacent databases can draw 399 ms and 200 ms and fire at 399 ms and 250 ms — closer together than the ladder intends, and several pairs can land on one tick. A worker exit can therefore burst the concurrent WebSocket/TLS setups Bound replication worker exit listeners (#357); membership-aware cluster_status.is_enabled (#217) #446 exists to space out. Verified by reading both sites. Tracked as Subscription-setup stagger is added on top of independent jitter draws, so the #446 spacing does not hold on stale-worker reassignment #805.
• A same-name URL migration can still force-reconnect the address the node left. The migration cancels the setup scheduler for the old URL, and its comment says so deliberately, but this branch introduces the owned entry.reDriveTimer and the old connectionReplicationMap entry survives the migration with its ownership guards intact — so an armed recovery timer can post a force-reconnect to the obsolete endpoint. The entry leak is pre-existing; the timer that now rides on it is not. Verified by reading the site.
• Pre-readiness unsubscribes are retained as tombstones. Before component readiness no connection can exist, so a subscribe→unsubscribe pair should cancel retained work rather than dispatch it. With a persistently rejecting readiness promise plus peer/database churn, one entry is retained per historic connection key and the pending map grows unbounded — in a change whose purpose is bounding a storm. Not independently verified. Tracked as Pre-readiness unsubscribes are retained as tombstones, so the worker-subscription admission map can grow unbounded #807.

Reviewer findings I declined, with the evidence:

• "refreshPending is a no-op after the timer fires, so a payload change is lost until the wedge reconcile." The early return it describes predates this branch and returned before reaching the old setTimeout for exactly the same entry state, so post-fire behavior is unchanged and refreshPending is purely additive.
• "The if (!(…)) reformat in the failover path is unrelated and prettier will revert it." The opposite: running this repo's prettier over origin/main's subscriptionManager.ts reports it as unformatted, and the shape in this branch is exactly what prettier emits. Reverting would fail prettier --check.
• "New test files must not add sinon." That rule is core/AGENTS.md, scoped to core; eight harper-pro replication unit tests already use sinon fake timers, including two this branch edits.
• "Remove the new comments that narrate history or restate declarations." Raised independently by gemini and codex. Read each named site: backoff.ts's docblock states why full jitter is the default and what budgetMs guarantees against a stalled event loop; knownNodes.ts says why NODE_WATCHER_HEALTHY_UPTIME_MS exists at all; replicationConnection.ts records why the growth curve changed (the previous ~0.4%/retry took >1000 retries, so a dead peer accumulated native TLS state faster than V8 reclaimed it); blobRepair.ts says why its cap is low rather than the 30 s replication cap. Each is a why or a constraint the code cannot state, and the issue refs are the evidence for a constant rather than narration. The test-scaffolding narration codex names separately is a fair hit and worth trimming when these findings are addressed.

Pre-existing problems surfaced by the review and deliberately not fixed here — each is filed rather than folded into a retry-pacing PR:

• connect() checks intentionallyUnsubscribed only before the createWebSocket await, and unsubscribe() closes this.socket?, which is undefined during it — so an unsubscribe landing inside a TLS handshake can leave an orphan live session with connected: true.
• The teardown sites build the connections key with the URL pair in the opposite order to the subscribe path. Identical for every ordinary subscription; under failover (connectToNextWorker subscribes node B over peer A's URL) the teardown lookup has always missed. Behavior preserved, the asymmetry is now documented at the helper.
• The revive path (unsubscribed = false) resets disconnectedAt and createdAt but not receiveStallReconnectAt, so an unsubscribe/re-add carries the old stall stamp and a connection that reads Receiving without advancing lastReceivedTime is invisible to the stall net until real progress. Same shape as the shouldFireStallKick release this PR adds, in a branch that predates it.

Coverage gap, stated plainly: startOnMainThread encloses onNodeUpdate, onDatabase, connectedToNode and reconcileWorkers, so every unit test here targets an extracted helper. The end-to-end evidence for the storm is the opt-in stress suite below, not a unit test.

Verification

Built (npm run build emits; its ~25 tsc errors are byte-identical to origin/main's, so the exit code is not a usable gate here — see the issue note below). npm run lint:required and prettier --check clean.

• Unit — npm run test:unit: 897 passing. New: the backoff matrix (growth, cap, full-jitter bounds and floor via injected RNG, reset, wall-clock vs attempt exhaustion, deadline clamping), the setup scheduler (a 60,000-event / 60 s storm collapsing to 9 dispatches with never more than one pending timer, hasRef() === false, payload refresh, cancellation, contained dispatch throw, reset-on-connect without cancellation), reconnect decorrelation with the floor and the unchanged first-sent-frame reset, the watcher's escalate-on-subscribe-then-throw and its uptime stamp, worker readiness admission (latest action per target, one armed re-attempt, the floor on a zero draw), and the stall-kick decision matrix.
• CI — everything green except Cluster Integration Tests 6/6, which fails on the same txnlogTearReplication assertion that main's own scheduled run fails on today (run 33650188518, all three Node versions, identical message). Not from this branch.
• Cluster integration, locally — npm run test:integration:cluster: green except cloneReadinessKeySet (QA-762) and txnlogTearReplication, both of which fail identically on origin/main (23b2e51) in this environment — verified by building and running that commit in the same worktree. oversizedFrameCursorSafety flaked once under full-suite load and passes in isolation.
• The replication: no-backoff subscription-setup retry storm on transient boot-time DNS failure ends in OOM #327 scenario end to end — integrationTests/stress/wedgedPeerSubscribeStorm.test.mjs under HARPER_RUN_STRESS_TESTS=1: passes. That is the suite that asserts a bounded Setting up subscription with leader rate, no MaxListenersExceededWarning, no OOM marker, RSS under cap, and a reconverging mesh while a peer stays wedged — the evidence a unit test cannot give for this path.
• The regression this caught — connectedBitRestartChurn (QA-587) went red mid-development with its chaos cycles converging in 5.3 s, 12.0 s, then not within the 25 s budget; that is what identified the connect-attribution mistake described above. Green after the fix (11.4 s / 13.4 s / 21.4 s, plus a 48 s outage converging in 24.6 s), and green again on the final head (28.2 s for the whole chaos test).

Pre-push review (HEG step 10, prepush-review.mjs --author claude): 12 rounds. Rounds 1-9 ran codex as the graded leg with harper-domain adjudicating and Cursor Grok contributing to its per-branch round cap, converging from a BLOCK through two CHANGES to COMMENTS; the Gemini leg failed auth throughout because the worker had no agy login. Once that was fixed, rounds 10-12 added the missing lens, and round 12 was the first full re-read since round 1 with both families running: ran=codex,gemini, verdict: CHANGES, adjudicated severity major, three substantive findings listed above and unfixed. Worth knowing why they appear this late rather than treating them as new: rounds 2-9 were delta reviews scoped to each round's own commits, so no leg re-examined these interactions after the branch changed around them. A note on the earlier rounds' budget too — codex was SIGKILLed mid-diff at the default 1500 s whole-run --timeout (about 975 s for finding legs after the adjudication reserve) and needed 983 s here at --timeout 3000.

Refs #327

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-composer; rounds=12 @ a5b6aa0}

_{Human-Review-Need: 4 (decisions: self-catchup-consumption-point, connect-report-resets-but-does-not-cancel, sticky-isleader-carry-forward, setup-delay-ceiling-200ms-to-30s, watcher-healthy-uptime-10s, blob-repair-continues-unpaced, isleader-carried-forward, connect-report-resets-not-cancels, subscribe-setup-ceiling-30s, stall-throttle-release-only-on-fire, shared-redrive-timer, watcher-health-signal, setup-cap-30s, one-jitter-draw-per-sweep, rider-consumed-at-dispatch, retrytime-as-derived-getter, stall-throttle-release-semantics, connect-lifecycle-shape, shouldfirestallkick-api, reset-on-unattributable-report, blob-repair-unpaced-tail, blob-sweep-single-flight-removed, payload-carried-on-the-schedule, send-auth-fail-closed-on-late-row) @ a5b6aa0}

[gemini-code-assist]

Code Review

This pull request introduces a unified, jittered backoff discipline across the replication system to mitigate thundering herd issues and prevent OOM conditions during retries. Key changes include the addition of a generic createBackoff utility, a SubscribeSetupScheduler to dedup and pace subscription setups, and a createWorkerSubscriptionAdmission gate to manage worker readiness. The changes are well-documented in DESIGN.md and supported by extensive new unit tests. I have kept the review comment regarding Map lookup optimization in subscriptionManager.ts as it provides a valid improvement opportunity.

[claude]

✅ Reviewed the full diff (backoff.ts, subscriptionManager.ts, replicationConnection.ts, knownNodes.ts, replicator.ts, cloneNode/*, blobRepair.ts, DESIGN.md) — no blockers found, and the sole prior review comment (gemini's Map-lookup suggestion) is resolved by commit a5b6aa0.

[cb1kenobi]

[High] Recompute leadership instead of preserving a stale leader bit

This OR makes isLeader: true persist even when the latest node state explicitly demotes that peer. The fast path then stores the stale flag, so later setup or wedge recovery can route through the former leader. Move the current leader calculation before this branch and assign its result instead of carrying forward the cached flag.

—
Reviewed a5b6aa0

[cb1kenobi]

[High] Scope stall cancellation to the reporting connection

Any connect report for this URL/database increments the generation, including reports from failover or superseded workers. Such a report can make shouldFireStallKick cancel recovery for a different connection that remains stalled, and repeated stale reports can keep suppressing its kicks. Include a worker/session identity in the report and advance the generation only for the connection being monitored.

—
Reviewed a5b6aa0