chore(oci): answer the remote fill's staging shed per the read's own path (upload-session-concurrency-cap plan: 7/12)

What

A container remote read whose cache fill is refused a staging session now answers 503 UNAVAILABLE with a Retry-After, instead of the fail-closed 500 INTERNAL that storage.ErrUploadSessionsBusy took before. This is the OCI arm of the upload-session concurrency cap.

One read is answered rather than refused: a blob read holding a cached copy under the digest it asked for. That path is content-addressed, so the copy is the requested layer and serving it takes none of the admission capacity the refusal protects.

internal/remote's fill lets that sentinel travel bare out of streamAndFill (Step 6, !2463 (merged)) precisely so a format arm can render it. This step is that arm.

Why 503 and not 500

The refusal is this pod's staging capacity, not a fault. 500 would say the service failed when it is shedding load on purpose, and it gives the client no reason to come back.

The arm is reached only after the upstream answered 200: streamAndFill has one caller, guarded on resp.StatusCode == http.StatusOK, and the session open sits inside it.

Whether a cached copy can answer instead turns on the read's own path, not on whether a copy exists. remote.FallbackEligible excludes the sentinel, so the arm never reaches the shared cache route and decides for itself in shedFallbackOrUnavailable: a copy under a content-addressed path answers the read, anything else is refused. So a stale-tag revalidation keeps the 503 — a tag names no bytes, so its copy is one the upstream has just superseded, and serving it would report replaced bytes rather than this pod's inability to stage the new ones — and so does a miss, which has nothing cached at all.

It is deliberately not cacheFallbackOrUnavailable, whose shape it borrows. That route mints through respondUnavailable, which carries no hint, so the refusal leg would drop the drawn window below and send the health-sweep interval instead.

Retry-After

Drawn per response from a jittered window, 28 to 42 seconds, centered on remoteFillShedRetryAfterBase. Not derived from the gate's wait budget: that budget is deliberately short while an admitted hold runs far longer, so a budget-derived hint would name a second at which no slot has freed. The magnitude is priced on internal/format/npm's inlineBuildTimeoutRetryAfter (rebuildTimeout + 5s = 35s) rather than shared with it, because the two hints wait on different work.

It is not priced on the hold this gate sees, and the doc now says so. The wait is on an admission unit, which a container fill holds for the whole artifact transfer — minutes for a large layer — while the magnitude comes from a packument rebuild. Getting it too short is not cheap either: streamAndFill closes the upstream response body when the session open is refused (internal/remote/fetch.go:464), so each premature return re-issues a complete upstream GET. Pricing it on the container blob-fill budget or a high percentile of upload_session_hold_duration_seconds is recorded on #1241, which wires the acquire at the session sites.

Both renderers, not just the blob arm

The decision is rendered twice, by writeBlobMissOutcome and by writeRefusedFill (which also serves stale-tag revalidation). Two defects in the manifest writer are fixed here rather than left for the arm's first production traffic:

  • It booked upstream_status from outcome.Status and logged "relaying the upstream's own refusal" for every status, including the ones this service mints. upstream_status is the field an on-call greps to decide whether the upstream is down, so a staging-saturation incident read as an upstream outage. The gate is the cause, not the upstreamStatus parameter: that parameter is 0 on the credentials arm, whose 401 is a real upstream verdict. The minted class is carried on the decision as RemoteFetchOutcome.Minted, written only by respondMinted, rather than derived in the renderer from a list of sentinels that would drift from the mapping every time an arm is added. It is wider than SelfDecided: the coalescing and shed arms mint a status too, but carry a hint of their own instead of the probe sweep's, which is what both renderers now state in place of a clause that had gone false. What a minted line withholds is outcome.Status, this pod's own 503, and it books no upstream status either: streamAndFill returns a zero FetchResult beside each sentinel it mints (internal/remote/fetch.go:469), so the arm is reached with upstream_status 0 and the field stays absent. Both shed tests assert that absence. The case upstreamAnswered arm is kept for the status a widened internal/remote contract would carry out, and has no coverage until then: deleting it leaves the full oci suite green.
  • The envelope told the puller "the upstream registry could not be reached" about an upstream that answered 200. remote_list.go already argues against that substitution. remoteFillShedMessage is the shed's own wording.
  • The blob arm's log line took msgBlobFillUnservable, "the upstream fill produced no servable payload", for every refusal it renders, so a fill this pod's own staging gate closed was filed against the upstream. It now takes msgBlobFillShed on the same storage.ErrUploadSessionsBusy predicate the envelope beside it already used. Keying it on outcome.Minted instead was measured and rejected: respondUnavailable routes through respondMinted, so every transport failure and unusable upstream status is minted too, and that split relabels eight of those subtests' genuine upstream refusals as decided on this side. A shed and an upstream outage both mint UNAVAILABLE and book no upstream_status, so the message is the only field between them.
  • commitUpstreamManifest passes contentAddressed as false rather than read.byDigest. S16 serves a shed from cache on the blob read alone, "never on a manifest or a tag read", so byDigest here left a RemoteActionServeCacheFallback that writeRefusedFill renders as a 500 through errRemoteFillUnrenderableOutcome. Nothing changes today, the full oci suite staying green, because fillCacheMiss passes hasCache=false and the stale-tag revalidation is the one true caller, on a tag. The latent 500 is what closes, and remote_errors.md drops the byDigest clause the S16 row contradicted.

Spec coverage

Acceptance criterion (plan Step 7) Test
Shed answers 503 UNAVAILABLE TestMapFetchOutcome_StagingShed_IsRetryableWhereNoCachedCopyNamesTheBytes
Retry-After asserted as bounds, not a literal TestMapFetchOutcome_StagingShedRetryAfterCarriesTheJitter, assertStagingShedRetryAfter
Hint survives the renderer's relay TestMapFetchOutcome_StagingShedRetryAfterSurvivesTheRelay
Positive hit on docker and oci TestRemoteBlobFill_StagingShed_ShedsOnEveryContainerFormat
A warm cache under a content-addressed path answers the read TestMapFetchOutcome_StagingShed_ServesACachedCopyTheReadsPathNames, TestRemoteBlobFill_StagingShed_IsAnsweredFromAWarmCache
A warm cache under a tag does not TestRemoteManifestRevalidate_StagingShed_IsMintedAndServesNoStaleCopy
Minted set on every minting arm, clear on every relayed status TestMapFetchOutcome_MintedIsWiderThanSelfDecidedAndOnlyOn503, TestMapFetchOutcome_MintedWithAHintOfItsOwn, TestMapFetchOutcome_MintedIsClearOnAStatusTheUpstreamReached
Fail-closed 500 still reachable TestMapFetchOutcome_StagingShedLeavesTheFailClosedInternalReachable
Manifest and revalidation arms render it as minted TestRemoteManifestFill_StagingShed_IsMintedRatherThanRelayed, TestRemoteManifestRevalidate_StagingShed_IsMintedAndServesNoStaleCopy
The renderer's minted line holds for every sentinel the bit covers, not just the shed TestRemoteManifestFill_CoalescingFailure_IsMintedRatherThanRelayed
The blob arm's shed line is not filed against the upstream TestRemoteBlobFill_StagingShed_IsLoggedUnderItsOwnMessage

The two manifest tests were mutation-checked: reverting the minted-class gate reddens both, and reverting the envelope override reddens all three rendered cases. The staging-shed guard was mutation-checked the same way: dropping either half of hasCache && contentAddressed reddens the suite, as does dropping outcome.Minted = true.

Re-measured at 14719a345, the renderer's own two lines part company. Restoring the relay line for every refusal reddens all three rendered cases. Deleting the minted arm's upstream_status reddens nothing in the package, for the zero-status reason above; an earlier revision of this description claimed that deletion reddens both shed tests, and it does not. The blob arm's new shed message reddens TestRemoteBlobFill_StagingShed_IsLoggedUnderItsOwnMessage on both mutations, deleting the arm and inverting its predicate.

Derivation basis

Every figure below is re-derived at 14719a345, this MR's head after the four review-fix commits of this round: the open-MR rescan and its overlap verdicts, the 13-file list, the LOC split, the shared-seams ruling and the chore classification.

The branch sits directly on main at 78ce1396c, which is !2560 (merged)'s own merge commit (step 8), carrying that step's 10e64cdac as its second parent. main has since moved past it to fa7c77de0, so every figure below is stamped at this branch's head and its own merge base, not at main's current tip. The branch still merges into fa7c77de0 cleanly, checked with git merge-tree, and the MR reports no conflicts, so only the base is behind and no further rebase is owed.

The previous basis, a935189f2, is still an ancestor but sits six commits behind this head, so every figure is re-stamped here rather than carried. Three older stamps stay dropped rather than corrected, because none of those commits is on the branch any more: fd2cd27fc, which the last rebase replaced, 61d0d32dd, which the previous one replaced, and c73c0b4c3, which an earlier history rewrite discarded.

Reviewable LOC: 969, past the 500 line

Split by file group, added lines, from git diff --numstat 78ce1396c...HEAD at 14719a345. The 13-file set, the 969 total and every row below were re-derived at that head rather than carried over. The file set is unchanged from last round; this round's four commits moved the two Go rows and the relocation row:

LOC Group
650 tests (remote_shed_test.go 274, remote_errors_test.go 230, remote_errors_selfdecided_test.go 145, remote_relay_internal_test.go 1)
179 a doc relocation, not new material: remote_errors.md +179, against 135 lines deleted from remote_errors.go, 130 of them comment lines
124 production Go (remote_errors.go 62, remote_manifest.go 32, remote_relay.go 13, remote_blob.go 17)
16 docs prose (internal/remote/errors.md 6, S16 5, docs/dev/observability.md 4, docs/dev/storage.md 1)

Splitting would not help. The relocation is forced and indivisible: MapFetchOutcome's doc comment states the classification order the new arm belongs to, and on main that comment is 85 lines (79 counted against the cap, which skips bare // separators) against an exported cap of 3, so it cannot be corrected in place: check-comment-caps.sh holds a block to its cap as soon as a diff touches it and offers no waiver. docs/dev/go-style.md prescribes a sidecar, and this package already keeps five others. Of the production Go, 70 added lines are neither a comment nor blank.

Merge order and open-MR overlap

!2507 (merged) (step 3), !2509 (merged) (step 9), !2513 (merged) (step 10), !2560 (merged) (step 8) and !2077 (merged) have all merged, and this branch sits on top of all five: 78ce1396c, its merge base with main, is !2560 (merged)'s own merge commit, carrying step 8's 10e64cdac as its second parent, and step 3's 6166ed4bf, step 9's 3411f6e59, step 10's c445318c2 and !2077 (merged)'s 0541936a2 are all ancestors of it. So the docs/dev/storage.md row now carries main's frame, step 8's npm remote-arm rewrite included, plus the clauses this branch claims, and !2077 (merged)'s S16 additions sit in the base.

Step 8 merging ahead of this step is what made the rebase conflict: it rewrote the same upload_session_admission_sheds_total row. It is the third sibling of this plan whose rewrite of that row this branch has had to compose against, after step 9 and step 10. The resolution rule was the same as the last two rounds, take main's row whole and port only this branch's delta onto it, but this time the merge was not symmetric.

Step 8 landed npm's remote arm, so main's row now reports three npm arms and names npm.writeRemoteUploadSessionShed and npm.virtualUploadSessionShed as written but not yet reachable. That falsified a clause this branch carried, so main's npm text was taken whole and the branch's npm clause was deleted rather than merged. Two sentences are gone from the row, and both are absent at 14719a345: this branch's "npm's own remote arm has not either, so a remote_fill shed on npm stays counter-only until it does", which is false now that the arm has landed, and main's own "While OCI has landed neither of its two arms", which this branch falsifies. What the branch contributes to that row now is the container arm through oci.mapRemoteFetchFailure, the cached-copy exemption, and OCI's two upload arms, which this round corrected from unlanded to landed: oci.UploadHandler.writeSessionOpenError answers the sentinel 503 with a drawn Retry-After on the create, mount-fallback and resume paths, so oci_upload and oci_resume carry both halves rather than the counter alone. It claims nothing about npm.

Three commits on the branch touch that row now, 7aaadafd5, 8c69c3db2 and this round's 4109bbedb. One message was amended, 8c69c3db2, which no longer claims the npm metrics-only note and now names the main sentence it replaces and the true half it keeps. One commit was dropped, the one previously titled docs(storage): compose the shed row onto main's npm-arm rewrite: its whole content was composing this branch's clause onto an older main row, one file and one line and nothing else, and main now carries that composition itself, so replaying it would have put the false npm clause back. That is the commit count going from 23 to 22 at the rebase; this round's four fixes bring it to 28. Nothing was squashed, and 28 of 28 report %G? = G.

Re-scanned at 14719a345 against all 71 other open merge requests, by intersecting each one's changed-file list with this branch's 13; every diff resolved, none unchecked. Six overlap, and the set has turned over almost whole since last round: !2542 (merged), !2545 (merged) and !2496 (merged) have left it, and five have opened into it. git merge-tree --write-tree was run for each of the six against main at fa7c77de0 and against this branch's head:

MR Overlap vs main vs this branch
!1598 (636-document-buffered-counter-dropped) docs/dev/observability.md conflicts conflicts, same file
!2622 (merged) (s13-resolution-budget) internal/remote/errors.md clean clean
!2630 (merged) (s04a-per-format-credential-columns-step-5) docs/specs/S16-container-remote.md conflicts, in 14 files clean
!2641 (merged) (s16-amend-credential-read-stage) docs/specs/S16-container-remote.md clean clean
!2645 (merged) (1271-listing-organization-gate) docs/dev/observability.md clean clean
!2647 (merged) (s04a-per-format-credential-columns-step-7) docs/dev/observability.md clean clean

This branch introduces no conflict. Only !1598 conflicts with it, in the one file it already conflicts with main in, so it owes a rebase whatever this branch does and that rebase resolves the same path. !2630 (merged) conflicts with main in 14 files, the overlap file among them, and is clean against this branch, so its rebase is likewise owed to main rather than to this step. No pipeline reports either column.

No open MR touches the shed-counter row any more, !2560 (merged) having merged, so merge order is now free of it. While none does, keeping that row's arm-state sentences correct is this branch's own job; once another sibling opens against it, whoever lands second re-reads those sentences instead of concatenating both halves. This round is why: each sentence names which formats have landed an arm, and step 8 wrote its npm text while this branch was unmerged, so the resolution had to delete this branch's npm clause rather than keep it beside step 8's.

Deviations and residuals

  • The plan's Step 7 lists two files; this branch changes 13. Step 7 names remote_errors.go and remote_errors_test.go. The other eleven, from git diff --name-only 78ce1396c...HEAD at 14719a345, are internal/format/oci/remote_errors.md (new, 179 lines, the relocated classification order), remote_blob.go, remote_manifest.go, remote_relay.go, remote_shed_test.go (new), remote_errors_selfdecided_test.go, remote_relay_internal_test.go, internal/remote/errors.md, docs/specs/S16-container-remote.md (the error-cases row for the new answer), docs/dev/storage.md and docs/dev/observability.md (the cache-events member list). The extra test files are what Step 7's own acceptance asks for, a rendered docker and oci positive hit, which the decision-level suite cannot give. The LOC table above accounts for all 13.
  • Step 7's Shared seams: none still holds, now ruled on the 13-file set rather than the two it was derived from. Checked at 14719a345: of the 71 other open MRs, !2630 (merged) and !2641 (merged) touch docs/specs/S16-container-remote.md, !1598, !2645 (merged) and !2647 (merged) touch docs/dev/observability.md, and !2622 (merged) touches internal/remote/errors.md. None touches docs/dev/storage.md, the four internal/format/oci production files, or remote_errors.md. Those overlaps are the ones the table above rules on, so the widened file set is a disclosure gap in the plan's derivation rather than a conflict.
  • The gate has no caller yet. At 14719a345, git grep '\.Acquire(' -- '*.go' finds one non-test reference, internal/storage/upload_admission.go:75, inside the gate's own method, so this arm is inert until Step 11 wires the session sites. Conformance tests are not re-run for that reason. The unrestricted grep also hits two Markdown files, docs/dev/writing-discipline.md and .claude/skills/golang-pro/references/concurrency.md, which are prose rather than call sites.
  • The over-capacity refusal is not covered. The gate raises the same sentinel for a reservation larger than the whole capacity, which no retry clears. It cannot fire at the remote-fill site, which asks for one unit, and #1241 owns keeping it unreachable or giving it its own answer.
  • The shed does not shed upstream load. The gate is consulted at the session open, past the upstream request, so a refused fill has already paid a round trip. Moving the acquire ahead of the fetch is what would change that, and #1035 owns it.
  • No OCI upstream_responses_total outcome books the shed. oci_remote_upstream_responses_total declines it deliberately, as docs/dev/observability.md records, where the Maven and npm families book cache_error. Left as-is rather than diverged in this step. The cache-events counter does book it, as miss beside the 503.
  • The same British spelling this round fixed survives outside this diff. internal/format/maven/upload_internal_test.go:493 carries the sentence "the base stopped being the centre" in another package, untouched by this branch, so it is left to a sweep that owns the whole repo.

type::maintenance, so the related-issue and e2e-catalog guardrails do not apply, and the title stays chore rather than becoming fix. What that rests on: uploadAdmissionGate.Acquire has no caller outside internal/storage, so nothing returns storage.ErrUploadSessionsBusy in production and no client receives a different answer when this merges. Step 11 wires the acquire, and that is the step whose merge makes every answer here reachable. Re-checked at 14719a345.

The stack

Merge order has not been step order, and does not need to be. Steps 3 (!2507 (merged)), 8 (!2560 (merged)), 9 (!2509 (merged)) and 10 (!2513 (merged)) all merged ahead of this one, and step 8's is the merge this round's rebase resolved. What a later step owes is a textual resolution of the shared docs/dev/storage.md row, not a position in the queue.

This is step 7 of 12, which is the row count of the plan's Status table at 14719a345, re-counted there rather than carried over. The plan file is byte-identical to main's copy, both at the merge base 78ce1396c and at main's current tip fa7c77de0.

# Step MR
1 config surface for the cap and the wait !2471 (merged) (merged)
2 admission gate primitive and shed sentinel !2408 (merged) (merged)
3 OCI upload arm and the S12 error contract !2507 (merged) (merged)
4 npm publish shed arm !2465 (merged) (merged)
5 Maven publish shed arm !2462 (merged) (merged)
6 the remote fill's own outcome, and the aggregate !2463 (merged) (merged)
7 OCI remote arm this MR
8 npm remote arm !2560 (merged) (merged)
9 Maven remote arm !2509 (merged) (merged)
10 npm packument rebuild arm !2513 (merged) (merged)
11 acquire and release at the five NewSession sites not opened
12 CancelSession, and the ResumeSession acquire not opened

Depends on Step 6 (!2463 (merged)), merged.

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading