feat(config): mark the GOMEMLIMIT fallback and cover the cap's OCI, remote fill and Maven arms, part 3 of 3 (upload-session-concurrency-cap plan: 11/12)

Adds the remaining test functions part 1 deferred: the OCI, remote-fill, Maven, storage and config cases. It also lands two things review found missing from the step: both upload-cap boot records now carry gomemlimit_fallback, true when GOMEMLIMIT is unset and the cap's budget came from the 400 MiB fallback, and config.example.yaml now says the cap also covers remote cache fills and npm packument rebuilds.

Part 3 of a three-MR stack. Parts 1 and 2 have merged, so this MR now targets main directly, and with it the stack carries the whole of the work.

The stack, in merge order

# MR State Contents
gate !2881 (merged) merged plan amendment authorising this split
1 !2882 (merged) merged production, docs, forced test edits
2 !2883 (merged) merged storage and npm test functions
3 !2884 (merged) (this one) open OCI, remote fill, Maven, storage and config test functions, plus the GOMEMLIMIT fallback marker and the config.example.yaml sentence

Merge order

handbook!21192, !2881 (merged), !2882 (merged) and !2883 (merged) have all merged, so nothing is left to land ahead of this MR.

No required order with open MRs, but one overlap: !2944 rewrites the header comments of internal/format/maven/upload_stream_test.go and internal/format/npm/publish_stream_integration_test.go. This MR renames the first to upload_stream_testsupport_test.go and had already rewritten its header, so whichever of the two lands second resolves that header. Its publish_stream_integration_test.go hunk does not touch the lines this MR changes there.

Conflict resolution

Parts 1 and 2 merged in their reviewed form, so this branch's local copies of them conflicted with main. The branch takes a merge of main rather than a rebase, so no force-push was needed, and with squash on the merge commit does not reach main. The branch merged main at 3200739f7, and that merge base is what GitLab diffs against. The merge commit 7ba17f8d0 is byte-identical to this part rebased onto 3200739f7 (8efbf95d9, kept on dmeshcharakou/ucap-step-11-part-3-rebased). Against the merge base it differed only in five _test.go files: internal/format/oci/upload_test.go, internal/format/maven/upload_test.go, internal/remote/fetch_test.go, internal/storage/pg_blobstore_test.go and internal/config/storage_test.go. main has moved on since, so a diff against its tip also picks up what landed there afterwards. main at 38225886d also edits three files this MR edits, config.example.yaml, docs/dev/configuration-reference.md and the plan, including the plan's ## Dependencies region, and git merge-tree --write-tree of head bc1b9ce87 with it exits 0, so the merge is clean and needs no resolution.

Part 1's review moved two helpers out of part 1 because only this part's tests call them, so they land here:

  • the Maven release-recording store, session and dispatcher, 296 lines of internal/format/maven/upload_test.go at 7ba17f8d0, which was the one textual conflict. This review has since folded the dispatcher into newStreamDispatcher (internal/format/maven/upload_stream_testsupport_test.go, renamed from upload_stream_test.go because it holds no test function) and moved the store and session into internal/format/maven/upload_release_testsupport_test.go. newUploadDispatcher, which built the same wiring by hand, is gone too, so newStreamDispatcher builds every upload dispatcher in the package;
  • the remote fake's newSessionSources field and its recording in NewSession, which admissionSources reads.

What it covers

OCI drives every enumerated post-admission path a request reaches and pins each path's release by the time it returns, plus a handler panic with the session live, whose body serves its whole payload before it panics. The rows live in internal/format/oci/upload_release_test.go. The one enumerated arm with no row is AppendChunk's own ErrOffsetMismatch, which resolvePatchHeadroom stops any request from reaching. Those rows run over stub.New, whose release frees no real unit, so one more case, TestUploadRelease_RealGateHoldsOneUnitWhileTheSinglePostReads, drives the single POST through storage.NewBlobStore over a real gate and reads the in-use gauge while the body stages.

The remote fill rows live in internal/remote/fetch_admission_test.go. They walk every terminal state a cacheFillTee reaches, the leaderBody wrapper, the vanished-row re-fetch, a panic while holding the leader body, and a FetchResult whose body is neither read to end nor closed. That last case keeps its unit charged, because nothing reclaims a hold. It pins the documented contract rather than a reclamation the gate does not have. It reddens if the release fires before any terminal transition. A finalizer cannot redden it, because the test's own result.Body keeps the tee reachable, and reclaiming the hold belongs to #1384. Those cases run over a fake store that books its own release closure, so TestFetcher_Fetch_Integration_RealGateHoldsOneUnitWhileTheFillBodyIsOpen drives a miss-200 fill through storage.NewBlobStore over a real gate: the in-use gauge reads exactly one more while the returned body is open, and is back once it closes.

Maven covers the deploy's stream, Commit and commit-outcome returns in TestUpload_Deploy_ReleasesAdmissionUnitOnStreamCommitAndOutcomeReturns: two ReadFrom failures, the Commit failure, and writeCommitOutcome's 201, 409 and default 500. Its byte-identical, ErrParentBlobMissing and ErrNotFound arms have no row, but run under the same deferred release. It also adds a Cancel-side probe to the release-ordering harness. Without that probe, moving Maven's defer release() to the top of its cancel closure leaves the suite green, which is the state npm publish was in before part 1 fixed it. The release rows wrap stub.New, whose release frees nothing, so TestUpload_Deploy_RealGateHoldsOneUnitWhileTheBodyStreams drives the deploy PUT through storage.NewBlobStore over a real gate and reads the in-use gauge while the body streams.

All three real-gate cases read the gauge through metrictest.SlotsInUse, build their gate with metrictest.WholeGateWait, and then ask it for its whole capacity in one call through metrictest.RequireWholeGateFree, each with its own message. The OCI and Maven cases read the gauge mid-body through metrictest.InUseSamplingBody. npm's publish release cases, already on main, now make the same ask through metrictest.RequireWholeGateFree and metrictest.WholeGateWait in place of their own copy. storage.RequireEveryUnitFree is the stricter twin internal/storage's own tests keep. It makes the same ask through acquireOK, which also checks that the reservation carries the issuing gate, an unexported field only the storage package can read, and that the in-use gauge rises by the ask. The two names differ so the shared helper does not claim that strength.

Storage pins NewBlobStore's boot guards untagged: a nil gate and the gate's zero value each panic, a nil client panics, and a wired gate is accepted and kept.

Config pins the sub-floor boot warning: a configured cap of 1 boots and warns, naming the value and the floor of 2, and a configured 2 boots with no warning. The derived caps at the driver defaults are pinned on main already, by TestLoad_Storage_UploadSessionCapDerivedFromTheDefaults.

The GOMEMLIMIT fallback marker is the one production change here. With GOMEMLIMIT unset, the cap's budget comes from the 400 MiB fallback, and neither upload-cap boot record said so outright. Both records, derived upload session concurrency cap and upload session concurrency cap set by configuration, now carry gomemlimit_fallback: true when the fallback is in force, false under a real limit. uploadSessionMemoryBudget returns the flag beside the budget, and each record logs the flag from the same call that sized its budget, so the marker cannot drift from the fallback. The Maven merge-budget record reads the same fallback and keeps no marker, as the plan's internal/config/storage.go bullet says. TestLoad_Storage_UploadSessionBootRecordsMarkTheGOMEMLIMITFallback asserts the marker on both records with the limit unset and set, and the max_concurrent_upload_sessions row of docs/dev/configuration-reference.md says both records carry it and what it means. No proto field changes. config.example.yaml gets the one sentence the plan's bullet for it asks for: the cap also covers remote-repository cache fills and npm packument rebuilds, so a read can answer 503 from it.

Size

2,033 reviewable LOC (1,866 added, 167 removed) against the merge base 3200739f7 at head bc1b9ce87, past the 500-LOC ceiling by 1,533. Of that, 1,916 is test code (thirteen _test.go files and the test-only metrictest helper package), 11 is production in internal/config/storage.go, and 106 is docs: 102 in the plan, 2 in config.example.yaml and 2 in docs/dev/configuration-reference.md. By group: OCI 585, Maven 561, remote 494, storage 152, config 112, npm 23, docs 106.

File Added Removed
internal/format/oci/upload_release_test.go 531 0
internal/format/oci/upload_finalize_integration_test.go 53 1
internal/format/maven/upload_test.go 148 69
internal/format/maven/upload_release_testsupport_test.go 276 0
internal/format/maven/upload_stream_testsupport_test.go (renamed from upload_stream_test.go) 24 31
internal/format/maven/upload_concurrent_test.go 2 9
internal/format/maven/metadata_erasure_integration_test.go 1 1
internal/format/npm/publish_stream_integration_test.go 6 17
internal/remote/fetch_admission_test.go 402 0
internal/remote/fetch_test.go 34 1
internal/remote/fetch_integration_test.go 56 1
internal/storage/pg_blobstore_test.go 85 0
internal/storage/metrictest/metrictest.go 64 3
internal/config/storage_test.go 101 0
internal/config/storage.go 7 4
docs/plans/2026-09-03-upload-session-concurrency-cap.md 73 29
config.example.yaml 2 0
docs/dev/configuration-reference.md 1 1

At head 7ba17f8d0 the figure was 1,419, all in the five files the conflict section names. The next review's test commits took it to 1,725: the OCI real-gate case and row edits (5cde5f593, 128e694ec, 902c9e75f), the Maven real-gate case (61aaffd95), the Maven release-support rework (439f49ae4, 6873212dc, e644a0660), and the remote real-gate case (db982a5e0). The round after moved it to 1,726: the dropped-body comment (35177d0e4), the nil-gate panic value (5da513648), the shared resolver helper (f8cefbd09) and the sub-floor record match (7d4146ee1). The fallback-marker commit (27affbacf) moved it to 1,787, and the sampling-body fix (43d6131d9) to 1,785. This round's eleven commits take it to 1,909. Moving the OCI and remote release blocks into files of their own adds each new file's package and import header (8a34a4c34 +21, 4ac629819 +17). Deleting newUploadDispatcher, which is on main, adds 29 removed lines (54a2c3750 +32). The upload_stream_test.go header rewrite adds 25 (dca8f97e7), and the plan corrections add 32 (8efb08541, b01cea2ec). The rest move it by 4 or less. The next round's eight commits take it to 2,013. The plan corrections add 58 (b575619ab). Folding npm's whole-gate ask into metrictest brings internal/format/npm/publish_stream_integration_test.go into the diff (d59d084d8 +25), the stream-harness rename and header fix touch two more Maven files (08c81fa9e +14), and the panic-body pin adds 8 (ce914b507). The rest move it by 2 or less. This round's four commits take it to 2,033: the plan corrections add 9 (a511df015 +7, bc1b9ce87 +2), the metrictest package doc adds 6 (2389a0125), and the Maven fixture doc adds 5 (ff87d058f).

Most removals are edits of lines already on main. In Maven, 439f49ae4 widened newStreamDispatcher to take the store and the upsert-fault flag, e644a0660 rewrote the upload_test.go file header and the uploadResolver and putPrimary docs, 54a2c3750 deleted newUploadDispatcher, and dca8f97e7 rewrote the upload_stream_test.go header, which still said the stream and commit branch returned 501. Before 7ba17f8d0 it was 1,117 against local part 2 commit 334a50ccf, and the step to 1,419 is the two helpers above, which now count here. Splitting further would not help: each helper ships with the only tests that call it, which golangci-lint run --enable-only=unused --build-tags=integration enforces. The marker lands here rather than in an MR of its own because Step 11 is not complete without it. The plan's ### Step 11 bullets now mark part 3's 1,117 at 368126733 superseded and point here for part 3's figure, since every review round moves this head.

Completeness

Parts 1 and 2 merged in their reviewed form, so the stack no longer matches the original unsplit branch file for file, and this description no longer claims it does. Every helper this part's tests call is present at this head, and none it adds goes unused.

This part departs from the plan's split in two places, and ## Plan and spec amendments below records the plan edit for both. The plan's ## Dependencies ruling on Step 11 has parts 2 and 3 carry "no production line and no doc line between them". Part 1 merged without the fallback marker that Step 11's Acceptance and its internal/config/storage.go bullet require, so the marker lands here instead, with its reference row and the config.example.yaml sentence. And the tests span more than five _test.go files, because review moved the release support into files of their own.

Plan and spec amendments

