docs(plans): name the admission source and the forged-reservation panic (!2794) · Merge requests · GitLab.org / ops / Artifact Registry

Why this exists

Step 11 of this plan could not be implemented as written. Both gaps surfaced while authoring its tests, before any test or implementation was committed, and both change the signature Step 11 lands. Neither is a reading ambiguity: the first is the plan requiring an observability property its own stated signature cannot produce.

Verified at 2043070c2be2d89390c6c10316ab326740efd8ee.

Gap 1: the admission source had no route to the store

UploadAdmissionGate.Acquire takes the source positionally with no default:

func (g *uploadAdmissionGate) Acquire(
    ctx context.Context, units int64, source AdmissionSource,
) (*SessionReservation, func(), error)

AdmissionSource and its six values exist only inside internal/storage. git grep -l AdmissionSource -- '*.go' returns four files, all in that package and two of them tests, so nothing carries a source from a caller in.

Step 11 puts the acquire inside NewSession for four of the five sites while naming only a *SessionReservation parameter. With that signature the store cannot tell npm_publish from maven_upload, oci_upload or remote_fill, so those four collapse to one label value. That falsifies two things at once: the six-way admission_source label the shed counter carries ("an unlabeled count cannot say which of those is happening", Step 2's metrics bullet), and the closed set docs/dev/storage.md:782 already publishes, which documents per-source behaviour in detail and states each format's arms have landed.

Resolved: NewSession takes the source as a second new parameter, and Step 12's ResumeSession takes it too.

NewSession(
    ctx context.Context,
    namespaceID, repositoryID uuid.UUID,
    res *SessionReservation,
    source AdmissionSource,
) (Session, func(), error)

A context-carried source was rejected for the same reason the gate itself is not an option: an omission would still compile and book a wrong label, where a parameter makes every omission a build error and lets the compiler enumerate the call sites. internal/storage does have a context-carrier precedent in WithClientIP, so this is a choice between two workable shapes rather than a rule, and the amendment says which and why.

Gap 2: the forged-reservation refusal named no observable

The plan requires NewSession to reject a non-nil reservation the gate did not issue, and ## Naming Conventions repeats it. No sentinel exists for it anywhere in the tree, so the assertion could be require.Panics or require.ErrorIs with nothing in between, and this is the one assertion that closes the fail-open door that passage exists to close.

Resolved: it panics. The condition is reachable only by building the struct in code, which puts it with the non-positive capacity, wait, and unit counts this plan already panics on, and no client input reaches it. It adds no sentinel to ## Naming Conventions and no row to docs/dev/storage.md's error table. An error return would have needed both, plus an arm in every format's mapping that this plan assigns to no step.

Sites touched

Both facts are fixed everywhere they live, not at one anchor:

the A zero reservation means "charge me" passage in ## Research Findings
### The release cannot be booked inside the store, the signature sentence
the ## Approach five-site release summary
Step 11 Scope
Step 11 Files, the internal/storage/blobstore.go bullet, which now carries the signature as a code block
Step 12 Files, the stub bullet and the narrowed-seam bullet
## Naming Conventions: the SessionReservation row, plus a new AdmissionSource row

Also corrected

Step 11's tail forecast said 28 fake NewSession declarations. Re-derived at 2043070c2 it is 36 declarations across 21 files. The plan already asks for this count to be re-derived when the step MR opens; the figure in the forecast is updated so the MR-size estimate is not low.

Step 11's internal/metrics/cardinality.go bullet was stale on all three of its numbers, and the second review round below rewrote it. On main the budget reads "outcome": 26 with the comment beside it saying the budget is spent, sessions_busy is in closedSetValues, and outcomeSessionsBusy is declared. The remaining work in that group is the outcomeForError arm in internal/storage/emit.go, which is still absent, so a shed still books outcome="error" today.

Review round: what the amendments added

Fourteen commits follow the original, each scoped to one review comment. Four came from operator decisions and change what the plan claims:

ADR-003 is now a deviation, not a conformance claim (e8dc102). The plan ran its per-session throughput test on the upload half of ADR-003's :144 projection and declared the download half met on a different argument. One standard now runs on both halves, and both fail on a default S3 install. The "ADR-003 is met and no amendment is owed" conclusion is removed rather than qualified. The per-session figures this bullet first carried were derived against one pod; the third round below re-derives them against the two-replica floor main has carried since 2026-09-07.
The staging soak has a written pass bar (e838918). The soak gates enabling the acquire and is the one instrument that can raise the 0.5 budget fraction, but it stated no load, no duration, no memory threshold and no shed bound, so whether it passed was an opinion. It now carries three bars and a rule for moving 0.5, and names the build it runs against: the Step 11 branch, before that MR merges. The third round below rewrote all three bars and both directions of that rule.
The wait budget's default and clamp ceiling are pinned (677be86). 5s and 15s, the second being half rebuildTimeout's 30 s, with the derivation stated once and the numbers carried into Step 1's Scope, its config.example.yaml and configuration-reference bullets, and two acceptance cases. Both are read at origin/main rather than at the plan's pin, because they arrived with Step 1's own MR.
The shipped S3 default's residual has an owner (fbc43d8d). The plan stated the over-admission at the S3 default but left nobody owning the shipped default, and implied #1171's buffer clamp would lift the cap. Four passages now say plainly that this plan reaches neither term that would close it: no step lowers defaultS3ChunkSize, and no step changes the S3 per-slot reservation. So on a default self-managed S3 install the floor binds and Step 1's boot warning is the expected steady state rather than a transient. The revisit is recorded on #1171, which already owns the S3 sizing work and is now assigned; no new issue is filed. Re-derived at origin/main: defaultS3ChunkSize = 64 << 20 (internal/config/storage.go:63) and GOMEMLIMIT: "400MiB" (.runway/values.yaml:74), so 129 MiB per slot against a 200 MiB budget, two slots at 258 MiB, 129% of the budget and 64.5% of GOMEMLIMIT. What #1171's clamp would do to the derived cap is left out: no reservation multiple for a clamped S3 writer is chosen anywhere, so no such figure re-derives.

Second review round

Eleven more commits, each scoped to one review comment, none of them changing what the plan claims. All are internal-consistency fixes:

!2514 (merged) recorded as merged in Step 8's Shared seams (494fe8c0e)
!2268 (merged) recorded as merged in the Dependencies overlap (14438b53e)
the container-remote fill paragraph's cap edit assigned to Step 11, matching both step bullets (cf2b2a71e)
both stale method-to-op row corrections deleted, since Step 2 already renamed the row (880ce3f76)
Step 11's cardinality.go bullet re-derived against origin/main (921016cc2)
Steps 11 and 12 Shared seams completed from their own Files lists (7a393ae1e)
the run-recipe SKILL.md added to Step 11's Files (6973128a7)
Step 1's release-note obligation carried in its Scope (fefe247c8)
the Dependencies 500 claim scoped to the arms that answer it (9a73b6a53)
"the two gauges above" pointed at Step 2, which adds them (1a0bf31ed)
the soak bar's provenance re-pointed at ## Context (50fbf92db)

Third review round

The five comments left open above were decided by the plan's owner and implemented, one scoped commit each.

The :144 check and the soak are re-derived at two replicas (66c0056cd). .runway/fairway.yaml has carried minInstances: 2 since c7a777185 (2026-09-07), later than this plan's own bases, so the one-pod premise under both was stale. The service-wide ceiling is the replica floor times one pod's cap: 56 sessions on GCS and 4 on S3. Re-derived per-session rates: upload 4 to 18 MB/s on GCS and 50 to 250 MB/s on S3; download 18 to 54 MB/s and 250 to 750 MB/s. The deviation stands, and the two halves now fail differently rather than being averaged. The soak targets one pod's gate rather than the service, because 56 offered to the service is 28 per pod, exactly each pod's cap, so no shed can appear.
The soak's memory bar is recalibrated from the measured decomposition (333dfda23). The 225 MiB bar was the 200 MiB staging budget plus the idle decayed baseline, and 056a4ad94 already peaked at 294.19 MiB while holding 25 sessions, under the cap of 28, so no run at the measured profile could clear it. Decomposed: ~25 MiB non-staging resident, 111 MiB the gap to the 182.77 MiB working set that the scavenger returns lazily, and ~158 MiB actually staged, which is 6.3 MiB across each of the 25 sessions against a 7 MiB reservation. The bar is 200 + 25 + 111 = 336 MiB, 84% of GOMEMLIMIT. That decomposition also retires the plan's reading of the whole 269 MiB arrival as staging, and with it the claim that the ceiling sits above 0.5; the fraction itself is unchanged. The push payload profile is pinned so the bar is reproducible.
The soak gained a fail-direction remedy (021e99e90). Only the raise direction was legislated, so a failed bar left Step 11 blocked with nothing written. A failed bar is now answered by two levers in a fixed order: lower uploadSessionBudgetFraction first, then raise the per-slot reservation multiple once the fraction bottoms out at 0.4375, where the 400 MiB fallback gives a 175 MiB budget and a GCS cap of exactly 25. A plan amendment is the vehicle either way, never a soak-time config override.
The shed-rate bar is a turnover criterion, and the soak has two load points (9ba6f1a8f). The shed-to-attempts ratio tracked mean hold duration rather than queue health, and "admission attempts" was undefined against the retries the shed counter books individually. The bar is now that every one of the 56 offered clients is admitted at least once within any 4-minute window, which is 112 admissions at the pinned 60-second hold. The same bullet had asked for a zero shed counter at the cap and a 2x load in one run; the soak is now run A at cap level for the zero-shed check and run B at 2x for the shed check, with the memory bar read on A.
The load-clearance test now runs on the S3 row (0995edf21). The plan validated the reservation multiple against dockerd's load on the GCS row only. On S3 it fails at every admissible chunk_size: at the shipped 64 MiB the cap is 2, so one ordinary docker push offers five concurrent layer uploads and three take a 503; at the 5 MiB minimum the cap is 18, still under the 25 sessions five pushes hold. 5 MiB is a hard lower bound (s3ChunkSizeMin aliases s3.MinChunkSize), so the chunk_size lever narrows the gap and cannot close it. The availability consequence is carried to the three surfaces that state the memory one: the derived-cap section, the sentence Step 1's floor-binding boot warning is owed, and Step 11's configuration-reference rows. The fourth round below moves that owed sentence off #1035 and into Step 11. Per the owner's decision this is document-only: defaultS3ChunkSize stays at 64 MiB, the floor stays at 2, and the revisit stays on #1171 beside the memory one.

Merge gate: a handbook ADR-003 amendment

The plan now states a deviation from ADR-003, so an amendment to that ADR is a merge gate on this MR, not a follow-up to track. It lands in the handbook decisions text, which is the authority; the local docs/adr/ copy is a CI-synced mirror and cannot carry it. Do not merge this MR before the amendment MR has merged, and filing an issue for it does not discharge the gate.

The amendment MR is open: handbook!21192, driven by Dzmitry (Dima) Meshcharakou (@dmeshcharakou), this plan's author and an S13 co-DRI. It qualifies ADR-003's Storage throughput projection to read as service-wide and bounded by the session count a pod admits, and records that a default S3 install at the two-replica floor meets neither stated range. That MR merging is what this gate waits on. It is opened in the handbook repository, not here.

The fourth round below widens the gate inside the plan: it now binds the steps that enact the deviation as well as the MR that states it, because step MRs land while a plan amendment is still open, so Steps 11 and 12 could otherwise have shipped the deviating behavior unamended. ## Dependencies names the owner and the amendment MR beside that gate, which was the one review comment the fourth round left open.

Fourth review round

Nine comments. The first eight took one scoped commit each; three of those change what the plan claims and the rest are internal-consistency fixes. The ninth, last below, took two.

The soak's load is sized in sessions, not pushes (9799e86). Both soak bars computed offered sessions as equal to offered pushes, but one docker push offers five concurrent layer uploads at dockerd's default, which this plan's own load-clearance test already records. Driving 28 real clients would have presented 140 sessions against 28 slots, shed on run A, and sent the failed-bar rule to lower the budget fraction on a spurious verdict. A fourth pinned load property now fixes one session per offered push, and the runs read 28 and 56 offered sessions.
The floor binds the derivation, not configuration (d8d6cb3). The plan said the floor keeps a capacity-1 gate unreachable from configuration. The merged Step 1 loader honors any positive configured value and refuses only a non-positive one, so max_concurrent_upload_sessions: 1 boots and the npm packument rebuild is refused from then on. Step 11's configuration-reference bullet gains that as an as-is row. The fifth round below corrects the mechanism this bullet first stated: the refusal is immediate, not a park that spends the wait budget.
The run-recipe duty has its operating trigger (18e0bbe). Both sites justified it by claiming a driver.sh boot fails without the two new keys. It does not: the cap carries explicit field presence, the wait decodes empty-string-as-default, and driver.sh boots today on a generated config carrying neither. Following the old bullet would have written both keys into that generated config, which the recipe's own comment forbids. The trigger is that Step 11 changes dispatch by turning the acquire on.
The ADR-003 gate binds the enacting steps (18960ae), not only the MR that states the deviation. Nine step MRs of this plan have already merged while this amendment stayed open.
The soak's failed-bar remedy is classified by failure shape (3e4328c). Both levers lowered the budget, so an availability-shaped failure was answered by shrinking the gate that had just refused a population the budget had room for. That case now raises the fraction, by at most the headroom the failed run's own memory series left under the 336 MiB bar, and re-soaks at the new derived cap.
The rebuild's gate gets its own identity case (4306f8e). Step 11's Acceptance pinned one-gate-per-process for the store and the factory seam only, so nothing reddened if SetRebuildAdmissionGate stopped receiving the process's instance.
The one-sided boot warning is closed in Step 11 (0479020) rather than deferred to #1035. The two deferrals were not on the same terms: the re-derive sentence waits on the comment-caps gate, while the warning's note is a log field value with no such cap and its deferral had no trigger at all.
The 500-today enumeration counts the OCI upload arm (361feef), at both occurrences. OCI's three CreateSession sites and its resumeSession helper are a third hosted path, not a fourth remote one.
The ADR-003 amendment gate names its driver and its MR (3ff02b3, bc81e0f). The gate named no person, issue, or MR to drive it, so it could have stalled Steps 11 and 12 or been waived quietly, against the plan's own standard of a named owner or issue on every other residual. ## Dependencies names @dmeshcharakou, this plan's author and an S13 co-DRI, as the owner who drives it, and both passages that state the gate cite handbook!21192: the ## Spec Reference bullet that declares the amendment owed and the ## Dependencies bullet that holds it.

Nothing from this round is left open. The gate itself stays open until handbook!21192 merges.

Fifth review round

Six comments, one scoped commit each. None changes what the plan builds. Two change what it claims about shipped behavior, and the sixth changes what it claims about ADR-003 conformance.

The over-capacity refusal is immediate, not a park (22cb1dd). Three passages said a configured cap of 1 parks every npm rebuild for the whole upload_session_wait_timeout and then sheds. The shipped gate does not: Acquire short-circuits ahead of the semaphore and units > g.capacity answers ErrUploadSessionsBusy at once (internal/storage/upload_admission.go:66), booking nothing on upload_session_admission_sheds_total, which docs/dev/storage.md already records as "answered at once without waiting" and "stays off this counter". Step 11's configuration-reference bullet now also states the signature an operator sees, so the row is not read as load: immediate rebuild 503s and the rebuild WARN, with the shed counter flat. The passage that motivates the floor is deliberately unchanged, because it reasons in the one-unit-per-NewSession world where a capacity-1 acquire does fit and does park.
The wait ceiling prices both of a rebuild's parks (d0e281e). The derivation priced one. A rebuild has two sequential park sites inside the one rebuildTimeout, the two-unit pre-acquire and the dist-tags session's own unit, so the worst case is twice the wait: 10s at the shipped 5s, leaving two thirds rather than five sixths, and the whole 30s at the 15s ceiling, leaving nothing. Recorded as a stated residual rather than by lowering uploadSessionWaitCeiling, which shipped with Step 1's !2471 and no step of this plan touches. The passage also gives the bound read as a sum, 7.5s.
A nil reservation charges; the zero value panics (f209640). "nil or zero means charge me" contradicted both the plan's own panic rule and Step 2's shipped doc comment, since a *SessionReservation's zero value is non-nil and unissued. "or zero" is dropped at all three sites the fact lives at, including the ## Research Findings passage whose lead sentence carried it and which the blobstore.go bullet cites by name.
Re-soaks re-derive their loads from the amended cap (012b01b). The remedy loop ordered a re-soak against bars stating run A as 28 sessions and run B over 56 clients, both GCS-default arithmetic, so a remedy lowering the fraction to a cap of 25 to 27 made run A shed by construction and the loop could not exit. Each re-soak now takes run A from the amended build's own boot-log cap, run B at twice it, and the 112 admissions at four times it. The 4-minute window stays fixed, and the passage says why: it falls out of the pinned 60-second hold and run B's doubling, so it holds at any cap.
A sub-floor configured cap warns at boot (e841ce1). The hazard was documented as as-is behavior whose only remedy was a configuration reference an operator who has already misconfigured is not reading. Step 11's internal/config/storage.go bullet now carries a second warning naming the configured value, the floor of 2, and the consequence. The bullet settles why it is a distinct condition rather than a field on the floor-binding warning, why it lands in Step 11, and what Acceptance owes it.
The GCS download half of :144 is rendered, and it passes (6efc1ed43). The passage gave that half its numbers, 18 to 54 MB/s per session over 56 sessions, and then rendered no verdict, while both S3 halves carried explicit fail verdicts. Per the owner's decision it passes, on the same basis the GCS upload half passes on: that per-session rate is one a single client transfer reaches. Both ends re-derived at 6efc1ed43: minInstances: 2 (.runway/fairway.yaml:31) times the derived cap of 28 is 56 sessions, and 1 to 3 GB/s service-wide over 56 sessions is 17.86 to 53.57 MB/s. Both GCS verdicts are now labelled a judgement rather than a measurement, so the two halves share one evidential standard as well as one numeric standard, and the passage states what overturns each: a single-session GCS download below 54 MB/s, or a single-session GCS upload below 18 MB/s. The S3 verdicts are unchanged, and the passage now states why the two configurations split: 250 to 750 MB/s per session is a rate no single session delivers, while 18 to 54 MB/s is one a single stream reaches. The posture is carried to the two other places it is stated, the ## Spec Reference deviation summary and the ## Dependencies merge-gate bullet. The comment's second half follows from that verdict: because the GCS default passes, handbook!21192's recording clause does not have to widen past the default S3 install, so its URL and the plan's citation are unchanged.

Nothing from this round is left open.

Sixth review round

Two comments, one scoped commit each. Neither changes what the plan builds. Both close a gap between a fact the fifth round landed and the surface that has to carry it.

Step 11's Acceptance takes the sub-floor boot-warning case (3804f7d4f). e841ce136 added the warning and its internal/config/storage.go Files bullet promised the case, but Acceptance had none and Tests named no internal/config file, so the warning shipped priced from a Files bullet: a refactor could drop it, or either of the two fields it must name, with nothing reddening. Acceptance now asserts both fields on the record, the configured value and the floor of 2, and carries its own negative in the shape Step 1 gives the floor-binding warning, so the pair reddens on a warning deleted outright and on one that fires on every boot. Tests names the host, internal/config/storage_test.go, which Step 1 already names for the floor-binding warning and which already reads boot records through captureBootLogs.
The two-park wait sum reaches the operator-facing row (c1f06932b). d0e281e40 put the residual in ## Research Findings and no step assigned it to a surface an operator reads, so the shipped upload_session_wait_timeout row still framed the 15s ceiling as half the npm rebuild timeout alone, which is the per-park figure that passage warns against doubling by hand. Step 11's configuration-reference bullet now owes that row one sentence: at a configured wait of W a rebuild can spend 2W of its 30-second render budget parked, the stay-short constraint binds on the sum, and 7.5s or less is the sum-safe figure. Step 1's own configuration-reference bullet names the wait row as one it keeps a sentence back on, beside the enforcement rows it already hands over, so the two bullets agree on who writes what.