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_files cache 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 uses npm_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. WithoutCancel severs that coupling (matching the packument handler on main and the buffered.go precedent); the timeout bounds the detached build's resource hold.
  • On a collapse, serveInlineBuild increments a new distTagsInlineBuildCollapsed counter (registered alongside distTagsCacheBlobMissing), 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 same shared/collapse-counter pattern is tracked as a follow-up; it is not yet on main.)

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 issues
  • go vet -tags integration ./internal/format/npm/... - clean (integration build compiles)

Related to #122 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading