Skip to content

feat(replication): gracefully drain in-flight blob sends before worker shutdown#529

Merged
kriszyp merged 5 commits into
mainfrom
kris/blob-send-drain
Jul 9, 2026
Merged

feat(replication): gracefully drain in-flight blob sends before worker shutdown#529
kriszyp merged 5 commits into
mainfrom
kris/blob-send-drain

Conversation

@kriszyp

@kriszyp kriszyp commented Jul 6, 2026

Copy link
Copy Markdown
Member

Summary

Fix (2) of the deploy-reload blob-divergence root cause (harper-pro#527 is the receiver-side fix (1)). A rolling http_workers restart (deploy reload) was tearing down a worker mid-stream while it was sending a replication blob to a peer; the peer then treated the blob as diverged until its source-idle watchdog fired and stamped a PENDING stub. This drains in-flight blob sends before the worker shuts down, so a healthy transfer finishes instead of being interrupted.

Requires the core drain hook — bumps the core submodule to HarperFast/harper#1621 (companion PR).

What changed

  • replication/blobSendDrain.ts (new): per-worker tracker of in-flight blob sends + their per-chunk progress, and drainBlobSends(deadline) — resolves when every send has finished, stalled (no bytes for ~5s), or the absolute deadline passes.
  • replication/replicationConnection.ts (sendBlobs): registers each send, marks progress as bytes are written (including around backpressure waits and the terminal finished:true frame flush), ends it in finally, and stops starting new sends once the worker is draining.
  • replication/replicator.ts: registers the drain against core's registerShutdownDrain, gated on hasProgressingBlobSends() (only a progressing send extends the shutdown backstops).

Design

  • Reset-on-bytes: a large transfer that keeps flowing is never treated as stalled and is allowed to finish, bounded by an absolute 10-minute ceiling (replication.blobSendDrainTimeout; 0 disables). Only a send that goes silent for the stall window is abandoned.
  • Availability-safe: the #1417 pre-started replacement worker is already serving, so a lingering draining worker costs no capacity.
  • Correctness backstop: any send that doesn't finish is still made correct by fix (1) — the receiver aborts + re-requests on reconnect (harper-pro#527). So this is a bounded optimization, not a correctness requirement.

Where to look / open items

  • Deploy-rollout latency: because the core restart throttle now waits on a draining worker, a large progressing send can delay the next worker's restart up to the ceiling (bounded; capacity preserved). Surfaced in the core PR; a follow-up could stop counting a draining worker against maxWorkersDown.
  • The terminal-frame flush is best-effort under no-backpressure (the frame may sit in the kernel TCP buffer); fix (1) covers the miss.

Unit tests cover the pure helpers (isSendProgressing, isDrainComplete, hasProgressingBlobSends) and the drain/quiesce behavior. Cross-model reviewed (Codex + Harper-domain adjudication; Codex's 1 blocker + 5 significant all addressed; the Gemini/agy leg hung out and was skipped).

Generated by Claude (Opus 4.8).

@kriszyp

kriszyp commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

📖 Docs: HarperFast/documentation#569 documents the new replication.blobSendDrainTimeout option. Core companion: HarperFast/harper#1621. (comment by Claude/KrAIs)

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a per-worker tracker (blobSendDrain.ts) to gracefully drain in-flight replication blob sends before a worker shuts down, preventing data divergence on the receiver side. It registers this shutdown drain in replicator.ts and integrates tracking hooks in replicationConnection.ts, accompanied by comprehensive unit tests. The review feedback suggests using optional chaining when accessing ws._socket (e.g., ws._socket?.writableNeedDrain) to prevent potential TypeError crashes if the socket is closed or destroyed mid-stream.

Comment thread replication/replicationConnection.ts Outdated
Comment thread replication/replicationConnection.ts Outdated
@kriszyp kriszyp force-pushed the kris/blob-send-drain branch from ab08bf1 to 336392c Compare July 6, 2026 20:07
@claude

claude Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

…r shutdown

Fix (2) of the deploy-reload blob-divergence root cause (harper-pro#527 is the
receiver-side fix (1)). A rolling http_worker restart (deploy reload) was tearing
down a worker mid-stream while it was SENDING a replication blob to a peer; the
peer then treated the blob as diverged until its idle watchdog fired and stamped a
PENDING stub.

replication/blobSendDrain.ts tracks in-flight blob sends and their per-chunk
progress. sendBlobs registers each send, marks progress as bytes are written
(including flushing the terminal frame), and stops starting new sends once the
worker is draining. The drain (registered against core's shutdownDrain hook) waits
only on sends still making progress (reset-on-bytes; stall = no bytes for ~5s),
bounded by a 10-minute ceiling; core extends the shutdown backstops only while a
send is actually progressing.

Bumps the core submodule to include the shutdownDrain hook (harper core PR).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kriszyp kriszyp force-pushed the kris/blob-send-drain branch from 336392c to df7b0dd Compare July 6, 2026 20:23
kriszyp added a commit to HarperFast/documentation that referenced this pull request Jul 6, 2026
New replication config option (HarperFast/harper#1621, HarperFast/harper-pro#529):
bounds how long a worker drains in-flight blob sends before shutting down during a
restart, so a rolling restart doesn't interrupt a transfer in progress.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kriszyp kriszyp marked this pull request as ready for review July 6, 2026 20:24
@kriszyp kriszyp requested a review from a team as a code owner July 6, 2026 20:24
@kriszyp kriszyp added the patch label Jul 8, 2026
Comment thread replication/replicationConnection.ts Outdated
kriszyp and others added 4 commits July 8, 2026 14:57
# Conflicts:
#	core
#	replication/replicationConnection.ts
cb1kenobi flagged that sendBlobs's `once('drain')` waits (mid-loop and the
terminal-frame flush) can hang forever if the peer closes/errors the
connection while parked on backpressure: `drain` never fires, so `finally`
never runs and `endBlobSend`/`outstandingBlobsBeingSent` cleanup is skipped,
leaking the drain token in blobSendDrain's module-global registry.

Add waitForDrainOrSocketEnd, which races the drain wait against the socket's
close/error events and the WebSocket wrapper's close event, cleaning up all
listeners once one fires. Use it at both call sites. Exported so the race is
unit-testable with plain EventEmitters.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@kriszyp kriszyp merged commit 1fb39f1 into main Jul 9, 2026
18 of 31 checks passed
@kriszyp kriszyp deleted the kris/blob-send-drain branch July 9, 2026 00:24
kriszyp added a commit that referenced this pull request Jul 9, 2026
cherry-pick: feat(replication): gracefully drain in-flight blob sends before worker shutdown (#529 → v5.1)
Ethan-Arrowood pushed a commit to HarperFast/documentation that referenced this pull request Jul 9, 2026
New replication config option (HarperFast/harper#1621, HarperFast/harper-pro#529):
bounds how long a worker drains in-flight blob sends before shutting down during a
restart, so a rolling restart doesn't interrupt a transfer in progress.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants