fix(blob): tolerate source-unavailable blobs in pre-commit so a missing blob can't wedge replication - #1353
Merged
Merged
Conversation
…ng blob can't wedge replication A replicated record whose blob is gone at the source (evicted/expired at the origin; the replication receiver flags the save rejection `sourceBlobUnavailable`, harper-pro#403) was aborting the record's apply: `startPreCommitBlobsForRecord().complete()` awaits the raw `saveBlob().saving`, whose rejection propagates out of the commit's pre-commit phase and is caught/swallowed as `error in subscription handler` (Table.ts). The record never commits, so on an expiration cache table full of TTL-evicted blobs every orphaned record re-threw, the base-copy never advanced, and backpressure pinned at ~100% — a permanent replication stall (observed on preprod.jjl). `complete()` now tolerates a `sourceBlobUnavailable` rejection (`isSourceBlobUnavailable`): the record commits with a diverged blob reference, left for proactive backfill (harper-pro#388), instead of aborting the apply. Local/transient save faults stay unmarked and still reject, so the write aborts and a reconnect retries it — no silent loss. This is the apply/commit-side complement to the receiver's resume-cursor advance (harper-pro#403/#405), which handles the durability-watermark side of the same blob. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request ensures that source-unavailable blobs do not abort replication commits, preventing replication copy streams from getting wedged. It introduces the 'isSourceBlobUnavailable' helper to identify unrecoverable source-missing errors and updates 'startPreCommitBlobsForRecord' to tolerate them while still failing on local or transient faults. Corresponding unit tests and design documentation have been added. The feedback suggests using 'await createBlob(...)' in the new tests to maintain style consistency with the rest of the test suite.
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.
Contributor
|
Reviewed; no blockers found. |
…ni review) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kriszyp
marked this pull request as ready for review
June 18, 2026 00:16
This was referenced Jun 18, 2026
kriszyp
added a commit
that referenced
this pull request
Jun 19, 2026
The harper-bench host serves harper + harper-pro via a repo-scoped just-in-time supervisor (one job at a time), not an org-level runner — org/enterprise runners aren't routed jobs for these repos. Also merges current main to pick up the blob fixes (#1353, #1364/#1369) the stale branch was missing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kriszyp
added a commit
that referenced
this pull request
Jun 19, 2026
…-saving blobs (#1353/#1376) The blob unit test asserting complete() "tolerates a source-missing blob but still rejects a transient fault" was RED on main. The assertion was a stale leftover from #1353, not a code regression. #1353 made complete() await EVERY pre-commit blob, so any save fault aborted the commit. #1376 then split pre-commit blobs into blobsNeedingSaving (saveInRecord / saveBeforeCommit — still awaited) and blobsToTrackOnly (already-saving, replication-received — deliberately NOT awaited, to avoid a WS deadlock when a back-pressured copy stream can't supply more chunks: harper-pro#414). Track-only durability moved off commit-abort onto the replication resume cursor (hasBlobGap pins lastDurableSequenceId so the blob is re-streamed on reconnect). The old test drove the transient-fault blob through the track-only path, which complete() no longer awaits, so the expected rejection never fired. This is a test-only contract correction (resources/blob.ts is unchanged): - the "transient fault must still abort" assertion now targets a needs-saving (saveBeforeCommit) blob, where abort is still the contract; - a new case asserts a transient fault on a track-only blob does NOT abort; - the source-unavailable→tolerated case is kept (now on the awaited needs-saving path, so it genuinely exercises the isSourceBlobUnavailable tolerance in complete()); - a comment records the #1353→#1376 history so it isn't "fixed" back. Not a v5.1.5 data-loss regression and no 5.1.6 code patch is needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Summary
startPreCommitBlobsForRecord().complete()(resources/blob.ts) now tolerates a blob whose save rejected because the replication source can no longer provide it — flaggedsourceBlobUnavailable(newisSourceBlobUnavailablehelper). The record commits with a diverged blob reference; local/transient save faults still reject and abort the write.Purpose
On a replicated expiration cache table with a
Blob!field, a base copy (e.g. after an upgrade) sources blobs the origin has since TTL-evicted.complete()awaited the rawsaveBlob().saving, whose ENOENT rejection propagated out of the pre-commit phase and was swallowed aserror in subscription handler(Table.ts), aborting that record's commit. With a contiguous run of orphaned blobs, every apply re-threw, the base copy never advanced, and the connection wedged at ~100% backpressure permanently.Confirmed live on
preprod.jjl: the receiver was both advancing the resume cursor past the blob (harper-pro#405, the durability-watermark side) and re-throwing the samesourceBlobUnavailableerror in the apply path. This fixes the apply/commit side that #405 didn't cover.Where to look
resources/blob.ts—isSourceBlobUnavailable(the classification gate) and the.catchincomplete(). The tolerance fires only on thesourceBlobUnavailableflag, which only the replication receiver sets (markSourceBlobUnavailable, harper-pro#403) — so local writes (which never set it) are unaffected and the no-silent-loss guarantee holds.Reviews / open items for the reviewer
agyCLI ran as an autonomous agent and timed out on environment setup without producing a review, and thegeminiCLI isn't available in this environment. A careful human read of the no-silent-loss reasoning (tolerate onlysourceBlobUnavailable, never a local/transient fault) is the main ask.RecordEncoderpath — and is tracked as a focused follow-up rather than bundled here.Tests
unitTests/resources/blob.test.js: theisSourceBlobUnavailableclassification gate, andcomplete()tolerates-marked / rejects-unmarked (faithfully, via a destroyed receive stream). Fullresourcessuite green locally (828 passing, 0 failing).🤖 Generated by Claude (Opus 4.8).