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. Within a stale-worker sweep, each setup's fire time, on a monotonic clock, slides past any the sweep already armed within RECONNECT_STAGGER_MS, so the 50 ms spacing that bounds concurrent TLS setup (#446) holds across independent jitter draws, and a pair whose own backoff has escalated is placed at its own delay without dragging the rest of the sweep out to its ceiling. Setups are cancelled on unsubscribe, node deletion and same-name URL migration — all reachable once a pending timer can live 30 s instead of 200 ms.

An armed setup never falls onto the main thread. If its worker exited while it waited, dispatchSubscriptionRequest defers it (with a warn) to the stale-worker reconcile, which rebinds the entry; main would have run subscribeToNode on the orchestrator. Only single-thread mode, where the main thread is the worker, dispatches there.

Self-catchup is claimed by the first dispatch for one connection entry, which keeps the rider for its life. Until the owning primary connection opens it rides on every recovery re-drive; after that, re-drives to that worker leave it off (its old startTime would make the leader rescan from there to now) and a replacement worker is sent it again, since the main thread cannot see catchup finish. The global claim is consumed only once the worker message is accepted, so cancellation, a long offline retry, or a synchronous postMessage throw cannot lose it.

The worker readiness boundary used to attach a continuation to the component-readiness promise per message (createWorkerSubscriptionAdmission). Before readiness, one insertion-ordered map 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. Subscribe, unsubscribe, force-reconnect and this map all derive identity through one getSubscriptionConnectionKey in replicator.ts, which replaces three hand-built url + '-' + … keys, including the missing nested-URL fallback.

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. A connect report resets the escalated setup delay and cancels nothing; each timer re-checks live state when it fires. The stall kick's half of that is shouldFireStallKick, which claims its entry through connectGeneration and the worker it captured.

Review-thread fixes in this round

• Leadership is recomputed, not carried forward. onDatabase derives it through deriveEffectiveLeader before the existing-entry fast path and assigns the result, so an explicit persisted isLeader: false demotes immediately. Recomputing exposed a second way to get it wrong: a component reload cleared routes before rebuilding it, so a recomputation could briefly see "no configured leader" while the previous hdb_nodes watcher stayed live. The reload now builds the list off to the side and publishes it in one splice, keeping the array identity.
• Only the monitored connection's open cancels a stall kick. connectReportAdvancesGeneration advances connectGeneration only for a socket-open edge (newSocket) from the owning worker's thread whose subscription URL (reported with the open) is the entry's primary. That last check matters because one worker can carry both an entry's primary connection and a proxied failover subscription over the same URL. Pongs, metadata posts, superseded workers' sockets and failover opens no longer move it. A shared-memory truth up-correction does, since it stands in for an open edge that never arrived. Look hardest here.

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 (monitorSync's leader version probe, 1 s → 4 s; fetchJWTKeyWithRetry, 250 ms → 1 s; attempt counts unchanged), the blob-repair per-record pause, and the sendBlobs in-place 503 re-read. DESIGN.md carries the per-site table and the list of what is deliberately excluded.

One definition per schedule. The subscription-setup parameters (200 ms floor, 400 ms → 30 s ceiling) live in createSubscribeBackoff, which the setup scheduler, the wedge/stall re-drive draw and the worker readiness re-attempt all call. The re-drive only ever takes the first draw, so its window is still [200, 400) ms. The scheduler's timing overrides (initialMs/maxMs/minMs/staggerMs) are removed; nothing passed them. The blob-send array [250, 500, 1000, 2000] is now BLOB_SEND_RETRY_BACKOFF (maxAttempts: 4, jitter: 'none'). It produces the same four waits, and the attempt bound now lives in the schedule instead of being split between shouldRetrySourceBlobRead and an array index. sendBlobs creates the backoff lazily on the first eligible 503, so a successful send allocates nothing. The stored-body, partial-send, closed-socket and draining guards are unchanged. It stays unjittered: the retried read is this node's own blob, so there is no fleet to decorrelate, and full jitter would halve the expected time the PENDING placeholder has to heal (DESIGN.md row). Planning review for this consolidation: Framing-Verdict: chosen-approach-sound (a4fea92b7e93).

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.

Merged with main

The branch was 76 commits behind and conflicting, so origin/main is merged in rather than rebased (no force-push; the conflict resolution is one commit). Resolutions that change behaviour: the socket-open report now uses main's threadId/newSocket fields plus this branch's subscriptionUrl, instead of two parallel sets; scheduled setups carry main's send-time exclusionOrigins; and main's hasDeadOwner note is corrected, since an unowned entry now defers instead of subscribing on the main thread.

For the human reviewer

Decisions worth disagreeing with:

• Explicit persisted isLeader: false beats a configured leader (HDB_LEADER_URL / cli / routes[0]); main ignored a persisted false. The owner chose this. One consequence: update_node { isLeader: false } is written LOCAL_ONLY only when true (setNode.ts), so a demotion issued on one node replicates to every peer and overrides their configuration too. In practice the derived flag only gates shouldReplicateFromNode for a database that does not exist locally, and the bootstrap loop that reaches that case already keys on the raw persisted flag. So the reach is narrow, but it is a policy change.
• 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 would restore the old mean and is a one-line change.
• 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.
• Self-catchup is re-sent per worker replacement, not per re-drive. One historical scan per worker replacement buys not losing the range when a worker dies mid-catchup. A same-worker wedge re-drive after the open still replaces the worker's subscription list without the rider, as main always did; closing that needs a worker→main "catchup complete" signal.
• The first reconnect is exactly 500 ms on every link (floor = initial ceiling), so after a shared blip where the peer stays up, the first redial is not decorrelated. Jittering it is one constant (initial 1000 ms, floor 500 ms).
• The hdb_nodes watcher backoff resets only after 10 s of live iteration. A stream that keeps ending sooner stops watching for up to 30 s at a time; the collection subscribe replays current rows on restart, so the cost is latency, not lost updates.
• A setup whose worker exited waits for the reconcile instead of running on the main thread — up to one reconcile interval with no subscription for that pair, in exchange for never doing replication work on the orchestrator.
• 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.
• NodeReplicationConnection.retryTime is removed. On main it was a mutable field; nothing outside the unit tests read it, and the tests now assert retryBackoff.ceiling.
• The blob-repair single-flight guard was removed: getRecord() has no timeout, so one wedged-but-open peer would hold the flag for the life of the thread and take away repair_blob_data until restart. Concurrent sweeps stack again, as on main.
• A hopeless blob-repair sweep 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. Both the per-record and per-peer warns are sampled once unpaced. A reviewer proposed keeping a capped delay instead; that holds the cursor open ~1 s per unrepairable record for the rest of the sweep, which is what the budget exists to end.

Still open, tracked or stated:

• A same-name URL migration can still force-reconnect the address the node left. The migration cancels the setup scheduler for the old URL, but the old connectionReplicationMap entry survives with its owned reDriveTimer. The entry leak predates this branch; the timer riding on it does not.
• Pre-readiness unsubscribes are retained as tombstones (Pre-readiness unsubscribes are retained as tombstones, so the worker-subscription admission map can grow unbounded #807): with a persistently rejecting readiness promise plus peer/database churn, the admission map keeps one entry per historic connection key.
• Pre-existing, surfaced by review, not fixed here: connect() checks intentionallyUnsubscribed only before the createWebSocket await, so an unsubscribe landing inside a TLS handshake can leave an orphan live session; and the revive path (unsubscribed = false) does not reset receiveStallReconnectAt, so an unsubscribe/re-add can hide a stalled connection from the stall net until real progress.
• Coverage gap, blob-send retry: no fixture shows a 503 healing during the in-place retry. blobGapEscalationBudget covers the 503-forever path (retry, then forward), and the unit test pins the exact waits and exhaustion.
• Coverage gap: startOnMainThread encloses onNodeUpdate, onDatabase, connectedToNode and reconcileWorkers, so every unit test targets an extracted helper. The end-to-end evidence is the cluster and stress suites below.

Review findings declined, with the evidence:

• "A deferred setup waits on the reconcile tick." findStaleNodeUrls flags a worker-less entry whenever live workers exist, and a worker exit triggers an immediate reconcile, so the wait is one tick at most — the same bound main had when its bare setTimeout posted to an exited worker.
• "Dropping !isWedged adds TLS churn." A URL that is both stale and wedged now also gets the full onNodeUpdate pass, but its wedged live-worker entries take the refresh-only early return (forceResubscribe is false there), so no extra dial happens; the pass only re-posts unsubscribe-from-node for databases not replicated from that peer, until the stale entries are rebound on that tick.
• "Drop the arithmetic comments in forceReconnect.test.mjs." They name the pinned draw's delays, which is what makes tick(500) / tick(998) readable.
• "canClearCapabilitiesForNewSocket ignores subscriptionUrl" and "pin the explicit-leader bootstrap gate with a test" — both pre-existing on main, recorded for follow-up rather than folded in here.

Verification

On the merged head (main at c2ad3351), then on the consolidation head bacd6b6c:

• Build — npm run build exits 0 with no tsc errors. npm run lint:required and prettier --check on changed files clean.
• Unit — npm run test:unit: 1412 passing, 1 pending; 1410 on bacd6b6c (the two removed tests were the retryTime getter's and the array-length bound's; blobSendRetry.test.mjs now pins [250, 500, 1000, 2000, undefined], and reconnectJitter/forceReconnect/connectReschedulesOnRejection read retryBackoff.ceiling). Covers the backoff matrix, the setup scheduler (a 60,000-event / 60 s storm collapsing to 9 dispatches with one pending timer, hasRef() === false, sweep spacing by fire time under colliding draws, staggered calls, and an escalated pair, deferral when the worker exits before fire), leadership derivation and atomic route publication, socket-open ownership (pong, superseded worker, proxied failover, single-thread), the rider re-sent to a replacement worker, worker readiness admission, and the stall-kick decision matrix.
• connectedBitRestartChurn (QA-587), locally — 5 of 6 runs green across this round's heads (1/1 on the final head), including the new rolling HTTP-worker replacement test (restart_service http_workers, then a write per database must reach the follower) in every run. The one red run was chaos cycle 3 missing its 25 s budget: one leg connected ~100 ms before the SIGKILL, never sent a frame, so its reconnect ceiling was not reset and it drew 29.6 s at the 30 s cap. main has the same reset-on-first-sent-frame rule and waits the full ceiling, so this is the test's budget against that rule, not this change.
• Test files. New: backoff.test.mjs (ceiling growth, factor, full-jitter range, floor, budget/attempt exhaustion on an injected clock), reconnectJitter.test.mjs (decorrelation, the 500 ms → 30 s ceilings, the 500 ms floor, onFrameSent reset), subscribeSetupScheduler.test.mjs (storm bound, dedup, sweep spacing, deferral), workerSubscriptionAdmission.test.mjs (connection-key derivation, latest-action retention, rejected-readiness re-attempt), subscriptionRoutingOwnership.test.mjs (atomic route publication, persisted isLeader precedence, socket-open ownership), shouldFireStallKick.test.mjs (the stall-kick decision matrix) and blobRepairPacing.test.mjs (the budget starts at the first unrepairable record). Extended: nodeUpdateWatcher.test.mjs (escalation when iteration throws at once, healthy-uptime reset, subscribe time not counted as uptime), shouldCloseSendAuthWatch.test.mjs (fail closed past the deadline, including a read that crosses it), findStalledReceivingNodeUrls.test.mjs (a fresh socket's grace period), and forceReconnect.test.mjs / connectReschedulesOnRejection.test.mjs (pinned draws against the jittered ceiling, read from retryBackoff.ceiling).
• Integration files. connectedBitRestartChurn.test.mjs gains the rolling HTTP-worker replacement test (and a comment now names the reconnect ceiling instead of the removed retryTime); fixture-blob-pending-source/resources.js changes only a comment, to the new constant's name.
• Consolidation head bacd6b6c, cluster — blobGapEscalationBudget (exercises the in-place 503 re-read, then the forward) and connectedBitRestartChurn: 6/6 pass.
• 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 on the merged head and again after the round-1 review fixes (bounded Setting up subscription with leader rate, no listener leak, no OOM marker, RSS under cap, reconverging mesh with a peer permanently wedged).

Pre-push review of this round (prepush-review.mjs --author claude --legs auto, codex + gemini + cursor-composer + harper-domain every round): a full round on the merged head (adjudicated minor), then delta rounds — nit, nit (COMMENTS), and nit (COMMENTS, on the pushed head) after a post-push human review showed the sweep compared relative delays from calls made at different instants (now absolute fire times). The full-round findings — the sweep floor dragged by an escalated pair, self-catchup lost when a worker dies mid-catchup, unbounded integration probes, the unsampled per-peer repair warn — are all fixed above, as is round 2's spacing gap next to a just-escalated pair. The consolidation commits got two delta rounds (all four lenses): nit (COMMENTS; a test title and a field comment, both fixed), then LGTM on bacd6b6c.

Refs #327
Fixes #805

🤖 Generated with Claude Code

https://claude.ai/code/session_017CTUbhGiW2AXhogHZLg8eo

— Claude Opus 5.5

Origin — the dispatch brief this PR was written from

Pace replication retries on one jittered schedule and bound subscription setup

LIVE CONVERSATION about #800.

You are answering a person, in a thread, one turn at a time. Every turn:

1. Read the whole thread in this dispatch file's # Log — it is the conversation so far, and
   each of your previous turns is in it. Read the PR/issue and the code as needed.
2. Answer the LAST message. Append your answer to # Log as your turn. Prose, not a report:
   they are talking to you, and a status template is not an answer.
3. Set status: needs-input and stop. The thread stays open; their next message resumes it.

Each turn arrives as ASK (answer it, change nothing) or PERFORM (do it, then say what you did) —
the person chose which when they sent it, and the run's own prompt tells you which one this is.
Never infer it from the wording: an unrequested commit in the middle of a discussion and a polite
description of work that was supposed to happen are the two failures this exists to prevent.

Never mark a PR ready and never merge from this conversation.

_{Dispatch: task chat-pr-harper-pro-800-kriszyp · queued by unknown · ran by claude/opus/xhigh · worker kzyp-xps-1}

_{Review-Coverage: authored=claude; ran=codex,cursor-composer,gemini; adjudicated=domain; declined=cursor-grok,cursor-kimi,cursor-muse; rounds=18; full=1 @ bacd6b6}

_{Human-Review-Need: 4 (decisions: self-catchup-retained-for-entry-life, reconnect-first-attempt-unjittered, full-jitter-at-cap, send-auth-fail-closed-at-deadline, blob-repair-budget-then-unpaced, watcher-healthy-uptime-reset) @ bacd6b6}

[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; no blockers found.