fix(blob): idle watchdog on writeBlobWithStream source to unwedge stalled replication receives - #1444
Conversation
…lled replication receives
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a source-stream idle watchdog for writeBlobWithStream to prevent replication apply consumers from wedging indefinitely when a stream goes silent without ending. It also adds corresponding regression tests. The review feedback highlights two important improvements: first, checking if the stream is paused to avoid false-positive timeouts during backpressure; second, explicitly removing the 'data' event listener upon stream completion to prevent memory leaks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Reviewed; no blockers found. Three new commits since the last review pass:
Full diff re-verified: All prior findings (backpressure false-positive, listener leak, type narrowing) remain addressed. The watchdog logic, cleanup in Ongoing observations (not blockers, same as prior pass):
|
…stener on finish (review)
|
The receiver-side watchdog is the load-bearing half here (it rescues independent of the sender), and the three regression tests nailing the destroyed/never-ended/torn-down PassThrough cases are great. 🙏 Same blocker as the sender PR (harper-pro#451): defaults to Live stakes: a customer cluster preprod 5.1.7 is wedged on exactly this (harper-pro#453) — a stalled source blob stream leaves Suggestion: give it a non-zero default in the replication receive path (mirror harper-pro — Claude (Opus 4.8), from live a customer cluster investigation |
The idle watchdog added in this PR defaulted to 0 (off), with nothing setting HARPER_BLOB_STREAM_IDLE_TIMEOUT_MS anywhere in harper or harper-pro — so it shipped inert and the replication-receive wedge it targets could still happen (observed live on JJill preprod 5.1.7, harper-pro#453: a stalled source blob stream left saveBlob.saving unsettled, outstandingBlobsToFinish pinned, the per-database apply consumer's drain await wedged, the follower stuck at lastReceivedStatus="Receiving"/ver=0, COPY_COMPLETE never reached, deploys blocked). Default it ON at 120000ms; HARPER_BLOB_STREAM_IDLE_TIMEOUT_MS still overrides (set 0 to disable). The only source that can stall this long is the replication-fed PassThrough (storageInfo.source) — local Readable.from(...) sources deliver immediately and finish — and the existing re-arm-on-pause guard means legitimate write backpressure never trips it, so the practical blast radius is the replication receive path this is meant to protect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Pushed an enablement commit ( One thing to confirm since this lives in core Nice fix — the destroyed/never-ended/torn-down PassThrough coverage is exactly the a customer cluster (#453) shape. 🙏 — Claude (Opus 4.8) |
…ead writer The lock is in-process and released on unlock() or DBHandle::close() (which reaps even expired-owner locks), so a dead/crashed writer cannot leave it held. The spin is pinned by a *live* in-progress write whose source stream stalled and never reached unlock() — the writeBlobWithStream pipeline that #1444 watchdogs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ault-on (#1458) Scopes harper#1444 so the watchdog only fires for the owning caller (replication arms it via harper-pro#459); HTTP/cache writers stay off. Off by default; env var is a process-wide kill switch. Sanitizes the timeout value (no 1ms-collapse). Tests: 5/5. Gemini review addressed.
…1444 format check) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…blob can't spin a worker at 100% CPU (#1454) (#1456) * fix(blob): bound the blob stream() incomplete-content wait so a truncated blob can't spin a worker at 100% CPU (#1454) FileBackedBlob.stream() → readMore() busy-spun a worker thread indefinitely when a blob file was present but truncated (its 8-byte header records a known final size larger than the bytes on disk) and the write-lock still read as held. In the bytesRead===0 branch, resumeIfWriterFinished() re-entered readMore() synchronously with no backoff and no deadline, so a read at EOF returned 0 every iteration and pegged the core. checkIfIsBeingWritten() caches its result, so a writer that died or stalled without releasing its lock pinned the spin forever. This is the prod-dyn/prod-gar GDI CPU storm; it is not addressed by #1423/#1424 (the spin branch is byte-identical 5.0.28 through 5.1.8). Add a lazily-set no-progress deadline (getBlobReadTimeout(), default 20s) and a 20ms backoff before the resume re-entry, failing fast with a retryable BlobReadError(503) once the deadline passes. The deadline is set only while no bytes are readable, so a genuinely slow but progressing in-progress write - which makes progress and resolves the pull each chunk, starting each new pull with a fresh budget - is unaffected. Also track both readMore() 20ms backoff timers via `timer` so cancel()/onError can clear a pending poll, avoiding a read on a closed/reused fd when a stream is aborted mid-wait. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(blob): correct #1454 stall wording — live stalled write, not a dead writer The lock is in-process and released on unlock() or DBHandle::close() (which reaps even expired-owner locks), so a dead/crashed writer cannot leave it held. The spin is pinned by a *live* in-progress write whose source stream stalled and never reached unlock() — the writeBlobWithStream pipeline that #1444 watchdogs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Kris Zyp <kris@harperdb.io> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Slow txn will expire test asserted trackedTxns.size === existingTxns, where existingTxns was sampled before SlowResource.get. trackedTxns is a module-level singleton shared across the suite; leftover open txns from earlier tests can expire (removed by the 20ms timer) during the following 50ms Promise.race window, dropping the count below existingTxns (the 0 == 5 / 1 == 6 AssertionError seen in CI). This branch's added blob tests shift suite timing enough to make the latent flake deterministic. Assert membership of the specific txn from SlowResource.get(3) instead, which is immune to background txn churn. Identical to #1459. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3 participants
Fixes #1443.
Summary
writeBlobWithStreamrunspipeline(stream, [compressedStream,] writeStream, finished).pipelinewaits for the source to emitendorerror. A source that has not ended and is not errored sits forever; nothing wakes the pipeline up,saveBlob.savingnever settles,outstandingBlobsToFinishretains the stuck promise, and the per-database replication apply consumer's drain await blocks indefinitely. In production this pins the affected (sender, receiver, database) tuple atlastReceivedStatus:"Receiving"with no log signature.Fix
Opt-in source-idle watchdog in
writeBlobWithStream, armed viaHARPER_BLOB_STREAM_IDLE_TIMEOUT_MS(default 0 = off so behavior is unchanged for callers that do not set it; replication's blob-receive path sets 120000ms). Watchdog is armed on everydataevent; on expiry it calls(stream as Readable).destroy(new Error(...))so the pipeline rejects cleanly withBlob source stream idle for <ms>ms (fileId=<id>). Cleared in the existingfinishedcallback so successful streams pay nothing past the lastdataevent.The replication layer's separate
blobsInFlighttimer only helps when the stuck stream is still in that map; this guard catches the case where a finishing chunk arrived but the source never ended.Test plan
npx mocha unitTests/resources/blob.test.js --grep "saveBlob with idle source stream"passes (3/3)saveBlob with idle source stream (replication wedge regression)covers the three prod-observed states: destroyed-before-save, chunks-but-never-ends, mid-stream-then-destroyBlob testsuite failures (#1423) are unrelated to this change (verified by running on clean main)Production observation
Akamai v4→v5 stage cluster (5.1.6-fixes2): post-upgrade catch-up on nl-ams-1 fired the watchdog 420 times, each catch unwedging an apply that would have stalled permanently on 5.1.6 stock; 6077 corresponding ENOENT blob-send errors on
/home/harperdb/harper/blobs/resilience/...confirm the upstream missing-blob source on the sender.Companion change
A complementary sender-side timeout lives in harper-pro (
replication/replicationConnection.tssendBlobsper-chunkPromise.raceagainstsetTimeout); both halves are bundled inharperfast/harper-pro:5.1.6-fixes2. That PR will be opened separately against harper-pro.