feat(oci): coalesce and cap container remote fills, compose virtual reads (S32 plan: 14/19)

This is S32 Step 14, the composition root. It builds the container virtual resolution at boot and puts the standalone container remote fill behind the shared per-repository flight registry. Standalone kind=2 container reads now coalesce, stop at the per-repository fill cap, and can answer 503 once the fill wait expires. Both virtual read routes still answer their interim 501: Steps 15 and 16 swap the handlers in.

Merge order. This MR targets main, and main is merged into this branch at f613aa170 (a merge, not a rebase, because Step 15 is stacked on this branch). Three open MRs overlap it:

  • !2947 (merged) (Maven virtual composition root) adds the same remoteAuditLogSink.RecordUpstreamExclusion. The method is byte-identical to !2947 (merged)'s at bd1ea55e5, so the text conflict in remote_audit_sink.go has a single answer. Whichever MR merges second collapses the S30 and S32 rows in docs/dev/observability.md into one, resolves .claude/skills/run-artifact-registry/SKILL.md, and settles the union placement of remote.UpstreamExclusionAuditSink and the auditSinkWithoutExclusion test.
  • !2856 (merged) conflicts in .claude/skills/run-artifact-registry/SKILL.md. Once its container upstream association routes land, seed_container_virtual's comment that no route writes the upstream list goes stale, so the MR that lands second updates it.
  • !2944 conflicts in docs/specs/S16-container-remote.md.

Each of the three merges cleanly into main, so whichever lands second resolves the conflict.

What changes

Composition (the plan's Step 14):

  • wire_oci_virtual.go builds one flight registry and one upstream authenticator per boot, shared by the standalone and virtual paths. It also builds the resolver over the production audit sink, and the upstream assembly over the shared upstreamhttp Doer.
  • VirtualPosition.RemoteRepository carries a bound remote position's row, and ociVirtualWiring.ReadCaches builds a winner's read cache from it, the two surfaces Steps 15 and 16 fill virtualServeRead.rows from.
  • A bad resolution knob fails the boot instead of silently serving the interim 501.
  • remoteAuditLogSink.RecordUpstreamExclusion records each excluded virtual position as virtual repository upstream exclusion, with position_index (the 0-based list index, never the stored position) and resolution_phase.

Behavior that changes on existing kind=2 container reads, because they now fetch through the shared flight:

  • Concurrent cold reads of one path coalesce onto one upstream GET, and fills cap at max_concurrent_fills_per_repo per pod.
  • A read queued past the cap, or behind a slow leader, answers 503 with Retry-After: 1 once single_flight_wait_timeout (default 5m) expires.
  • A follower whose leader leaves re-issues its fetch once. A follower that has written nothing when it learns of the loss mid-stream answers 503 with Retry-After: 1 instead of 500. A follower already streaming the layer gets a truncated response.
  • The response write deadline is armed before the fetch, with the wait term, and re-armed once the fetch returns. The second arm is remote.FillTailBudget, 75s at the defaults, so a client that stops reading holds its connection for the tail, not for 5m45s.
  • Coalesced followers book no upstream-response or fill samples. oci_remote_proxy records carry coalesced.
  • A new counter, oci_remote_fill_wait_expiries_total{fill_wait="slot"|"follower"}, shows fill-slot saturation.
  • A follower that gets the leader's committed blob (remote.FetchResult.CommittedRead) closes it unread and answers from the row, as a cache hit does. A plain GET follower in redirect mode therefore answers 307, and the manifest arm no longer reads the committed manifest twice.

Plan and spec amendments

  • The plan's Step 14 entry does not list the committed-read skip or the injected mint constructors (ociRemoteMints). These came from a branch review.
  • The exclusion audit record keeps the plan's virtual repository upstream exclusion message. Its keys, position_index and resolution_phase, come from neither the spec nor the plan, and !2947 (merged) writes the same ones, so one audit query covers container and Maven virtual.
  • remote.SingleFlight.SetBeforeFollowerWait moves from export_test.go into production code, and oci.FlightRegistryDeps.BeforeFollowerWait installs it. Both are documented as test-only, so the leader-loss integration tests can park followers on a signal instead of a sleep.
  • S13, S15 and S16 are restated where they still described the container fill as uncoalesced: the #752 (closed) gates on S16's coalescing criteria, the request builder called unhoistable, the follower-model widening, coalesced as not emitted, and Container answering 500 on an abandoned fill. S16 §Metrics gains oci_remote_fill_wait_expiries_total.
  • S16's Follow-ups bullet for #596 (closed) is dropped on purpose. #596 (closed)'s work, a request builder that holds only its authenticator and position, landed with Step 8 (internal/format/oci/remote_requestbuilder.go is identical on main), so the issue can be closed.
  • The composition builder is named buildOCIVirtualSlots, not the plan's buildOciVirtualSlots: Go initialism casing only.
  • The probe/resolve ordering check stays in virtual.NewResolver, so the boot, not config load, refuses a bad pair. This follows the placement from c0f0e5d4b.

Size

3,889 changed lines against main, over the 500-line guideline (guardrail 18). By file group:

Group Lines
Go tests 2,479
Go source 924
Run recipe (driver.sh, SKILL.md) 220
Docs and config 214
Specs 52

Splitting would not help review. The composition and the behavior it changes on the standalone arms have to land together, or the flight exists with nothing in front of it. Tests are about two thirds of the diff, most of it integration cases against a real database.

Test evidence

  • go test for cmd/artifact-registry, internal/format/oci/..., internal/remote/..., internal/virtual/..., internal/metrics/... and internal/config/...: pass, and cmd/artifact-registry also passes under -race.
  • go test -tags=integration ./cmd/artifact-registry/ -run 'TestWireOCI|TestBuildOCI|TestRemoteAuditLogSink': pass. The abandoned-leader and canceled-leader cases fail against the pre-fix code, and the two leader-loss cases passed three runs in a row once parked on follower joins.
  • golangci-lint, untagged and with --build-tags=integration: clean on every file in the diff. scripts/ci/check-comment-caps.sh: OK.
  • driver.sh smoke from a fresh run dir after merging main at f613aa170: 80 passed, 0 failed, including the six virtual-route interim-501 checks.
  • Conformance: .conformance-oci does not exercise kind=2 or kind=1 reads. The integration cases above cover the coalescing behavior instead.
  • The run-artifact-registry edits are here because guardrail 19 and the plan's Files entry require them in this MR. skill-benchmark results have not been run yet.

Scenario and user docs

  • docs/testing/e2e/oci.md gains e2e.oci.remote.blob-single-flight and e2e.oci.remote.blob-fill-cap-wait, because composing the flight changes standalone remote behavior.
  • docs/user/troubleshooting.md names the fill-wait 503 as a second cause of a 503 from a remote repository, and says a pull already receiving a layer gets a truncated response when the shared fetch's client disconnects.

Related to #291 (closed)

Closes #752 (closed)

🤖 Generated with Claude Code

Edited by Sylvia Shen

Merge request reports

Loading
Loading