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.goat7ba17f8d0, which was the one textual conflict. This review has since folded the dispatcher intonewStreamDispatcher(internal/format/maven/upload_stream_testsupport_test.go, renamed fromupload_stream_test.gobecause it holds no test function) and moved the store and session intointernal/format/maven/upload_release_testsupport_test.go.newUploadDispatcher, which built the same wiring by hand, is gone too, sonewStreamDispatcherbuilds every upload dispatcher in the package; - the remote fake's
newSessionSourcesfield and its recording inNewSession, whichadmissionSourcesreads.
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 theconfig.example.yamlsentence, and its tests span more than five_test.gofiles. The ruling now says part 2 shipped that way, and part 3's departure is marked**Amended:**.- Step 11's Tests bullet (
8efb08541): namesinternal/format/oci/upload_finalize_integration_test.goandinternal/remote/fetch_integration_test.gofor the real-gate cases, in place of the untaggedinternal/format/oci/upload_test.goandinternal/remote/fetch_test.go. The untagged files have no Postgres client forNewBlobStore. The bullet also names the two new release-row files,internal/format/oci/upload_release_test.goandinternal/remote/fetch_admission_test.go.b575619abadds the three it still left out:internal/format/maven/upload_release_testsupport_test.go(Create), the Maven stream harness (Modify), andinternal/remote/fetch_test.go(Modify).08c81fa9erecords the harness's rename toupload_stream_testsupport_test.go, andbc1b9ce87names it by that path first. - Step 11's Shared seams bullet (
b01cea2ec,8ffed23e5,bc1b9ce87):unusedproves 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 isgolangci-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 against334a50ccfis marked superseded, anda511df015also 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 mergedmainat3200739f7instead, 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 ininternal/config/storage.goanddocs/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 vetis clean untagged and with-tags=integrationoverinternal/remote,internal/format/maven,internal/format/oci,internal/configandinternal/storage/....- The unit suites of those packages pass under
-race. - With
-tags=integration: all ofinternal/remotepasses under-race; the Maven^TestUploadtests pass, including all six rows ofTestUpload_Deploy_ReleasesAdmissionUnitOnStreamCommitAndOutcomeReturnsand 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=falseit 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_uploadreddens all five remote unit cases this MR adds. Booking the remote fill's release inFetchbefore 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'sdefer 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 theCancel-side probe is removed. - At head
7d4146ee1: theinternal/remote,internal/storageandinternal/configunit suites pass under-race,go vetis clean over them untagged and with-tags=integration, and golangci-lint reports 0 issues untagged over them. Mutations: deleting thegate == nilguard reddens the nil-gate row, and dropping the value field from the sub-floor warning reddens that subtest's value assertion. - At head
27affbacf: theinternal/configunit suite passes under-race,go vetis 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=integrationis clean overinternal/format/oci, golangci-lint with--build-tags=integrationreports 0 issues there, and the OCI real-gate case passes. It also passes with its body wrapped iniotest.OneByteReader. - At head
b01cea2ec:go vetis clean untagged and with-tags=integrationoverinternal/remote,internal/format/maven,internal/format/oci,internal/config,internal/storage/...andcmd/artifact-registry. The unit suites ofinternal/remote,internal/format/oci,internal/configandinternal/storage/...pass under-race. With-tags=integrationand-race, all ofinternal/remote, the Maven^TestUploadtests and the OCI^TestUploadRelease_tests pass. golangci-lint reports 0 issues untagged overinternal/remote,internal/format/maven,internal/format/oci,internal/config,internal/storageandinternal/storage/metrictest, and with--build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=falseit reports none in this MR's files. Mutations: setting the budget call's fallback flag to false reddensTestLoad_Storage_UploadSessionBootRecordsMarkTheGOMEMLIMITFallback, and makingmetrictest.RequireWholeGateFreeask for one unit past the capacity reddens the remote real-gate case. - At head
8ffed23e5:go vetis clean untagged and with-tags=integrationoverinternal/remote,internal/format/maven,internal/format/oci,internal/format/npmandinternal/storage/.... The unit suites ofinternal/remote,internal/format/ociandinternal/storage/...pass under-race. With-tags=integrationand-race, all ofinternal/remote, the Maven^TestUploadtests, the OCI^TestUploadRelease_tests and the npm^TestPublishStreamIntegration_tests pass. golangci-lint reports 0 issues untagged overinternal/remote,internal/format/maven,internal/format/oci,internal/format/npm,internal/storageandinternal/storage/metrictest. With--build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=falseit reports none on a line this MR changes: the four it reports in this MR's files sit on untouched lines ofinternal/format/npm/publish_stream_integration_test.go.golangci-lint run --enable-only=unused --build-tags=integrationreports 0 issues over those packages plusinternal/configandcmd/artifact-registry, and it reports a stray unused helper appended tointernal/remote/fetch_integration_test.go, which the untagged run does not. Mutations: revertingpanickingBodyto a single-shot copy reddensTestUploadRelease_HandlerPanicBooksItsRelease, with 512 of 131,072 bytes served; makingmetrictest.RequireEveryUnitFreeask for one unit past the capacity reddens the npm hangup case and the OCI real-gate case; and makingmetrictest.InUseSamplingBodyread the gauge one low reddens the OCI and Maven real-gate cases. - At head
bc1b9ce87:go vetis clean untagged and with-tags=integrationoverinternal/remote,internal/format/maven,internal/format/oci,internal/format/npmandinternal/storage/.... With-tags=integrationand-race, the remote real-gate case, the Maven^TestUploadtests, the OCI^TestUploadRelease_tests and the npm^TestPublishStreamIntegration_tests pass. golangci-lint reports 0 issues untagged overinternal/remote,internal/format/maven,internal/format/oci,internal/format/npm,internal/storageandinternal/storage/metrictest. With--build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=falseoverinternal/remote,internal/format/maven,internal/format/oci,internal/format/npmandinternal/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 ofinternal/format/npm/publish_stream_integration_test.go.
Documentation and e2e scenarios
docs/user/: no change. It does not documentstorage.max_concurrent_upload_sessions,GOMEMLIMITor the boot log. The operator reference for them isdocs/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 indocs/testing/reads. The new config test pins it.
Related to #1035