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.MaxBytesReader takes and returns io.ReadCloser, server.UploadBody(r) returns one, and npm installs its body by assigning r.Body, so a bare io.Reader fits 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 ceilingAt is an instant and recomputing it later would place it past the deadline in force.
  • A successful read is n > 0, not err == nil. io.Reader permits (0, nil), so err == nil would 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 that writeBodyFault exists 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 PATCH is one request per chunk and already re-arms, so the absolute window now bites only on monolithic PUT and single POST. The issue text predates this. Step 2 still installs at both OCI wrap sites, so chunked PATCH tightens 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. handlePut and handleSinglePost both stream through appendFinalChunk, which owns the wrap.
  • Five routes arm a read deadline; the plan covers three. OCI manifest PUT keeps 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_000 over 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 the MavenConfig tag 5 Step 3 takes and is not one: theirs lands in ResolutionConfig, and they collide with each other rather than with this. !2266 (merged)'s config.proto hunk is comment-only. !1011 (closed) does add a field, Config.pypi = 17 plus a new PypiConfig message inserted adjacent to ContainerConfig, and it edits config.example.yaml too, 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.yaml has no container: 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 beside container_remote: rather than after maven:, which keeps the hunk clear of Step 3's insert into the maven: block and of !1011 (closed)'s pypi: block.
  • The three format steps share three documents, and the plan says what keeps them mergeable in any order. internal/server/body_size.md lines 97 to 99 and line 113, and the server.timeouts.write row of docs/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.proto is one file but three different messages, and internal/config is 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)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading