feat(npm): dist-tags inline-build singleflight (S11 Step 10, MR 4/5)
📦 What
S11 Step 10 (dist-tags GET endpoint) is a five-MR stack, in dependency order (each targets the one above; GitLab auto-retargets to main as they merge):
- !622 (merged) - dist-tags handler primitives + in-package unit tests.
- !623 (merged) - inline-build handler (resolve -> 404 -> build
{tag:version}-> serve). - !624 (merged) - kind=1
npm_metadata_filescache fast-path + conditional-GET 304-without-blob-read. - !625 (merged) - inline-build singleflight (collapse concurrent cache misses). <- this MR
- !626 (merged) - integration suite (real datastore finders + stub
BlobStore).
This MR now targets main (GitLab auto-retargeted it as its stack parents merged).
Slice 4 - wraps the inline build (the cache-miss path) in a golang.org/x/sync/singleflight so concurrent misses for the same package collapse to one builder + N waiters, completing the handler.
- The singleflight is scoped to the inline-build only: package resolution, the visibility read, and the kind=1 cache read all run per request, outside
inflight.Do. - The key is
(namespace_id, npm_package_id, kind=1). It usesnpm_package_id, not the package name: a name is not unique within a namespace (one namespace hosts multiple npm repositories, each may hold a package of the same name), so a name-based key would collide across repositories and hand one waiter another repository's dist-tags. - The build runs on a context detached from the leader's request (
context.WithTimeout(context.WithoutCancel(ctx), distTagsInlineBuildTimeout)): singleflight shares one build across N waiters, so coupling it to the leader's request would let a leader disconnect cancel the shared build and 500 waiters whose own requests are still live.WithoutCancelsevers that coupling (matching the packument handler onmainand thebuffered.goprecedent); the timeout bounds the detached build's resource hold. - On a collapse,
serveInlineBuildincrements a newdistTagsInlineBuildCollapsedcounter (registered alongsidedistTagsCacheBlobMissing), so the collapse rate is observable: a regression that breaks the key and stops collapsing is alertable rather than green-on-dashboards. (Aligning the packument handler onto the sameshared/collapse-counter pattern is tracked as a follow-up; it is not yet onmain.)
✅ Spec coverage
| Behaviour | Test |
|---|---|
| 10 concurrent inline-builds collapse to exactly one build, all return 200 | TestDistTagsHandler_Singleflight_CollapsesInlineBuilds |
Reliable collapse under -cpu=1 (barrier rendezvouses at the cache finder) |
TestDistTagsHandler_Singleflight_CollapsesInlineBuilds |
Same name in different repositories (distinct npm_package_id) does NOT collapse |
TestDistTagsHandler_Singleflight_DistinctPackagesDoNotCollapse |
| A leader canceled mid-build does NOT fail the collapsed group: the detached build still serves all N waiters their 200s | TestDistTagsHandler_Singleflight_CanceledLeaderDoesNotFailGroup |
⚠️ Known gap - handler not yet wired into the route table
DistTagsHandler is implemented and tested, but nothing mounts it: internal/format/npm/handler.go still serves the 501 Not Implemented placeholder for the dist-tags route, and the tests construct the handler directly with an injected Resolution. This is an unassigned plan-level gap (the npm hosted plan creates handler.go with placeholders in Step 8 and never assigns the placeholder->real-handler swap to a handler step); it surfaces no later than Step 23 (real npm-CLI conformance) and is closed via a plan amendment, not bolted onto this stack. Deliberately out of scope.
🧪 Testing
go test ./internal/format/npm/...- pass (incl.-race; singleflight reliable at-cpu=1)golangci-lint run ./internal/format/npm/...- 0 issuesgo vet -tags integration ./internal/format/npm/...- clean (integration build compiles)
Related to #122 (closed)