feat(maven): emit storage-accounting deltas on remote cache fills

Moaz Khalifarequested to merge
mkhalifa3/835-maven-remote-fill-accounting into main

Why

#835 (closed): the Maven remote cache fill wrote maven_remote_versions and maven_remote_files rows plus their blobs without recording any storage-accounting delta, so all four counters its rows feed read low against their recomputes until a reconciliation pass corrects them:

  • repositories.artifacts_count — the remote version walk counts live rows of the repository, and the fill inserts them;
  • repositories.size_bytes — the remote size walk sums distinct blob sizes over the remote file table;
  • namespace_statistics.components_count — six bare row counts, maven_remote_versions among them;
  • namespace_statistics.deduplicated_size_bytes — the by-namespace shadow trigger fires on every blob_storage_blobs insert, cache-filled blobs included.

The counting is deliberate on every one of those recomputes today (and #437's billable-dimension question, if it ever lands, belongs in the recompute rather than here), so this closes the gap between the fast path and what the recomputes already answer. This is the fill arm of the gap family: #775 covers the npm/Maven eviction arms, #834 the npm fill, #836 (closed)/#837 (closed) the container ones — none are touched here.

Process note

The operator approved landing this without a plan MR — an acknowledged deviation from the plan-before-code guardrail. The research that would have gone into that plan is folded into this description and into the doc comments the diff adds.

What (non-obvious)

  • One emit site, by construction. The emit lives in RemoteCacheStore.UpsertCacheEntry, strictly after the backing store's RunInTx returns nil. Every production fill write reaches the backing store through that wrapper — the flight adapter builds a fresh wrapper per call — so the streaming GET, the HEAD drain, and the vanished-blob recovery are covered without touching any of their dispatch arms. A future direct caller of the wrapper inherits the emit rather than silently reopening this gap.
  • Deltas come from facts the transaction read, not post-commit recounts. UpsertCacheEntry now returns MavenRemoteUpsertResult: whether this statement's own INSERT produced the maven_remote_versions row (derived from the version upsert itself — the statement binds a candidate id minted outside it and RETURNING id, so the insert arm returns the candidate and the conflict-update arm returns the existing row's id; no pre-read is taken), whether the repository already referenced the incoming digest, and what this write displaced with its credited size. The membership probes run over exactly the digest set recomputeMavenRemoteFilesSizeStmt walks — files joined through packages alone, every level namespace-scoped, no soft-delete predicate, because neither does the walk — so an emit taken here and the reconciliation pass that follows answer one number.
  • Displacement credit settles after the delete. The superseded digest costs the byte total only when no row of the repository still references it after the repoint; the probe runs post-write because the upsert's repoint changes the answer — it moves this coordinate's row off the old digest. The attachment delete reads blob_storage_attachments alone and cannot flip the probe, which joins files to packages. Its size reads from blob_storage_blobs_by_namespace, not blob_storage_blobs, on the parity argument findDisplacedBlob records: the recompute sums the shadow, so a credit taken from it answers the same number.
  • The namespace half rides remote.CacheEntry. Whether the namespace already held the bytes is decided by storage.Session.Commit before the upsert transaction opens and exists nowhere in the database by the time it runs, so the fact travels: CacheEntry gains Size and Deduplicated, populated in completeFill from the committed StoreResult. npm and container stores ignore the new fields until their fill gaps close (#834, #836 (closed)).
  • Dispatch reuses the upload machinery, not its handler. The shed-or-spawn body moves out of dispatchHandler.dispatchCounterEmit into dispatchCounterEmitCore; the upload dispatcher delegates unchanged, and the new fillCounters bundle (emitter + process-wide cap) drives the same core from inside the stores, which cannot reach back into a handler. Nil bundle or nil emitter stays inert with no metering — the partially wired contract Deps.Emitter already buys the upload path. Wiring needs nothing at the composition root: the same emitter wire_maven.go already builds serves both arms.
  • Accepted drifts, mirroring the hosted upload's documented ones: a Commit whose upsert then fails strands the blob row unattached and leaves deduplicated_size_bytes low until reconciliation; and the probes are not exclusive against concurrent fills, so two replicas can transiently both charge or credit one digest.
  • #437 caveat recorded where the deltas live: if remote-cache footprint stops being a counted dimension, the exclusion belongs in these recomputes, and this emit changes shape with them.

Test plan

  • Unit: delta arithmetic table (committedRemoteFillDeltas) across every discriminating combination including negative and moves-nothing readings; scope split (repository scope names repositories.id, zero scopes skipped); inertness without an emitter; dispatched work runs and returns its slot.
  • Integration (internal/datastore): the three facts pinned against real Postgres across first fill, identical refill, byte-differing refill, shared digest, and package-level file; and the issue's done-when as a reconciliation-parity walk — after four fills over one repository, RecomputeArtifactsCount, RecomputeSizeBytes, and RecomputeComponentsCount each moved by exactly what the returned facts sum to.
  • Integration (internal/format/maven): the wired wrapper records both scopes with the first-fill deltas naming repositories.id, and dispatches nothing on an identical refill or on a rolled-back upsert (missing parent blob).
  • Mirror-suite check against upload_emit_test.go: no dropped subtest shape — rollback-emits-nothing, zero-delta suppression, scope split, and the recording fixture all have fill-side counterparts; the shed/panic/metering arms stay covered through the shared core by upload_emit_dispatch_test.go.
  • Local: go build ./..., unit suites green for internal/format/maven, internal/datastore, internal/remote; integration suites green for all three under -tags=integration.
  • golangci-lint 2.x (--build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false): zero findings in every line this diff touches.

Diff size

+1480/−83 over 25 files: tests ≈806 (two new integration suites, one new unit suite, mechanical signature updates across eight existing datastore/maven test files), code ≈757 — of which the majority is the doc-comment density this repo requires (delta semantics stated once beside the walks they mirror). No split ships a widened return with no reader or an emit with no test; the seam widening, the probes, the wrapper emit, and the suites pinning them reference one another.

e2e catalogs

No scenario added. docs/testing/e2e/maven.md places virtual and remote Maven repositories out of the catalog's scope ("owned by S30 and S14"), and there is no e2e.maven.remote.* section to extend; the change is also invisible to any Maven protocol response — the counters move through the internal buffered pipeline and become observable only through management-API statistics reads. The correctness an e2e row would assert (counters match recomputes after a fill) is pinned here directly by the reconciliation-parity integration test.

Related to #835 (closed)

Database Review Evidence

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17.11 container (GL_PG_CURR_VERSION = 17 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
MavenRemoteCacheStore.mavenRemoteVersionExistsForFill Limit → Index Scan maven_remote_versions_p55_namespace_id_maven_remote_package_idx (partition-local twin of the partial unique arbiter) 1 / 1 8.30 0.015ms 3 / 0 1/64
MavenRemoteCacheStore.remoteRepositoryHoldsBlob Limit → Nested Loop (Index Scan + Seq Scan on pruned single-row side) maven_remote_files_p39_namespace_id_blob_sha256_idx 1 / 1 9.33 0.011ms 4 / 0 1/64, 1/64
MavenRemoteCacheStore.displacedRemoteShadowSize Index Only Scan blob_storage_blobs_by_namespace_p03_pkey (INCLUDE (size) PK) 1 / 1 8.30 0.020ms 3 / 0 1/64
MavenRemoteCacheStore.mavenRemoteVersionExistsForFill

Summary: Plan matches the method intent exactly: Index Scan over the partition-local twin of the partial unique arbiter index (WHERE soft_deleted_at IS NULL), with the bound namespace pruning maven_remote_versions to one of 64 partitions. Actual row (1) matches the estimate and the read costs 3 buffer hits at 0.015ms over 5000 seeded version rows. The scan is Index rather than Index Only because freshly seeded rows are not yet all-visible; steady-state vacuum makes the projection index-only. No anomalies.

Seed shape: namespaces=1, repositories=1, maven_remote_repositories=1, maven_remote_packages=1, maven_remote_versions=5000 (single partition)

Rendered SQL:

        
        SELECT maven_remote_versions.id AS "maven_remote_versions.id"
        FROM public.maven_remote_versions
        WHERE (((maven_remote_versions.namespace_id = $1::uuid) AND (maven_remote_versions.maven_remote_package_id = $2::uuid)) AND (maven_remote_versions.version = $3::text)) AND (maven_remote_versions.soft_deleted_at IS NULL)
        LIMIT $4;

Bound args: [8e6bab9c-5e0f-4227-b320-a2ac6924ef36, c54ff232-3a19-4515-bc46-c17ca55f9e6d, review-prep-v-002500, 1]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

                                                                                                       QUERY PLAN                                                                                                       
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.28..8.30 rows=1 width=16) (actual time=0.007..0.008 rows=1 loops=1)
   Buffers: shared hit=3
   ->  Index Scan using maven_remote_versions_p55_namespace_id_maven_remote_package_idx on maven_remote_versions_p55 maven_remote_versions  (cost=0.28..8.30 rows=1 width=16) (actual time=0.007..0.007 rows=1 loops=1)
         Index Cond: ((namespace_id = '8e6bab9c-5e0f-4227-b320-a2ac6924ef36'::uuid) AND (maven_remote_package_id = 'c54ff232-3a19-4515-bc46-c17ca55f9e6d'::uuid) AND (version = 'review-prep-v-002500'::text))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=474
 Planning Time: 0.812 ms
 Execution Time: 0.015 ms
(9 rows)

Timings: planning 0.812ms, execution 0.015ms, total 0.827ms.

MavenRemoteCacheStore.remoteRepositoryHoldsBlob

Summary: Plan matches the method intent: the digest probe drives the join via Index Scan on the (namespace_id, blob_sha256) index, pruning both joined tables to one of 64 partitions each, and the repository filter resolves against the single surviving package row. Actual rows match estimates (1 / 1) at 4 buffer hits and 0.011ms over 5000 seeded files. The packages side Seq Scan is the correct choice after pruning — the surviving partition holds one row. An initial seed pass binding one digest shared by all 5000 rows produced a rational Seq Scan at full selectivity; the seed was reshaped to per-file distinct digests before this plan was taken. No anomalies.

Seed shape: namespaces=1, repositories=1, maven_remote_repositories=1, maven_remote_packages=1, blob_storage_blobs=5000, blob_storage_attachments=5000, maven_remote_files=5000 (single partition, one digest per file)

Rendered SQL:

        
        SELECT maven_remote_files.id AS "maven_remote_files.id"
        FROM public.maven_remote_files
             INNER JOIN public.maven_remote_packages ON ((maven_remote_packages.id = maven_remote_files.maven_remote_package_id) AND (maven_remote_packages.namespace_id = maven_remote_files.namespace_id))
        WHERE (((maven_remote_files.namespace_id = $1::uuid) AND (maven_remote_packages.namespace_id = $2::uuid)) AND (maven_remote_packages.maven_remote_repository_id = $3::uuid)) AND (maven_remote_files.blob_sha256 = $4::bytea)
        LIMIT $5;

Bound args: [97ebbf47-2c2a-4d97-a698-24b6ce513662, 97ebbf47-2c2a-4d97-a698-24b6ce513662, 142d24c7-c91f-4b1a-b49c-8c5c62a8a62c, \x000009c400000000000000000000000000000000000000000000000000000000, 1]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

                                                                                                 QUERY PLAN                                                                                                 
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.28..9.33 rows=1 width=16) (actual time=0.006..0.007 rows=1 loops=1)
   Buffers: shared hit=4
   ->  Nested Loop  (cost=0.28..9.33 rows=1 width=16) (actual time=0.006..0.006 rows=1 loops=1)
         Join Filter: (maven_remote_packages.id = maven_remote_files.maven_remote_package_id)
         Buffers: shared hit=4
         ->  Index Scan using maven_remote_files_p39_namespace_id_blob_sha256_idx on maven_remote_files_p39 maven_remote_files  (cost=0.28..8.30 rows=1 width=48) (actual time=0.003..0.003 rows=1 loops=1)
               Index Cond: ((namespace_id = '97ebbf47-2c2a-4d97-a698-24b6ce513662'::uuid) AND (blob_sha256 = '\x000009c400000000000000000000000000000000000000000000000000000000'::bytea))
               Buffers: shared hit=3
         ->  Seq Scan on maven_remote_packages_p39 maven_remote_packages  (cost=0.00..1.01 rows=1 width=32) (actual time=0.002..0.002 rows=1 loops=1)
               Filter: ((namespace_id = '97ebbf47-2c2a-4d97-a698-24b6ce513662'::uuid) AND (maven_remote_repository_id = '142d24c7-c91f-4b1a-b49c-8c5c62a8a62c'::uuid))
               Buffers: shared hit=1
 Planning:
   Buffers: shared hit=307
 Planning Time: 0.354 ms
 Execution Time: 0.011 ms
(15 rows)

Timings: planning 0.354ms, execution 0.011ms, total 0.365ms.

MavenRemoteCacheStore.displacedRemoteShadowSize

Summary: Plan matches the method intent: Index Only Scan over the shadow table primary key, whose INCLUDE (size) covering columns answer the read without a heap visit beyond the single fetch of the not-yet-all-visible seeded row. The bound namespace prunes the 64-way hash layout to one partition; actual rows match (1 / 1) at 3 buffer hits and 0.020ms over 5000 seeded shadow rows. No anomalies.

Seed shape: namespaces=1, blob_storage_blobs_by_namespace=5000 (single partition)

Rendered SQL:

        
        SELECT blob_storage_blobs_by_namespace.size AS "blob_storage_blobs_by_namespace.size"
        FROM public.blob_storage_blobs_by_namespace
        WHERE (blob_storage_blobs_by_namespace.namespace_id = $1::uuid) AND (blob_storage_blobs_by_namespace.sha256 = $2::bytea);

Bound args: [489f3e7d-6c2d-41d0-b0cb-2bbfbd9c1bc4, \x0000000100000000000000000000000000000000000000000000000000000000]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

                                                                                                    QUERY PLAN                                                                                                     
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Index Only Scan using blob_storage_blobs_by_namespace_p03_pkey on blob_storage_blobs_by_namespace_p03 blob_storage_blobs_by_namespace  (cost=0.28..8.30 rows=1 width=8) (actual time=0.011..0.011 rows=1 loops=1)
   Index Cond: ((namespace_id = '489f3e7d-6c2d-41d0-b0cb-2bbfbd9c1bc4'::uuid) AND (sha256 = '\x0000000100000000000000000000000000000000000000000000000000000000'::bytea))
   Heap Fetches: 1
   Buffers: shared hit=3
 Planning:
   Buffers: shared hit=49
 Planning Time: 0.172 ms
 Execution Time: 0.020 ms
(8 rows)

Timings: planning 0.172ms, execution 0.020ms, total 0.192ms.

Edited by Moaz Khalifa

Merge request reports