OCI remote by-digest fill can truncate a 200 when the armed write deadline fires mid-copy
Problem
A cold by-digest manifest pull arms a response write deadline sized to the
upstream budgets plus a grace, then streams the payload to the client after the
cache fill commits. A client slow enough to still be reading when that deadline
fires gets a truncated 200: the status line and part of the body have
already gone out, so there is no way to answer 503 with Retry-After. Docker
then fails digest verification and retries immediately — the re-hammer the
deadline exists to prevent.
At 512 KiB/s a 4 MB manifest needs roughly 8s. A stall in the database or object store past the grace lands the same way, on a connection that already holds a committed row.
A warm hit does not arm the deadline at all, so a cold pull carries a truncation risk a warm one does not. That asymmetry is the part worth keeping in mind: the failure is only reachable on the first pull of a reference, which is also the pull least likely to be retried by a warm cache.
What the armed deadline does and does not bound
Worth writing out, because the margin is smaller than "a 5s grace against a client-paced copy" suggests, and the arithmetic is what a fix has to be sized against.
WithManifestFill arms token_exchange_timeout + request_total_timeout + remoteReadResponseGrace once, before the fetch starts — 10s + 30s + 5s = 45s
at the config.example.yaml defaults. What runs inside that window after the
upstream is done is not covered by the two configured bounds in the sum:
storage.Session.Commitruns on a context with no deadline at all.internal/remote/fetch.godetaches the session withcontext.WithoutCancel, and its own comment says a datastore or object-store stall "parks every in-flight fill on its terminal operation with nothing to free it". That gap is #396.CacheStore.UpsertCacheEntrythen runs underinternal/remote's owndetachedCacheOpTimeout, a separate 30s that is not part of the armed sum. For the container slice that call also containsRemoteCacheStore.classifyCommittedManifest's object-store GET plus a read of up tocontainer.manifest_max_payload— a leg the Maven and npm callers of the same seam do not have.- Only then come the second
LookupRow, the serve'sOpenBlob, and the copy the client paces.
So the grace is not protecting a copy against 5s of slack; it is the last 5s of a 45s window that an unbounded commit and a separate 30s budget can already have consumed. A fill can stay inside every configured bound and still pass the armed deadline.
The truncation is indistinguishable from client churn
There is no signal that says a deadline fired. armRemoteReadResponseDeadline
returns nothing, and a copy that dies mid-stream is reported by
remoteReadSubject.logStreamFailure as a Warn whose own comment names "a client
disconnecting mid-download" as the common cause. OCI has no counter here
either.
npm solved both halves on the path this one was modelled on: its
armRemoteReadResponseDeadline returns the armed instant so the tarball route
can attribute a kill, and it emits
gitlab_artifact_registry_npm_remote_tarball_deadline_kills_total. Whatever
shape the fix takes, it should leave the kill countable — otherwise the fix
cannot be shown to have worked, and a regression reads as ordinary churn.
Why this is not a correctness bug
Nothing is corrupted. The row and the object are committed and correct, so the client's retry is served from the cache and succeeds. The defect is the wasted round trip and the immediate retry, not a bad artifact.
Not the same as a departed client
A client that hangs up before the serve's storage legs run is a separate case and is already answered: those legs check for a closed request context and stamp 499 rather than minting a 500. That guard cannot reach the truncation described here, because by then the status and headers are committed and there is nothing left to change about the response.
Candidate fixes
- Re-arm the write deadline after the commit, sized to the remaining tail rather than to the upstream budget that has already been spent.
- Or widen the armed sum to include the bound the post-fetch cache write
actually runs under. This is only a partial answer:
Session.Commitahead of it has no bound to add, so no sum can be complete until #396 lands. - Either way, make a deadline kill attributable — return the armed instant and count the kill, the way npm's tarball route does.
- Cover it with an end-to-end test that drives a fill to the budget edge against a throttled client, which nothing does today.
Sibling
#776 is the same defect class on the Maven upload path — a deadline armed once and never re-armed, so a long transfer loses its status line. #31 is the S01 follow-up that first noted upload handlers arming write deadlines alongside read deadlines. A fix here should read those two first; the shape is shared even though the paths are not.