chore(storage): upload session admission gate and shed sentinel (upload-session-concurrency-cap plan: 2/12)

What

Adds the upload-session admission gate and its shed sentinel. The gate bounds how many upload sessions this process holds open at once, which bounds the per-session object-storage staging buffers behind them.

Nothing calls it, so the cap is inert on this branch. The gate takes its capacity and wait budget as parameters, the four metric vectors register at boot, and the acquire is wired at the NewSession sites by a later step. The three label-free vectors read 0 or empty from registration; the shed counter is a CounterVec, so until its first child Gather omits the family altogether and a dashboard reads "No data" rather than 0.

  • internal/storage/upload_admission.go — the gate over golang.org/x/sync/semaphore.Weighted, its constructor, and Acquire, which parks at most the wait budget and returns an idempotent release closure. Weighted rather than a counting channel so an operation needing two sessions takes both units or none: two one-unit acquires can each strand a unit and wait on the other.
  • internal/storage/errors.goErrUploadSessionsBusy.
  • internal/storage/metrics.go — the shed's outcome constant, the AdmissionSource type and its six values, and four vectors: the units-in-use gauge, the derived-capacity gauge that gives it a denominator, the hold-duration histogram observed at release, and the shed counter labeled by call site.
  • internal/metrics/cardinality.go — pins admission_source at 6 with its closed set, and pins the shed's sessions_busy in closedSetValues["outcome"], which takes that label to 26 against its budget of 26 and spends it: the next family reaching for outcome raises both numbers rather than adding a value for free. Both pins land before anything emits the value, because accumulateLabels rejects a gathered value its closed set omits, so a pin arriving after the first emission arrives too late.
  • AGENTS.md: the ten conventions this MR's review asked for with remember:, each landed as its own commit beside the change it came from. Nine are their own section; the tenth corrected a section an earlier round added, splitting the absent-vector reading from the flat-counter one.

Which failures a caller can tell apart

Acquire reads the caller's context, not the derived one, so three outcomes stay distinct:

Condition Returns Books a shed
Wait budget expired, caller still connected ErrUploadSessionsBusy, unwrapped yes
Caller's own context done that error, wrapped no
units past the whole capacity ErrUploadSessionsBusy, at once, no parking no

A disconnect is the dominant abort, so booking it would fill the counter an operator alerts on and render a 503 to a client that is already gone. A request larger than the gate cannot be satisfied at any load, so parking only burns the caller's budget and booking it would read as a full gate.

units <= 0 panics: it reaches the gate only from a composition bug, and a zero-unit acquire would otherwise succeed on a full gate and hand back a gate-issued reservation covering nothing, which the ownership check planned for NewSession accepts as "already admitted".

Tests

internal/storage/upload_admission_internal_test.go, in-package because the gate is unexported. Every case that can park runs inside a testing/synctest bubble, so a budget expiry is an ordered event rather than a race and no acquire that must succeed hangs on a real-time window. This is the first use of testing/synctest in the tree.

Coverage was measured rather than reasoned about: of 28 mutations of upload_admission.go run against go test ./internal/storage/ at 9f0715466, 27 redden. The set was re-enumerated at that commit rather than carried forward, one mutant per decision, constant, emission and ordering in the file, which is why the denominator moved from the 20 an earlier revision of this section reported.

The one survivor is releasing the semaphore before the in-use gauge decrement, green over three runs against a baseline also green over three. That window is visible only to a scrape landing between two statements, no deterministic test can open it, and the comment there says what the ordering buys rather than implying a guard.

Reviewable size

1055 added lines, over the 500 ceiling docs/dev/development-model.md sets, so here is the split rather than a claim that it could not be smaller. Read from git diff --numstat 63d2b8809...c650279bb, so re-run it if the branch moves:

Group Added LOC
Production Go (upload_admission.go 106, metrics.go 86, cardinality.go 17, errors.go 5) 214
Tests (upload_admission_internal_test.go 693, metrics_test.go 25) 718
Conventions (AGENTS.md) 222
Docs (docs/dev/storage.md) 8

62% is test code, 693 lines of it the one gate suite, and the eight acceptance criteria in the plan's Step 2 are what size it: two context arms, cancel-while-parked, the capacity round trip, the double-release no-op, the floor-progress case, the parked-waiter admission, and the budget expiry, plus the value assertions on both gauges and the histogram. The 214 production lines are one primitive and one metric block; splitting them would put the vectors in one MR and their only writer in another, and both cardinality pins have to land with the labels they govern. The AGENTS.md sections are conventions rather than product code, and each is a separate commit, so any one can be reviewed or reverted on its own.

Six plan corrections this step owes

