Repository navigation
fix(replication): recover open-but-idle wedged subscriptions via watchdog-driven reconnect - #424
Merged
Merged
Conversation
…hdog-driven reconnect A per-DB receive socket that connects and then goes idle (copy stalls, no transport close) never fires 'close', so the close-handler retry never arms and the wedge reconciler skips the still-connected:true entry — the (peer, db) pair makes zero further connection attempts (harper-pro#420; blocks replicated deploys when 'system' is among the wedged sockets). Drive recovery from the receive watchdog instead: NodeReplicationConnection gains forceReconnect(), invoked from onSilence on the client side. It flips the node entry to connected:false (reconciler backstop), tears the socket down, and schedules one fresh connect() independent of whether 'close' ever fires. A reconnectScheduled flag (cleared in connect()'s finally once the new socket is installed) plus a socket-identity guard in the close handler keep a late close from the superseded socket from double-scheduling or tearing down the live connection. Adds a unit test for forceReconnect's contract and an env-gated, one-shot fault-injection hook + cluster integration regression test that wedges a socket open-but-idle and asserts automatic recovery with no restart. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Contributor
|
Reviewed; no blockers found. |
kriszyp
marked this pull request as ready for review
June 19, 2026 05:49
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
This was referenced Jun 19, 2026
Merged
Merged
kriszyp
added a commit
that referenced
this pull request
Jun 25, 2026
) Adds the deferred third recovery layer from #466 / PR #467. While a receive leg is paused for back-pressure the byte-silence receiveWatchdog is stopped (ws.pause() freezes bytesRead) and the active sendPing is exempt, so a leg that dies mid-pause — e.g. a system base copy stalled at ~100% back-pressure whose peer restarted — had no recovery driver and could wedge connected:false forever, removing the #424 forceReconnect path for exactly that case. A pause-stall watchdog (createPauseStallWatchdog, a thin wrapper over the existing createReceiveWatchdog stall-timer) now guards the paused window, keyed on a local consumerProgress counter that advances on signals surviving ws.pause(): onCommit (apply loop drained a queued batch) and blob-stream drains. Armed on pause, stopped on resume; exactly one of {receiveWatchdog, pauseStallWatchdog} is armed at a time. It fires forceReconnect only after a sustained window of ZERO consumer progress, so a pause that is legitimately making progress re-arms every window and never trips. Cross-model review (Codex + agy): also makes resetPingTimer pause-aware so a frame handler can't re-arm the byte watchdog while paused; threshold defaults to max(PING_TIMEOUT*2, blobTimeout*2) and the slow-single-operation residual (benign: re-streams from the durable cursor) is documented. Tests: unitTests/replication/pauseStallWatchdog.test.mjs. Replication unit suite green (187 passing). Single-repo (no core change). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 1, 2026
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
1 participant
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.
Closes #420.
Summary
After a cluster-wide simultaneous restart, a per-DB receive socket can connect and then go open-but-idle — copy stalls, no bytes flow, and no
closeevent ever fires. Because the close handler only retries onclose, and the wedge reconciler only re-drives entries already markedconnected:false, the(peer, db)pair makes zero further connection attempts and stays wedged until a manual staggered restart. Whensystemis among the wedged sockets, replicated deploys then fail with the 120shdb_deployment row did not replicatetimeout. Observed live on JJill preprod (5.1.5, 4-node, RocksDB).The root gap: an open-but-idle socket never flips its per-DB entry to
connected:false(noclose), and even if it did, the cached connection is still "reusable" (replicator.isReusableConnection) so a reconciler re-subscribe just hands back the same dead socket. So recovery has to be driven from the one thing that does fire on silence — the receive watchdog.What changed
NodeReplicationConnection.forceReconnect()— the receive watchdog'sonSilencenow calls this on the client (subscription) side instead of a barews.terminate(). It notifiesdisconnectedFromNode(flipping the entry toconnected:falsefor the reconciler backstop), tears the socket down, and schedules one freshconnect()— independent of whethercloseever fires. Server-accepted connections (no connection object) keepterminate(); the remote client reconnects.reconnectScheduledflag — set byforceReconnect, cleared inconnect()'sfinallyonce the new socket is installed. The close handler returns early if it's set, so the close path andforceReconnectnever both arm a connect for the same drop.if (this.socket !== socket) return) — a lateclosefrom a socketforceReconnectalready replaced can no longer tear down the live connection.forceReconnect's contract (single reconnect, double-arm guard, no-revive on teardown, backoff, stale-listener cleanup), plus an env-gated one-shot fault-injection hook and a cluster integration regression test that wedges a socket open-but-idle and asserts automatic recovery with no restart (validated against a negative control: pre-change code times out).Scope notes (per the issue's investigator handoff)
connected:falsesockets;forceReconnectfires only on the receive watchdog (true byte-silence — a live-but-slow copy keeps ponging, so it won't loop-restart a system base-copy gated on huge hdb_analytics (analytics.replicate:true) blocks hdb_deployment convergence → deploys fail after any copy #421-style stalled-but-liveReceivingsocket).Database not openunhandledRejectionand thecreateWebSocket-no-try/catch latent item were explicitly ruled out of scope by the investigator (already-caught / not the trigger) — not addressed here.Where to look
forceReconnect()and the two close-handler guards inreplication/replicationConnection.ts— the reconnect lifecycle is the subtle part; the interleavings (fast close, late close during thecreateWebSocketawait, concurrent normal close) are what to scrutinize.armReplicationWedgeForTestlives inreplicationConnection.ts(env-gated onHARPER_TEST_REPLICATION_WEDGE_DB, one-shot, no-ops in production). It's there because there's no clean way to inject an open-but-idle wedge from outsidereplicateOverWS; flag it if you'd prefer it factored out.(peer, db); the receiver de-dupes by sequence id and the sender's own watchdog reaps the idle side.A multi-model review (Codex + a Harper-domain replication/concurrency pass) ran on this change; its findings — a stale-close race, a
subscriptions-updatedlistener leak, an await-window double-schedule, and a test-hook production-arming footgun — are all addressed in the current diff.🤖 Generated by Claude (Opus 4.8, 1M context).