feat(maven): emit the upload's four counter deltas after the commit (S22 plan: 19/21)

What this step delivers

The Maven primary-file upload records its four storage-accounting deltas after its own transaction commits, and it records them off the request goroutine.

Before this change, maven.UploadStore carried a repository-scoped BumpRepoCounters stub that ran inside the upload transaction. This step removes that stub from the format seam, from mavenStoreAdapter, and from MavenRepositoryStore. No .go file in the tree names the symbol any more. The new site is emitCommittedCounters in internal/format/maven/upload_emit.go. commitFileRow reaches it only on the arm where datastore.RunInTx returned nil. A rolled-back upload and a byte-differing release re-PUT therefore emit nothing.

The step also adds the namespace-scoped site that the old signature cannot carry. The plan states why that site is not optional. S20-a's purger emits namespace-scoped component decrements for every format, Maven included. An omitted increment therefore leaves Maven decrementing a counter that nothing increments.

Four values, three conditions:

  • When this request's own INSERT created the maven_versions row, the repository's artifact count and the namespace's component count each move by one. FindOrCreateVersion returns that flag beside the row. A pre-read cannot tell a row this request created from one a concurrent request created first.
  • The repository's byte total takes the blob's whole length on the blob's first attach in that repository. A new datastore read, MavenFileStore.RepositoryHoldsBlob, answers that question inside the commit transaction.
  • The namespace's deduplicated byte total takes the same length on the blob's first attach in that namespace, which storage.StoreResult.Deduplicated reports.

A scope whose two deltas are both zero is not called at all. Such a call moves no counter and still marks the scope dirty, which buys the scope a drain chunk per no-op upload.

The credit for a displaced blob

On the operator's answer to validation finding VB1, the repository-scoped size delta is a charge less a credit.

A mutable overwrite replaces the digest that a maven_files row carries. UpsertMutableFile reads that row before its ON CONFLICT DO UPDATE and reports the displaced digest with its length. The upload path probes RepositoryHoldsBlob again for that digest after the upsert. If no other row of the repository reaches the digest, the delta credits its bytes back. An overwrite onto new bytes therefore carries both terms, and it emits the difference.

The length comes from blob_storage_blobs_by_namespace, the shadow table, and not from blob_storage_blobs. blob_storage_blobs is PARTITION BY HASH (sha256). The digest is what this read looks for rather than something it can bind, so the planner built an Append over all 64 partitions. The shadow is PARTITION BY HASH (namespace_id), so the namespace literal on the join prunes it to one partition at plan time. The digest then resolves index-only inside that partition. This read runs inside the upload commit transaction, where planning sits in the lock hold.

The two findDisplacedBlob rows in ## Database Review Evidence below carry the plans: one partition of 64 each, and settled planning of 0.17 ms to 0.51 ms. The first review run measured 2.46 ms to 4.26 ms over the Append. The swap is scoped to findDisplacedBlob, and its projection restores the old column alias, so the two browse callers keep reading blob_storage_blobs unchanged.

Without the credit, repositories.size_bytes stays overstated forever. The recompute sums the blob lengths behind the distinct digests that the repository's rows reach. It drops the displaced digest at the instant of the swap.

The credit adds no method to UploadStore. It reuses RepositoryHoldsBlob, so the predicate that has to match the recompute walk exists exactly once.

The emit runs off the request goroutine

commitFileRow called the buffered pipeline inline, before writeCommitOutcome wrote the response. accounting.Emitter's record discards the caller's deadline and bounds its own two Redis calls at detachedEmitTimeout, which is 5 s. The site cannot shorten that wait from its own side. What a stalled pipeline costs there is the response, not latency.

Go arms the connection write deadline once, at the request start plus server.timeouts.write, which is 10 s in the deployed configuration and in config.example.yaml. An upload that emits to both scopes against a stalled Redis therefore writes its 200 or 201 after that deadline passes. The flush fails, and the connection closes with the client having seen no status line. The client then retries an upload whose rows and blob already landed.

dispatchCounterEmit in internal/format/maven/upload_emit_dispatch.go runs the emit on a goroutine instead. It takes a slot from a semaphore without blocking, detaches the context with context.WithoutCancel, and recovers a panic inside the goroutine. NewHandler hands every handler the same semaphore, so one ceiling of 64 covers the process rather than one ceiling per handler. A handler with no emitter dispatches nothing at all. The shape follows the counter dispatch that step 18's !1793 (merged) introduces in internal/format/oci, and merge-order note 5 states where that sibling is today. It diverges in one place: it sets no per-worker deadline, because record re-bounds what it is handed and a deadline set here reaches nothing.

The dispatch pays for the shorter request in deltas it can lose. A saturated cap sheds a delta, and process exit takes whatever has not run. An inline emit sat inside http.Server.Shutdown's drain and did not. A reconciliation pass repairs either loss, and nothing in the tree schedules one. accounting.RegisterAsynqHandlers has an empty body, and accounting.RegisterRiverJobs registers the two drain chunk workers alone. The shed branch says so where a reader finds it.

The dispatch meters the deltas it loses

On the operator's answer to finding S2, the two arms that lose a delta record it. meterCounterEmit increments the Maven package's existing bufferedCounterUpdates vector under a new column value, size_bytes. A shed at a saturated cap records result="dropped", and a recovered panic in the emit seam records result="panic".

The path records no result="ok". CounterEmitter returns nothing that separates a recorded delta from one the pipeline dropped, so the series carries no denominator. The series is a rate, not a ratio. Deltas that process exit takes reach no series at all. This dispatcher has no drain to observe them, and bufferedUpdateWaitGroup is test-only and belongs to bufferedUpdate.

One shed dispatch can lose up to four deltas, across repositories.artifacts_count, repositories.size_bytes, namespace_statistics.components_count and namespace_statistics.deduplicated_size_bytes. Both metered arms drop the closure without running it, so the dispatcher never learns which delta it was carrying. The column value therefore names one counter rather than an inventory of what the loss cost.

The change adds no collector and moves no cap. bufferedCounterUpdates is already built, already listed in mavenCollectors, and already carries both label names. internal/metrics/cardinality.go budgets column at 10 and pins no closed set for it, and size_bytes is already an emitted column value elsewhere in the tree. bufferedUpdateMaxPerLabel stays at 21, because meterCounterEmit increments the vector directly and bufferedUpdateSemByLabel is consulted inside bufferedUpdate alone.

docs/dev/observability.md gains the new column value on the Maven counter's row, which is the paired-doc obligation for the change.

What a failed upload leaves behind

On the operator's answer to finding W2, the emit sites are unchanged and emitCommittedCounters' doc names every reading the absence leaves open.

Two durable things a failed upload can leave behind reach a recompute. The first is a maven_versions row. FindOrCreateVersion runs on the pool before the body streams, so a row it inserts is committed before the commit transaction opens. Both count-shaped deltas are gated on that transaction returning nil. The second is a blob. session.Commit makes the blob durable before the format transaction opens, and the trigger on blob_storage_blobs writes the by-namespace shadow row the deduplicated recompute sums.

Neither is emitted for. A delta issued for rows that rolled back is wrong the other way round, and nothing at the site can take a delta back.

Which counter drifts depends on the route out of the upload. An orphan maven_versions row leaves repositories.artifacts_count and namespace_statistics.components_count each reading one low against their recompute. On a mid-upload soft-delete of the version or the package only the second drifts, because recomputeMavenVersionsStmt predicates on both those levels being live. A soft-deleted repository is a third case, and the repositories row the counter lives on is going away with it. A committed blob the namespace did not already hold leaves namespace_statistics.deduplicated_size_bytes reading low. repositories.size_bytes drifts in neither, because recomputeMavenFilesSizeStmt walks maven_files and neither shape writes a row there.

Two upload shapes cost nothing whatever they fail on, because neither owed a count. They are a package-level maven-metadata.xml, which creates no version row, and an upload onto a version that already existed. Bytes the namespace already holds write no shadow row and drift nothing. Reconciliation's from-source recompute is the correction for every drift here, and merge-order note 7 states what schedules it.

Wiring

wireAccounting builds one *accounting.Emitter over the counter buffer and holds it on the wiring struct. mountSlugAnchoredFormats threads that instance to buildMavenDispatcher. One long-lived emitter per process is what CLAUDE.md's shared-instance seam convention asks for. accounting.NewEmitter over a nil buffer is inert, so a process with no cache Redis still reaches the emit and logs a dropped delta. A handler wired with no emitter at all still serves the upload.

Spec coverage

Spec: docs/specs/S22-storage-accounting.md Plan: docs/plans/2026-08-04-s22-storage-accounting.md, Step 19: Maven call sites

This step owns acceptance criterion 20 and criterion 25's Maven upload row. Every other row names the step of the same plan that owns it, and carries no test here.

Criterion 20 is amended in this merge request, in 08914e5af. It scoped the upload's repo-scoped delta to a blob's first attach in the repository, which left the overwrite-displacement credit this step emits undescribed. The criterion now names the last detach an overwrite causes, and says why the namespace scope carries no matching credit. The counter-model table row for repositories.size_bytes and the criterion's own Verified-by sentence carried the same gap and are corrected with it. The precedent for amending a criterion inside a numbered step MR is 3580c8628 (!1793 (merged), criterion 19), ef6878ea1 (criteria 5 and 31) and cba75168c (criterion 10), all on main.

Acceptance criteria

# Criterion Tests
1 Repo-scoped increment reaches repositories.artifacts_count/size_bytes after a drain Step 10 owns it.
2 Namespace-scoped increment reaches namespace_statistics after a drain Step 10 owns it.
3 Concurrent increments to one scope sum exactly Steps 5 and 10 own it.
4 Re-marked claimed scope: no lost delta, no double count, overlap narrowed Steps 5, 8, 10 and 14 own it.
5 Stale chunk re-adds its scopes, bumps the metric, issues no UPDATE Step 8 owns it.
6 Chunk failing on every attempt re-adds before the terminal error Step 8 owns it.
7 Redis down at increment time does not fail the write; delta dropped Step 6 (drop half) and Step 14 (restore half) own it.
8 Reconciliation clears buffered state before its scan Step 14 owns it.
9 Recompute soft-delete visibility per format Steps 11, 12, 13 and 14 own it.
10 Positive-hit recompute per version-type table Steps 11 and 13 own it.
11 Drift recorded on the unit-matched histogram before the overwrite Step 14 owns it.
12 Crash between HINCRBY and SADD loses no increment Step 14 owns it.
13 Hash TTL refreshed on every write; no expiry at normal cadence Steps 5 and 10 own it.
14 Migrations apply and roundtrip through jet Steps 1, 2a and 2b own it.
15 Shadow-table triggers stay exactly consistent Step 2b owns it.
16 Every namespace has a zero-valued statistics row by construction Step 1 owns it.
17 npm publish and unpublish call sites Step 17 owns it.
18 OCI increment call sites Step 18 owns it.
19 OCI decrement call sites at the delete handler Step 18 owns it.
20 Maven's upload emits all four increments from a post-commit site, and BumpRepoCounters is retired rather than swapped TestUploadEmit_ArtifactNewToRepositoryAndNamespace_MovesAllFourCounters, TestUploadEmit_SecondVersionOverHeldBlob_MovesCountsNotSizes, TestUploadEmit_SecondRepositoryInNamespace_MovesRepoSizeNotDedupSize, TestUploadEmit_JarAndPomForOneVersion_MoveCountsByOne, TestUploadEmit_PackageLevelMetadata_MovesSizesOnly, TestUploadEmit_SnapshotOverwriteWithNewBytes_ChargesTheNewBlobWhole, TestUploadEmit_IdempotentSnapshotRePut_EmitsNothing, TestUploadEmit_NilEmitter_StillServesTheUpload; probe and flag: TestMavenFileStore_RepositoryHoldsBlob, TestMavenVersionStore_FindOrCreateVersion; retirement: TestMavenStoreAdapterHandleRouting, TestMavenRepositoryStore_CounterStubsAreNoOps, TestMavenRepositoryStore_CounterStubsLeaveColumnsUnchanged
21 Repository cascade hard-delete emits at the purger Travels to the S20-a plan.
22 One asynq task per namespace candidate Steps 14 and 15 own it.
23 First pass UPSERTs a missing statistics row Step 13 owns it.
24 In-flight reconciliation cap honored Step 14 owns it.
25 Source-first ordering, per site — Maven upload row only Committed-before: TestUploadEmit_RowsAreCommittedBeforeTheEmit. Zero emission: TestUploadEmit_RolledBackUpload_EmitsNothing, TestUploadEmit_ConflictingReleaseRePut_EmitsNothing, TestUploadEmit_ByteIdenticalReleaseRePut_EmitsNothing, TestUploadEmit_ReleaseConflictRendezvous_LosesTheVersionCount. Contract half: Step 6. npm rows: Step 17. OCI rows: Step 18.
26 last_reconciled_at stamped only after every repository is written back Step 14 owns it.
27 Trigger selects only namespaces stale beyond the interval Step 15 owns it.
28 A namespace with an outstanding task is enqueued at most once Step 15 owns it.
29 reconciliation_backlog behind a single-writer lease Step 16 owns it.
30 Reconciliation guard predicate skips a reconciled scope Steps 7 and 14 own it.
31 counter_dirty_set_size sampled once per tick before SPOP Step 10 owns it.
32 Config load rejects each invalid configuration Step 3 owns it.
33 Six metrics registered with bounded label values Steps 8, 10, 14 and 16 own the registration half; the alert-wiring half is not verifiable in this plan.
34 Reconciliation saturation policy enforced Step 14 owns it.
35 Namespace-scoped chunk drains a namespace with no statistics row Steps 7, 8 and 14 own it.
36 A failing recovery SADD loses no delta Steps 8 and 14 own it.
37 Management-API deletes emit once their transaction commits Deferred; ships with #313 (closed).
38 A persistently failing reconciliation task stays re-enqueueable Step 14 owns it.

Error cases

Condition Tests
Redis unavailable at increment time Owned by internal/accounting (Step 6), which drops and logs. This step's site cannot surface it either way: CounterEmitter returns nothing, so no emit can fail an upload. TestUploadEmit_NilEmitter_StillServesTheUpload pins the neighbouring case, a handler wired with no emitter at all.
Redis unavailable at drain-trigger time Step 10 owns it.
Chunk UPDATE fails Step 8 owns it.
:flushed DEL fails after the UPDATE succeeded Step 8 owns it.
Chunk exhausts all retry attempts Step 8 owns it.
Recovery SADD itself fails Steps 8 and 14 own it.
Chunk dequeued later than drain_chunk_stale_timeout Step 8 owns it.
Worker dies mid-chunk after merging into :flushed Step 14 owns it.
Two chunks run one scope concurrently Steps 8 and 14 own it.
Trigger's EnqueueTx fails while the process is alive Step 10 owns it.
Crash between a trigger's SPOP and its EnqueueTx Accepted crash-only gap; Step 14's reconciliation is the backstop.
Assigned repository or namespace row hard-deleted before its chunk drains Step 7 owns it.
Namespace-scoped chunk drains a namespace with no statistics row Steps 7 and 8 own it.
Crash between a scope's HINCRBY and its SADD Step 14 owns it.
Crash between reconciliation's clear and its SET Step 14 owns it.
Reconciliation scan races a concurrent increment Step 14 owns it.
A drain chunk and a reconciliation process one scope concurrently Steps 7 and 14 own it.
Reconciliation finds a discrepancy Step 14 owns it.
Namespace has no statistics row when its task runs Steps 13 and 15 own it.
Reconciliation task fails before its final UPSERT Step 14 owns it.
A namespace can never be reconciled Step 14 owns it.

Security considerations

Concern Tests
Redis keys carry only internal UUID segments, so no key injection Owned by internal/accounting/counterbuf (Step 5). This step passes uuid.UUID values straight through; TestMavenStoreAdapterHandleRouting pins that the ids reach the store rather than being reconstructed from request text.
Counter values are non-secret but feed billing, so drift has financial impact Owned by Steps 14 to 16 (drift metrics and reconciliation schedule). This step's contribution is that each delta is emitted exactly once per committed row change, which criterion 20's rows above cover.
No new credential surface Nothing to test; this step adds no client, no config key and no credential.

One test renamed and ten added after that table was written

The table above is the test(maven) commit's own, pasted unchanged. VB1's credit then split one case in two, a later pass added two more tests, the dispatch commit added five, W8's answer added one, and F4's answer added one. Row 20 reads against these names on the branch.

In the table On the branch
TestUploadEmit_SnapshotOverwriteWithNewBytes_ChargesTheNewBlobWhole TestUploadEmit_SnapshotOverwriteWithNewBytes_ChargesTheDifference
TestUploadEmit_SnapshotOverwriteDisplacingAHeldBlob_CreditsNothing
TestMavenFileStore_RepositoryHoldsBlob_ArgumentGuards
TestMountSlugAnchoredFormats_ThreadsMavenCounterEmitter
TestDispatchCounterEmit_RunsTheWorkOffTheCallersGoroutine
TestDispatchCounterEmit_StripsTheCallersCancellation
TestDispatchCounterEmit_ShedsPastTheCap
TestDispatchCounterEmit_ContainsAPanickingSeam
TestDispatchCounterEmit_UnwiredHandlerDispatchesNothing
TestUploadEmit_SnapshotOverwriteOntoEqualBytes_EmitsTheNamespaceScopeOnly
TestUploadEmit_ReleaseConflictRendezvous_LosesTheVersionCount

TestMavenFileStore_UpsertMutableFile also gains two subtests, one for the displaced blob and its length, one for an overwrite onto the digest the row already carried. TestMountSlugAnchoredFormats_ThreadsMavenCounterEmitter builds the dispatcher through the real composition path, drives a PUT through the mounted mux, and reads the recorded deltas. It covers a dropped wiring edge, a fault that boots green and still serves every request. The five TestDispatchCounterEmit_* cases are in internal/format/maven/upload_emit_dispatch_test.go, over a handler built on a semaphore of its own.

The dispatch also reworked the emit suite rather than adding to it. Every upload in internal/format/maven/upload_emit_test.go now goes through emitEnv's put, putTo or putMetadata. Each of the three waits on the dispatch's in-flight slots before the case reads the recorder. TestUploadEmit_RowsAreCommittedBeforeTheEmit records its probe's read error and asserts it on the test goroutine, rather than calling require from the dispatched one.

W8's case is TestUploadEmit_SnapshotOverwriteOntoEqualBytes_EmitsTheNamespaceScopeOnly, in internal/format/maven/upload_emit_test.go. It reaches the one upload where the two emit scopes disagree. An overwrite that attaches a blob exactly as long as the one it displaces creates no version row. The repository's charge and its credit then cancel, so that scope is not called. The namespace has not seen the replacement bytes, so its scope is called. No case reached that branch before, because every case emitting on a single scope did so with a non-zero count.

F4's case is TestUploadEmit_ReleaseConflictRendezvous_LosesTheVersionCount, in internal/format/maven/upload_emit_test.go. It holds the request that created the maven_versions row at its conflict rendezvous while a second request commits the file row, so the interleaving is ordered rather than raced. The committing request carries versionCreated false, the held one answers 409, and neither emits the count. It also adds putBytesWithHook, the one put helper that does not wait on the dispatch, because a case driving two uploads at once waits once after both return.

S2's metering added no test name. TestDispatchCounterEmit_ShedsPastTheCap and TestDispatchCounterEmit_ContainsAPanickingSeam each gained an assertion on the metered increment. Each also dropped t.Parallel, because an exact delta on a process-global vector must not run beside another writer.

e2e scenario catalogs

No e2e scenario is affected: no catalogued Maven scenario asserts a storage-accounting counter, and moving the emit off the request goroutine changes only when the delta reaches the buffer, not the response or the rows. Metering the dispatch changes no catalogued scenario either. It adds label values to a Prometheus series, and no file under docs/testing/ names a metric at all: grep -rn 'metric' docs/testing/ matches nothing. The branch changes no file under docs/testing/.

The one Maven row that names size accounting is e2e.maven.lifecycle.delete-package at docs/testing/e2e/maven.md:133. It defers freed space to S22, on a delete path this step does not touch. e2e.npm.lifecycle.unpublish-package at docs/testing/e2e/npm.md:127 defers it the same way. An upload-side counter assertion is not runnable end to end until the drain trigger lands (plan step 10, !1751 (merged)), because nothing on main pops the dirty set.

A third row, e2e.npm.lifecycle.unpublish-package-counters at docs/testing/e2e/npm.md:129, carries the npm package counters tags_count and versions_count. Those are not storage accounting, so the line above holds. The row is named here because a reader who greps docs/testing/ for "counter" finds it.

Diff size

Measured at ae82e9a85 with git diff --shortstat af5027a60...HEAD, where af5027a60 is the merge base: 45 files changed, 4057 insertions, 344 deletions. That is past the 500 reviewable lines that docs/dev/development-model.md sets, so the split and the reason follow. The figures in this section come from git diff --numstat af5027a60...HEAD at the same commit.

File group Files Added Deleted
internal/format/maven/ 13 2643 123
internal/datastore/ 15 818 130
cmd/artifact-registry/ 10 570 76
docs/ 3 15 5
internal/metrics/ 2 9 8
internal/managementapi/ 2 2 2

Tests carry 2648 of the 4057 added lines, across 25 of the 45 files. Five files add 2477 of those test lines, and four of the five are new:

Test file Added New file
internal/format/maven/upload_emit_test.go 1351 yes
internal/format/maven/upload_emit_dispatch_test.go 392 yes
cmd/artifact-registry/wire_maven_accounting_integration_test.go 361 yes
internal/datastore/maven_files_holds_blob_integration_test.go 250 yes
internal/datastore/maven_files_integration_test.go 123 no

The other 20 test files add 171 lines and remove 101 between them. That change is the repair of the stub retirement and of the two signature changes, plus the two records of which package emits which column value.

Go production is 17 files, 1394 insertions and 238 deletions. Four files carry the new behavior and 1113 of those insertions: internal/datastore/maven_files.go (+309/−38), internal/format/maven/upload_emit_dispatch.go (+283/−0), internal/format/maven/upload_emit.go (+266/−0), and internal/format/maven/upload.go (+255/−67). The other 13 add 281 lines and remove 133. internal/datastore/maven_repositories.go is a deletion of 28 lines and nothing else, the stub itself.

Three files make up the docs/ group, 15 added and 5 removed between them. docs/dev/observability.md (+1/−1) restates one table row for the metering's new column value, which is the paired-doc obligation for a new metric label value. docs/specs/S22-storage-accounting.md (+9/−3) is the criterion 20 amendment described under ## Spec coverage. docs/dev/storage-accounting.md (+5/−1) corrects one sentence, described in ## Merge-order notes item 1.

internal/metrics/ (+9/−8) is two files and no behavior: cardinality.go's two ownership paragraphs and column_budget_test.go's maven owner row, both records of which package emits which column value.

Five reasons a split does not help:

  • The retirement is one edit across three packages. BumpRepoCounters sits on the format seam, on mavenStoreAdapter, and on MavenRepositoryStore. A part that removes it from one of the three does not compile.
  • The composition-root files cannot land alone. maven.Deps.Emitter has to receive a real emitter in production, and the only scope that builds one is the wiring struct. An emitter wired to a handler that does not emit is dead code, and a handler that emits with no wired emitter moves no counter.
  • The dispatch, its metering, and the emit share one test suite. 51b435853 rewrote how every case in internal/format/maven/upload_emit_test.go waits before it reads the recorder. c93624ce4 then took two dispatch cases out of the parallel phase, so that their metric assertions hold. A split therefore leaves one part with a suite that passes for the wrong reason.
  • The test half is what a reviewer of an accounting change most wants in one place. A split puts the emit cases, the probe cases, the dispatch cases, and the composition-root case in four different reviews.
  • The last ripples are three changed lines and two small datastore edits. Each internal/managementapi/ test file changes one line, for FindOrCreateVersion's new return value, and docs/dev/observability.md changes one. internal/datastore/container_manifest_deleter.go (+11/−7) restates blobSHA256's invariant as a property of its argument rather than as a list of callers. internal/datastore/blob_storage_attachments.go (+11/−3) routes its own digest conversion through that helper instead of converting the slice inline. A separate merge request for those costs more review time than it saves.

Merge-order notes

1. docs/dev/storage-accounting.md

Two separate sentences of that file are in play, and this step touches one of them.

The emitter's composition-root sentences, at :153-154, are left alone. They say that the emitter has no composition-root construction. Their check is that grep -rn 'accounting\.NewEmitter' matches only files under internal/accounting/. This branch adds accounting.NewEmitter to cmd/artifact-registry/wire_accounting.go, which that grep matches, so neither sentence survives the merge. The correction is step 17's, and it is on step 17's branch already, in !1798 (merged). !1798 (merged) targets main directly, so the correction reaches main when !1798 (merged) merges. While the correction is unmerged, those two sentences describe the pre-correction behavior and disagree with this step's code. Once it merges, the file and this step's code agree. This step leaves them alone because the correction already exists on that branch, and two copies of it would conflict. Checked at !1798 (merged) 1912e5953 and !1751 (merged) fe7f74d1f. The line numbers moved from :144-145 to :153-154 when this branch rebased onto af5027a60; the sentences are unchanged.

The exactness sentence, at :135, is corrected here, in ae82e9a85. It asserted tree-wide that Δartifacts and Δcomponents "come from a statement's own committed outcome and are exact under concurrency". That holds only where the delta is bound to the statement that commits the rows. This step's count-shaped value is decided by FindOrCreateVersion on the pool, before the transaction that commits the file row, so two requests racing one release coordinate can leave the value with the request that does not commit. The correction is written format-neutrally rather than as a note about Maven's upload: it names the condition under which the pair is exact, the condition under which it is not, the under-count shape, and the remedy at the emit site. !1798 (merged) also changes this file, so a Maven-only wording would have collided with it.

2. cmd/artifact-registry/wire_accounting.go

Step 10's !1751 (merged) has merged, as a570c673f under merge commit af5027a60, which is the base this branch now sits on. Its hunks in this file are therefore behind this step rather than ahead of it.

That merge produced this branch's one rebase conflict, and the rebase resolves it. Both sides rewrote the wireAccounting doc block, this branch for the emit side and !1751 (merged) for the drain side. The resolution keeps both: the block now names the emitter, the two chunk kinds and the two scheduled trigger kinds as one entry each, and four sequencing rules rather than three. Nothing outside that comment differed, and git range-diff --creation-factor=100 pairs the other fifteen commits byte for byte.

Two open merge requests still change this file: step 17's !1798 (merged) and step 15's !1803 (merged).

This branch and !1798 (merged) each add the same two things to it, byte for byte. Those two are the line w.counterEmitter = accounting.NewEmitter(newEmitBuffer(cacheRedis, saConfig)), and the newEmitBuffer helper with its doc comment. Whichever merges second carries a duplicate of that hunk, and its author drops the hunk on rebase. The two copies of the whole file are no longer identical. cmd/artifact-registry/wire.go holds the same shape: both branches declare counterEmitter *accounting.Emitter on the wiring struct, with the same name and type and a different doc comment.

!1803 (merged) changes both files and adds no emitter to either. Its cmd/artifact-registry/wire_accounting.go hunks are the asynq client holder, the reconciliation trigger's dependency set, and the River kind list. Its cmd/artifact-registry/wire.go hunks carry no counterEmitter line at all. That collision is textual proximity rather than a duplicate declaration, so a rebase settles it with no decision to make.

Checked at !1798 (merged) 1912e5953 and !1803 (merged) 67dbdef7b; !1751 (merged) is merged at a570c673f.

3. !1754

!1754 declares its own counterEmitter *accounting.Emitter field on the same wiring struct as this step, with a different doc comment. Its construction site is different too: cmd/artifact-registry/wire.go rather than cmd/artifact-registry/wire_accounting.go. That is a collision on the field, not only on the documentation. Two identically named fields on one struct do not compile, so whichever merges second reconciles them into one field and one construction site. Checked at !1754 (merged) 77a1f0ae1.

4. internal/datastore/query_names.go

16 other open merge requests append constants to this file's single sorted const block, measured at 2026-08-21T09:25Z over all 95 open merge requests. None of the 16 adds a maven_files_* constant, so this step's own neighborhood in the block is unclaimed. The file stays a known collision point.

This step adds two constants: maven_files_select_repository_holds_blob, and maven_files_select_displaced_blob from VB1's credit.

5. The counter dispatch in three format packages

Three merge requests each add a counter dispatch of this shape, one per format package: this one adds internal/format/maven/upload_emit_dispatch.go, step 18's !1793 (merged) added internal/format/oci/emit_dispatch.go, and step 17's !1798 (merged) adds internal/format/npm/counter_emit_dispatch.go. No two of the three add the same file, so the dispatches themselves collide with nothing and no merge order binds them.

All three now meter their lost deltas under one column value, size_bytes, each on its own format package's bufferedCounterUpdates vector. That brings one collision the dispatch files do not have. !1793 (merged) has merged, as 3580c8628, and !1751 (merged) as a570c673f. Both of their docs/dev/observability.md rows are on main and so in this branch's base, which settles the three-way collision in that table. This branch rewrites one row, the line directly below !1793 (merged)'s.

upload_emit_dispatch.go names none of the other two. An earlier revision of it justified its own missing per-worker deadline by contrasting itself with internal/format/oci's counter dispatch, in the present tense, and main carried no such dispatch when that revision was written. Guardrail 19 bars that shape, so the contrast is gone and the standalone reason is what remains: everything a worker runs is accounting.Emitter's record, which detaches the context it is handed and applies a bound of its own, so a deadline set at the dispatch would reach nothing.

The unification pointer in that file is TODO(buffered), the marker bufferedUpdate already carries for the cross-format buffered-write client. It is not issue #559: that issue derives detached-write in-flight caps from the shared Postgres pool's total capacity, over five consumers that each hold a pooled connection, and this dispatch holds none. Checked at !1798 (merged) 1912e5953; !1793 (merged) is merged at 3580c8628.

6. ADR-007's cross-partition claim, and handbook !20835

docs/adr/007_database_schema.md:1773 states that joins to blob_storage_blobs via (namespace_id, blob_sha256) do not cross-partition scan, because the planner prunes the format-table partition on namespace_id and the blob partition on sha256 independently. This branch's own EXPLAIN evidence is what shows that reading too broad. blob_storage_blobs is PARTITION BY HASH (sha256), and a column-to-column predicate leaves sha256 unknown at plan time, so the planner builds an Append over all 64 blob partitions and prunes per outer row at execution instead. Commit 96e8d3921 moves findDisplacedBlob onto blob_storage_blobs_by_namespace for exactly that reason, and TestMavenFileStore_FindDisplacedBlob_PrunesToOnePartition now pins the resulting plan.

The correction to the ADR is handbook !20835: gitlab-com/content-sites/handbook!20835 (merged). docs/adr/ in this repository is a CI-synced mirror that must not be hand-edited, so this branch does not touch it. While !20835 is unmerged, the mirrored ADR sentence reads against the swap this branch relies on, and a reviewer quoting docs/adr/007_database_schema.md:1773 is quoting the merged authority. Once !20835 merges and the daily sync job runs, the mirror states the plan-time distinction the swap rests on, and the two agree. Checked at !20835 e017baf47.

7. !1803 and the drift a failed upload leaves

emitCommittedCounters' doc names two durable things a failed upload can leave behind. The first is a maven_versions row that FindOrCreateVersion commits on the pool before the body streams. The second is a blob that session.Commit makes durable before the format transaction opens. Neither is emitted for, so repositories.artifacts_count, namespace_statistics.components_count and namespace_statistics.deduplicated_size_bytes can each read low against their own recompute. A delta this branch's dispatch sheds or loses to a panicking seam has the same shape: metered now, but not recoverable at the site.

Nothing on main schedules the recompute that corrects any of them. accounting.RegisterAsynqHandlers has an empty body, and accounting.RegisterRiverJobs registers the two drain chunk workers alone. Step 15's !1803 is the merge request that changes both. It registers the reconciliation triggers on River, and it fills RegisterAsynqHandlers with the per-namespace pass those triggers fan out onto. While !1803 is unmerged, each drift stands for as long as its counter row does. Once !1803 merges, the first pass to reach a scope overwrites its columns from the rows, and the drift is bounded by storage_accounting.reconciliation_interval instead.

No merge order binds the two. This step's comments state the condition rather than !1803's state, so they read correctly on either side of that merge. !1803 changes no file under internal/format/maven/. The two branches do overlap on cmd/artifact-registry/wire.go, cmd/artifact-registry/wire_accounting.go and internal/datastore/query_names.go, which notes 2 and 4 cover. Checked at !1803 (merged) 67dbdef7b.

8. internal/metrics/cardinality.go and its budget test

This step now touches internal/metrics/cardinality.go and internal/metrics/column_budget_test.go, which it did not before. Seven other open merge requests change cardinality.go: !1011, !1754 (merged), !1761 (merged), !1798 (merged), !1799 (merged), !1805 (merged) and !1811 (merged), measured over all 93 open merge requests at 2026-08-22T07:40Z.

This step's hunks there are prose and one test literal, and they add no map entry. cardinality.go gains two corrected ownership paragraphs and nothing else: maven now owns four column values rather than three, and it is named as the third emitter of size_bytes. column_budget_test.go gains "size_bytes" to maven's owner row. "column": 11 is unchanged and the pinned closedSetValues set is unchanged, because size_bytes is already a member of it and the union does not move when a second owner starts emitting a value the set already holds. That is the staleness column_budget_test.go's own doc comment predicts and asks to be repaired anyway, and it is why nothing failed CI before the repair.

!1798 (merged) is the collision to watch, because it rewrites the same two paragraphs. It takes the budget from 11 to 10, retires repository_publish_counters and artifacts_count from npm's list, adds last_updated_at, and restates both paragraphs around the new counts. Whichever of the two merges second reconciles the counts, in that one file. No merge order binds them, and this step's own conclusion in internal/format/maven/upload_emit_dispatch.go states no count at all, so it reads correctly whichever lands first. OCI's owner row in column_budget_test.go is stale on main for the same reason, left behind by !1793 (merged), and this step does not fold that correction in. Checked at !1798 (merged) 94f5d45c8.

Database Review Evidence

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17 container (matching GL_PG_CURR_VERSION from .gitlab-ci-other-versions.yml), with synthesized seed data rolled back per query and the container torn down at the end of the run. Numbers reflect moderate cardinality and do not capture production-scale effects. See Database review evidence for seed sizing, methodology, and the anomalies the skill flags. Expand each row's details for the seed shape, rendered SQL, bound args, and raw plan.

Method Plan node Index Rows (plan / actual) Cost Time Buffers (hit / read) Partitions
datastore.MavenFileStore.RepositoryHoldsBlob Limit index_maven_files_on_ns_id_blob_sha256 1 / 1 9.33 0.018ms 4 / 0 1/64 maven_files, 1/64 maven_packages
datastore.MavenFileStore.findDisplacedBlob.PackageLevel Limit unique_maven_files_ns_id_package_id_file_name_when_ver_null, pk_blob_storage_blobs_by_namespace 1 / 1 16.62 0.039ms 6 / 0 1/64 maven_files, 1/64 blob_storage_blobs_by_namespace
datastore.MavenFileStore.findDisplacedBlob.VersionLevel Limit unique_maven_files_ns_id_version_id_file_name, pk_blob_storage_blobs_by_namespace 1 / 1 16.62 0.040ms 6 / 0 1/64 maven_files, 1/64 blob_storage_blobs_by_namespace
datastore.MavenFileStore.RepositoryHoldsBlob

Summary: The plan matches the method's intent. The planner reaches maven_files through index_maven_files_on_ns_id_blob_sha256 and prunes it to one partition of 64. It then joins to the matching maven_packages row, also in one partition of 64. The Seq Scan on maven_packages_p57 is an artifact of the seed, which inserts a single package. A second transaction with 5001 packages in that partition moved the join to pk_maven_packages, and both tables stayed at one partition. No anomalies.

Seed shape: namespaces=1, repositories=1, maven_repositories=1, maven_packages=1, maven_versions=1, blob_storage_blobs=5000, blob_storage_blobs_by_namespace=5000, blob_storage_attachments=5000, maven_files=5000 The 5000 blob_storage_blobs_by_namespace rows are not inserted directly. The AFTER INSERT trigger on blob_storage_blobs writes one shadow row per blob, inside the same transaction.

Rendered SQL:

SELECT maven_files.id AS "maven_files.id"
FROM public.maven_files
     INNER JOIN public.maven_packages ON ((maven_packages.id = maven_files.maven_package_id) AND (maven_packages.namespace_id = maven_files.namespace_id))
WHERE (((maven_files.namespace_id = $1::uuid) AND (maven_packages.namespace_id = $2::uuid)) AND (maven_packages.maven_repository_id = $3::uuid)) AND (maven_files.blob_sha256 = $4::bytea)
LIMIT $5;

Bound args: [ba387b8a-fdca-47fb-a02a-2a34a8a484ae, ba387b8a-fdca-47fb-a02a-2a34a8a484ae, 0ba556ec-8325-4b28-9f52-3f8fd7565d28, \x00000000000000000000000000000000000000000000000000000000000009c4, 1]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

Limit  (cost=0.28..9.33 rows=1 width=16) (actual time=0.018..0.018 rows=1 loops=1)
  Buffers: shared hit=4
  ->  Nested Loop  (cost=0.28..9.33 rows=1 width=16) (actual time=0.017..0.017 rows=1 loops=1)
        Join Filter: (maven_packages.id = maven_files.maven_package_id)
        Buffers: shared hit=4
        ->  Index Scan using maven_files_p57_namespace_id_blob_sha256_idx on maven_files_p57 maven_files  (cost=0.28..8.30 rows=1 width=48) (actual time=0.013..0.013 rows=1 loops=1)
              Index Cond: ((namespace_id = 'ba387b8a-fdca-47fb-a02a-2a34a8a484ae'::uuid) AND (blob_sha256 = '\x00000000000000000000000000000000000000000000000000000000000009c4'::bytea))
              Buffers: shared hit=3
        ->  Seq Scan on maven_packages_p57 maven_packages  (cost=0.00..1.01 rows=1 width=32) (actual time=0.004..0.004 rows=1 loops=1)
              Filter: ((namespace_id = 'ba387b8a-fdca-47fb-a02a-2a34a8a484ae'::uuid) AND (maven_repository_id = '0ba556ec-8325-4b28-9f52-3f8fd7565d28'::uuid))
              Buffers: shared hit=1
Planning:
  Buffers: shared hit=284 read=1
Planning Time: 1.316 ms
Execution Time: 0.031 ms

Timings: planning 1.316ms, execution 0.031ms, total 1.347ms. That planning figure is the first plan of a psql session. On later plans of the same statement, in the same session, planning settles at 0.22ms to 0.52ms.

datastore.MavenFileStore.findDisplacedBlob.PackageLevel

Summary: The plan matches the method's intent. The namespace literal on the join prunes blob_storage_blobs_by_namespace to one partition of 64 at plan time. The digest then resolves inside that partition through pk_blob_storage_blobs_by_namespace, as an Index Only Scan. The planner reaches maven_files through the partial unique index unique_maven_files_ns_id_package_id_file_name_when_ver_null, also in one partition of 64. The single heap fetch comes from the seed rows, which the transaction created, so the visibility map does not cover them. No anomalies.

Seed shape: namespaces=1, repositories=1, maven_repositories=1, maven_packages=1, maven_versions=1, blob_storage_blobs=5000, blob_storage_blobs_by_namespace=5000, blob_storage_attachments=5000, maven_files=5000 The 5000 blob_storage_blobs_by_namespace rows are not inserted directly. The AFTER INSERT trigger on blob_storage_blobs writes one shadow row per blob, inside the same transaction.

Rendered SQL:

SELECT maven_files.namespace_id AS "maven_files.namespace_id",
     maven_files.id AS "maven_files.id",
     maven_files.maven_package_id AS "maven_files.maven_package_id",
     maven_files.maven_version_id AS "maven_files.maven_version_id",
     maven_files.blob_storage_attachment_id AS "maven_files.blob_storage_attachment_id",
     maven_files.soft_deleted_at AS "maven_files.soft_deleted_at",
     maven_files.blob_sha256 AS "maven_files.blob_sha256",
     maven_files.sha1 AS "maven_files.sha1",
     maven_files.sha512 AS "maven_files.sha512",
     maven_files.md5 AS "maven_files.md5",
     maven_files.file_name AS "maven_files.file_name",
     blob_storage_blobs_by_namespace.size AS "blob_storage_blobs.size"
FROM public.maven_files
     INNER JOIN public.blob_storage_blobs_by_namespace ON ((blob_storage_blobs_by_namespace.namespace_id = $1::uuid) AND (blob_storage_blobs_by_namespace.sha256 = maven_files.blob_sha256))
WHERE ((((maven_files.namespace_id = $2::uuid) AND (maven_files.file_name = $3::text)) AND (maven_files.soft_deleted_at IS NULL)) AND (maven_files.maven_package_id = $4::uuid)) AND (maven_files.maven_version_id IS NULL)
LIMIT $5;

Bound args: [81eb0699-3030-4180-b564-3753ab7fc590, 81eb0699-3030-4180-b564-3753ab7fc590, review-prep-002500.xml, 18a05eca-e571-4aac-a874-6233521f7b14, 1]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

Limit  (cost=0.56..16.62 rows=1 width=262) (actual time=0.039..0.039 rows=1 loops=1)
  Buffers: shared hit=6
  ->  Nested Loop  (cost=0.56..16.62 rows=1 width=262) (actual time=0.038..0.039 rows=1 loops=1)
        Buffers: shared hit=6
        ->  Index Scan using maven_files_p29_namespace_id_maven_package_id_file_name_idx on maven_files_p29 maven_files  (cost=0.28..8.30 rows=1 width=254) (actual time=0.015..0.016 rows=1 loops=1)
              Index Cond: ((namespace_id = '81eb0699-3030-4180-b564-3753ab7fc590'::uuid) AND (maven_package_id = '18a05eca-e571-4aac-a874-6233521f7b14'::uuid) AND (file_name = 'review-prep-002500.xml'::text))
              Buffers: shared hit=3
        ->  Index Only Scan using blob_storage_blobs_by_namespace_p29_pkey on blob_storage_blobs_by_namespace_p29 blob_storage_blobs_by_namespace  (cost=0.28..8.30 rows=1 width=41) (actual time=0.022..0.022 rows=1 loops=1)
              Index Cond: ((namespace_id = '81eb0699-3030-4180-b564-3753ab7fc590'::uuid) AND (sha256 = maven_files.blob_sha256))
              Heap Fetches: 1
              Buffers: shared hit=3
Planning:
  Buffers: shared hit=326 read=1
Planning Time: 1.627 ms
Execution Time: 0.058 ms

Timings: planning 1.627ms, execution 0.058ms, total 1.685ms. That planning figure is the first plan of a psql session. On later plans of the same statement, in the same session, planning settles at 0.28ms to 0.51ms.

datastore.MavenFileStore.findDisplacedBlob.VersionLevel

Summary: The plan matches the method's intent. The namespace literal on the join prunes blob_storage_blobs_by_namespace to one partition of 64 at plan time. The digest then resolves inside that partition through pk_blob_storage_blobs_by_namespace, as an Index Only Scan. The planner reaches maven_files through the partial unique index unique_maven_files_ns_id_version_id_file_name, also in one partition of 64. The single heap fetch comes from the seed rows, which the transaction created, so the visibility map does not cover them. No anomalies.

Seed shape: namespaces=1, repositories=1, maven_repositories=1, maven_packages=1, maven_versions=1, blob_storage_blobs=5000, blob_storage_blobs_by_namespace=5000, blob_storage_attachments=5000, maven_files=5000 The 5000 blob_storage_blobs_by_namespace rows are not inserted directly. The AFTER INSERT trigger on blob_storage_blobs writes one shadow row per blob, inside the same transaction.

Rendered SQL:

SELECT maven_files.namespace_id AS "maven_files.namespace_id",
     maven_files.id AS "maven_files.id",
     maven_files.maven_package_id AS "maven_files.maven_package_id",
     maven_files.maven_version_id AS "maven_files.maven_version_id",
     maven_files.blob_storage_attachment_id AS "maven_files.blob_storage_attachment_id",
     maven_files.soft_deleted_at AS "maven_files.soft_deleted_at",
     maven_files.blob_sha256 AS "maven_files.blob_sha256",
     maven_files.sha1 AS "maven_files.sha1",
     maven_files.sha512 AS "maven_files.sha512",
     maven_files.md5 AS "maven_files.md5",
     maven_files.file_name AS "maven_files.file_name",
     blob_storage_blobs_by_namespace.size AS "blob_storage_blobs.size"
FROM public.maven_files
     INNER JOIN public.blob_storage_blobs_by_namespace ON ((blob_storage_blobs_by_namespace.namespace_id = $1::uuid) AND (blob_storage_blobs_by_namespace.sha256 = maven_files.blob_sha256))
WHERE (((maven_files.namespace_id = $2::uuid) AND (maven_files.file_name = $3::text)) AND (maven_files.soft_deleted_at IS NULL)) AND (maven_files.maven_version_id = $4::uuid)
LIMIT $5;

Bound args: [ecf5c936-92b2-4616-947e-595e71ce480c, ecf5c936-92b2-4616-947e-595e71ce480c, review-prep-002500.jar, 9d3c99a0-6db5-4b39-a75a-b605ec4d81e2, 1]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

Limit  (cost=0.56..16.62 rows=1 width=262) (actual time=0.040..0.040 rows=1 loops=1)
  Buffers: shared hit=6
  ->  Nested Loop  (cost=0.56..16.62 rows=1 width=262) (actual time=0.039..0.039 rows=1 loops=1)
        Buffers: shared hit=6
        ->  Index Scan using maven_files_p22_namespace_id_maven_version_id_file_name_idx on maven_files_p22 maven_files  (cost=0.28..8.30 rows=1 width=254) (actual time=0.014..0.015 rows=1 loops=1)
              Index Cond: ((namespace_id = 'ecf5c936-92b2-4616-947e-595e71ce480c'::uuid) AND (maven_version_id = '9d3c99a0-6db5-4b39-a75a-b605ec4d81e2'::uuid) AND (file_name = 'review-prep-002500.jar'::text))
              Buffers: shared hit=3
        ->  Index Only Scan using blob_storage_blobs_by_namespace_p22_pkey on blob_storage_blobs_by_namespace_p22 blob_storage_blobs_by_namespace  (cost=0.28..8.30 rows=1 width=41) (actual time=0.023..0.023 rows=1 loops=1)
              Index Cond: ((namespace_id = 'ecf5c936-92b2-4616-947e-595e71ce480c'::uuid) AND (sha256 = maven_files.blob_sha256))
              Heap Fetches: 1
              Buffers: shared hit=3
Planning:
  Buffers: shared hit=322
Planning Time: 2.544 ms
Execution Time: 0.062 ms

Timings: planning 2.544ms, execution 0.062ms, total 2.606ms. That planning figure is the first plan of a psql session. On later plans of the same statement, in the same session, planning settles at 0.17ms to 0.21ms.

Review threads declined

One review thread on this merge request is declined rather than answered. This section records the decision, so a later pass does not re-open it.

The author declines the GitLab Duo summary note (note 3715482857). That note reports no findings and asks for no change. It gets no reply and no resolve.

Related to #515

This is a bot message 🤖 — /smurfit

Edited by Pawel Rozlach

Merge request reports

Loading
Loading