chore(datastore): the windowed tag-names read over container_tags (S17 Container Manifest Reads plan: 8/20)

Hayley Swimelarrequested to merge
1150/step-8-tag-names-by-manifest-ids into main

Why

Step 8 of the S17 container manifest reads plan. The redesigned container version-list and version-detail pages carry each manifest's tags, and no read in the service can answer that: container_tags has no statement scoped by container_manifest_id. This adds one, batched over a page's manifest ids, returning each manifest's first ten names beside the true total from the same statement, which is what lets AC #130 (closed) ask for a preview and a count together inside AC #138 (closed)'s four-query page bound. Step 10 is the first caller.

What

The window needs a derived table. ROW_NUMBER() OVER (PARTITION BY container_manifest_id ORDER BY name) cannot be filtered in WHERE, so the rn <= $N bound crosses a subquery. Complete drops the row number and its filter instead of passing a large limit, so one builder serves both shapes under one query name.

The zero value is rejected, not read. A PreviewLimit of zero read as "the complete array" would turn a forgotten field into the unbounded read the window exists to prevent: index-backed, inside AC #138 (closed)'s statement count, and past every EXPLAIN pin. Everything but {PreviewLimit: N} and {Complete: true} returns errContainerTagPreviewShape before any database access.

The projection is aliased under a manifest_tag_row. prefix. qrm keys an untagged destination field on <type>.<field>, so a model.ContainerTags embed beside a sibling Total leaves Total zeroed with no error.

Complete carries no LIMIT. The write path caps a manifest at container.manifest_max_tags (1,000, ADR-004) and the batch limit caps the call at 100 ids, so 100,000 rows bounds it.

Measured at the ceiling (AC #139 (closed))

At that ceiling, 100 manifests at 1,000 tags each, the page shape runs 58 ms and the complete shape 51 ms, worst of three EXPLAIN (ANALYZE, BUFFERS) runs on PostgreSQL 17.10 under CI's .pg-service-options flags. Both inside the 100 ms budget, so the family ships as a batched count rather than the counter D4's ladder would otherwise force. The figure lands in the "Measured at the ceiling" table this MR adds under Timing budget.

The plan still rides container_tags_pNN_namespace_id_container_manifest_id_idx on one partition at that size, and Postgres pushes the row-number bound into the inner WindowAgg as a Run Condition, so the outer filter drops 99,000 of 100,000 rows without a second pass.

Treat the 42% headroom as optimistic. Review reproduced 57.2 ms at this exact fixture, but the same shape rebuilt with 200,000 extra rows of noise on different hardware measured 71 to 97 ms. The budget holds for the documented ceiling on the stated setup, and it is hardware-sensitive. The index note below is the lever if that margin turns out to matter.

Spec coverage

# Criterion Covered by
AC #130 (closed) Ten names ascending, the true total beside them, the complete array on the detail shape TestContainerTagStore_ListTagNamesByManifestIDs, _TagCapShapedManifest. Serialization is Step 10's
AC #138 (closed) One statement per family, invariant to page size TestListTagNamesByManifestIDsStmt pins one builder serving both shapes. The page-level query count is Step 10's
AC #139 (closed) The statement rides the family's index and never scans the partition TestListTagNamesByManifestIDsStmt_RidesTheManifestIDIndex, on a fixture under the partition-share threshold. The timing half is above, not a test (D4)

AC #128 (closed), #129 (closed), and #131 (closed) through #137 (closed) belong to Steps 6, 7, 10, 12, 14, 16, 18, 19, and 20.

Reviewable LOC

Group Lines
Source: container_tag.go, query_names.go, list.go 163
Tests: container_tag_test.go, container_tag_integration_test.go, list_internal_test.go 525
Docs: database-query-patterns.md 16
Total 704

Past guardrail 18's line on tests, and past the plan's own ~390 forecast. Splitting does not help: the statement, its guard table, its integration suite, and AC #139 (closed)'s pin are one read, so landing the pin alone would put the criterion in an MR with nothing to pin, and landing the tests alone would break test-first authorship. Source is 163 lines.

internal/managementapi is not in the plan's file list for this step, and the 9 lines here are the smallest edit that keeps it true. list.go names the datastore constants that copy maxPageSize and says those four are the whole list. This branch adds the fifth, so the comment and the failure message an engineer follows when raising the cap both go stale on merge (guardrail 17). The same message now also names the ceiling row below, whose figure a raise invalidates.

Test plan

mise exec -- env -u GOROOT GOEXPERIMENT=jsonv2 go test -count=1 ./internal/datastore/...

ARTIFACT_REGISTRY_DATABASE_TEST_DSN="postgres://<user>:<pw>@<host>:<port>/<db>?sslmode=disable" \
  mise exec -- env -u GOROOT GOEXPERIMENT=jsonv2 \
  go test -count=1 -tags=integration -run 'ListTagNamesByManifestIDs' ./internal/datastore/

mise exec -- env -u GOROOT GOEXPERIMENT=jsonv2 golangci-lint run \
  --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false \
  ./internal/datastore/...

To reproduce the figure: seed 100 manifests under one image, batch-insert 1,000 tag rows each with generate_series, ANALYZE, then EXPLAIN (ANALYZE, BUFFERS) what listTagNamesByManifestIDsStmt builds for {PreviewLimit: 10} over all 100 ids.

Context for LLM agents

Design rationale, and what was rejected

  • Batched window over a lateral subquery per row. The spec hands the choice to implementation with a recorded prior: the lateral rewrite of a neighboring shape measured three times the plan work and forty-four times the buffers (docs/dev/database-query-patterns.md, repository statistics live counts). A lateral would also fold the family into the list statement, reading as zero further queries under AC #138 (closed) and leaving the timing measurement no per-family figure to report.

  • Batched window over "fetch the capped array and slice ten in Go". That alternative has an exact in-tree precedent in NpmDistTagNamesByVersionIDs and satisfies the same two contract sentences. It fails on the parents family (Step 15), whose array is uncapped per manifest and bounded only by container.image_max_manifests, so a hundred-row page would pull up to 2.5 million digest rows into handler memory to produce a thousand. Tags would survive it, but taking the window for parents alone would carry two shapes for one job.

  • A flag rather than the configured cap for the complete shape. Passing container.manifest_max_tags would silently truncate an array the contract calls complete on manifests pushed under a higher value, and it would pull config.ContainerConfig into a store call with no other use for it.

  • Guard precedence is namespace, then the id-set guards, then the shape guard, mirroring NpmDistTagNamesByVersionIDs. Each guard-table row violates exactly one guard, so no test over-asserts the order.

  • The batch limit is inclusive at 100, rejecting at 101, mirroring npm's npmDistTagVersionIDsBatchLimit.

  • The EXPLAIN pin deliberately omits the sort-free assertions that TestContainerTagStore_ListContainerTagsPage_DeepPageIsIndexBacked carries. A manifest-scoped read's per-manifest name order is not index-backed: the measured plan shows an Incremental Sort with Sort Key: container_tags.container_manifest_id, container_tags.name and Sort Method: quicksort, so copying either assertion would fail against correct code. D4 predicted this and it is confirmed empirically.

  • The pin's fixture sits under the partition-share threshold and the timing fixture at the ceiling. They are different fixtures on purpose: past that share the planner stops probing the page's rows and scans the partition, which would fail the pin for the one reason AC #139 (closed) excludes.

  • The shared-jet-value hazard was checked and does not apply. internal/AGENTS.md warns that handing a package-level jet value to a mutating wrapper races across concurrent requests. Here Column.From(subQuery) returns a fresh column rather than mutating the generated one, and COUNT(pg.STAR) stores STAR in a parameter slice without calling setRoot on it. Verified as well as read: 200 concurrent builds of both shapes under -race produced byte-identical SQL.

  • The pin asserts an Index Cond binding both namespace_id and container_manifest_id. Without it, an implementation that bound the namespace alone and filtered the ids in the heap would print the same Index Scan using ... line and pass.

  • An index that would remove the sort was measured and deliberately not taken. Widening index_container_tags_on_namespace_id_and_container_manifest_id to (namespace_id, container_manifest_id, name) turns the plan into an Index Only Scan with Heap Fetches: 0 and drops buffers from 100,107 to 938, removing the Incremental Sort entirely. It is not in this MR because S17's fetch mechanism states the patch adds no index and owes ADR-007 nothing, so taking it needs a spec amendment rather than an implementation decision. Recorded here with its numbers so the amendment, if anyone opens one, starts from a measurement.

Non-goals

  • No production caller. Step 10 serves tags_preview, tags_count, and the detail tags. The plan's accepted-code-smells section takes this explicitly, with !1131 and !1133 as the precedent for the same interval under the same chore(datastore) prefix. Wiring a caller here would pull in Step 10's facts seam.
  • No remote arm. Step 9 mirrors this read over container_remote_tags with a constant window on its complete shape, and reuses ManifestTags. That is why the result type's doc says a second read will return it.
  • No deadline on the read. No read path in internal/datastore or internal/managementapi arms a context.WithTimeout today, 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. The plan declines it and states the exposure.
  • No index. The spec's fetch mechanism names index_container_tags_on_namespace_id_and_container_manifest_id, which exists, so this owes ADR-007 nothing.
  • No configuration. Guardrail 14: the window's constant and the batch limit are constants beside the read, not knobs.
  • No plan-file edit. The plan's Status table has one owner MR.

Falsified while building

  • The plan and the spec both describe the pin as asserting a truncated index suffix. The child index is container_tags_pNN_namespace_id_container_manifest_id_idx at 57 characters, under NAMEDATALEN - 1, so it is not truncated. The pin matches the full auto-generated child name.
  • The plan cites go-jet v2.15.0. The tree is on v2.16.0. Every window API it names is present.

Database Review Evidence

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17.10 container (matching GL_PG_CURR_VERSION from .gitlab-ci-other-versions.yml) under CI's .pg-service-options flags, seeded at this MR's documented ceiling and ANALYZEd, with the container torn down at the end of the run. Numbers reflect that ceiling on one host 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 Exec time Buffers (hit / read) Partitions
datastore.ListTagNamesByManifestIDs.Preview Subquery Scan container_tags_p26_namespace_id_container_manifest_id_idx 100000 / 1000 16327.40 55.71 ms 1640 / 0 1/64
datastore.ListTagNamesByManifestIDs.Complete Incremental Sort container_tags_p26_namespace_id_container_manifest_id_idx 100000 / 100000 13577.40 54.10 ms 1640 / 0 1/64

Query notes:

  • ListTagNamesByManifestIDs (both shapes): the ceiling figure moves with the order tag names arrive in, and two of the four fixture layouts miss the 100 ms budget. All four seed the same ceiling (100 manifests, 1,000 tags each, 100,000 rows, one namespace), take the same plan (Index Scan on container_tags_p26_namespace_id_container_manifest_id_idx, one partition of 64, Incremental Sort, quicksort at 28 kB and 71 kB), and differ only in insert order. Worst of three runs each:

    Heap order Name order inside a manifest Buffers Preview Complete
    Grouped by manifest ascending 1,640 55.7 ms 54.1 ms
    Grouped by manifest arbitrary 1,640 109.0 ms 102.1 ms
    Interleaved across manifests ascending 100,101 74.4 ms 69.7 ms
    Interleaved across manifests arbitrary 100,101 129.7 ms 124.9 ms

    The Incremental Sort absorbs the whole difference (25.9 ms against 76.3 ms on the preview shape), because a manifest's names reach the sort already ordered in the ascending rows and unordered in the arbitrary ones. A manifest holding 1,000 tags collects them over time, so arbitrary arrival is the ordinary case and ascending arrival is the favorable one. Choosing what to do about it is the spec owner's call, and this note proposes nothing.

  • ListTagNamesByManifestIDs.Preview: the root node estimates 100,000 rows against 1,000 actual, a 100x overestimate. Postgres cannot estimate the selectivity of a filter over a window function's output, so it assumes every row survives rn <= $102. ANALYZE ran, and the plan Postgres picks is already the indexed one, so nothing is wrong today. It matters when a caller nests this statement inside a join, where a 100x row estimate can pick the wrong join strategy. Step 10 consumes the result in Go rather than in SQL, which avoids it.

datastore.ListTagNamesByManifestIDs.Preview

Summary: The plan matches the method's intent. The namespace literal prunes to one partition of 64, both key columns bind in the Index Cond rather than filtering in the heap, and Postgres pushes the row-number bound into the inner WindowAgg as a Run Condition. Two anomalies stand above, both recorded in the notes: the root's 100x row overestimate, and the ceiling figure's dependence on tag-name insert order.

Seed shape: namespaces=1, repositories=1, container_repositories=1, container_images=1, blob_storage_blobs=1, blob_storage_attachments=1, container_manifests=100, container_tags=100000

Rendered SQL (the 100 manifest-id placeholders $2 through $101 are elided from the IN list for length, and are otherwise verbatim from .Sql()):

SELECT t."container_tags.container_manifest_id" AS "manifest_tag_row.container_manifest_id",
     t."container_tags.name" AS "manifest_tag_row.name",
     t.total AS "manifest_tag_row.total"
FROM (
          SELECT container_tags.container_manifest_id AS "container_tags.container_manifest_id",
               container_tags.name AS "container_tags.name",
               COUNT(*) OVER (PARTITION BY container_tags.container_manifest_id) AS "total",
               ROW_NUMBER() OVER (PARTITION BY container_tags.container_manifest_id ORDER BY container_tags.name ASC) AS "rn"
          FROM public.container_tags
          WHERE (container_tags.namespace_id = $1::uuid) AND (container_tags.container_manifest_id IN ($2::uuid, ..., $101::uuid))
     ) AS t
WHERE t.rn <= $102
ORDER BY t."container_tags.container_manifest_id" ASC, t."container_tags.name" ASC;

Bound args: $1 = 01900000-7000-7000-8000-000000000001 (the seeded namespace), $2 through $101 = the 100 seeded manifest ids 0190000N-7000-7000-8000-00000000000N for N in 1 to 100 hexadecimal, $102 = 10.

Plan (EXPLAIN (ANALYZE, BUFFERS) output, id array elided as above):

Subquery Scan on t  (cost=260.72..16327.40 rows=100000 width=37) (actual time=0.830..55.626 rows=1000 loops=1)
  Buffers: shared hit=1640
  ->  WindowAgg  (cost=260.72..15327.40 rows=100000 width=45) (actual time=0.830..55.567 rows=1000 loops=1)
        Filter: ((row_number() OVER (?)) <= '10'::bigint)
        Rows Removed by Filter: 99000
        Buffers: shared hit=1640
        ->  WindowAgg  (cost=108.68..13827.40 rows=100000 width=37) (actual time=0.286..36.707 rows=100000 loops=1)
              Run Condition: (row_number() OVER (?) <= '10'::bigint)
              Buffers: shared hit=1640
              ->  Incremental Sort  (cost=108.54..12077.40 rows=100000 width=29) (actual time=0.283..25.939 rows=100000 loops=1)
                    Sort Key: container_tags.container_manifest_id, container_tags.name
                    Presorted Key: container_tags.container_manifest_id
                    Full-sort Groups: 100  Sort Method: quicksort  Average Memory: 28kB  Peak Memory: 28kB
                    Pre-sorted Groups: 100  Sort Method: quicksort  Average Memory: 71kB  Peak Memory: 71kB
                    Buffers: shared hit=1640
                    ->  Index Scan using container_tags_p26_namespace_id_container_manifest_id_idx on container_tags_p26 container_tags  (cost=0.29..5842.51 rows=100000 width=29) (actual time=0.015..8.711 rows=100000 loops=1)
                          Index Cond: ((namespace_id = '01900000-7000-7000-8000-000000000001'::uuid) AND (container_manifest_id = ANY ('{...100 manifest ids, $2 through $101...}'::uuid[])))
                          Buffers: shared hit=1640
Planning:
  Buffers: shared hit=37
Planning Time: 0.535 ms
Execution Time: 55.710 ms

Timings: planning 0.535 ms, execution 55.710 ms, total 56.245 ms.

datastore.ListTagNamesByManifestIDs.Complete

Summary: The plan matches the method's intent. It rides the same index on the same single partition, and with no outer filter the planner flattens the derived table and returns all 100,000 rows the documented ceiling allows. Estimate and actual agree at 100,000. The Incremental Sort carries the whole cost, so this shape moves with tag-name insert order the same way the preview shape does.

Seed shape: namespaces=1, repositories=1, container_repositories=1, container_images=1, blob_storage_blobs=1, blob_storage_attachments=1, container_manifests=100, container_tags=100000

Rendered SQL (same elision as above):

SELECT t."container_tags.container_manifest_id" AS "manifest_tag_row.container_manifest_id",
     t."container_tags.name" AS "manifest_tag_row.name",
     t.total AS "manifest_tag_row.total"
FROM (
          SELECT container_tags.container_manifest_id AS "container_tags.container_manifest_id",
               container_tags.name AS "container_tags.name",
               COUNT(*) OVER (PARTITION BY container_tags.container_manifest_id) AS "total"
          FROM public.container_tags
          WHERE (container_tags.namespace_id = $1::uuid) AND (container_tags.container_manifest_id IN ($2::uuid, ..., $101::uuid))
     ) AS t
ORDER BY t."container_tags.container_manifest_id" ASC, t."container_tags.name" ASC;

Bound args: $1 = 01900000-7000-7000-8000-000000000001, $2 through $101 = the same 100 seeded manifest ids. No $102, because Complete drops the row number and its filter.

Plan (EXPLAIN (ANALYZE, BUFFERS) output, id array elided as above):

Incremental Sort  (cost=196.16..13577.40 rows=100000 width=37) (actual time=0.735..51.542 rows=100000 loops=1)
  Sort Key: container_tags.container_manifest_id, container_tags.name
  Presorted Key: container_tags.container_manifest_id
  Full-sort Groups: 100  Sort Method: quicksort  Average Memory: 28kB  Peak Memory: 28kB
  Pre-sorted Groups: 100  Sort Method: quicksort  Average Memory: 79kB  Peak Memory: 79kB
  Buffers: shared hit=1640
  ->  WindowAgg  (cost=73.64..7342.51 rows=100000 width=37) (actual time=0.281..30.774 rows=100000 loops=1)
        Buffers: shared hit=1640
        ->  Index Scan using container_tags_p26_namespace_id_container_manifest_id_idx on container_tags_p26 container_tags  (cost=0.29..5842.51 rows=100000 width=29) (actual time=0.025..10.689 rows=100000 loops=1)
              Index Cond: ((namespace_id = '01900000-7000-7000-8000-000000000001'::uuid) AND (container_manifest_id = ANY ('{...100 manifest ids, $2 through $101...}'::uuid[])))
              Buffers: shared hit=1640
Planning Time: 0.415 ms
Execution Time: 54.095 ms

Timings: planning 0.415 ms, execution 54.095 ms, total 54.510 ms.

No migration files changed, so migration mode did not run.

🤖 Generated with Claude Code

Related to #1150 (closed)

Edited by Hayley Swimelar

Merge request reports