Guardrail 4 stops a step MR editing the plan, the Status table included, so none of the six is fixed on this branch. They land together in one docs(plans) MR off main, opened by the operator, carrying all six plus the empty step-2 row in the plan's Status table, in the shape of !2412 (merged). That MR is not this branch's to open, and the Status row arrives from it rather than from here.

Two move a file or a rationale between steps:

  • docs/dev/storage.md gets rows for the four vectors, because they register from this MR onward and the authority doc otherwise omits them, plus a ## Supporting types and errors row for ErrUploadSessionsBusy on the same argument. The plan's file bullets give that file to the step that wires the acquire, so that step's bullets are correspondingly partly spent. While editing the table, four rows carried pre-rename label names (method for both op and chunk_api, result for cache_result, backend for cdn_backend) alongside blobstore_calls_total, a family with no vector; those are corrected too.
  • The outcome budget comment in internal/metrics/cardinality.go reads 24 distinct values on main, where main's closed set holds 25. This branch pins a 26th value, and the comment now reads 26 against a budget of 26. The later step's bullet justifies itself with "the comment beside that budget says 24", which is stale against either number.

Three were recorded only in a commit body on this branch (0865c5510) and belong here too:

  • Acquire takes a third parameter the plan does not. The plan writes Acquire(ctx, 2) and Acquire(ctx, n); the shipped signature is Acquire(ctx, units, source), because source is what labels the shed counter. The step that wires the acquire therefore has to thread an AdmissionSource through NewSession at its five sites, which changes that step's scope and not only its prose.
  • AdmissionSource has no row in the plan's ## Naming Conventions table. It is a new exported type in internal/storage, declared in internal/storage/metrics.go, and that table lists ErrUploadSessionsBusy and SessionReservation but not this one. A symbol named in prose and absent from that table has no declaring step.
  • The floor-progress case does not guard what three plan passages say it guards. All three say it is what reddens if the two-unit acquire is split into two one-unit ones. TestUploadAdmissionGate_TwoUnitAcquiresAllMakeProgressAtTheFloor calls Acquire(ctx, 2, source) itself, so the split it can catch is one written in the test body. The caller-side split those passages argue from has no production call site until the acquire is wired, so nothing on this branch reddens for it.

One is a ceiling the plan's census does not price:

  • The wait budget's third ceiling is the response write deadline. The plan's ### Two existing semaphores are held while parking on this one prices the wait against rebuildTimeout (30s) and single_flight_wait_timeout, and prices the hold against the per-request read deadline, evidencing that with UploadHandler.setUploadDeadlines alone. Nothing prices the response write deadline. net/http arms that one once per request, when the header is read, from http.Server.WriteTimeout: that is server.timeouts.write, 10s in config.example.yaml, wired through httpserver.Config.WriteTimeout in internal/server/server.go. Past that instant the shed cannot be written at all, so the client gets a connection reset rather than the 503 and Retry-After the counter booked. Only the OCI arms re-arm it: setUploadDeadlines sets read and write, while maven.dispatchHandler.extendReadDeadline and npm.PublishHandler.ServeHTTP set the read deadline only and reach NewSession right after, so on those two arms the park runs under the original 10s. Either the wait's ceiling becomes the minimum of the rebuildTimeout-derived one and server.timeouts.write, or the steps wiring those two arms re-arm the write deadline before the session opens, the way setUploadDeadlines and npm.armInlineBuildBodyDeadline already do. The plan's hold-side paragraph does not already cover this: it prices how long an admitted session may hold a unit, not how long a parked waiter has left to write a response. The two arms are the npm-publish and Maven-publish rows of the ## Stack table.

Deliberate residual

Nothing bounds how long a unit is held. The wait budget bounds the queue; the caller's defer is the entire reclamation mechanism, and the hold-duration histogram is how a leaked slot is told apart from genuine saturation. The step that wires the acquire owns the alert.

Testing

No e2e scenario is added or affected: the gate has no caller, so no route behaviour changes and no request can reach it. The scenario catalogs in docs/testing/ become relevant when the acquire is wired.

Stack

upload-session-concurrency-cap, in merge order. An empty cell means the row is not yet recorded, not that the step has not started.

# Step MR
1 config surface for the cap and the wait
2 admission gate primitive and shed sentinel this MR
3 OCI upload arm and the S12 error contract
4 npm publish shed arm
5 Maven publish shed arm
6 the remote fill's own outcome, and the aggregate
7 OCI remote arm
8 npm remote arm
9 Maven remote arm
10 npm packument rebuild arm
11 acquire and release at the five NewSession sites
12 CancelSession, and the ResumeSession acquire

Depends on: nothing. This step takes capacity and wait as parameters, so it does not wait on Step 1's config surface. Steps 3, 4, 5, 6, 10, 11 and 12 all depend on this one for the gate type and the sentinel.

Plan: docs/plans/2026-09-03-upload-session-concurrency-cap.md

Related to #1035

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading