feat(oci): remote repository store finder + health seams (S16 Step 6)

Sylvia Shenrequested to merge
sshen/container-remote-step-6 into main

What

Step 6 of the S16 container remote slice: the container_remote_repositories row store — the kind=2 finder, both S13 health seams, and the container entries for remote.RemoteRepositoryEnumerator. Mirrors Maven's own Step 6 (!1192 (merged)) over the container table.

  1. FindRemoteRepository resolves the remote row bound to the parent repositories.id, returning the row id, url, auth_url, auth_status, cache_validity_hours, and the health columns. This is the finder the read-path kind-dispatch seam (Step 10) attaches to Resolution.
  2. HealthStatus / SetHealthStatus implement remote.HealthStatusReader and remote.HealthStatusWriter over last_health_status / last_health_checked_at, keyed by (namespace_id, repository_id), so S13's on-demand and scheduled probes drive container health without knowing the table.
  3. ListRemotes implements remote.RemoteRepositoryEnumerator for the container protocol family — repositories.format IN (docker, oci), kind=2 — the entry point S13's scheduled sweep enumerates through.

No HTTP surface and no client-visible behavior yet: nothing calls the finder until Step 10 wires the kind-dispatch seam, and nothing calls ListRemotes until S13's scheduled sweep reaches it.

Plan: docs/plans/2026-07-30-container-remote.md, "Step 6: datastore read — remote-repository finder and health status".

Notes for the reviewer

namespace_id is pinned in both health queries, not only the finder. container_remote_repositories is PARTITION BY HASH(namespace_id) × 64, and the Maven sibling's own finder-plus-health MR originally planned both health queries as an Append over all 64 partitions with no supporting index — the gap #445 (closed) was filed for and has since merged. Every statement here — HealthStatus, the CAS SELECT, and the CAS UPDATE — binds namespace_id and prunes to one partition through the existing UNIQUE (namespace_id, repository_id) index; no (repository_id)-only index is needed.

SetHealthStatus is a locked compare-and-set, not a plain write: it locks the row (SELECT ... FOR UPDATE inside RunInTx), applies only when the stored last_health_checked_at is NULL or older than the incoming checkedAt, and reports (previous, applied, error). A probe that loses the race neither overwrites a newer status nor moves the timestamp backwards, without an orchestrator serializing probes — TestContainerRemoteRepositoryStore_SetHealthStatus_CASSerializesConcurrentProbes pins this against two concurrent transactions blocked on the same row lock.

The read gates soft_deleted_at IS NULL on the parent repositories row, not on container_remote_repositories itself — the table carries no soft_deleted_at column of its own. containerRemoteParentPredicate is the one spelling of "an active container-family remote parent" both the finder's JOIN and ListRemotes' sweep gate on, kept out of the shared activeParentPredicate helper because that helper's Format field takes exactly one format and this protocol family spans two (docker, oci).

ListRemotes gets a constant ceiling, not a page. remote.RemoteRepositoryEnumerator.ListRemotes(ctx) ([]RemoteRepositoryRef, error) is a signature S13 already merged, with no pagination or limit parameter, so a bounded page is not available here. The query carries LIMIT maxListContainerRemotes+1 instead, and a result past the ceiling returns errContainerRemoteTooManyRemotes rather than a truncated page. Failing loud has a cost worth naming: HealthSweepWorker.collect discards a source's refs on any error, so past the ceiling the sweep probes none of that source's remotes. Narrowing this properly needs a signature change to RemoteRepositoryEnumerator itself — S13's seam to widen, not this step's to work around. Accepted because the sweep this method feeds runs on its own schedule outside any request path.

ListRemotes skips a row whose url remote.ParseUpstreamBase refuses, rather than handing it to the outbound sweep to fail on less legibly later. The gate is ParseUpstreamBase rather than a scheme test of the store's own so the skip set is exactly the refusal set of the code that would dial the URL. Skips accumulate into one WARN per call carrying the exact count and a bounded sample, because what produces an unusable URL is systemic rather than per-row.

The two Maven companion edits are comment-only. maven_remote_repositories.go and its concurrency-test twin each gain a //nolint:dupl comment naming this step's container twin — no logic change. The three in maven_remote_repositories.go are load-bearing: the container methods are near-identical to Maven's, which trips golangci-lint's dupl detector on the pre-existing Maven code once the twin lands beside it. The one in maven_remote_repositories_concurrency_integration_test.go suppresses nothing — that file is behind //go:build integration and .golangci.yaml leaves run.build-tags unset, so no linter analyzes it. It is kept as the cross-reference telling whoever edits one twin that the other exists.

The two health-probe acceptance criteria are asserted at the store seam only. Per the plan's Research Findings, this spec contributes no container-specific probe path — the probe itself, its status transitions, and its failure counter are all S13's and already on main. The scheduled sweep itself (#336 (closed), closed) is also live: the job, schedule, and worker exist on main. What is still missing is the container remote.HealthSweepSource that Step 13 appends to the aggregate; until that lands, every sweep is a no-op for container remotes.

No e2e scenario is added or affected. This step adds no route, no response, and no client-visible behavior, so neither docs/testing/e2e/oci.md nor docs/testing/e2e/docker.md changes.

Size

Over the 500-line ceiling in development-model.md and over the plan's own Est. ~450 for this step. The production file is past the ceiling on its own; the rest is the internal/datastore/container_remote_repositories_test.go constructor guard and the two integration suites the plan template requires to ship in the same step as the code they cover, so no split is available that does not separate the store from its tests. The suites' size is the enumerated matrices the mirrored-suite guardrail asks for: every isolation case on the finder's parent gate (hosted parent, virtual parent with a remote binding, wrong-format parent, missing row, missing parent, soft-deleted parent, cross-namespace), both container formats (docker, oci) as positive hits against Maven and npm as negative hits, all three health-taxonomy round trips, the transient-error and no-silent-success guards on every method, and the partition-pruning EXPLAIN harness over the finder and both health queries.

Review passes

Two passes ran on this branch before /validate-step's coherence check:

  • A behavior-preserving simplification pass over the store and its suite — no query, error-identity, or assertion changes. It collapsed ListRemotes' scan target from an unneeded anonymous wrapper struct to the same []model.ContainerRemoteRepositories shape every other list read in the package uses, dropped sprint identifiers and a spec-section pointer from comments in favor of naming the Go symbol each contract rests on (remote.HealthStatus, remote.HealthStatusReader, remote.HealthMonitor) per the "comments do not cite process state" rule, and collapsed four near-identical ListRemotes exclusion subtests into one table.
  • Writing the CAS-under-concurrent-probes twin (container_remote_repositories_concurrency_integration_test.go) and the constructor's nil-client panic guard (container_remote_repositories_test.go) surfaced two plan-text corrections folded into this branch: the Tests bullet had named only the main integration suite, and the Acceptance bullet had claimed a soft_deleted_at gate on this table's own row rather than only on its parent.

/validate-step then ran all five coherence categories (plan adherence, acceptance criteria, spec adherence, ADR adherence, dev-guidelines adherence) in parallel against the resulting diff — all five passed with no blockers or warnings.

Related to #288

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.ContainerRemoteRepositoryStore.FindRemoteRepository Limit over Nested Loop container_remote_repositories_..._namespace_id_repository_id_idx, repositories_p52_pkey 1 / 1 16.62 0.022ms 6 / 0 2 (1 per table)
datastore.ContainerRemoteRepositoryStore.HealthStatus Limit over Index Scan container_remote_repositories_p2_namespace_id_repository_id_idx 1 / 1 8.30 0.020ms 3 / 0 1
datastore.ContainerRemoteRepositoryStore.compareAndSetHealthStatus.Select Limit over LockRows over Index Scan container_remote_repositories_p2_namespace_id_repository_id_idx 1 / 1 8.31 0.022ms 4 / 0 1
datastore.ContainerRemoteRepositoryStore.compareAndSetHealthStatus.Update Update over Index Scan container_remote_repositories_p2_namespace_id_repository_id_idx 1 / 1 (underlying scan; root reports 0, standard for UPDATE) 8.30 0.794ms 20 / 0 1
datastore.ContainerRemoteRepositoryStore.ListRemotes Hash Join over two Appends none — full scan on both sides 5063 / 5000 473.70 8.301ms 5359 / 0 128 (64 per table; by design, see notes)

No anomalies on the first four: each is a single-row lookup keyed on (namespace_id, repository_id), prunes to one partition through the existing UNIQUE (namespace_id, repository_id) index, and plan/actual rows match exactly. ListRemotes is flagged below — the scan shape is a known, already-documented trade-off, not new drift.

Important

The ListRemotes row above and the plan inside its details block were captured before the ceiling and the id column landed. The rendered SQL and bound args in that block are re-rendered from the current builder; the plan, cost, time, buffer, and row numbers are the original capture, so they show neither the Limit node the ceiling adds above the join nor id in the scan's output list. Neither changes the scan shape this row is evidence for — the ceiling sits far above the 5,000 seeded rows — but treat the numbers as the pre-ceiling measurement rather than a current one.

datastore.ContainerRemoteRepositoryStore.FindRemoteRepository

Summary: Plan matches the method's intent: the namespace_id literal prunes both hash-partitioned tables to one partition each, and the join resolves as a Nested Loop over two unique-index probes — mirroring TestContainerRemoteRepositoryStore_FindRemoteRepository_PrunesToOnePartition's in-suite assertion. Actual rows match the estimate (1 / 1) and execution stays at 0.022ms with 5000 seeded rows per table. No anomalies.

Seed shape: namespaces=1, repositories=5000 (one partition), container_remote_repositories=5000 (one partition, 1:1 with repositories)

Rendered SQL:

SELECT container_remote_repositories.namespace_id AS "container_remote_repositories.namespace_id",
     container_remote_repositories.id AS "container_remote_repositories.id",
     container_remote_repositories.repository_id AS "container_remote_repositories.repository_id",
     container_remote_repositories.url AS "container_remote_repositories.url",
     container_remote_repositories.auth_url AS "container_remote_repositories.auth_url",
     container_remote_repositories.auth_status AS "container_remote_repositories.auth_status",
     container_remote_repositories.cache_validity_hours AS "container_remote_repositories.cache_validity_hours",
     container_remote_repositories.last_health_status AS "container_remote_repositories.last_health_status",
     container_remote_repositories.last_health_checked_at AS "container_remote_repositories.last_health_checked_at"
FROM public.container_remote_repositories
     INNER JOIN public.repositories ON ((repositories.id = container_remote_repositories.repository_id) AND (repositories.namespace_id = container_remote_repositories.namespace_id))
WHERE (((container_remote_repositories.namespace_id = $1::uuid) AND (repositories.namespace_id = $2::uuid)) AND (container_remote_repositories.repository_id = $3::uuid)) AND (((repositories.format IN ($4, $5)) AND (repositories.kind = $6)) AND (repositories.soft_deleted_at IS NULL))
LIMIT $7;

Bound args: [f58d0dad-d5d8-4c88-86b1-39a81313b23d, f58d0dad-d5d8-4c88-86b1-39a81313b23d, 9febc0e6-7b4a-4cba-90a0-a5be6a09a8e8, 0, 3, 2, 1]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Limit  (cost=0.56..16.62 rows=1 width=160) (actual time=0.022..0.022 rows=1 loops=1)
   Buffers: shared hit=6
   ->  Nested Loop  (cost=0.56..16.62 rows=1 width=160) (actual time=0.021..0.021 rows=1 loops=1)
         Buffers: shared hit=6
         ->  Index Scan using container_remote_repositories__namespace_id_repository_id_idx47 on container_remote_repositories_p52 container_remote_repositories  (cost=0.28..8.30 rows=1 width=160) (actual time=0.013..0.013 rows=1 loops=1)
               Index Cond: ((namespace_id = 'f58d0dad-d5d8-4c88-86b1-39a81313b23d'::uuid) AND (repository_id = '9febc0e6-7b4a-4cba-90a0-a5be6a09a8e8'::uuid))
               Buffers: shared hit=3
         ->  Index Scan using repositories_p52_pkey on repositories_p52 repositories  (cost=0.28..8.31 rows=1 width=32) (actual time=0.007..0.007 rows=1 loops=1)
               Index Cond: ((id = '9febc0e6-7b4a-4cba-90a0-a5be6a09a8e8'::uuid) AND (namespace_id = 'f58d0dad-d5d8-4c88-86b1-39a81313b23d'::uuid))
               Filter: ((soft_deleted_at IS NULL) AND (format = ANY ('{0,3}'::smallint[])) AND (kind = '2'::smallint))
               Buffers: shared hit=3
 Planning:
   Buffers: shared hit=286
 Planning Time: 0.925 ms
 Execution Time: 0.041 ms

Timings: planning 0.925ms, execution 0.041ms, total 0.966ms.

datastore.ContainerRemoteRepositoryStore.HealthStatus

Summary: Single-row read of one remote's health status, keyed by namespace_id + repository_id. Both predicates are bound, so the planner prunes to one hash partition and Index Scans the unique (namespace_id, repository_id) index; LIMIT 1 caps the result. Actual rows match the estimate (1 / 1) and execution is ~20us with 5000 rows in the target partition. No anomalies.

Seed shape: namespaces=1, repositories=5000 (one partition), container_remote_repositories=5000 (one partition)

Rendered SQL:

SELECT container_remote_repositories.last_health_status AS "container_remote_repositories.last_health_status"
FROM public.container_remote_repositories
WHERE (container_remote_repositories.namespace_id = $1::uuid) AND (container_remote_repositories.repository_id = $2::uuid)
LIMIT $3;

Bound args: [9476aec5-b4ac-4dc3-936b-4f7dfed82e25, 89ecf56e-e5a6-4d5a-ad0f-f6af47ebefb6, 1]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Limit  (cost=0.28..8.30 rows=1 width=2) (actual time=0.009..0.010 rows=1 loops=1)
   Buffers: shared hit=3
   ->  Index Scan using container_remote_repositories_p2_namespace_id_repository_id_idx on container_remote_repositories_p20 container_remote_repositories  (cost=0.28..8.30 rows=1 width=2) (actual time=0.009..0.009 rows=1 loops=1)
         Index Cond: ((namespace_id = '9476aec5-b4ac-4dc3-936b-4f7dfed82e25'::uuid) AND (repository_id = '89ecf56e-e5a6-4d5a-ad0f-f6af47ebefb6'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=84
 Planning Time: 0.416 ms
 Execution Time: 0.020 ms

Timings: planning 0.416ms, execution 0.020ms, total 0.436ms.

datastore.ContainerRemoteRepositoryStore.compareAndSetHealthStatus.Select

Summary: The compare-and-set transaction's lock step: reads last_health_status + last_health_checked_at FOR UPDATE, keyed by namespace_id + repository_id. Prunes to one partition and Index Scans the unique index before locking the single row (LockRows). Actual rows match the estimate (1 / 1) and execution is ~22us. No anomalies.

Seed shape: namespaces=1, repositories=5000 (one partition), container_remote_repositories=5000 (one partition)

Rendered SQL:

SELECT container_remote_repositories.last_health_status AS "container_remote_repositories.last_health_status",
     container_remote_repositories.last_health_checked_at AS "container_remote_repositories.last_health_checked_at"
FROM public.container_remote_repositories
WHERE (container_remote_repositories.namespace_id = $1::uuid) AND (container_remote_repositories.repository_id = $2::uuid)
LIMIT $3
FOR UPDATE;

Bound args: [9476aec5-b4ac-4dc3-936b-4f7dfed82e25, 89ecf56e-e5a6-4d5a-ad0f-f6af47ebefb6, 1]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Limit  (cost=0.28..8.31 rows=1 width=20) (actual time=0.011..0.012 rows=1 loops=1)
   Buffers: shared hit=4
   ->  LockRows  (cost=0.28..8.31 rows=1 width=20) (actual time=0.011..0.011 rows=1 loops=1)
         Buffers: shared hit=4
         ->  Index Scan using container_remote_repositories_p2_namespace_id_repository_id_idx on container_remote_repositories_p20 container_remote_repositories  (cost=0.28..8.30 rows=1 width=20) (actual time=0.007..0.007 rows=1 loops=1)
               Index Cond: ((namespace_id = '9476aec5-b4ac-4dc3-936b-4f7dfed82e25'::uuid) AND (repository_id = '89ecf56e-e5a6-4d5a-ad0f-f6af47ebefb6'::uuid))
               Buffers: shared hit=3
 Planning:
   Buffers: shared hit=3
 Planning Time: 0.124 ms
 Execution Time: 0.022 ms

Timings: planning 0.124ms, execution 0.022ms, total 0.146ms.

datastore.ContainerRemoteRepositoryStore.compareAndSetHealthStatus.Update

Summary: The compare-and-set transaction's write step: single-row UPDATE of last_health_status + last_health_checked_at, keyed by namespace_id + repository_id. Prunes to one partition and Index Scans the unique index to locate the row; both FK triggers (namespace, repository) fire as the row is checked. No anomalies; execution includes trigger overhead.

Seed shape: namespaces=1, repositories=5000 (one partition), container_remote_repositories=5000 (one partition)

Rendered SQL:

UPDATE public.container_remote_repositories
SET (last_health_status, last_health_checked_at) = ($1, $2::timestamp with time zone)
WHERE (container_remote_repositories.namespace_id = $3::uuid) AND (container_remote_repositories.repository_id = $4::uuid);

Bound args: [1, 2026-08-06 13:xx:xx+00, 9476aec5-b4ac-4dc3-936b-4f7dfed82e25, 89ecf56e-e5a6-4d5a-ad0f-f6af47ebefb6]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Update on container_remote_repositories  (cost=0.28..8.30 rows=0 width=0) (actual time=0.189..0.189 rows=0 loops=1)
   Update on container_remote_repositories_p20 container_remote_repositories_1
   Buffers: shared hit=20
   ->  Index Scan using container_remote_repositories_p2_namespace_id_repository_id_idx on container_remote_repositories_p20 container_remote_repositories_1  (cost=0.28..8.30 rows=1 width=20) (actual time=0.007..0.007 rows=1 loops=1)
         Index Cond: ((namespace_id = '9476aec5-b4ac-4dc3-936b-4f7dfed82e25'::uuid) AND (repository_id = '89ecf56e-e5a6-4d5a-ad0f-f6af47ebefb6'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=3
 Planning Time: 0.155 ms
 Trigger for constraint fk_container_remote_repositories_namespace_id_namespaces on container_remote_repositories_p20: time=0.014 calls=1
 Trigger for constraint fk_container_remote_repositories_repository_id_repositories on container_remote_repositories_p20: time=0.394 calls=1
 Execution Time: 0.794 ms

Timings: planning 0.155ms, execution 0.794ms (0.4ms of it FK trigger checks), total 0.949ms.

datastore.ContainerRemoteRepositoryStore.ListRemotes

Summary: Plan matches the method's own doc comment: with no namespace_id predicate to prune on, the planner Appends over all 64 partitions of both repositories and container_remote_repositories, joining via Hash Join. Plan and actual rows are close (5063 est / 5000 actual) and execution is 8.3ms at 5000 seeded rows — proportional to table size, exactly the "grows with the number of live container-format remotes system-wide" that ListRemotes' doc comment and the plan's Step 6 "Accepted trade-off" both call out. Not a new finding. See the pre-ceiling caveat above the table: the SQL and args below are current, the plan below is not.

Seed shape: namespaces=1, repositories=5000 (one partition), container_remote_repositories=5000 (one partition, 1:1 with repositories)

Rendered SQL (re-rendered from the current builder):

SELECT container_remote_repositories.namespace_id AS "container_remote_repositories.namespace_id",
     container_remote_repositories.id AS "container_remote_repositories.id",
     container_remote_repositories.repository_id AS "container_remote_repositories.repository_id",
     container_remote_repositories.url AS "container_remote_repositories.url"
FROM public.container_remote_repositories
     INNER JOIN public.repositories ON ((repositories.id = container_remote_repositories.repository_id) AND (repositories.namespace_id = container_remote_repositories.namespace_id))
WHERE ((repositories.format IN ($1, $2)) AND (repositories.kind = $3)) AND (repositories.soft_deleted_at IS NULL)
LIMIT $4;

Bound args: [0, 3, 2, 100001]

Plan (EXPLAIN (ANALYZE, BUFFERS) output, abridged — 64-partition Append repeated on both sides of the join):

 Hash Join  (cost=256.52..473.70 rows=5063 width=98) (actual time=3.694..6.792 rows=5000 loops=1)
   Hash Cond: ((repositories.id = container_remote_repositories.repository_id) AND (repositories.namespace_id = container_remote_repositories.namespace_id))
   Buffers: shared hit=5359
   ->  Append  (cost=0.00..190.59 rows=5063 width=32) (actual time=0.096..2.538 rows=5000 loops=1)
         Buffers: shared hit=237
         ->  Seq Scan on repositories_p00 repositories_1  (cost=0.00..0.00 rows=1 width=32) (actual time=0.015..0.016 rows=0 loops=1)
               Filter: ((soft_deleted_at IS NULL) AND (format = ANY ('{0,3}'::smallint[])) AND (kind = '2'::smallint))
         -- ... 61 more per-partition Seq Scans / one Index Scan (repositories_p23, using
         --     repositories_p23_namespace_id_format_name_idx) on repositories, all but one
         --     partition returning 0 rows ...
         ->  Seq Scan on repositories_p14 repositories_15  (cost=0.00..157.00 rows=5000 width=32) (actual time=0.012..0.531 rows=5000 loops=1)
               Filter: ((soft_deleted_at IS NULL) AND (format = ANY ('{0,3}'::smallint[])) AND (kind = '2'::smallint))
               Buffers: shared hit=82
   ->  Hash  (cost=180.58..180.58 rows=5063 width=98) (actual time=3.502..3.522 rows=5000 loops=1)
         Buckets: 8192  Batches: 1  Memory Usage: 699kB
         Buffers: shared hit=5122
         ->  Append  (cost=0.00..180.58 rows=5063 width=98) (actual time=0.044..2.335 rows=5000 loops=1)
               Buffers: shared hit=5122
               -- ... 61 more per-partition Seq Scans / one Index Scan
               --     (container_remote_repositories_p23, using
               --     container_remote_repositories__namespace_id_repository_id_idx21) ...
               ->  Seq Scan on container_remote_repositories_p14 container_remote_repositories_15  (cost=0.00..147.00 rows=5000 width=98) (actual time=0.005..0.457 rows=5000 loops=1)
                     Buffers: shared hit=97
 Planning:
   Buffers: shared hit=2205 read=121
 Planning Time: 34.187 ms
 Execution Time: 8.301 ms

Timings: planning 34.187ms (128 partition children examined), execution 8.301ms, total 42.488ms.

Query notes:

  • ContainerRemoteRepositoryStore.ListRemotes: full Append over 64 partitions on both repositories and container_remote_repositories. Already documented and accepted — remote.RemoteRepositoryEnumerator.ListRemotes takes no per-namespace parameter, so narrowing the scan needs a signature change to that S13-owned interface rather than a query-level fix in this store. Not a new finding; recorded here as the empirical confirmation of the trade-off named in ListRemotes' doc comment and the plan's Step 6 "Accepted trade-off" entry. The rendered SQL does carry a LIMIT (maxListContainerRemotes+1), which the plan above predates; at 100,001 against 5,000 seeded rows it adds a Limit node over the join and changes nothing else about the shape. TestContainerRemoteRepositoryStore_ListRemotes_PlanShape pins the rest of the shape, including that no repositories index serves the global kind filter; #531 (closed) tracks the partial index that changes that.
Edited by Sylvia Shen

Merge request reports