feat(npm): bound a rebuild's open blob sessions and outlive its budget
🎯 What this MR does
Closes two bounds part 3 left open, both consequences of a render that holds blob sessions across its whole walk rather than opening one per kind and closing it.
Part 4 of 5 of Step 2 in the npm packument streaming generation plan. Depends on !1719 (merged), !1720 (merged) and !1721 (merged) — merge in that order.
| Part | MR | What it delivers |
|---|---|---|
| 2a | !1719 (merged) | the bounded reads the rebuild will walk |
| 2b | !1720 (merged) | blob writes routed through a hashing tee |
| 2c | !1721 (merged) | the rebuild streams into its blob sessions |
| 2d · this MR | session lifetime and the two-session bound | |
| 2e | !1723 (merged) | fence re-checked per page |
🧩 The three changes
Sessions outlive the rebuild's budget by rebuildSessionGrace.
storage.Session.Cancel refuses outright once the context the session was opened
with is done, returning before it touches either the staging object or the
upload_sessions row. Opening the sessions on the rebuild's own
rebuildTimeout-bounded context therefore made a budget expiry mid-render — the
one failure the budget exists to cause — the single case where nothing is
reclaimed. Nothing collects those: ADR 025's gc:staging-sweep is still
proposed.
The grace is measured from the budget's deadline, not from the call. A window opened at the call would close during the render on any rebuild whose reads are quick, which is the leak arriving earlier and on exactly the packages streaming was built to serve.
The store phase moves onto that same context. Session.Commit runs on the
context its session was opened with, so leaving the fence pre-check and the
upsert on the budget put a deadline boundary in the middle of the three steps: a
budget expiring during the Commit's storage move let the commit finish and then
refused the upsert, stranding a committed blob that nothing references and
nothing reclaims (#498).
The streamed kinds move into their own function, and that is a memory bound.
Committing a session does not release its driver-side staging buffer —
pgSession keeps its writer and the GCS writer keeps the chunk-sized slice it
allocated at open — so a rebuildBlob still reachable from the caller's frame
keeps 64 MiB alive with it. Holding the two streamed kinds in a function that
returns before the dist-tags session opens is what makes the two-open-sessions
bound real rather than nominal, and takes rebuildMaxRendering's ceiling from
three buffers per rebuild to two.
📊 Worth knowing
A worker's real lifetime is now rebuildTimeout + rebuildSessionGrace, not
rebuildTimeout, because the store phase and the teardown both run inside the
worker on the grace context. The capacity arithmetic pairing the timeout with
the in-flight cap derives from that sum, so every comment that asserted the old
bound moved with it.
rebuildSessionGrace is 30 s, derived from pg_session.go's cleanupTimeout
rather than picked. The grace covers the store phase plus up to two Cancel
calls, and cleanupTimeout gives one session's identical detached teardown
30 s; internal/remote/fetch.go already mirrors that constant for the same
class of work. A worker therefore lives up to 60 s, which is the window
rebuildMaxPerPackage now states.
The reasoning behind the caps moved into a new
internal/format/npm/packument_cache.md, next to metrics.md: the occupancy
model, the grace sizing, and the chunk-size ceiling arithmetic that the
comment-caps trim had left with no home in-tree. Each affected Go block is one
line pointing there, because editing any of them in place makes
check-comment-caps.sh require that whole block be cut to its cap, 59 lines
down to 1 for rebuildMaxPerPackage.
Deliberately not done here: the file header's two pointers at lines 10 and 16,
and packument_get.go:96. Both sit inside blocks the same gate would collapse,
48 lines in the header's case.
📏 Size
Roughly 690 reviewable LOC. Production Go is about 130 lines and shrank by
125 more as comment blocks moved out; the new packument_cache.md is 230 lines
of relocated prose, and the tests are the rest. Splitting would separate a
constant from the doc that sizes it, so it stays together.
✅ Testing
- Unit,
-race, and integration suites green;golangci-lint0 issues, including a--build-tags=integrationrun. - npm conformance (
mise run conformance:npm) passes on the head of this stack. - Covers the grace anchored on the budget's deadline rather than the call, budget expiry mid-render still reclaiming its sessions, at most two sessions open, and a dist-tags session failure leaving the streamed kinds stamped.
- No e2e scenario is added or affected: the grace is an internal deadline on a
detached rebuild goroutine, not a request-path behavior the catalogs in
docs/testing/describe.
Related to #241