feat(maven): delivery-gated cache-hit serve path — artifact and metadata (S14 Step 11)

What

Lands the cache-hit serve path for Maven remote repositories — Step 11 of the S14 Maven remote vertical slice — and the delivery-gated remote last_downloaded_at bump on top of it. The bufferedUpdate dispatch helper, the metrics surface, and the downloads_count delivery-gate activation landed with the split downloads-count MR !1480 (merged); this MR was rebased onto it and consumes the delivery verdicts that MR's download.go reports.

serveRemoteArtifact / serveRemoteMetadata (replacing the Step 10 stubs in remote_stub.go) resolve a fresh cache row and serve straight from CAS with the S10 header contract, with no upstream revalidation or fetch: a fresh hit never touches the upstream. A stale or missing row still returns 501 — the fill path is Steps 13–15's, and Steps 14/15's sidecar fills remain 501 stubs declared in remote_stub.go.

The fresh-hit serve path:

  • Resolves (namespace_id, path)maven_remote_files via the Step 8 backing store (lookupRowFresh), applying Step 8's freshness rule — cache_validity_hours for artifacts, metadata_cache_validity_hours for metadata, snapshot versions always revalidate (ForceStale).
  • Serves through the shared hosted primitives: servePrimaryGet / servePrimaryHead with locally composed headers (Content-Type per the S10 mapping, Content-Disposition from the row's file_name, ETag: "sha256:<hex>" from blob_sha256, SHA-1/MD5 per S06's resolver-parsed header names), Range→206, client conditional→304.
  • Bumps maven_remote_packages.last_downloaded_at (and maven_remote_versions when the row is version-level) when the response delivers — a full 200 body copy in serveFull or a written 302 in serveRedirect, mirroring npm's last_downloaded_at — through the id-keyed direct-write primitive BumpLastDownloadedAtByID (called via bumpRemoteAccess). A HEAD, a 304, a 206/416 partial, a CAS miss, or a failed or abandoned copy delivers no artifact and does not bump.
  • Archive-class GET increments the live downloads_count via the activated BumpRepoDownloadCount — but only on real delivery, under the same delivery verdict.

The plan entry records the landed drift and the spec fix in full: the parent-id projection the bump needs, the BumpRepoDownloadCount activation (landed with !1480 (merged)), and the async-dispatch mechanism with TODO(buffered) rebinding markers for the helper and TODO(s18-buffered-counters) on the datastore primitive.

Counter bumps on the read path

Two counters move on a fresh hit, both dispatched off the request path through bufferedUpdate (buffered.go):

  • last_downloaded_at on the remote package row (always) and the remote version row (version-level files only), delivery-gated — remote counter, remote rows. This divergence from the hosted bumpAccess (which stays an access signal recording GET and HEAD) is deliberate and per-format, and it is the point of the S14 resolution rewrite — Download-signal counting split by signal.
  • repositories.downloads_count for archive-class GET only (.jar, .war, .ear, .aar, .zip), under the same delivery gate — the no-op stub activated to its live UPDATE.

bufferedUpdate is the Maven twin of npm's same-named helper: a fire-and-forget goroutine with context.WithoutCancel, a 5-second per-worker deadline, a 64-deep in-flight semaphore that sheds excess dispatches, panic recovery, and Warn-not-Error failure logging. A datastore brownout can neither fail a served download nor extend its latency — the counter columns keep their best-effort contract. The new bufferedCounterUpdates metric (buffered_counter_updates_total), Maven's first Prometheus surface, meters every dispatch by target column and outcome (ok / error / panic / dropped), registered at boot beside npm's via RegisterMetrics.

Delivery gate: both counters now move only when the artifact actually reached the client — a full 200 body copy in serveFull or a written 302 in serveRedirect, the same signal that fires artifact_pulled. A 206/416 partial, a CAS 404/500, or an abandoned transfer delivers no artifact and counts nothing (mirroring npm's deliveryModeNone gate). The hosted bumpAccess keeps its access-signal semantics (GET and HEAD both record) per S10; the remote parents' last_downloaded_at is delivery-gated like npm's — a HEAD probe no longer refreshes ADR-010 retention for a cached row, which the operator accepted in exchange for counting actual deliveries.

The interim helper, metric, and markers carry TODO(buffered); the datastore primitive carries TODO(s18-buffered-counters), the literal maven_invariants_test.go's scan pins. The cross-format buffered-write subsystem owns the eventual rebinding of the direct-write primitives themselves.

Tests

Every acceptance clause has a named asserting test:

Acceptance clause Test
Fresh artifact hit serves 200 from CAS with the full S10 header contract, no upstream call TestRemoteServe_ArtifactFreshHit_ServesFromCache
Archive-class GET increments downloads_count (awaited through the bufferedUpdate wait hook); .pom does not TestRemoteServe_ArtifactFreshHit_ArchiveBumpsDownloadsCount / TestRemoteServe_ArtifactFreshHit_PomDoesNotBumpDownloadsCount
Redirect delivery: a written 302 advances the remote parents' last_downloaded_at and, for an archive-class file, downloads_count TestRemoteServe_FreshHit_RedirectDeliveryBumpsBothCounters
The remote path never calls the hosted bumpAccess (projected ids address the maven_remote_* tables) TestRemoteServe_FreshHit_NeverCallsHostedBumpAccess
Non-archive GET never dispatches a bump TestPrimaryGET_CounterBumpRules zero arm
Abandoned transfer / over-long object: no downloads_count, no last_downloaded_at bump, no event TestPrimaryGET_ProxyMidStreamFailure_EmitsNoEvent / TestPrimaryGET_OverLongObject_EmitsNoEvent
Pinned upstream (cache_validity_hours=0) never revalidates TestRemoteServe_ArtifactPinnedNeverRevalidates
Range on a fresh hit → 206 TestRemoteServe_ArtifactRangeOnHit
HEAD on a fresh hit — full headers, empty body, no redirect; never bumps the remote parents or downloads_count TestRemoteServe_FreshHitHEAD_DoesNotBumpRemoteParentsLastDownloadedAt / TestRemoteServe_ArtifactHeadOnHit / TestRemoteServe_MetadataHeadOnHit
Client conditional on a fresh row → 304, no body, no counter, no last_downloaded_at bump TestRemoteServe_ArtifactConditionalRequest_304 / TestRemoteServe_MetadataConditionalRequest_304 / TestRemoteServe_Conditional304_DoesNotBumpLastDownloadedAt
Fresh metadata hit serves within the metadata freshness window TestRemoteServe_MetadataFreshHit_ServesFromCache
last_downloaded_at bump (awaited through the wait hook): a delivered GET advances package+version (version-level) or package only (package-level metadata) TestRemoteServe_FreshHitGET_BumpsRemoteParentsLastDownloadedAt / TestRemoteServe_FreshHitGET_PackageLevelMetadataBumpsOnlyPackage
Stale or missing row still returns 501 (fill path is Steps 13–15) TestRemoteServe_ArtifactStaleRow_501 / TestRemoteServe_ArtifactMiss_501 / TestRemoteServe_MetadataStaleRow_501 / TestRemoteServe_NoBackingFallsThroughToFillStub_501
If-None-Match fuzzy-match unit behavior, method-scoped to GET/HEAD TestIfNoneMatchMatches table in remote_serve_internal_test.go
Datastore primitive: bumps advance parents to the DB clock; lossy no-op on missing/soft-deleted; argument guards; both UPDATE failure wraps TestMavenRemoteCacheStore_BumpLastDownloadedAtByID_* (integration + unit in maven_remote_cache_test.go)
Wire adapter pool inventory: poolMethods lists each pool-resolved store method once wire_maven_adapter_test.go inventory

Run: go test ./internal/format/maven/... -tags=integration and go test ./internal/datastore/... -tags=integration.

MR size

2,500 insertions and 157 deletions across 20 files (38 commits) against main. Production surface: the serve adapter (remote_serve.go), the datastore bump primitive (maven_remote_cache.go), the wire adapter seam, and the stub deletions. The remainder is same-step tests (integration + unit), the plan's landed-drift and spec-fix amendments, the observability row, and the spec/plan amendments. The bufferedUpdate dispatch helper, the metrics surface, and the downloads_count activation are on main via !1480 (merged) and are not in this diff. Split executed (recorded on the size escalation): the buffered-dispatch, metrics, main.go registration, and the downloads_count delivery gate (with buffered_test.go, download_internal_test.go, and the two usage-data flips) moved to the dedicated downloads-count MR !1480 (merged), which lands ahead of this one. This MR was rebased onto it: the delivery verdicts this MR's remote last_downloaded_at gate consumes now come from that MR's download.go; the merge order is !1480 (merged) first, this MR second.

End-to-end scenario catalogs

No Maven e2e catalog exists in docs/testing/ yet — authoring the first one is a separate docs concern, per the plan's testing strategy. This MR's observable behavior (fresh-hit serve, counters, conditional handling, async dispatch) is pinned by the handler integration suite; Step 18's hermetic proxy harness is the automated e2e coverage for the S14 read paths. No scenario is added or invalidated here.

Conformance

The hosted Maven surface is byte-identical on this branch except for the delivery-gated downloads_count activation (which splits out with the downloads-count MR), pinned by the pre-existing hosted suite; the remote arm's fresh-hit serve is new behavior. Conformance for the proxy read paths runs against Steps 11 and 14 through 16 as they land, with Step 18's harness the end-to-end check, per the plan's conformance strategy.

Notes for reviewers

The one deliberate behavior change to flag: both remote-path counters move at delivery, not at request arrival. The review-driven delivery-gate commits (the last two before this description was refreshed) moved the remote parents' last_downloaded_at behind the same ADR-012 signal that gates downloads_count — a full 200 body copy or a written 302 — so a HEAD, 304, 206/416 partial, CAS miss, or abandoned transfer no longer advances the remote columns. The hosted bumpAccess stays per-request. The divergence between hosted and remote bump semantics is deliberate and per-format, recorded in S14's Download-signal counting split by signal resolution, whose trade-off the operator accepted; the S10 counting-rule contradiction and its resolution ship with the split downloads-count MR. Both bumps are fire-and-forget: a brownout sheds result=dropped meter events instead of failing or stalling a served download.

Review commits landed in plan order; the plan's step entry track keeps the running ledger of landed-drift records citing their commits.

Related to #286 (closed)

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.BumpLastDownloadedAtByID Update → Index Scan maven_remote_packages_p05_pkey 1 / 1 0.28..8.30 1.072ms 33 / 2 1/64
datastore.BumpLastDownloadedAtByID.VersionNonNil Update → Index Scan maven_remote_versions_p40_id_maven_remote_package_id_namesp_idx 1 / 1 0.28..8.30 0.438ms 37 / 0 1/64
datastore.BumpLastDownloadedAtByID

Summary: Plan matches the method's intent — a single-row UPDATE keyed on the partition key (namespace_id) plus the package PK (id), with soft_deleted_at IS NULL. At 5000 seeded rows in the target partition the planner uses the partition-local PK index (maven_remote_packages_p05_pkey) with exactly one partition pruned and 1 row updated; actual rows match the estimate (1 / 1) and the FK-check triggers fire. (At the skill's default write-target seed of 50 rows the planner correctly prefers a Seq Scan — the index path only becomes the cheaper plan once the partition holds enough rows. The seed was raised here so the production index shape is what the plan shows.) No anomalies.

Seed shape: namespaces=1, repositories=1, maven_remote_repositories=1, maven_remote_packages=5000

Rendered SQL:

UPDATE public.maven_remote_packages
SET last_downloaded_at = NOW()
WHERE ((maven_remote_packages.namespace_id = $1::uuid) AND (maven_remote_packages.id = $2::uuid)) AND (maven_remote_packages.soft_deleted_at IS NULL);

Bound args: [namespace id (seeded), maven_remote_packages.id (seeded)]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

Update on maven_remote_packages  (cost=0.28..8.30 rows=0 width=0) (actual time=0.144..0.145 rows=0 loops=1)
   Update on maven_remote_packages_p05 maven_remote_packages_1
   Buffers: shared hit=33 read=2
   ->  Index Scan using maven_remote_packages_p05_pkey on maven_remote_packages_p05 maven_remote_packages_1  (cost=0.28..8.30 rows=1 width=18) (actual time=0.008..0.009 rows=1 loops=1)
         Index Cond: ((id = 'b9392c04-4207-4230-9e84-892e180486f9'::uuid) AND (namespace_id = '439c8996-6f55-40b5-9cf3-ec0c14a88c7e'::uuid))
         Filter: (soft_deleted_at IS NULL)
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=306
 Planning Time: 0.566 ms
 Trigger for constraint fk_maven_remote_packages_maven_remote_repository_id on maven_remote_packages_p05: time=0.604 calls=1
 Trigger for constraint fk_maven_remote_packages_namespace_id_namespaces on maven_remote_packages_p05: time=0.213 calls=1
 Execution Time: 1.072 ms

Timings: planning 0.566ms, execution 1.072ms, total 1.638ms.

datastore.BumpLastDownloadedAtByID.VersionNonNil

Summary: Plan matches the method's intent — the version-row counterpart of the package bump, keyed on namespace_id + the version PK (id) with soft_deleted_at IS NULL. At 5000 seeded version rows the planner uses the composite index maven_remote_versions_p40_id_maven_remote_package_id_namesp_idx (index-leading id), one of 64 partitions pruned, 1 row updated; actual rows match the estimate (1 / 1) and FK-check triggers fire. (As with the package bump, the skill's default 50-row write-target seed yields a Seq Scan; the seed was raised so the plan shows the index path.) No anomalies.

Seed shape: namespaces=1, repositories=1, maven_remote_repositories=1, maven_remote_packages=5000, maven_remote_versions=5000

Rendered SQL:

UPDATE public.maven_remote_versions
SET last_downloaded_at = NOW()
WHERE ((maven_remote_versions.namespace_id = $1::uuid) AND (maven_remote_versions.id = $2::uuid)) AND (maven_remote_versions.soft_deleted_at IS NULL);

Bound args: [namespace id (seeded), maven_remote_versions.id (seeded)]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

Update on maven_remote_versions  (cost=0.28..8.30 rows=0 width=0) (actual time=0.116..0.116 rows=0 loops=1)
   Update on maven_remote_versions_p40 maven_remote_versions_1
   Buffers: shared hit=37
   ->  Index Scan using maven_remote_versions_p40_id_maven_remote_package_id_namesp_idx on maven_remote_versions_p40 maven_remote_versions_1  (cost=0.28..8.30 rows=1 width=18) (actual time=0.007..0.007 rows=1 loops=1)
         Index Cond: ((id = 'eb7480cc-f4cf-4c3e-8e2d-51b9c24b359f'::uuid) AND (namespace_id = '2d87497e-283f-4b08-af8c-5da98dd26606'::uuid))
         Filter: (soft_deleted_at IS NULL)
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=430
 Planning Time: 1.093 ms
 Trigger for constraint fk_maven_remote_versions_maven_remote_package_id on maven_remote_versions_p40: time=0.201 calls=1
 Trigger for constraint fk_maven_remote_versions_namespace_id_namespaces on maven_remote_versions_p40: time=0.012 calls=1
 Execution Time: 0.438 ms

Timings: planning 1.093ms, execution 0.438ms, total 1.531ms.

Edited by Moaz Khalifa

Merge request reports

Loading
Loading