docs(plans): add the upload inactivity deadline plan
What
#1043 (closed) says the upload read deadline is armed once, before the body read, so it bounds the whole transfer rather than the gap between reads. A client that keeps sending is cut anyway when the window elapses, with its bytes already staged.
This plan turns it into an inactivity window on the three routes that carry artifact-sized bodies.
Shape
Four steps. The re-arming reader lands on its own, then one step per format, and those three are siblings that merge in any order.
| Step | Scope |
|---|---|
| 1 | the re-arming reader in internal/server, with its unit suite and benchmark; no route installs it and no route behavior changes |
| 2 | the container inactivity key and the two OCI blob wrap sites |
| 3 | the Maven inactivity key and the artifact PUT site, which is where #776 (closed) closes |
| 4 | the npm inactivity key and the publish envelope read, once !2259 (merged) and !2228 (merged) free internal/format/npm/publish.go |
Both decisions #1043 (closed) raised are settled in the plan: new *_inactivity_timeout
keys beside the existing ceilings rather than a redefinition, and the write
deadline re-arms alongside the read.
The one number worth arguing with
The three new keys default to 5m. That changes behavior for every
deployment, including one that sets nothing: today an upload that goes twenty
minutes without a byte survives inside its one-hour ceiling, and from its
format's own step it is cut. The existing keys keep their meaning, so a value an operator has
already set does not change under them, but the new default is not a no-op
upgrade and the plan says so rather than leaving it implied.
Five minutes sits far above any ordinary stall and far below every ceiling: one
twelfth of the OCI and Maven ceilings, one sixth of npm's, and five times
nginx's sixty-second client_body_timeout default. If it reads as too tight or
too loose, that belongs here rather than on a step MR.
Notes for review
Measured at d5ee57ffd. Every cited file is byte-identical to 5bc5679d5, the
basis the first revision used, except docs/dev/configuration-reference.md,
whose server.timeouts.write row !2283 (merged) rewrote elsewhere in the same line.
- The wrapper is an
io.ReadCloser, and it sits under the size cap.http.MaxBytesReadertakes and returnsio.ReadCloser,server.UploadBody(r)returns one, and npm installs its body by assigningr.Body, so a bareio.Readerfits nowhere in that chain. - The wrapper arms once at construction, not only after a read. Re-arming
only after a read left the gap before the first byte unbounded: a client that
sent headers and then nothing kept the full one-hour ceiling, which is the
slow-client case ADR-004 asks for this timeout for. The construction arm also
gives the amortization rule a defined starting state, and both arming helpers
now return the instant they armed, because
ceilingAtis an instant and recomputing it later would place it past the deadline in force. - A successful read is
n > 0, noterr == nil.io.Readerpermits(0, nil), soerr == nilwould re-arm on a read that carried nothing, which is the case the window exists to cut. This revisits what 60d5d00d settled; the placement reasoning that commit gave is kept as it stands. - The re-arm is amortized. Unamortized it is one deadline operation per
driver read: about 1.5 million for a blob at
container.blob_max_size, and double that with the write deadline. The wrapper skips the re-arm while the standing deadline is more than half the window away, which costs precision at the cut and the acceptance criteria are written against the resulting range rather than an exact instant. - An inactivity cut is made distinguishable from a client hangup. OCI
already books a fired read deadline as
bodyFaultClientGone, answered with a bare 499 thatwriteBodyFaultexists to keep off the 5XX rate and out of the ERROR log. That is the wrong record for a cut this plan's own default caused, so the wrapper marks it and each site carries the discriminator. No status code moves. - !2223 (merged) landed first and narrows the OCI half. Chunked
PATCHis one request per chunk and already re-arms, so the absolute window now bites only on monolithicPUTand singlePOST. The issue text predates this. Step 2 still installs at both OCI wrap sites, so chunkedPATCHtightens too, and the plan says that is deliberate rather than leaving it a side effect. - There are two OCI wrap sites behind the three arms.
handlePutandhandleSinglePostboth stream throughappendFinalChunk, which owns the wrap. - Five routes arm a read deadline; the plan covers three. OCI manifest
PUTkeeps its absolute deadline on the arithmetic: 4 MB in a minute needs 67 KB/s. npm single-version unpublish keeps its own on scope, not arithmetic, because the arithmetic does not support it:100_000_000over five minutes needs 333 KB/s, five times the manifest figure and above the 278 KB/s the plan itself calls a correctness gap. The residual is named and goes as a note on #1043 (closed). - Seams were checked hunk by hunk. !2289 (merged) and !2255 (merged) each add
resolve_timeout = 5, which looks like a collision with theMavenConfigtag 5 Step 3 takes and is not one: theirs lands inResolutionConfig, and they collide with each other rather than with this. !2266 (merged)'sconfig.protohunk is comment-only. !1011 (closed) does add a field,Config.pypi = 17plus a newPypiConfigmessage inserted adjacent toContainerConfig, and it editsconfig.example.yamltoo, so it is a same-region conflict on two more files than the first revision of this description claimed. No proto tag collides. config.example.yamlhas nocontainer:block. Step 2 creates it, with both the ceiling and the new key, so an operator does not read a gap bound with no ceiling beside it. It goes besidecontainer_remote:rather than aftermaven:, which keeps the hunk clear of Step 3's insert into themaven:block and of !1011 (closed)'spypi:block.- The three format steps share three documents, and the plan says what keeps
them mergeable in any order.
internal/server/body_size.mdlines 97 to 99 and line 113, and theserver.timeouts.writerow ofdocs/dev/configuration-reference.md. The rule is a rebase in landing order with the last to land owning the counts, because that row breaks by count rather than by phrasing.config.protois one file but three different messages, andinternal/configis three different files, so neither is a seam between the steps. - The plan names one accepted duplication. The three sites arm the deadline with three different error postures (OCI 500s, Maven and npm log and proceed), so they construct the shared reader separately instead of sharing one helper. Unifying the postures would be a behavior change nobody asked for. The wrapper's own mid-read arm-failure posture is the fourth, and it is shared, so the wrapper owns it: a failed re-arm never fails the upload.
The same re-arm closes #776 (closed) and the Maven and npm halves of #31. #31 prescribed
the once shape and declined to pick a correction; this plan supersedes that,
and ## Dependencies records the reversal.
Label note: this spans OCI, Maven and npm, and format::* is scoped to one per
MR (labels.md),
so artifact-registry::formats cannot be completed honestly here. The stream
label is artifact-registry::server instead, which is where the reader now
lives.
Related to #1043 (closed)