chore(datastore): widen the container_tags manifest-id index (S17 Container Manifest Reads plan: 21/21)

Why

Step 21 of the S17 Container Manifest Reads plan. Step 8's hosted tag read rides index_container_tags_on_namespace_id_and_container_manifest_id, which answers its search and not its order, so PostgreSQL sorts every matched row before the preview keeps ten. That sort's cost tracks the order the names arrived in rather than the row count, and a manifest collects its thousand tags over time. Measured at the ceiling on one host: 64 ms preview with the names arriving ascending and the heap grouped, 138 ms with them arriving in arbitrary order and the heap interleaved, against the 100 ms budget AC #139 (closed) gates closed beta on. Step 8 recorded the favorable end of that spread. This migration widens the index to (namespace_id, container_manifest_id, name), which makes the preview shape index-only and sort-free and drops the ordinary figure to 45 ms.

What is not obvious

PostgreSQL 16 keeps a sort, and still gains. PG 17's nbtree array-key rework is what lets an = ANY index scan return rows in index order. On PG 16, with a page of 100 manifests against a 500-manifest namespace, the plan is the same Index Only Scan with both key columns bound and no heap fetch, plus a full Sort of the page's 100,000 rows: 71 ms against 138 ms before the index, inside the same budget. PG 17.10 and PG 18.4 are sort-free at 48 ms.

The complete shape keeps an Incremental Sort, and the index is not why. WindowAgg truncates its output ordering to its own window's, and the complete shape's one window orders by container_manifest_id alone, so the name pathkey the index supplies is dropped above the scan. The preview shape's second window orders by both and keeps it. That falsifies two sentences written before the index existed, neither fixable from a step MR: docs/specs/S17-rest-management-api.md:748 and the plan's Step 21 Acceptance both promise no sort node for both shapes. Index-only and heap-fetch-free hold for both. Sort-free holds for the preview shape, which is what the pin asserts, and both shapes clear the budget. Both corrections are open: the spec's on docs(specs): scope the tag-names index claims t... (!2660 - merged) • Hayley Swimelar • 19.5, the plan's on docs(plans): explicit partition-child names and... (!2651 - merged) • Hayley Swimelar • 19.5.

The wider index costs 9x the bytes. At 100,000 rows in one partition, both freshly built on committed data: 6,648 kB against the two-column index's 728 kB, on a 12 MB heap. The two-column index deduplicates a manifest's thousand identical keys into one posting list, and the wider one carries a distinct key per row, because name is unique inside an image.

Spec coverage

Contract Where Test
The hosted tags read rides a covering (namespace_id, container_manifest_id, name) index (Fetch mechanism) 20260916090417_extend_container_tags_manifest_id_index_with_name.sql TestListTagNamesByManifestIDsStmt_RidesTheManifestIDIndex
AC #139 (closed): the read plans index-only and sort-free, over one partition of 64 same same
The retired index's other two readers keep an index plan on the two-column prefix same measured, see Database Review Evidence
The parent index is valid, keyed in order, and 64 children are attached same container_tags_manifest_id_name_index_schema_integration_test.go
The subsumed two-column index is gone, unattached orphans included same same
The migration applies up, down, and up again on every supported PostgreSQL version same test:integration:migrations on the PG 16, 17, 18 matrix

Reviewable LOC

Group Added Removed
Migration SQL 547 0
Tests (new schema suite 542, tightened pin 80, two stale sibling comments 3, head-version pin 1) 626 23
Docs (the query-patterns row) 1 1
Reviewable total 1,174 24
structure.sql (generated, excluded) 129 129

Guardrail 18: past 500 and not splittable, and past the plan's own forecast of roughly 580 source and 600 test. 388 of the migration's 547 lines are the per-partition build and attach, 64 times three statements over one index, against the 574 lines 20260820224727_add_container_blobs_blob_sha256_index.sql spends on the same shape. The plan rejected splitting the create from the drop: the drop is only safe once the wider index is attached on every partition, and two MRs would leave main in the window where both indexes exist and the write amplification is paid for nothing. The schema suite's 542 lines are nine tests over one index, against 1,212 in the precedent suite for two indexes across two tables.

Test plan

test:integration:migrations on the PG 16, 17, and 18 matrix covers the up, down, and re-up walk plus the new schema suite. Locally, against private postgres:{16,17,18}-alpine containers under CI's .pg-service-options flags, the migration applies, rolls back, and re-applies on all three, and the schema suite and the moved EXPLAIN pin pass on all three.

Non-goals

container_remote_tags is out of scope, deliberately. Step 9's remote read measures 110 to 144 ms with the cached names in arbitrary order, the same shape of miss. No write path caps that table, the spec forecloses a counter over the cached rows because that is a write to the cache tables S16 owns, and a remote miss goes to the fill-side cap question at #669. Recorded as the follow-up.

No knob and no counter (plan D3 and D4): what missed is the array read's sort, and a counter leaves the sort where it is.

Three stale mentions of the retired index stay. docs/db/S12-container-oci-hosted-review.md names it twice and is a measurement record pinned to commit 107ba5f46, so renaming an index inside it would falsify its own attribution rule. docs/specs/S12-container-oci-hosted.md:253 lists it in another spec's index table, which a step MR editing would be scope creep. The fix for that line rides docs(specs): scope the tag-names index claims t... (!2660 - merged) • Hayley Swimelar • 19.5, and the measurement record keeps the old name by design.

ADR-007 amendment

docs/adr/007_database_schema.md:537 lists container_tags with index on (namespace_id, container_manifest_id), which this migration replaces. Guardrail 22 wants an escalation and a handbook amendment MR for an ADR deviation. The escalation was already on record in S17's Follow-ups section on main, and the amendment is now open: Amend AR ADR-007: two container index lookups g... (gitlab-com/content-sites/handbook!21119 - merged) • Hayley Swimelar • 19.5. Guardrail 13 puts docs/adr/ outside this repository, so it cannot land here.

That one amendment covers both bullets, this line and the container_manifest_relationships parents line changed by chore(datastore): the batched parent-digest rea... (!2579 - merged) • Hayley Swimelar • 19.5, and it trails both MRs.

Merge order

The spec amendment that declares this index, docs(specs): add the covering index the hosted ... (!2533 - merged) • Hayley Swimelar • 19.5, merged on 2026-09-14, so this migration implements text already on main.

This stamp is ahead of every migration on main, after a collision moved it. Four landed while this branch was open, and each time an earlier stamp fell behind the target's latest and turned the required lint:migration-ordering red. The fourth went further: 20260915160000_drop_container_remote_plaintext_credentials.sql took this file's exact version. Two files with the same version are not a git conflict, because the names differ, so has_conflicts stayed false and the pre-train pipeline stayed green against the older main. goose rejects it outright, panic: goose: duplicate version 20260915160000 detected on every job that migrates, which is what dropped the merge train's car. This file is now 20260916090417, ahead of main's newest rather than merely distinct from it, and ahead of every open migration. scripts/ci/check-migration-immutability.sh permits the re-stamps, because it freezes only migrations already on the target branch. Another open migration, 20260912104512 on feat(datastore): drop Maven's interim plaintext... (!2630 - merged) • Dzmitry (Dima) Meshcharakou • 19.5, targets a different branch, so its stamp is its parent's to order.

Ordering the stamps rather than merely keeping them distinct is what internal/datastore/migrations/migrations.go:76 requires: it sets goose.WithAllowOutofOrder(false) on the provider the boot path drives through migrations.Up, so a migration offered after a later one has already applied is refused with found 1 missing migrations before current version.

Database Review Evidence

Migrations

Note

Both rows are apply / rollback order. The local row ran against private postgres:{16,17,18}-alpine containers under CI's .pg-service-options flags, goose verbose, at PG 16.15, 17.10, and 18.4. The CI row is read off each db:migrate leg's migration-review.log on pipeline 2853504732.

Migration Run PG 16 PG 17 PG 18
20260916090417_extend_container_tags_manifest_id_index_with_name.sql local OK (203ms / 148.69ms) OK (95.25ms / 76.91ms) OK (171.35ms / 58.94ms)
same CI OK (342.31ms / 194.8ms) OK (245.41ms / 65.02ms) OK (333.28ms / 69.61ms)

Idempotency: re-applying after the rollback takes 311.42ms (PG 16), 228.15ms (PG 17), and 140.48ms (PG 18) locally, and 383.88ms, 280.84ms, and 397.58ms in CI's Re-Up pass. All six end with the parent index valid, 64 children attached, and the two-column index gone including unattached children.

Against a populated table, 100,000 tag rows in one of the 64 partitions, PG 17.10: apply 247.53ms and 246.18ms over two cycles, rollback 114.71ms and 107.60ms. The Up's 64 CREATE INDEX CONCURRENTLY builds each wait on the database's own snapshot drain rather than on this table's rows, which the precedent migration measures and this one inherits: 0.35s to 0.7s against an idle server, and unbounded behind a stream of long write transactions. Drain long writers before applying.

Queries

No statement changed. The read is Step 8's, re-measured under the new index.

ListTagNamesByManifestIDs, ceiling fixture: one namespace, one image, 100 manifests at container.manifest_max_tags (1,000) each, 100,000 rows, no sibling namespace in the partition. PG 17.10, EXPLAIN (ANALYZE, BUFFERS), worst of three, host load average per run given below.

The seed is committed data followed by VACUUM (ANALYZE), with relallvisible = relpages read back, rather than db-review-prep's rolled-back transaction. CI runs autovacuum=off, so a rolled-back seed leaves the heap bloated and the visibility map unset, and an index-only scan then prices a heap fetch per row that production would not pay.

Fixture Shape Before After Plan after
Names arbitrary, heap interleaved preview 138 ms, 73,937 buffers 45 ms, 1,092 buffers Index Only Scan, Heap Fetches: 0, no sort node
Names arbitrary, heap interleaved complete 132 ms 50 ms Index Only Scan, Heap Fetches: 0, Incremental Sort above the scan
Names arbitrary, heap grouped by manifest preview 117 ms, 1,646 buffers 47 ms, 1,051 buffers as above
Names arbitrary, heap grouped by manifest complete 105 ms 52 ms as above

Arrival order is what the before column tracks. On the two-column index the same 100,000 rows cost 64 ms preview and 69 ms complete with the names ascending and the heap grouped, and 92 ms and 90 ms ascending and interleaved, against the 117 to 138 ms arbitrary-order figures above. Loads on the one host, per run: the arbitrary-order before column at 3.7 to 4.2, the after column at 2.8 to 3.5, and these ascending figures at 8.2 to 9.6. So the arrival-order gap is understated, and the before-to-after gap was measured at comparable loads with a slight advantage to the after column.

Version spread, measured on a page of 100 manifests against a 500-manifest namespace so the id set is a share rather than the whole tenant, preview shape: PG 16.15 71 ms with a full Sort above the scan, PG 17.10 48 ms sort-free, PG 18.4 sort-free. Every version scans exactly the page's 100,000 rows, with both key columns in the Index Cond and Heap Fetches: 0, so no version walks the namespace.

Partition pruning: one partition of 64 in every arm, container_tags_p11, from the namespace_id equality. Planner estimate 63,397 rows before the swap and 100,000 after against 100,000 actual: the two-column index's estimate came off = ANY selectivity over 100 array elements, and the wider index's own statistics correct it.

The two other readers of the retired index, re-measured on the same fixture under the new index:

  • countTagsForManifest (SELECT COUNT(*) ... WHERE namespace_id = $1 AND container_manifest_id = $2): Index Only Scan on the new child index, both columns in the Index Cond, Heap Fetches: 0, 15 buffers, 0.37 ms. It gains an index-only plan it did not have.
  • ContainerTagStore.DeleteByManifestID: Index Scan on the new child index, both columns in the Index Cond, one partition, 2.60 ms for 1,000 rows.
Context for LLM agents

Why an index rather than a counter. Plan D4's ladder sends a family that misses its budget to a denormalized counter. It does not reach this family: what missed is the array read's sort, and a counter replaces the count, not the order. Naming that explicitly is what keeps the ladder from reading as covering this case.

Why the three-phase build on a schema with no confirmed production rows. The 2026-06-02 migration on this same table used a single blocking parent-level CREATE INDEX and its own comment says to replace that with the per-partition path against populated tables. Whether this schema carries rows is unconfirmed, and unconfirmed is what picks the safe path.

Why the 64 child index names are spelled out. The per-partition DROP guards and the ATTACH statements both need a deterministic target. A child's name is 62 characters, inside the 63-character limit, so nothing truncates and nothing takes the global uniquifier a truncated name carries, and each is byte-identical to the name PostgreSQL generates, verified against a real partitioned table.

Why DROP INDEX CONCURRENTLY guards precede each build. A crash during the build phase leaves child indexes built but never attached, in the partitions schema, invisible to a check on the parent name. A Down reaches attached children only, so the guards are what make a plain re-run the safe first move after an interruption. The Down comment carries the query that finds the orphans.

Rejected: adding name to the window's PARTITION BY to kill the complete shape's Incremental Sort. That changes COUNT(*) OVER (PARTITION BY container_manifest_id) into a different aggregate. The plan's Acceptance holds the statement constant: only the index under it moves, so query_names.go and TestQueryNames stay untouched.

Rejected: a Heap Fetches: 0 assertion in the pin. It is not deterministic in a shared test database. Three consecutive runs on one fixture, each preceded by VACUUM (ANALYZE) from a fresh session, returned 4000, 0, 4000: a concurrent transaction in another test holds OldestXmin back and VACUUM then cannot set the visibility map. The scan node and the absence of a sort are stable across the same runs, so the pin asserts those and the no-heap-fetch property lives in the evidence above. A VACUUM issued on the connection that inserted the rows never sets the map at all.

Why the pin's fixture grew. Rows per manifest, not total rows, decides whether the planner takes the ordered index scan. On PG 16 at the old fixture's 4 manifests of 50 rows the plan is a bitmap over the primary key plus a sort, and at 10 or 20 manifests of 100 rows a bitmap over the covering index plus a sort. From 250 rows per manifest up it is the index-only sort-free scan on every supported version, so the pin seeds 4 manifests of 1,000. The two sort assertions carry no version gate: all three versions plan the production-shaped ceiling sort-free, measured. The qual assertion carries one. At this fixture PG 16 filters the manifest ids above the scan where PG 17 and later bind them in the Index Cond, and accepting a Filter on every version would pass a regression that stopped binding on 17.

Non-goal: ADR-007's index list. It carries the two-column form. Guardrail 13 keeps docs/adr/ out of this repository's MRs, and S17's Follow-ups section already records the handbook amendment.

🤖 Generated with Claude Code

Related to #1150 (closed)

Edited by Hayley Swimelar

Merge request reports

Loading
Loading