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.DB and resolves s.client.DB() itself. The ContainerManifestStore doc 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 named ListContainerManifests as 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 joins rawSQLTimedFunctions and times with a deferred metrics.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 through database/sql/driver.Valuer rather than a pq.ByteaArray type assertion, because lib/pq is an indirect dependency and asserting the concrete type would promote it to direct and force a go.mod edit.
  • The image id is a predicate, not a filter. index_container_manifests_on_ns_id_ci_id_subject_digest_digest leads 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 empty manifests array 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 the container_image_id log field that rule moves to the caller, because the management API's 500 writers restore only namespace_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. ListReferrersPage and ChildManifestsExist do not check either, and the column CHECK fixes digest at 32 bytes, so an over-long element is unmatchable rather than dangerous.
  • Sort Method: is not asserted. Plain EXPLAIN never emits it, so the assertion would be unreachable. Nine other files carry it against plain EXPLAIN for 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_digest third 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 = OFF removed 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 HashAggregate is 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 400ef5976 and have drifted: childManifestsExistStmt is at :549 not :547, and ChildManifestsExist at :471 not :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 smells takes the interval explicitly, with !1131 and !1133 as the precedent under the same chore(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/datastore or internal/managementapi arms a context.WithTimeout, the service sets no statement_timeout, and no http.TimeoutHandler wraps 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.

🤖 Generated with Claude Code

Related to #1150 (closed)

Merge request reports

Loading
Loading