feat(maven): charge and recompute the remote cache version's size

What this delivers

maven_remote_versions.size_bytes now has both of the writers ADR 007 asks for: the cache fill charges it, and the cached file's own delete recomputes it.

On the fill side, a committed Maven remote cache fill derives the version's byte delta from the entry's size and the facts its upsert returns. It applies that delta through MavenRemoteVersionStore.AddMavenRemoteVersionSizeBytes, off the request path, under a buffered label of its own. The delta is deduplicated within the version, which is a narrower set than the repository set. A digest that a sibling version of the same cached package already holds is new to this version, and it charges the blob's whole length.

On the removal side, MavenRemoteEvictor.DeleteMavenRemoteFile locks the cached version row, deletes the file row, and then recomputes the column from the rows that survived, inside that same transaction. That recompute replaces the stored value rather than moving it, so it repairs a drifted counter in both directions.

Two gaps in datastore.MavenRemoteUpsertResult left the column without a caller. Its membership facts were repository-scoped. mavenRemoteRepositoryHoldsBlobStmt names no maven_remote_version_id, so a digest a sibling version already holds reads as held. ADR 007 deduplicates within the version, so that reading is the wrong one for this counter. upsertCacheEntry also computed the maven_remote_versions row id for its parent lock and its two SQL builders, then dropped it at return res, nil. committedRemoteFillDeltas therefore saw no difference between a first attach to a version and a repeat. It also held no row id to key a bump on.

The change produces the version-scoped facts beside the repository-scoped ones, rather than narrowing the repository probe. A version predicate on that probe drops every cached package-level entry from the walk it mirrors.

The seams this change touches:

  • counterDeltas gains a versionSize field, so the shared movesNothing() guard sees a version-only movement.
  • movesNoEmitterScope() is new. It stops a version-only fill from spawning a CounterEmitter worker that declines both scopes.
  • buffered.go gains the label remote_version_size_bytes and its own per-label quota.
  • internal/datastore/maven_remote_version_size.go is new. It carries LockVersionForSizeRefoldTx and RefoldVersionSizeTx, the cache-side twins of the hosted walk's two transaction-taking halves.
  • internal/datastore/maven_remote_eviction.go composes those two halves around its DELETE, the way MavenFileDeleter.deleteFileTx does.
  • internal/datastore/query_names.go gains five names: the version-scoped membership probe, the refold's digest read, the refold's shadow-size read, the refold's version lock, and the refold's writeback.
  • internal/metrics/cardinality.go gains the new column value in the column closed set and in the count budget.
  • cmd/artifact-registry/wire_maven.go supplies Deps.RemoteVersionSizes. Without it the fill arm ships inert while every unit test passes.
  • docs/dev/database-query-patterns.md gains the section The Maven remote file delete's lock order.
  • docs/specs/S14-maven-remote.md, docs/dev/storage-accounting.md and docs/dev/observability.md carry the matching prose corrections.

The hosted upload path is unchanged. Its filler never sets versionSize.

The removal side landed after the phase-8 review, on an operator decision. The section ## The removal side, added by an operator decision is the record of it.

The read path is unchanged, deliberately

The management API still serializes size as null for every remote row. versionFromMavenRemoteModel sets no Size, exactly as on main. A non-null size on a remote version is a regression, not an improvement.

Three readings share one column value, and no code separates them. A row cached before this emitter landed, and reached by no file delete, is charged only for the digests filled into it after that point. It undercounts whatever it already held. Whenever the row was cached, a shed, failed or lost fill bump drifts the value in either direction. A repoint whose negative delta is shed leaves the row above the version's true footprint. A stored 0 is therefore one of three things: never measured, genuinely empty, or left behind by fill bumps that never landed. The genuine empty is reachable because the delete's recompute writes what the surviving rows are worth, so a version's last digest takes the row to zero.

The recompute is the only thing that replaces any of these readings, and it reaches a version only when a file of that version is deleted. Nothing reaches the rest. internal/datastore/reconcile_repository.go carries a live-version row count and a repository-scoped byte sum, and no version-scoped size recompute. No backfill exists.

ADR 007's read side is not a precondition this branch closes

ADR 007 says the display path and the version list read the indexed column directly. That sentence is not a precondition this change closes, and the measurement says why.

No version list sorts or filters by size, for any of the four version tables. MavenVersionSortColumn, NpmVersionSortColumn and MavenRemoteVersionSortColumn each carry only CreatedAt and Version. The one size_bytes sort key in the tree is the repository list's, at internal/managementapi/list.go:68, which holds at origin/main and at this head. docs/specs/S17-rest-management-api.md:526 scopes the management-API size field to hosted versions.

This change lands both writers and leaves the read side where the spec already put it.

Two comment corrections diverge from the issue body

internal/managementapi/version_file_resources.go. The issue body's ## Work names lines :92 to :94 as the one comment that needs no correction. Those lines carry that text at the merge base f9f9184e4. This branch deleted that sentence rather than restoring it, because its consequent is now false. The sentence promised that once the cache fill starts writing the counter, the mapper reads the column the way versionFromMavenModel does. Both writers now move the counter and the mapper still sets no Size. The run does not edit issues, so this description is where a reviewer who reads both documents finds the divergence.

internal/datastore/maven_remote_versions.go. The issue body's ### Emit route cites lines :93 to :94 as one of three sources for the off-request-path contract. Those lines also carry that text at the merge base. This branch deleted them under the comment caps. The surviving off-path statement is the maven_remote_versions.size_bytes paragraph of docs/dev/storage-accounting.md, in its ## Accounting layers section.

The claims this branch corrected

The issue's ## Work named five sites. Three more were ruled in on guardrail 22, because a comment must describe the code as the same change leaves it. Line numbers on files this branch edits are given at the merge base f9f9184e4, which is where the false text stands.

Site What went false
internal/datastore/maven_remote_cache.go, upsertMavenRemoteVersion's doc Four sentences, and one was already false before this branch: it said remote.CacheEntry carries no size field, while internal/remote/interfaces.go:63 declares Size int64 and :71 declares Deduplicated bool, both already read by committedRemoteFillDeltas
internal/datastore/maven_remote_versions.go, the writer's doc and its TODO(s18-buffered-counters) Named the cache-fill path as an unlanded future caller
internal/managementapi/version_file_resources.go "no code writes it" and "has no caller outside its own tests"
internal/datastore/maven_remote_cache.go, MavenRemoteUpsertResult's doc "All three are meaningful once UpsertCacheEntry has returned nil" — the struct now carries more than three fields. Ruled in on guardrail 22, not in the issue's list
Five comments promising repair by a reconciliation pass On Deps.RemoteVersionSizes, emitCommittedFillVersionSize, labelRemoteVersionSizeBytes, remote_fill_emit.go's file doc, and MavenRemoteUpsertResult.VersionDisplaced. No reconciliation pass repairs this column: reconcile_repository.go carries only recomputeMavenRemoteVersionsStmt (a live-version row count) and recomputeMavenRemoteFilesSizeStmt (a repository-scoped byte sum), and ADR 007 scopes its artifact-level recompute walk to a maven_version_id or an npm_version_id. Each now points at docs/dev/storage-accounting.md's ## Accounting layers, which states in full what does repair the column and when
internal/managementapi/maven_remote_reads_test.go:1026 "whose column exists but has no emitter"
docs/dev/storage-accounting.md:2368 "nothing writes maven_remote_versions.size_bytes". Now says the per-artifact figure is a different number on the same arithmetic its hosted twin follows, and an incomplete source besides
docs/specs/S14-maven-remote.md:135 and :146 The column's "no emitter owns this column yet" note and the index's "sorts on zeros" note

internal/format/maven/handler.go's pre-existing Deps.Emitter sentence was left alone, and the reason was checked rather than assumed: reconcile_repository.go does carry a recompute for every column that emitter moves, so its promise is true.

The phase-8 review found more stale claims, and this branch corrects each one.

  • internal/managementapi/version_file_resources.go, the Version doc: "whose column exists but is never written". This branch is what writes it.
  • internal/managementapi/version_file_resources.go, versionFromMavenRemoteModel's doc: "until a repair lands" made the null Size conditional on a repair the documents it cites said did not exist.
  • internal/managementapi/bulk_maven_worker.go, the MavenRemoteBulkEvictor doc: "Each method reports what its own transaction moved". The version refold moves the column and reports nothing.
  • docs/dev/storage-accounting.md, the reconciliation sentence: it read ADR 007 as scoping the whole provision to a maven_version_id or an npm_version_id. The ADR puts maven_remote_versions in the counter set, and it is the recompute walk that carries the hosted scoping.
  • docs/dev/storage-accounting.md, the three-evictions paragraph: a bare size_bytes that reads as both columns once this branch writes the version one. It now says repositories.size_bytes.
  • docs/dev/storage-accounting.md and docs/specs/S14-maven-remote.md, four size_bytes notes: they read as if the cache fill were the column's only writer, which is what kept the eviction gap out of the prose.

The fill joins the shed path, and the delete is what repairs it

Deps.RemoteVersionSizes is nil-tolerant by design, so an unwired build leaves the fill arm inert rather than fatal. TestEmitCommittedFillVersionSize_InertWhenUnwired and TestNewRemoteVersionSizeTxRunner_NilClientYieldsNoRunner pin that state. The seam fails open on purpose, and TestBuildMavenDispatcher_DepsLiteralCarriesEverySeam is what stops a dropped Deps field from booting green.

Issue 632 published a classification that names maven_remote_versions.size_bytes as joining its shed set the moment this caller lands. This work confirms that classification for the fill's charge. The bump dispatches through bufferedUpdate under labelRemoteVersionSizeBytes, which sheds at a saturated per-label semaphore. Neither a version-scoped reconciliation pass nor a backfill repairs the loss. What does repair it is the next file delete on that version, which replaces the value from source. The result="error" log line is the only record of a lost bump, so it now carries the delta as well as the identifiers, which the sibling call sites on self-healing columns do not need.

bufferedUpdateWorker released its per-label slot one statement before its global slot, which left the global shed arm reachable in that window. This branch reverses the two releases. The quotas sum to the cap, so a dispatcher holding a label slot holds one that no bufferedUpdateSem holder holds, and a free global slot always exists. That is what makes the pre-existing bufferedUpdateSem doc true, and with it this branch's own claim that a flood on another column cannot shed this one.

The removal side takes no shed path. MavenRemoteEvictor.DeleteMavenRemoteFile recomputes the column inside the file delete's own transaction, so the new value commits with the delete or not at all. The TODO(s18-buffered-counters) marker stays in place, and its text now names the cross-format subsystem swap rather than a missing caller. updateMavenRemoteVersionSizeStmt carries the noinspect:s18-buffered-counters synchronous exemption for the same reason: the refold must commit with the delete it derives from, so the swap has to leave it a direct UPDATE.

The buffered quota re-split

The fifth buffered label, remote_version_size_bytes, is seated by re-splitting the four existing per-label quotas (last_downloaded_at 20 to 16, hosted_last_downloaded_at 12 to 8, downloads_count 20 to 16, version_size_bytes 12 to 8). The re-split is unavoidable: bufferedUpdateSem's doc requires the per-label quotas to sum to bufferedUpdateMaxInFlight, TestBufferedUpdate_PerLabelQuotaFitsGlobalCap pins that sum at equality, and raising the cap would trade an in-scope quota change for an out-of-scope resource change. Tier membership is unchanged from origin/main. Shedding thresholds on the hosted upload and download paths therefore move, even though the issue lists the hosted arm under ## Out of scope.

That disclosure stands as written, and the phase-8 review is right that one clause of it inverts. Raising bufferedUpdateMaxInFlight is not the only out-of-scope resource change on offer. Cutting four live shed thresholds by up to a third is one too, and it is the larger one. downloads_count fires on every archive-class GET, and last_downloaded_at on every delivered remote hit. Nothing in this branch measures the shed-rate change those cuts produce.

The derivation this description carried before was wrong, and it is corrected here. That text said a fifth loss-sensitive label gives three labels at X, two at Y, so 3X + 2Y = 64, and that the equation has one integer solution. It has ten, and four of them keep the loss-sensitive tier above the windowed one: (20, 2), (18, 5), (16, 8) and (14, 11). Separate what the code forces from what a person chose:

Forced By what
The five quotas sum to 64 bufferedUpdateSem's doc, and TestBufferedUpdate_PerLabelQuotaFitsGlobalCap now asserting equality where it asserted <=
The new label is loss-sensitive Nothing repairs a shed bump on it until a file delete on that version lands, which the quota map's own tier comment states
Tier membership of the four incumbents Unchanged from origin/main
The incumbents' prior 20:12 ratio cannot survive X:Y = 5:3 gives 21k = 64, which has no integer root

16 and 8 is a choice among the four tier-preserving solutions, not an arithmetic result. A later reader seating a sixth label gets the same shape: the sum is forced, the split is not, and the cost of the split is the shed-threshold change disclosed above.

TestBufferedUpdate_PerLabelQuotaFitsGlobalCap now asserts equality where it asserted <=. Every future label must therefore take its quota from an existing one rather than grow the cap. That policy outlives this merge request, which is why the review records the re-split as a decision rather than as a defect.

Coverage

Rows are the issue's ten ## Done when clauses, stated by the issue itself rather than derived by the enricher. No spec governs the behavior; docs/specs/S14-maven-remote.md governs only the column and index declarations.

# Clause Tests
DW-1 A committed version-level fill of a digest the version did not hold raises that version's size_bytes by the blob's byte count TestRemoteCacheStore_FillWritesVersionSizeBytes/a first version-level fill charges the blob's whole length, TestCommittedRemoteFillDeltas/first version-level fill of bytes new to the namespace, TestMavenRemoteCacheStore_UpsertCacheEntry_AccountingFacts/a first version-level fill names its version row and holds nothing, TestEmitCommittedFillVersionSize_AppliesTheDelta/a charge reaches the writer keyed on the version row, and TestBuildMavenDispatcher_DepsLiteralCarriesEverySeam for the production wiring the clause needs to hold outside a test
DW-2 A refill of the same coordinate with byte-identical content leaves the counter unchanged TestRemoteCacheStore_FillWritesVersionSizeBytes/an identical refill leaves the counter unchanged, TestCommittedRemoteFillDeltas/identical refill of a version-level entry moves nothing, TestMavenRemoteCacheStore_UpsertCacheEntry_AccountingFacts/an identical refill reports held and displaces nothing
DW-3 A refill that repoints the coordinate to a different digest moves the counter by the new blob's size less the displaced blob's size TestRemoteCacheStore_FillWritesVersionSizeBytes/a repoint that empties the digest from the version credits it back, .../a repoint over a digest the version keeps charges only the new bytes, TestCommittedRemoteFillDeltas/byte-differing refill charges new bytes and credits the displaced, .../displacement can exceed the charge and go negative, TestMavenRemoteCacheStore_UpsertCacheEntry_AccountingFacts/a byte-differing refill displaces the superseded digest at its shadow size, .../a repoint a sibling version absorbs credits the version and not the repository, TestEmitCommittedFillVersionSize_AppliesTheDelta/a credit reaches the writer as a negative delta
DW-4 A fill of a digest the version already holds under a different file name leaves the counter unchanged (ADR 007's dedup-within-the-version rule) TestRemoteCacheStore_FillWritesVersionSizeBytes/a digest the version already holds under another name is deduplicated, TestCommittedRemoteFillDeltas/digest the version already holds under another file name moves nothing, TestMavenRemoteCacheStore_UpsertCacheEntry_AccountingFacts/a digest the version already holds under another file name reports version-held
DW-5 A fill of a digest new to the version, but one the repository and namespace hold, still moves the counter and reaches the dispatch TestRemoteCacheStore_FillWritesVersionSizeBytes/a digest the repository and namespace hold still charges the version, TestCommittedRemoteFillDeltas/digest new to the version but held by the repository and the namespace still moves (asserts movesNothing() false), .../a repoint a sibling version absorbs on both sides moves only the version, TestMavenRemoteCacheStore_UpsertCacheEntry_AccountingFacts/a digest another coordinate holds reports held
DW-6 A package-level file creates no version row and moves no version's counter TestRemoteCacheStore_FillWritesVersionSizeBytes/a package-level fill creates no version row and moves no counter, TestCommittedRemoteFillDeltas/package-level fill of bytes new to the namespace, .../identical refill of a package-level entry moves nothing, TestMavenRemoteCacheStore_UpsertCacheEntry_AccountingFacts/a package-level file claims no version row, TestEmitCommittedFillVersionSize_AppliesTheDelta/a package-level fill has no version to bump
DW-7 An upsert whose transaction rolled back moves no counter TestRemoteCacheStore_FillWritesVersionSizeBytes/a rolled-back upsert moves no counter; the emitter scopes stay covered by the pre-existing TestRemoteCacheStore_FillEmitDispatchesAfterCommit/a rolled-back upsert dispatches nothing
DW-8 A failed counter write leaves the fill successful TestRemoteCacheStore_FillWritesVersionSizeBytes/a failed counter write leaves the fill successful — strengthened with an attempt counter so it cannot pass on a fill that skipped the bump — and TestEmitCommittedFillVersionSize_FailedBumpDoesNotEscape
DW-9 The same merge request corrects the merged claims listed under ## Work Not test-covered; the claims are prose. The Go half is in 968a60a83, the Markdown half in 845e8c1dd, and the test-comment half in 7026d8af3. Later commits correct the further claims the reviews found. See ## The claims this branch corrected for the site list.
DW-10 No GC-side emitter is required, and none is added Not assertable — a negative about code that does not exist. No test demands one, and none was added. See ## The removal side, added by an operator decision for what the premise behind this clause now rests on.

Seam mechanics the clauses rest on

Concern Tests
The new column label has a bounded quota and the quotas still sum to the global cap TestBufferedUpdate_EveryLabelHasBoundedSemaphore, TestBufferedUpdate_PerLabelQuotaFitsGlobalCap (now asserts equality, not <=)
The label passes the metric cardinality audit and its closed set TestRegisterMetrics_PassesCardinalityAudit, TestColumnBudget_MatchesPinnedColumnValues, TestColumnClosedSet_HoldsEveryOwningPackagesColumns
The bump meters under its own label, not the hosted recompute's TestEmitCommittedFillVersionSize_MetersItsOwnLabel
A partially wired handler stays inert on this arm TestEmitCommittedFillVersionSize_InertWhenUnwired, TestNewRemoteVersionSizeTxRunner_NilClientYieldsNoRunner
The version-scoped membership probe prunes to one partition TestMavenRemoteCacheStore_VersionHoldsBlobStmt_PrunesToOnePartition
A field dropped from buildMavenDispatcher's maven.Deps literal, or set from a bare nil, fails rather than booting green TestBuildMavenDispatcher_DepsLiteralCarriesEverySeam
The refold's two halves reject a nil context, a nil transaction and a zero id before any database work TestMavenRemoteVersionStore_SizeRefoldHalves_FiresBeforeAnyDBWork
Each of the refold's four statements is the SQL the composer means to issue TestLockMavenRemoteVersionForSizeRefoldStmt_SQL, TestMavenRemoteVersionDistinctBlobsStmt_SQL, TestMavenRemoteVersionBlobSizesStmt_SQL, TestUpdateMavenRemoteVersionSizeStmt_SQL
The refold's digest read and its shadow sum each prune to one partition TestMavenRemoteVersionDistinctBlobsStmt_PrunesToOnePartition, TestMavenRemoteVersionBlobSizesStmt_PrunesToOnePartition
The catalog test names every declared maven_remote_files query label TestMavenRemoteFileQueryNames_AreDistinct, which now lists all 16

Vacuous passes, flagged rather than hidden

Five subtests pass against a no-op stub because they are negative guards, and they become load-bearing only after the fix: TestEmitCommittedFillVersionSize_AppliesTheDelta/{a package-level fill has no version to bump, a delta that moves nothing opens no transaction}, _InertWhenUnwired, TestNewRemoteVersionSizeTxRunner_NilClientYieldsNoRunner, and ..._AccountingFacts/a package-level file claims no version row. The test author reported these itself rather than letting them read as coverage.

Three subtests landed after that table was written, and it does not list them:

  • TestCommittedRemoteFillDeltas/a repoint onto a digest the version already holds credits the displaced one, for a purely negative delta the original table had no case for.
  • a fill that moves only the version dispatches nothing and a version-only movement calls neither scope, the unit and integration coverage for the movesNoEmitterScope() guard.

One tagged run caught a defect nothing else did

An earlier tagged run of internal/datastore failed, and the failure was a real defect. TestInvariant_MavenCounterUpdatesCarrySwapMarker rejected the refold's new UPDATE because it carried no swap marker. The statement now carries the noinspect:s18-buffered-counters synchronous exemption, which is correct rather than a silencing: the refold must commit with the delete it derives from. Untagged lint, go vet and go build all passed while that was broken.

e2e scenario catalogs (guardrail 12)

No e2e scenario is added or affected. The recompute is datastore-internal, and the management API still serializes size as null for every remote version, so no HTTP-visible behavior moves.

docs/testing/e2e/maven.md declares remote Maven repositories out of scope besides. Its ## Scope section lists "Virtual and remote (proxy/cache) Maven repositories — owned by S30 and S14" under "Out of scope until the capability ships", and its In-scope line names local (hosted) Maven repositories only. This change adds a per-version byte counter on the remote cache-fill path and a per-version recompute on the remote eviction path. A catalog that excludes remote repositories has no journey row for either one, and adding one contradicts that scope statement. The catalog's two counter rows are e2e.maven.setup.repository-statistics and e2e.maven.lifecycle.management-delete-storage-counters. Both are repository- or namespace-scoped and neither names a version's byte total, so neither changes.

The credited column has no reader on any API surface: versionFromMavenRemoteModel sets no Size, and MavenRemoteVersionSortColumn carries no size member.

The conformance-test trigger does not fire either. This change touches no Maven wire surface.

Diff size (guardrail 23)

Measured at head e42565b9b against merge base f9f9184e4: 33 files, +2061 / −459.

Group Files Lines
Production Go 14 +559 / −394
Go tests 15 +1458 / −60
Docs 4 +44 / −5

Grouping: *.md is docs, *_test.go is test, and everything else is production. Three files are new, and all three are Go: internal/datastore/maven_remote_version_size.go, internal/datastore/maven_remote_version_size_test.go and internal/datastore/maven_remote_version_size_integration_test.go. docs/dev/database-query-patterns.md is an existing file that gained a section.

Nine files joined the diff after the phase-8 review, with the fixes that review asked for and with the removal side the operator then decided:

  • The three new maven_remote_version_size files, which carry the refold and its coverage.
  • internal/datastore/maven_remote_eviction.go and internal/datastore/maven_remote_eviction_accounting_integration_test.go, the composition and its coverage.
  • internal/datastore/maven_remote_eviction_integration_test.go, one comment compressed under the caps and one clause moved to the assertion it belongs to.
  • internal/format/maven/remote_artifact_integration_test.go, which gains TestRemoteArtifact_Miss_FillWritesVersionSizeBytes. It reads maven_remote_versions.size_bytes back after a mux-driven miss fill, so NewHandler's own projection of the version-size seams carries the bump rather than a hand-built bundle.
  • internal/managementapi/bulk_maven_worker.go, one comment that the removal side made wrong.
  • docs/dev/database-query-patterns.md, the lock-order section the refold owes.

The production group still deletes more Go comment text than it adds, and the comment caps are the reason. At the same head and base, the Go diff deletes 376 whole-line // comments and adds 293.

A split does not help, and that is measured rather than assumed. scripts/ci/check-comment-caps.sh charges every line of any comment block the diff touches. The caps are 1 for an unexported top-level doc, 2 for any block in a _test.go file, and 3 for an exported one. docs/dev/go-style.md states that the caps have no waiver, and lint:comment-caps runs as a blocking CI job. The gate first read 69 violations on this branch's uncompressed text: 737 charged comment lines against 128 of allowance. It reads 0 at e42565b9b against f9f9184e4, re-run for this description.

The gate charges a block as the diff leaves it, so a preparatory style(comments) merge request buys nothing. Such a merge request brings the blocks to cap on main. This branch then writes its own lines back into the same 69 blocks, and the gate charges all 69 again.

About 552 of those 737 charged lines were this branch's own new text, and 7 of the 69 blocks are legacy-dominated. The compression is mostly of prose this branch wrote, rather than of merged documentation. The floor is not elective either. 24 of the 69 blocks are forced by the issue's own acceptance and by the code this change edits. Those 24 charge 408 lines against 43 of allowance, so 365 lines have to go in the minimal case.

Precedent keeps this work inside the feature branch. git log --grep '^style(comments)' origin/main returns 13 merged commits, all inside feature-step branches, deleting between 41 and 1897 lines. A floor this size is exceptional: 14 of 14 recent Go-touching merges clear the caps gate with zero findings, against a control at 447334389, the commit before the s17-p6-step-20 caps fix, where the same harness returns rc=1 with 7 findings and rc=0 after.

Caps outcome disclosure

docs/dev/go-style.md asks for a disclosure on outcomes 3, 4 and 5.

Outcome 3 was taken at least once. The cross-package reconciliation claim now lives in docs/dev/storage-accounting.md, under ## Accounting layers, and five Go comments point at it.

Seven of the 69 blocks are legacy-dominated. Three of the seven are the large ones, and each was compressed rather than restored. Measured between f9f9184e4 and e42565b9b: upsertCacheEntry's doc went from 45 comment lines to 1, upsertMavenRemoteVersion's from 35 to 1, and RemoteCacheStore.UpsertCacheEntry's from 24 to 3. The test half took outcome 1 and outcome 2 only, with sub-claims relocated into assertion messages, so it owes no disclosure.

One compression removed a load-bearing clause, and the fix is on this branch. At the merge base, the doc on emitCommittedFillCounters stated that a scope whose two deltas are both zero is not called at all. The same branch widened counterDeltas and falsified that invariant, and the compression had already deleted the sentence that contradicted it. The movesNoEmitterScope() guard restores the invariant in code, the doc states it again, and the two subtests named above pin it.

Merge order

No merge order binds this change, and two files carry a conflict risk that is worth naming.

The sweep covers all 88 open merge requests, at origin/main 719845590 on 2026-09-02, with zero fetch failures. It diffs each open merge request's head against that merge request's own merge base, and matches those paths against this branch's 33. Two merge requests return more than 100 files and were paged to completion. The result is 16 overlapping merge requests, on 7 files.

Branch file Overlapping merge requests Count
docs/dev/observability.md !1598, !2169 (merged), !2204 (merged), !2210 (merged), !2224 (merged), !2261 (merged), !2263 (merged), !2266 (merged) 8
docs/dev/storage-accounting.md !2039 (merged), !2162 (merged), !2169 (merged), !2202 (merged), !2210 (merged), !2232 (merged), !2262 (merged) 7
internal/datastore/query_names.go !2162 (merged), !2202 (merged), !2262 (merged) 3
internal/metrics/cardinality.go !1011 (closed), !2228 (merged), !2261 (merged) 3
internal/datastore/maven_remote_files_test.go !1859 (merged) 1
internal/format/maven/handler.go !2169 (merged) 1
internal/managementapi/bulk_maven_worker.go !2169 (merged) 1

The other 26 files carry no in-flight overlap, docs/dev/database-query-patterns.md among them. The open set moves, so every reading here holds at the sweep and not at merge.

docs/dev/observability.md is likely to conflict rather than merge. This branch changes one line in that file, the gitlab_artifact_registry_maven_buffered_counter_updates_total row, at line 54 in the merge base and at line 54 in origin/main. !1598 inserts its own row directly below that one, with no unchanged line between the two, so a three-way merge gets no separating context. That adjacency was re-read from !1598's own diff at this sweep and still holds. None of the other seven merge requests on that file edits or borders that row: !2263 (merged) inserts a row higher in the table, and !2261 (merged) and !2266 (merged) rewrite the authz row.

docs/dev/storage-accounting.md is the second one. This branch's edits there are +22 / −2 in three hunks: 16 added lines inside ## Accounting layers, one word on a line in the three-evictions paragraph, and 4 added lines with 1 replaced in the Maven remote reap section. Seven other open merge requests touch the file, and a previous rebase of this branch already conflicted in that region. Whoever lands second resolves it by hand. No open merge request rewrites any of the sentences this branch rewrites: !2162 (merged) rewrites a neighbouring sentence about version-less cache files, which stays true after this change, because this counter never covers version-less files.

The rest are textual and separated.

  • internal/datastore/query_names.go: this branch adds five names and !2162 (merged) adds three. They share one gofmt alignment run, the maven_remote_versions one. This branch also realigns unchanged lines in the maven_remote_files run, which !2162 (merged), !2202 (merged) and !2262 (merged) do not touch. !2202 (merged) inserts into the maven_packages run.
  • The three single-overlap files each carry their hunks away from this branch's.

Work on !2162 (merged) runs at the same time as this branch, and !2162 (merged) is not a draft, so its head moves. Read the reading above as measured at this sweep, not as settled.

One integer needed a re-measurement, and it came back clean. This branch raises internal/metrics/cardinality.go's hand-maintained column budget from 11 to 12 for remote_version_size_bytes. Two merge requests that each raise one integer merge cleanly, and both end up wrong. TestColumnBudget_MatchesPinnedColumnValues asserts the count against the closed set length, so the second one lands as a red suite rather than as a silent defect. At this sweep no other open merge request goes near that budget: !1011 (closed) raises handler and code for PyPI, !2228 (merged) rewrites a closedSetValues comment, and !2261 (merged) adds denial_reason. The five query names this branch adds raise no budget of their own. labelName: 600 is a declared ceiling, and its own comment states that a count past it fails nothing.

Plan guardrail

This work lands with no plan file, which is a stated deviation from the project guardrail. Neither candidate plan declares it. Step 3 of docs/plans/2026-08-10-maven-version-size-accounting.md hands the cache-fill caller to S14 Track B, and the Step 9 amendment in docs/plans/2026-07-27-maven-remote.md hands it back.

This merge request amends that Step 9 bullet, so it carries a docs/plans/** path. Guardrail 4 binds step merge requests and names no other class, and this is not one: the title carries no step marker, and no numbered step of either plan allocates this work. The guardrail's single-writer clause is about the Status table specifically, which this merge request does not touch; the S14 plan's twenty-one Status rows assert nothing about this column or its emitter, so none of them goes false at merge. docs/plans/README.md's write-once clause is scoped the same way, to editing in place from a step branch. The amendment and both spec annotations were authored together by one feat merge request, f245a42e, and this one already carries the spec half, so splitting the retirement across two merge requests is what would leave the plan and the spec asserting opposites for that window. One cost is accepted rather than avoided: the amendment lands inside a feat commit, so it is invisible to git log --grep 'docs(plans)', which AGENTS.md names as the grep target for plan-tracking commits. git log -- docs/plans/2026-07-27-maven-remote.md still finds it.

Verified against a running service

The hazard case ran first. A digest already cached under a sibling version moved com.example:remote 2.0 from size_bytes 1500 to 4572. That is the whole 3072-byte length, and the run established all three CounterEmitter-scope deltas as zero beforehand. movesNoEmitterScope() was true, the emitter arm returned early, and the per-version dispatch still fired. The negative ran too. The management API returned size null against a row whose size_bytes is 4096, so the read path is unchanged. driver.sh smoke from zero state reported 72 passed and 0 failed against the pristine generated config.

That run used head a4b8a7ed7, which this round's rebase replaced with 958acd56d. It ran before the phase-8 fixes and before the removal side. It did not exercise the delete-path recompute, which landed after it. TestMavenRemoteEvictor_DeleteMavenRemoteFile_RepairsAShedFillCharge is the integration coverage that does.

The version-scoped probe needs no query plan

TestMavenRemoteCacheStore_VersionHoldsBlobStmt_PrunesToOnePartition asserts partition pruning and nothing about the access method. No EXPLAIN output is offered here, and none is owed. ADR 007 :2203 puts a Maven version at 4 to 15 files, and reports both compared probe forms sub-millisecond at that row count. Neither candidate index covers the whole predicate, so the plan keeps a heap filter on the third column either way. At 4 to 15 rows the access method does not decide the cost. The pruning assertion is therefore not asked to prove more than it can.

The removal side, added by an operator decision

The phase-8 review established that the Maven remote arm's own file delete did not maintain the column the cache fill now charges. The operator decided the arm: recompute maven_remote_versions.size_bytes inside the remote file eviction's own transaction, in this merge request. The scope extension is the operator's as well. internal/datastore/maven_remote_eviction.go was not in the file set this work started from, and the operator put it there.

What ADR 007 asks for, and what this branch now does

ADR 007 carries two statements about this column, one about the value and one about the mechanism, and this branch meets both.

:2176 gives the semantic. The column "increments when a blob_sha256 first becomes attached to the version and decrements when the last attachment of that sha256 leaves the version (deduplicated within the version, matching the DISTINCT blob_sha256 used in reconciliation), and two paths remove it".

:2177 gives the mechanism. "A format's own file delete removes the row in its own transaction and recomputes the column there, and the lifecycle purger removes a file that no such route can address."

:2183 states the same mechanism again, in the paragraph on what ends a soft-deleted file's contribution. "A Maven or npm file delete removes the row in its own transaction and recomputes the version's size_bytes there."

Both sit under the :2174 bullet, whose subject is the column "on maven_versions, maven_remote_versions, npm_versions, and npm_remote_versions", so both reach this table.

An earlier revision of this branch met :2176 through a gated decrement, and an earlier revision of this description called that conformance. Both are gone. MavenRemoteEvictor.deleteMavenRemoteFileTx now recomputes inside the delete transaction, which is what :2183 prescribes word for word. No handbook ADR amendment follows, because this arm conforms rather than deviates.

The mechanism

MavenRemoteEvictor.deleteMavenRemoteFileTx runs three steps in one transaction: lock the cached version row, delete the file row, recompute. internal/datastore/maven_remote_version_size.go is new and carries the two halves it composes, which are the cache-side twins of the hosted walk's:

  • LockVersionForSizeRefoldTx takes the maven_remote_versions row FOR NO KEY UPDATE and answers its size_bytes as the recompute's baseline.
  • RefoldVersionSizeTx reads the version's distinct surviving digests, sums them against blob_storage_blobs_by_namespace, and writes the result back when it differs from that baseline.

MavenFileDeleter.deleteFileTx composes its hosted equivalents in the same order, and the lock order is documented in docs/dev/database-query-patterns.md, section The Maven remote file delete's lock order. Removed with the old mechanism: the gated decrement, creditVersionSizeTx, versionStillHoldsBlobTx, lockVersionForCreditTx, and two query names. queryMavenRemoteVersionsSelectLockForEvict became queryMavenRemoteVersionsSelectLockForSizeRefold.

Why a recompute and not a decrement

A gate makes the arithmetic right only when the stored value is already right. The acceptance case works in both directions, and it is the case the two mechanisms answer differently.

A version holds a 3072-byte jar and a 900-byte pom, and the jar's fill charge was shed at a saturated remote_version_size_bytes semaphore, so the column reads 900 against a true 3972.

Eviction Gated decrement writes Recompute writes
The jar 0, clamped by GREATEST on a version still holding the pom 900
The pom 900 3072, an upward repair no credit can ever make

TestMavenRemoteEvictor_DeleteMavenRemoteFile_RepairsAShedFillCharge carries both rows.

The one behavior change

A soft-deleted cache version's size_bytes now moves at a file delete, where the gated decrement left it alone. The gate was never a decision about marked versions. It was a property of AddMavenRemoteVersionSizeBytes, whose soft_deleted_at IS NULL predicate exists so a buffered charge does not move a row on its way out, and this change removes the eviction's call to that method. Nothing replaces the gate, deliberately.

The hosted arm made the same choice and documents it on LockVersionForSizeRefoldTx: "a marked version's files stay deletable, and its counter refolds like a live one's." The reasoning carries over exactly. The walk counts a tombstoned file row's bytes either way, so marking a version moves its sum by nothing. The only thing the recompute takes off is the hard-deleted file's bytes, which is what ADR 007 says ends the contribution. Refusing to write would leave a stale value on a row that is still readable until the purger reaps it.

The hazard this mixed regime carries

The fill's charge is a relative increment run detached after commit. The delete's recompute is a transactional replacement. The two are unordered. A fill that committed its file row before the delete took its lock, and whose emit runs after the delete commits, double-adds those bytes, because the recompute had already counted the row.

The hazard is inherent to the mixed regime and this description does not claim otherwise. It is bounded and self-healing: the next delete on that version replaces the value again. That self-healing property is the strongest thing this arm buys, and it is what the gated decrement could never offer. docs/dev/storage-accounting.md records the shape as one of four steady-state drift shapes, and a fifth shape there covers the rolling-deploy and rollback window, during which a pod running a build without the recompute deletes a file row and leaves the column high.

What was not mirrored from the hosted mechanism, and why

Each of these is deliberate, and each rests on schema evidence rather than on preference.

Hosted feature Not mirrored, because
Keyset paging over the digest set index_maven_files_on_ns_id_ver_id is (namespace_id, maven_version_id, blob_sha256), so a hosted page is an index-ordered scan. index_maven_remote_files_on_ns_id_ver_id is (namespace_id, maven_remote_version_id) with no digest column, so every page would re-read and re-sort the version's whole row set. The cache side takes a different bound instead: mavenRemoteVersionFootprintDigestsStmt is a DISTINCT subquery that reconcileBlobSizeStmt joins to the shadow, so the statement answers one row whatever the version's file count, which is the exemption docs/dev/database-query-patterns.md's ## Bounding result sets names for a single-row aggregate. It also holds the version row lock for one round trip where a paged walk holds it for one per page. SumMavenVersionFootprint and SumDistinctNpmFileSizesByVersion compose their per-version reads the same way.
The version-keyed advisory lock The hosted lock lets a pool-side RecomputeVersionSize serialize against a delete without taking the version row a served upload waits on. The cache side has no such caller, so the advisory lock would serialize nothing. The version row lock is the only thing serializing this recompute, and that is stated deliberately, in the lock-order section of docs/dev/database-query-patterns.md.
The base-table size source Hosted sums blob_storage_blobs bound as sha256 = ANY(bytea[]). This joins the blob_storage_blobs_by_namespace shadow. Hosted parity is not the argument here. That claim was made earlier in this work and is retracted. What supports the shadow is ADR 007's own per-version measurement, which resolves each digest's size "against blob_storage_blobs_by_namespace, the same shadow the namespace- and repository-level walks read" (:2201), plus parity with the sibling reads in this package.

A premise this change repairs

This work recorded an answer on the garbage-collection question before implementation: no garbage-collection-side emitter is needed, because ADR 007 gives the removal side to the format's own delete. The review then established that the remote arm's own delete did not do it. After this change it does, so that premise holds again, and DW-10 rests on a true one.

Coverage for the removal side

TestMavenRemoteEvictor_DeleteMavenRemoteFile_RefoldsTheCachedVersion carries seven cases:

  • the version's last row for the digest leaves it holding nothing
  • a sibling row of the same version holding the digest keeps its bytes
  • a tombstoned sibling row of the same version keeps its bytes
  • a package-level row holding the digest is no row of this version
  • another version of the same package holding the digest stays out of this sum
  • a delete that removed nothing writes nothing
  • a marked cache version refolds like a live one

TestMavenRemoteEvictor_DeleteMavenRemoteFile_RepairsAShedFillCharge carries the two-direction acceptance case above, and TestMavenRemoteEvictor_DeleteMavenRemoteFile_SumsTheSurvivingDistinctDigests pins the distinct-digest arithmetic. TestMavenRemoteEvictor_DeleteMavenRemoteFile_ReportsTheBytesItFreed is unchanged, which is the point: the repository-scoped freed-byte figure and the version-scoped recompute answer different questions on the same delete.

Test and lint evidence

  • golangci-lint 2.13.2, through mise exec --: 0 issues untagged over the three packages, and nothing on the new or edited files with the integration build tag.
  • Tagged suites at the pre-rebase head 23e3d8bc2, which this round's rebase replaced with e42565b9b: internal/format/maven ok 35.9 s, internal/datastore ok 501.2 s.
  • Pipeline 2816149856 on the rebased head e42565b9b: success — https://gitlab.com/gitlab-org/ops/artifact-registry/-/pipelines/2816149856
  • b44a007f bounds the refold's digest read, which moves the Keyset paging over the digest set row above; golangci-lint --new-from-merge-base=origin/main reads 0 issues and the comment-caps gate reads 0 on it.
  • gofmt clean. go build and go vet exit 0, tagged and untagged.
  • Comment caps and Markdown checked by hand, and the caps gate re-run for this description reads 0.

Closing form

Quoted from validation pass 2, which established it:

Acceptance: 10 of 10 items have evidence
Closes-eligible: yes — verdict is PASS, N equals M at 10, and no acceptance item was derived: the card's `acceptance` entry diffs byte-identical against the re-scoped issue body's own `## Done when`, so all three conditions hold.

Closes #550 (closed)

This is a bot message 🤖 — /smurfit

Edited by Pawel Rozlach

Merge request reports

Loading
Loading