feat(npm): route the rebuild's blob writes through a hashing tee
🎯 What this MR does
Routes each rebuilt document's bytes into its blob session through a tee that
hashes them on the way, and hands that digest to Commit as the expected one.
The rebuild still materializes every kind exactly as it does today; only the path
the bytes take to the store changes.
It also consults the rebuild fence once before any blob session opens, which is the consultation Step 2's design asks for and the one the store-path rewrite had otherwise pushed behind the upload. Added in review.
Part 2 of 5 of Step 2 in the npm packument streaming generation plan. Depends on !1719 (merged), which has merged; this branch is rebased onto it.
| Part | MR | What it delivers |
|---|---|---|
| 2a | !1719 (merged) | the bounded reads the rebuild will walk |
| 2b · this MR | blob writes routed through a hashing tee | |
| 2c | !1721 (merged) | the rebuild streams into its blob sessions |
| 2d | !1722 (merged) | session lifetime and the two-session bound |
| 2e | !1723 (merged) | fence re-checked per page |
🧩 Why this is its own part
The digest check is the piece that has to move to the sink before a render can stop holding the document whole, and it can move while the generator is still materializing. Landing it here is behaviour-preserving apart from the gate itself, and it leaves 2c as a much smaller diff against a store path that already has the shape it needs.
rebuildBlob owns the session, the SHA-256, and the io.MultiWriter that feeds
both. rebuildStoreKind takes a *rebuildBlob rather than a Document, and
rebuildStoreOneKind owns open-write-store-cancel for one kind.
🔍 Two things worth a close read
The gate is not cosmetic. The session's own digest describes whatever
reached it, so on its own it cannot detect a write path that dropped,
duplicated, or reordered bytes between the caller and the store. Recording it
against a document nobody produced would key the cache row — and every client's
ETag — on bytes no one should be served. The rendered digest is now passed to
Commit, so the session refuses before it finalizes. That check is what a caller
handing over one []byte got for free from Document.SHA256.
One kind at a time, deliberately. On GCS a session preallocates
storage.gcs.chunk_size (64 MiB by default) the moment it is created,
independent of how large the document turns out to be, and neither committing
nor closing it gives that slice back. Staging one kind at a time keeps a rebuild
at one live staging buffer, which is main's profile. 2c is where two overlap,
and 2c is where the cap that bounds them lands.
📊 Metric change
packument_rebuild_total{result="superseded"} keeps its meaning, but the refusals
behind it get cheaper: the consultation before the render turns the common
supersede into one SELECT with no session opened. A widening gap between it and
the orphaned-blob counter is the signal both consultations are working.
npm_packument_rebuild_orphaned_blobs_total widens: it now counts every stamp
failure past a successful commit rather than fence refusals alone. A rotated
fence and a database fault strand the identical blob, and counting only one left
the other running with the dashboard clean. Which cause it was stays readable on
packument_rebuild_total's result label. Name and label set are unchanged, so
no new series; alert thresholds keyed to "fence refusals only" should be revisited.
The Help string and metrics.md say so, and both now also name the two shapes
this counter does not cover: a staging object left by a cancel that could not
run, and an object left by a Commit that failed after the storage move.
🔁 Changed in review
Four threads from @10io, all addressed on this branch:
- A fence consultation before any session opens.
rebuildRenderAndStorenow consults the advisory fence before it renders or stages anything, so a rebuild a concurrent write has already superseded costs oneSELECTrather than a full packument upload.rebuildCommitAndStampkeeps its own consultation immediately before the commit, for a rotation that arrives while the document stages. Both go throughrebuildFenceStillHolds, which names the moment in its error, and the two log lines differ so the cheap refusal and the expensive one stay apart on a dashboard. - The widened orphan counter is pinned. Reverting the widening left the
suite green.
TestRebuildCommitAndStamp_CountsTheOrphanAFaultedUpsertLeavesdrivesfailingUpsertWriterovercountingCommitBlobStoreand asserts a counter delta of 1 on an error that is not a supersede; the same mutation now fails. rebuildStoreAllKindsis inlined. It was always called with one element. The call chain is three deep now, the supersede logging and the early-stop rationale sit on the loop that implements them, andrebuildStoreKindis renamedrebuildCommitAndStamp.TestRebuildPackumentCache_MemoryBoundis restored, withversionStringand the file header's Memory-bound bullet. The 20 MiB growth bound is the merged npm hosted plan's committed enforcement, and this MR is behaviour-preserving on that axis, so nothing forced the deletion.
Two comments that described the end state rather than this part were corrected at the same time: "that is what streaming the render into the session costs" on the store step, and the mid-render supersede test's account of where the refusal lands.
📏 Size
1940 reviewable LOC: roughly 420 production, the rest tests and metric docs. Past the 500-line guidance. The blob type, its digest gate, the store-path rewrite and their suite are one unit; splitting the type from the call site that uses it would leave a part that cannot be reviewed on its own.
✅ Testing
- Unit,
-race, and integration suites green.golangci-lintis clean on./internal/format/npm/...; the--build-tags=integrationrun with--max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=falsereports the same 69 pre-existing package findings before and after this branch's changes, so it adds none. - npm conformance (
mise run conformance:npm) passes on the head of this stack. - The orphan-counter widening and the pre-render consultation were both verified by mutation: reverting either fails its new test.
- Digest divergence is covered per kind, as are cancel-on-write-error,
cancel-on-commit-error, the stranded-object warn line and its fields, and
ErrSessionTerminatednot being reported as a strand.
Related to #241