This part corrects the passages of Step 11, and the Step 12 text that relies on them, that its own review proved wrong. No step is added, removed, split or reordered, so no plan-amendment MR is needed, and the merge order is unchanged.

  • ## Dependencies, the Step 11 split ruling (8efb08541, b575619ab): adds that part 3 departs from "no production line and no doc line between them". It carries the fallback marker part 1 merged without, with its production lines, the reference row and the config.example.yaml sentence, and its tests span more than five _test.go files. The ruling now says part 2 shipped that way, and part 3's departure is marked **Amended:**.
  • Step 11's Tests bullet (8efb08541): names internal/format/oci/upload_finalize_integration_test.go and internal/remote/fetch_integration_test.go for the real-gate cases, in place of the untagged internal/format/oci/upload_test.go and internal/remote/fetch_test.go. The untagged files have no Postgres client for NewBlobStore. The bullet also names the two new release-row files, internal/format/oci/upload_release_test.go and internal/remote/fetch_admission_test.go. b575619ab adds the three it still left out: internal/format/maven/upload_release_testsupport_test.go (Create), the Maven stream harness (Modify), and internal/remote/fetch_test.go (Modify). 08c81fa9e records the harness's rename to upload_stream_testsupport_test.go, and bc1b9ce87 names it by that path first.
  • Step 11's Shared seams bullet (b01cea2ec, 8ffed23e5, bc1b9ce87): unused proves where a test function lands only on a lint run that compiles the file. CI's lint sets no build tags, so for an integration-tagged file the run that proves placement is golangci-lint run --enable-only=unused --build-tags=integration; the full tagged run reports other linters' findings in the integration-tagged files CI never lints.
  • Step 11's Shared seams and Forecast bullets (b575619ab, a511df015): part 3's 1,117 against 334a50ccf is marked superseded, and a511df015 also marks it where Shared seams lists the split. It was to be re-derived once part 3 rebased onto part 2's head, and part 3 merged main at 3200739f7 instead, so both bullets point here for its figure.
  • Step 12's Shared seams bullet and the shared-paths paragraph above ### Step 1 (b575619ab): they no longer call parts 2 and 3 test-only. Part 3 carries the marker in internal/config/storage.go and docs/dev/configuration-reference.md, which Step 12 edits too, so a Step 12 branch stacked on a part still open rebases onto that part's tip.

Step 11's ## Status row stays empty in this MR. This plan's rows are batch-filled from a docs(plans) branch, as rows 1 to 10 were (8e63bd37c, 67f6a2467, c032e5463), and the plan's own note under ## Status says step MRs leave the column to that batch. Row 11 gets !2882 (merged), !2883 (merged) and !2884 (merged) in the next batch after this MR merges.

Verification

At head db982a5e0, with heads 7d4146ee1, 27affbacf, 43d6131d9, b01cea2ec, 8ffed23e5 and bc1b9ce87 re-run in the last six bullets:

  • go vet is clean untagged and with -tags=integration over internal/remote, internal/format/maven, internal/format/oci, internal/config and internal/storage/....
  • The unit suites of those packages pass under -race.
  • With -tags=integration: all of internal/remote passes under -race; the Maven ^TestUpload tests pass, including all six rows of TestUpload_Deploy_ReleasesAdmissionUnitOnStreamCommitAndOutcomeReturns and the Maven real-gate case; the OCI ^TestUploadRelease_ tests pass, including the OCI real-gate case.
  • golangci-lint reports 0 issues untagged over the same packages. With --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false it reports none in the ten files; everything it reports sits in other integration-tagged files CI never lints.
  • Mutations: switching the remote fill's admission source to oci_upload reddens all five remote unit cases this MR adds. Booking the remote fill's release in Fetch before it returns reddens the remote real-gate case's open-body reading, and dropping that release reddens its after-close reading and its whole-capacity ask. Moving Maven's defer release() to the top of its cancel closure reddens three rows (commit failure, mid-body read failure, undeclared over-limit body), and they stay green once the Cancel-side probe is removed.
  • At head 7d4146ee1: the internal/remote, internal/storage and internal/config unit suites pass under -race, go vet is clean over them untagged and with -tags=integration, and golangci-lint reports 0 issues untagged over them. Mutations: deleting the gate == nil guard reddens the nil-gate row, and dropping the value field from the sub-floor warning reddens that subtest's value assertion.
  • At head 27affbacf: the internal/config unit suite passes under -race, go vet is clean over it untagged and with -tags=integration, and golangci-lint reports 0 issues untagged over it. Mutations: dropping the marker from the derived record reddens both derived cases, dropping it from the configured record reddens both configured cases, and hardcoding it true or false reddens the two set cases or the two unset cases.
  • At head 43d6131d9: go vet -tags=integration is clean over internal/format/oci, golangci-lint with --build-tags=integration reports 0 issues there, and the OCI real-gate case passes. It also passes with its body wrapped in iotest.OneByteReader.
  • At head b01cea2ec: go vet is clean untagged and with -tags=integration over internal/remote, internal/format/maven, internal/format/oci, internal/config, internal/storage/... and cmd/artifact-registry. The unit suites of internal/remote, internal/format/oci, internal/config and internal/storage/... pass under -race. With -tags=integration and -race, all of internal/remote, the Maven ^TestUpload tests and the OCI ^TestUploadRelease_ tests pass. golangci-lint reports 0 issues untagged over internal/remote, internal/format/maven, internal/format/oci, internal/config, internal/storage and internal/storage/metrictest, and with --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false it reports none in this MR's files. Mutations: setting the budget call's fallback flag to false reddens TestLoad_Storage_UploadSessionBootRecordsMarkTheGOMEMLIMITFallback, and making metrictest.RequireWholeGateFree ask for one unit past the capacity reddens the remote real-gate case.
  • At head 8ffed23e5: go vet is clean untagged and with -tags=integration over internal/remote, internal/format/maven, internal/format/oci, internal/format/npm and internal/storage/.... The unit suites of internal/remote, internal/format/oci and internal/storage/... pass under -race. With -tags=integration and -race, all of internal/remote, the Maven ^TestUpload tests, the OCI ^TestUploadRelease_ tests and the npm ^TestPublishStreamIntegration_ tests pass. golangci-lint reports 0 issues untagged over internal/remote, internal/format/maven, internal/format/oci, internal/format/npm, internal/storage and internal/storage/metrictest. With --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false it reports none on a line this MR changes: the four it reports in this MR's files sit on untouched lines of internal/format/npm/publish_stream_integration_test.go. golangci-lint run --enable-only=unused --build-tags=integration reports 0 issues over those packages plus internal/config and cmd/artifact-registry, and it reports a stray unused helper appended to internal/remote/fetch_integration_test.go, which the untagged run does not. Mutations: reverting panickingBody to a single-shot copy reddens TestUploadRelease_HandlerPanicBooksItsRelease, with 512 of 131,072 bytes served; making metrictest.RequireEveryUnitFree ask for one unit past the capacity reddens the npm hangup case and the OCI real-gate case; and making metrictest.InUseSamplingBody read the gauge one low reddens the OCI and Maven real-gate cases.
  • At head bc1b9ce87: go vet is clean untagged and with -tags=integration over internal/remote, internal/format/maven, internal/format/oci, internal/format/npm and internal/storage/.... With -tags=integration and -race, the remote real-gate case, the Maven ^TestUpload tests, the OCI ^TestUploadRelease_ tests and the npm ^TestPublishStreamIntegration_ tests pass. golangci-lint reports 0 issues untagged over internal/remote, internal/format/maven, internal/format/oci, internal/format/npm, internal/storage and internal/storage/metrictest. With --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false over internal/remote, internal/format/maven, internal/format/oci, internal/format/npm and internal/storage/metrictest, it reports none on a line this MR changes: the four it reports in this MR's files still sit on untouched lines of internal/format/npm/publish_stream_integration_test.go.

Documentation and e2e scenarios

  • docs/user/: no change. It does not document storage.max_concurrent_upload_sessions, GOMEMLIMIT or the boot log. The operator reference for them is docs/dev/configuration-reference.md, updated here.
  • docs/testing/: no scenario added or affected. The rest of this MR is unit and integration test code, and the marker is a boot-log field that no scenario in docs/testing/ reads. The new config test pins it.

Related to #1035

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading