Set Retry-After from the probe-sweep interval on every proxy 503
Problem
remote.RetryAfterSeconds derives the value a 503 for an unavailable upstream carries in its Retry-After header: health_check.scheduled_interval rounded up to the next whole second, never below 1, so 300 under the default 5m. It lives in internal/remote/retryafter.go and merged in !1678 (merged), as item 3 of #609 (closed).
When this issue was filed nothing called it, so the first proxy path to render one of the covered 503s would pick its own number. The npm remote read routes have since taken it. No other path has: each one either has no covered 503 to render yet, or renders one from a value of its own. Two such values sit in internal/format/oci and internal/format/maven today.
Proposal
Every 503 a proxy path answers for an unavailable upstream takes its value from remote.RetryAfterSeconds, applied to the configured health_check.scheduled_interval. The rule covers the verdicts behind remote.ErrUpstreamUnavailable, remote.ErrAllUpstreamsUnhealthy and remote.ErrRemoteUnhealthy, per S13's Retry-After on an unavailable upstream.
The derivation is done. What each path still needs is the configured interval reachable at the point it writes the header, and no second helper deriving it again. That wiring is what this issue tracks.
Blocked by
This issue closes only when four other slices ship, so it is a tracking umbrella rather than a unit of work. The linked blockers, and which item each one gates:
| Item | Blocked by | Why |
|---|---|---|
| 2. Container remote fill | #288 | The fill that renders the first covered 503 is an S16 step, not a change this issue can make on its own. |
| 4. npm virtual mapping | #290 | No npm virtual slice exists: internal/format/npm/handler.go answers kind=1 with a blanket notImplemented, so there is no mapping to give a value to. |
| 5. Container virtual | #291 | Blocked in turn by #239. docs/specs/ holds no S32-*.md, and docs/specs/README.md lists S32 as Planned. |
| 5. Maven virtual | #289 | Blocked in turn by #284. S30 has no spec file at all, and docs/specs/README.md lists it as Planned. The furthest-out item here. |
Items 1, 3 and 6 are not blocked and can proceed in any order. Two of them contend with open merge requests on the files they touch, which is a rebase to sequence rather than a blocker: item 1 with #820 and with the Maven remote cache-fill accounting work, and item 3 with the open S16 wave on internal/format/oci/remote_relay.go and internal/format/oci/remote_list.go. Item 6's spec file also carries an open edit, at a hunk far from the ## Follow-ups entry.
Already landed
Measured at f50fb5ed. Symbols rather than line numbers, so the claims stay checkable by grep.
- npm remote (S15). All three read routes take the value:
internal/format/npm/remote_packument.go,remote_disttags.goandremote_tarball.goeach build theirremoteReadPolicywithremote.RetryAfterSeconds(deps.HealthCheckInterval), andwriteRemoteProxyEnvelopeininternal/format/npm/remote_read_errors.gostamps the header on a503only. The interval reaches them fromcmd/artifact-registry/wire.gothroughwire_npm_remote.go. Asserted in the construction tests (internal/format/npm/remote_packument_internal_test.go,remote_disttags_internal_test.go), the route tests, and the composition-root boot testcmd/artifact-registry/wire_npm_remote_boot_integration_test.go. - The npm interim value is gone.
remoteProxyRetryAfter = 5 * time.Secondis no longer anywhere in the tree; the renderer takes the seconds as a parameter instead. - First correction to the S13 follow-up entry, in !1708 (merged). Item 6 below is the second pass it now needs.
What still needs to be done
1. Maven remote (S14): collapse the duplicate helper
upstreamRetryAfterSeconds in internal/format/maven/remote_artifact.go re-derives what remote.RetryAfterSeconds already derives (ceiling to the whole second, floor of 1), with a different formula and the same result. Maven's behavior is therefore already correct, which makes this a de-duplication rather than a fix, and it is the item most likely to be mistaken for done.
Call sites: remote_artifact.go's relayed-value fallback and writeArtifactFillUnavailable, plus the matching pair in internal/format/maven/remote_metadata.go. The interval already reaches all four: cmd/artifact-registry/wire.go passes HealthCheck.ScheduledInterval through wire_root_dispatcher.go and wire_maven.go onto maven.Deps.UpstreamRetryAfter, held as the handler's upstreamRetryAfter. Note that Maven applies its helper at render time on every 503, where npm computes once at construction; either is fine, but the shapes differ, so this is not a copy of the npm diff.
Removing the helper also removes TestUpstreamRetryAfterSecondsRounding in internal/format/maven/remote_artifact_dispatch_test.go, which duplicates internal/remote/retryafter_test.go case for case.
Overlap to settle before opening an MR. #820 rewrites the same four call sites to bound the relayed upstream value, and it is a security issue, so its MR goes to the security mirror rather than here. Whichever change lands second rebases onto the first, and no pipeline reports the collision.
2. Container remote (S16): the fill arm's unavailable 503
No OCI path renders a covered 503 yet. The two wired read arms serve from cache and answer an interim 501 on anything else (writeRemoteReadFillNotImplemented, reached from internal/format/oci/remote_serve.go and remote_blob_serve.go), and internal/format/oci/remote_stub.go still answers 501 for the unfilled tag-list and referrers slots.
The decision layer is written and has no production caller: MapFetchOutcome and respondUnavailable in internal/format/oci/remote_errors.go deliberately leave UpstreamRetryAfter empty for a renderer to supply. Nothing in internal/format/oci reads health_check.scheduled_interval, so the interval is not yet wired into the package at all.
!1894 (merged) wires the first container remote blob fill. While it is open, no covered 503 is reachable in OCI; once it merges, that fill's unavailable arm is the first one, so it is the MR that owes both the interval wiring and the header assertion. Blocked by #288.
3. Container: split defaultRemoteRetryAfter
internal/format/oci/remote_relay.go holds defaultRemoteRetryAfter = "5". It has two kinds of caller, and only one of them is this issue's:
- In scope.
writeRemoteReadUnavailablein the same file, andwriteRemoteListUnavailableandwriteRemoteListUnusableResponseininternal/format/oci/remote_list.go, write it on a503for an upstream that produced nothing to serve. That is the covered verdict, and the value should come from the interval. - Out of scope.
remoteRetryAfterin the same file falls back to it when a relayed upstream status carried noRetry-After, or one in neither RFC 9110 form. That is the relayed case under Out of scope below, which #687 reconciles.
So the fix is to split the constant, not to replace it: a shared rename would drag the relayed fallback into a rule that does not cover it. The doc comment's "a floor rather than an estimate" rationale belongs to the relayed half.
None of the in-scope writers is reachable from production today: remoteReadSubject.writeUnavailable has no caller, and the two list routes are unwired. This is a hardcode to replace before a caller arrives, not a shipped defect, and it is the OCI twin of the npm remoteProxyRetryAfter that item 2 of Already landed removed.
4. npm virtual (S31): the resolution error mapping
internal/format/npm/handler.go answers kind=1 with a blanket notImplemented, so there is no mapping to give a value to. This item opens when the npm virtual slice does. Blocked by #290.
5. Container virtual (S32) and the Maven virtual paths
Neither slice exists. internal/format/maven/handler.go answers kind=1 through writeNotImplemented in internal/format/maven/remote_stub.go, and OCI has no virtual dispatch. Same shape as item 4: the slice's own MR carries the value and the assertion. Blocked by #291 for the container half and #289 for the Maven half.
remote.ErrAllUpstreamsUnhealthy has no HTTP mapping anywhere in production. It is raised in internal/virtual/resolve.go and documented in internal/remote/errors.go, and no format package matches it. It is one of the three verdicts this rule covers, and it is reachable only from a virtual path, so whichever of items 4 and 5 lands first owes it a mapping and a Retry-After assertion, not just the two verdicts a remote path sees. A virtual slice that maps only ErrUpstreamUnavailable leaves this item open.
6. The S13 ## Follow-ups entry
The entry in docs/specs/S13-virtual-remote-foundation.md that names this work is deleted by the last of the items above. Before then it needs a second correction, on top of the one !1708 (merged) made, because two of its claims no longer hold:
- "no handler calls it" is falsified by the three npm remote routes in item 1 of Already landed.
- "no path reaches ... the blocked Fetch path on an unhealthy remote" is falsified by npm's tarball health gate in
internal/format/npm/remote_tarball.go, which feeds theErrRemoteUnhealthyarm ofinternal/format/npm/remote_read_errors.go.
Still accurate in that entry, and to be kept: the Maven-local-helper sentence, "No virtual-resolution path sets the header", and "no path reaches the AllUpstreamsUnhealthy verdict". That correction is its own docs MR and does not wait for any path above.
Not a gap
A 503 mapped from remote.ErrLeaderCanceled or remote.ErrSingleFlightTimeout carries Retry-After: 1, per S13's #### Retry-After on a coalescing failure. No probe has to run before that condition can clear, so the sweep interval does not apply. npm's remoteCoalescingRetryAfterSeconds and the literal "1" in internal/format/maven/remote_errors.go are that rule rather than a violation of this one. #825 (closed) tracks the npm remote tarball route answering the sweep interval on that condition instead of 1.
Out of scope
A 503 that relays a status the upstream itself produced. S13, S15, S16 and S31 disagree on what such a response carries, and remote.RetryAfterSeconds's doc comment records the disagreement rather than settling it. Reconciling the four specs is #687. OCI's remoteRetryAfter in internal/format/oci/remote_relay.go is that relay path (RFC 9110 grammar check, one-hour clamp), and #820 covers Maven's, which relays unbounded.
Scope note
Each item lands with the slice or fill-handler MR that renders the 503, not as a separate sweep, and asserts the header value in that MR's own tests. #609 (closed) states the same rule. Items 1, 3 and 6 are the exceptions: no new path is involved, so each is its own MR.
Related to #609 (closed).