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 overgolang.org/x/sync/semaphore.Weighted, its constructor, andAcquire, 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.go—ErrUploadSessionsBusy.internal/storage/metrics.go— the shed'soutcomeconstant, theAdmissionSourcetype 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— pinsadmission_sourceat 6 with its closed set, and pins the shed'ssessions_busyinclosedSetValues["outcome"], which takes that label to 26 against its budget of 26 and spends it: the next family reaching foroutcomeraises both numbers rather than adding a value for free. Both pins land before anything emits the value, becauseaccumulateLabelsrejects 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 withremember:, 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.mdgets rows for the four vectors, because they register from this MR onward and the authority doc otherwise omits them, plus a## Supporting types and errorsrow forErrUploadSessionsBusyon 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 (methodfor bothopandchunk_api,resultforcache_result,backendforcdn_backend) alongsideblobstore_calls_total, a family with no vector; those are corrected too.- The
outcomebudget comment ininternal/metrics/cardinality.goreads 24 distinct values onmain, wheremain'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:
Acquiretakes a third parameter the plan does not. The plan writesAcquire(ctx, 2)andAcquire(ctx, n); the shipped signature isAcquire(ctx, units, source), becausesourceis what labels the shed counter. The step that wires the acquire therefore has to thread anAdmissionSourcethroughNewSessionat its five sites, which changes that step's scope and not only its prose.AdmissionSourcehas no row in the plan's## Naming Conventionstable. It is a new exported type ininternal/storage, declared ininternal/storage/metrics.go, and that table listsErrUploadSessionsBusyandSessionReservationbut 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_TwoUnitAcquiresAllMakeProgressAtTheFloorcallsAcquire(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 oneprices the wait againstrebuildTimeout(30s) andsingle_flight_wait_timeout, and prices the hold against the per-request read deadline, evidencing that withUploadHandler.setUploadDeadlinesalone. Nothing prices the response write deadline.net/httparms that one once per request, when the header is read, fromhttp.Server.WriteTimeout: that isserver.timeouts.write,10sinconfig.example.yaml, wired throughhttpserver.Config.WriteTimeoutininternal/server/server.go. Past that instant the shed cannot be written at all, so the client gets a connection reset rather than the503andRetry-Afterthe counter booked. Only the OCI arms re-arm it:setUploadDeadlinessets read and write, whilemaven.dispatchHandler.extendReadDeadlineandnpm.PublishHandler.ServeHTTPset the read deadline only and reachNewSessionright after, so on those two arms the park runs under the original10s. Either the wait's ceiling becomes the minimum of therebuildTimeout-derived one andserver.timeouts.write, or the steps wiring those two arms re-arm the write deadline before the session opens, the waysetUploadDeadlinesandnpm.armInlineBuildBodyDeadlinealready 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## Stacktable.
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