chore(datastore): the batched referrers count over container_manifests (S17 Container Manifest Reads plan: 11/20)
Why
Step 11 of the S17 container manifest reads plan. The redesigned image-detail and version-list pages carry each manifest's referrer count, and nothing in the service can answer that for a page: ListReferrersPage reads one subject's referrers as full sixteen-column rows, a statement per row on a hundred-row page. This adds the batched count, driven from the page's digests rather than its manifest ids, because the partial referrers index is keyed on subject_digest and carries no manifest id (AC #139 (closed)). Step 12 is the first caller.
What
The count needs no window. The other four families carry a preview array beside their count, so their statements number rows and filter that number across a derived table (D3). A count has no array, so this is a plain GROUP BY subject_digest under the namespace and image equality prefix, the repository's first production GROUP BY in a Go statement builder.
A digest with no referrers is absent from the map, not present with a zero. AC #133 (closed)'s 0 lives in Step 12's serializer, which renders a missing key. A zero-valued entry would be indistinguishable in the response and would hide a statement that counted nothing, so the integration case asserts the map's length rather than a value.
The EXPLAIN pin runs the planner unaided, and its sort-free half binds from PostgreSQL 17. It seeds a 2,000-row sibling image so the target holds about 0.5% of its partition, rather than forcing the index with enable_seqscan = OFF, under which the no-seq-scan assertion could not fail. TestListContainerManifests_ViewsRideTheirOwnIndex 70 lines above uses the same ballast for the same reason. PG 17's nbtree rework keeps a btree scan's ordering across an = ANY condition and PG 16 drops it, so the sort-free assertions sit behind a version gate and D4's sort-free premise is version-scoped.
Two files sit outside the plan's declared list. internal/managementapi/list.go and its pin both said six constants copy maxPageSize, and this adds the seventh (guardrail 17). ## Bounding result sets said a GROUP BY "grows unbounded as keys accumulate", which read literally demands a LIMIT here, so it now states the bound a capped list on the grouping column carries.
Measured at the ceiling (AC #139 (closed))
25,000 referrer rows image-wide, container.image_max_manifests, spread 250 per digest over a hundred-digest page. PostgreSQL 17.10 under CI's .pg-service-options flags, EXPLAIN (ANALYZE, BUFFERS), worst of three across two arrival orders.
| Arm | Plan | Buffers | Worst of 3 |
|---|---|---|---|
| All-visible, image owns its partition | Seq Scan, HashAggregate | 929 | 15.2 ms |
| All-visible, image at 9% of its partition | Index Only Scan, sort-free GroupAggregate | 508 | 10.3 ms |
| Pre-vacuum, rows clustered | Index Only Scan, 25,000 heap fetches | 25,534 | 15 ms |
| Pre-vacuum, rows interleaved | Index Only Scan, 25,000 heap fetches | 25,534 | 94 ms |
The family clears D4's 100 ms budget, and the pre-vacuum interleaved arm clears it by 6%. By the spec's letter the read ships as a batched count and the counter ladder stays unclimbed. Six percent is noise rather than headroom, and that arm is the shape a page reads right after a push, on fast local storage, so whether Step 11 still clears D4 is a ruling rather than a reviewer's call. This MR builds no counter and adds no index either way, and both figures are in the query-patterns row. What moves is the heap fetches, not the row count: 535 buffers all-visible against 25,534 pre-vacuum, after which the cost tracks how many of those pages the buffer pool holds (204 MB heap, 128 MB pool).
Spec coverage
| # | Criterion | Covered by |
|---|---|---|
| AC #133 (closed) | The per-image count, sibling images and other namespaces excluded, hidden referrer rows included | TestContainerManifestStore_CountReferrersBySubjectDigests (5 subtests), _CountsRowsTheDefaultListHides, _NamespaceIsolation, _BatchLimitBoundary. The 0 is Step 12's, the route walk Step 14's |
| AC #139 (closed) | Rides the partial referrers index, never scans the partition, reads sort-free from PG 17 | TestCountReferrersBySubjectDigests_PartialIndexScan, on a fixture under the partition-share threshold. The timing half is above, not a test (D4) |
| AC #138 (closed) | One statement per family, invariant to page size | TestCountReferrersBySubjectDigestsStmt pins one builder and three binds whatever the digest count. The page-level count is Step 12's |
AC #128 (closed) through #132 (closed) and #134 (closed) through #137 (closed) belong to Steps 6, 7, 10, 12, 14, 16, 18, 19, and 20.
Reviewable LOC
| Group | Added |
|---|---|
Source: container_manifest.go, query_names.go, list.go |
115 |
Tests: container_manifest_integration_test.go, container_manifest_test.go, queries_test.go, list_internal_test.go |
598 |
Docs: database-query-patterns.md |
17 |
| Total | 730 |
Past guardrail 18's line on tests, and past the plan's ~280 test forecast. Splitting does not help: the statement, its guard table, its integration suite, and AC #139 (closed)'s pin are one read, so the pin alone would land in an MR with nothing to pin and the tests alone would break test-first authorship. Source is 115 against the plan's ~110.
Test plan
mise exec -- env -u GOROOT go test -count=1 ./...
ARTIFACT_REGISTRY_DATABASE_TEST_DSN="postgres://<user>:<pw>@<host>:<port>/<db>?sslmode=disable" \
mise exec -- env -u GOROOT go test -count=1 -tags=integration ./internal/datastore/...
mise exec -- env -u GOROOT golangci-lint run --build-tags=integration \
--max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false \
--new-from-rev origin/main ./internal/datastore/... ./internal/managementapi/...The pin ran against all three CI PostgreSQL legs on the shipping fixture: 16.15 and 17.10 at -count=5, 18.4 at -count=3, plus -count=20 in one progressively bloating database.
This step adds no route and changes no response, so no e2e scenario or usage-data row is added or affected (guardrail 12).
Context for LLM agents
Design rationale, and what was rejected
- The method takes no
db qrm.DBand resolvess.client.DB()itself. TheContainerManifestStoredoc comment sets the convention by role rather than by store: protocol methods take a handle so callers compose inside a format-handler transaction, management reads deref the client so no management handler holds a transaction open across an HTTP response. Steps 8 and 9 do the same. Both of that comment's blocks namedListContainerManifestsas the only management read and were corrected here. - A positional row loop rather than a qrm model scan, following
ChildManifestsExist: the result is a map over a two-column projection with no generated model, and a PK-less projection into a model slice sits in qrm's silent row-grouping territory. That route means the method joinsrawSQLTimedFunctionsand times with a deferredmetrics.InstrumentQuery, a fourth file beyond the plan's Files list. - The digest set binds as one
bytea[], not a parameter per digest, so the statement is a single round trip whatever the page size. The unit test asserts three binds throughdatabase/sql/driver.Valuerrather than apq.ByteaArraytype assertion, becauselib/pqis an indirect dependency and asserting the concrete type would promote it to direct and force ago.modedit. - The image id is a predicate, not a filter.
index_container_manifests_on_ns_id_ci_id_subject_digest_digestleads with it, and without it the count would include a sibling image's referrers on the same subject, which AC #133 (closed) scopes out. - The batch limit is inclusive at 100, rejecting at 101, mirroring
containerTagManifestIDsBatchLimit. Guard order is context, namespace, image, empty set, over-limit, and the guard table constructs a zero-valued store so a guard reaching the pool would nil-panic rather than return its sentinel. - The empty set rejects rather than returning an empty map, unlike
ChildManifestsExist, whose empty case is legitimate (an index with an emptymanifestsarray references nothing). A page with no manifests never reaches this call, so an empty set is a caller bug. - The error string carries no identifiers, per
docs/dev/database-query-patterns.md, with an integration case pinning it. Step 12's call site owes thecontainer_image_idlog field that rule moves to the caller, because the management API's 500 writers restore onlynamespace_id. That obligation is recorded nowhere in tree. internal/managementapi/list.go's three-line comment lost the words "by value" and "deliberately". The seventh identifier is 41 characters, the block is capped at 3 lines and 160 columns, and the original text needs 474 characters against 457 usable. Three independent packing searches over trimmed variants found no arrangement that keeps both.- Digest elements are not length-checked.
ListReferrersPageandChildManifestsExistdo not check either, and the column CHECK fixesdigestat 32 bytes, so an over-long element is unmatchable rather than dangerous. Sort Method:is not asserted. PlainEXPLAINnever emits it, so the assertion would be unreachable. Nine other files carry it against plainEXPLAINfor the same reason, and fixing one in isolation would diverge from the convention.
Falsified while building
- D4's ground for the sort-free assertion is necessary but not sufficient. The plan says the index carries
subject_digestthird under a two-column equality prefix and covers every column the count reads. True, and sort-freedom also needs the alternative plans to be unattractive, which is what the sibling ballast buys and what PG 16 does not deliver at all. One clarifying sentence in D4, not an amendment. - The first pin fixture was on the wrong side of AC #139 (closed)'s threshold. Ten rows alone in a partition is 100% share, and it reached the index only because
enable_seqscan = OFFremoved the alternative. It failed 9 of 30 encounters in a shared test database. The selective shape (200 rows over 100 digests, requesting 3) is not the better reading either: its image also holds 100% of its partition, so it is the same defect with a different digest distribution. - Arrival order does not move the all-visible figure. Both all-visible plans are order-insensitive, so grouped and interleaved differ by host load, which the identical buffer counts within each arm confirm. That is the opposite of Step 8, where an Incremental Sort absorbed the whole difference. Order matters only pre-vacuum, where it is 10 to 15 ms against 78 to 94 ms.
- The plan shape turns on the image's share of its partition, not on a sibling namespace. Seeded alone the image owns its partition, the digest set matches 25,000 of 25,100 rows, and scanning under a
HashAggregateis cheapest. At 9% share the planner switches to the index. A production partition pools roughly 1/64 of all namespaces, so the index arm is the production shape. - The plan's line cites are at
400ef5976and have drifted:childManifestsExistStmtis at:549not:547, andChildManifestsExistat:471not:472. - The integration-tag lint baseline for these two packages is 5,659 findings, not the ~17,600 output lines a line count suggests. Zero new.
- Test LOC came in at 598 against the plan's ~280 forecast, roughly 2x. Source came in at 115 against ~110.
Non-goals
- No production caller. Step 12 serves
referrers_count. The plan's### Accepted code smellstakes the interval explicitly, with!1131and!1133as the precedent under the samechore(datastore)prefix. - No counter and no index. D4's ladder is climbed on a budget miss, and the measurement clears. The covering-index question is the spec owner's and lives in Step 21.
- No configuration (guardrail 14). The batch limit is a constant beside the read.
- No deadline on the read. No read path in
internal/datastoreorinternal/managementapiarms acontext.WithTimeout, the service sets nostatement_timeout, and nohttp.TimeoutHandlerwraps the mux. Arming one here would be the first, decided inside one family's step. - No plan-file edit. The Status table has one owner MR, and the row is owed from the standing
docs(plans)branch. - No spec or ADR edit. The mirror is stale by one upstream ADR-009 commit (a repository-delete in-use refusal) which does not reach this surface.
Related to #1150 (closed)