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_statusfromoutcome.Statusand logged "relaying the upstream's own refusal" for every status, including the ones this service mints.upstream_statusis 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 theupstreamStatusparameter: that parameter is0on the credentials arm, whose401is a real upstream verdict. The minted class is carried on the decision asRemoteFetchOutcome.Minted, written only byrespondMinted, 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 thanSelfDecided: 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 isoutcome.Status, this pod's own503, and it books no upstream status either:streamAndFillreturns a zeroFetchResultbeside each sentinel it mints (internal/remote/fetch.go:469), so the arm is reached withupstream_status0 and the field stays absent. Both shed tests assert that absence. Thecase upstreamAnsweredarm is kept for the status a widenedinternal/remotecontract would carry out, and has no coverage until then: deleting it leaves the fullocisuite green. - The envelope told the puller "the upstream registry could not be reached"
about an upstream that answered
200.remote_list.goalready argues against that substitution.remoteFillShedMessageis 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 takesmsgBlobFillShedon the samestorage.ErrUploadSessionsBusypredicate the envelope beside it already used. Keying it onoutcome.Mintedinstead was measured and rejected:respondUnavailableroutes throughrespondMinted, 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 mintUNAVAILABLEand book noupstream_status, so the message is the only field between them. commitUpstreamManifestpassescontentAddressedasfalserather thanread.byDigest. S16 serves a shed from cache on the blob read alone, "never on a manifest or a tag read", sobyDigesthere left aRemoteActionServeCacheFallbackthatwriteRefusedFillrenders as a500througherrRemoteFillUnrenderableOutcome. Nothing changes today, the fullocisuite staying green, becausefillCacheMisspasseshasCache=falseand the stale-tag revalidation is the onetruecaller, on a tag. The latent500is what closes, andremote_errors.mddrops thebyDigestclause 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.goandremote_errors_test.go. The other eleven, fromgit diff --name-only 78ce1396c...HEADat14719a345, areinternal/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.mdanddocs/dev/observability.md(the cache-events member list). The extra test files are what Step 7's own acceptance asks for, a rendereddockerandocipositive hit, which the decision-level suite cannot give. The LOC table above accounts for all 13. - Step 7's
Shared seams: nonestill holds, now ruled on the 13-file set rather than the two it was derived from. Checked at14719a345: of the 71 other open MRs, !2630 (merged) and !2641 (merged) touchdocs/specs/S16-container-remote.md, !1598, !2645 (merged) and !2647 (merged) touchdocs/dev/observability.md, and !2622 (merged) touchesinternal/remote/errors.md. None touchesdocs/dev/storage.md, the fourinternal/format/ociproduction files, orremote_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.mdand.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_totaloutcome books the shed.oci_remote_upstream_responses_totaldeclines it deliberately, asdocs/dev/observability.mdrecords, where the Maven and npm families bookcache_error. Left as-is rather than diverged in this step. The cache-events counter does book it, asmissbeside the503. - The same British spelling this round fixed survives outside this diff.
internal/format/maven/upload_internal_test.go:493carries 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.