feat(datastore): namespace_statistics table and last_reconciled_at (S22 plan: 1/20)
Adds the reconciliation-bookkeeping schema the storage-accounting counters rest
on: a namespace_statistics row per namespace, existing by construction, plus
the repository-level timestamp the counter drain's guard reads.
Implements Step 1 of the S22 storage accounting plan, against the S22 spec.
This MR is code only. Every spec, plan, and docs/dev/ change that used to live
here is in !1442 (merged), including the decisions this code implements — the 'epoch'
default, the schema corrections, and the (last_reconciled_at, namespace_id)
index.
What this ships
namespace_statistics—namespace_id(PK, FK tonamespaces),deduplicated_size_bytes,components_count,last_reconciled_at. Not partitioned, and the reason is the shape of the key rather than the size of the table: the whole primary key isnamespace_id, with no separateidcolumn, so the table cannot represent a second row for a namespace. "Small" and "1:1 with its parent" are rejected as reasons by name.trg_namespaces_create_statistics, anAFTER INSERTtrigger onnamespaces, created before the one-time seed so the seed's snapshot covers every row the trigger will not fire for.repositories.last_reconciled_at timestamptz NOT NULL DEFAULT 'epoch'.datastore.NamespaceStatisticsStore.FindByNamespaceID, the namespace-scoped read.
No production caller yet, and none in this plan: Steps 13, 15, and 16 each reach
namespace_statistics through their own store file rather than this accessor,
and the plan's store inventory lists NamespaceStatisticsStore as Step 1's
alone. The counters' named consumer is S24 Billing, which has no spec yet. Every
path here is driven directly from tests until a caller lands, and that first
caller owes a namespace_id structured log field, because FindByNamespaceID's
errors carry no identifier by design.
Three things a reviewer should look at
1. The foreign key carries ON DELETE CASCADE, and it is the only
REFERENCES namespaces (id) in the tree that names an action. Neither the
spec nor ADR-007 specifies one. The others omit it so a namespace still holding
content rows fails to delete, which is how those rows are caught leaking
(internal/datastore/migrations/migrations_test.go). That rationale does not
reach a row a trigger creates for every namespace and nothing removes — a
restricting action there would make every namespace permanently undeletable.
2. last_reconciled_at defaults to 'epoch', not '-infinity', and that is
what keeps the rows readable. -infinity states the intent more plainly, and
was the first choice, but no time.Time holds it: jet types a NOT NULL timestamptz as time.Time, so the generated model cannot scan a row still
carrying the default — which is every row until its first reconciliation pass.
Working around that cost three pieces in namespace_statistics.go (a reduced
row type, a column list omitting the timestamp, a converter between them) plus a
named exclusion weakening TestRepositoryColumns_MatchesGeneratedProjection
from an exact equality. All four are gone: the accessor projects every column
and returns the generated model, and repositoryColumns projects
last_reconciled_at like everything else.
What 'epoch' gives up is the guarantee that no real pass could stamp the
sentinel value. Stamping 1970-01-01 needs a system clock wrong by decades, and
this column selects reconciliation candidates rather than feeding a counter, so
a misread would delay or repeat a pass rather than corrupt a billing input.
Recorded in ADR-007 by handbook!20738.
One consequence worth knowing while reading the schema test: PostgreSQL resolves
DEFAULT 'epoch' to a constant at DDL time and renders it in the session's time
zone, so information_schema reads '1970-01-01 00:00:00+00' under UTC and
'1969-12-31 19:00:00-05' under America/New_York — the same instant, two
strings. '-infinity' rendered literally under any zone, so the old textual
assertion was safe where a new one would not be. assertDefaultsToEpoch pulls
the literal back out of the default expression and compares instants.
structure.sql carries the same rendered constant, which is stable because the
dump runs against a container with no TimeZone set, in both
db:dump-structure and CI.
3. The index is (last_reconciled_at, namespace_id), and the second column is
correctness rather than tuning. Review of handbook!20738 established that the
candidate-selection query as previously specified could not use this index at
all — the IS NULL disjunct of its namespaces join is a post-join filter, not
an index condition — and that the same page costs 34,308 buffers joined against
4 reading namespace_statistics alone. !1442 (merged) changes both consumers to read one
table; this MR changes the index they read. The trailing column is what makes
the keyset cursor well-defined: until a row's first pass every row holds the
'epoch' default, so the timestamp alone orders nothing and a cursor over it
cannot advance through the tie. Measured in ## Database Review Evidence below:
with the composite index the cursor predicate is an index condition and each
page costs 10 buffers; with a single-column index it is demoted to a filter,
each page re-reads the whole tie group, and the cost does not fall as the cursor
advances. The schema test pins the column order, not just the leading column.
4. Both migrations mix concerns, knowingly. The namespace_statistics
migration combines table, trigger, and seed in one file, against
docs/dev/database-migrations.md's "Do not combine schema changes with data
changes". The plan's Approach section accepts it: the consistency argument
depends on the trigger and the seed committing together, and separating them
reintroduces the gap the spec rules out.
One deviation from the plan text: the plan asks that
assertNoBatchingDoBlock's third parameter be renamed to sql. It is named
content, because sql shadows database/sql imported in the same file, and
content matches what readMigrationFile returns.
Documents amended here
None. Every document this work touches is in !1442 (merged): the S22 spec, the S22 plan,
and docs/dev/database.md. That split is deliberate — the schema and query
decisions carry their own approval and should not review as incidental to a
migration.
Review feedback addressed
- The trigger-and-seed rationale was wrong, and is corrected. The migration
claimed the two "only work as one transaction" and that splitting them would
open a window losing statistics rows. The seed is
INSERT … ON CONFLICT (namespace_id) DO NOTHING, so table-plus-trigger followed by a backfill loses nothing: rows created in between come from the trigger and the backfill skips them. The comment now says what is true — the order inside the file is load-bearing, and one transaction is preferred because no intermediate state is observable, not because a split would lose data. The spec and plan halves of the same wrong claim are corrected in !1442 (merged). - The seed's
ON CONFLICThad no test. Every case ran it against an empty table (a fresh migration, or aDownTo-then-Upcycle), so deleting the clause would have passed the whole suite.TestNamespaceStatisticsMigration_SeedIsIdempotentre-runs the seed against a table that already holds every row, lifting the statement out of the embedded migration text so it exercises what the migration carries rather than a copy, and asserts both no error and unchanged counters — the second half pinningDO NOTHINGrather than aDO UPDATEthat would zero a namespace's accounting. Falsified by deleting the clause: the test then fails onduplicate key value violates unique constraint "pk_namespace_statistics". - The
ADD COLUMNmigration gave two opposite premises. Line 1 saidrepositoriesis empty; the body said the change is safe because it is the firstNOT NULL-with-a-default column added to a table that already holds rows. The body's claim was false on its own terms too —sql/*_add_npm_versions_size_bytes.sqlis already onmainaddingsize_bytes bigint NOT NULL DEFAULT 0. Both premises are gone: the metadata-only argument depends on the default's volatility rather than the row count, and the squawk rationale now follows that precedent, which rejects the empty-table framing by name. - The index comment overstated its own plans. It claimed both consumers of
the index are index-only scans; the backlog gauge's unbounded count is a
sequential scan while every row still holds
'epoch'. Measured and corrected, with the numbers in## Database Review Evidence. - ADR-007's amendment must land first. handbook!20738 carries it and is open;
this MR should not merge until it has merged and
sync:adrshas run. The plan text stating that dependency now lives in !1442 (merged).
Corrected: what pgx does with -infinity
The spec claimed pgx "refuses to scan -infinity into *time.Time and to
encode it back as a parameter". Only the scan half is true, measured against
the pinned github.com/jackc/pgx/v5 v5.10.0 through pgtype.Map:
| Direction | Behavior |
|---|---|
Scan -infinity → *time.Time |
errors, cannot scan -Infinity into *time.Time, in both binary and text format; database/sql fails equivalently |
Encode a zero time.Time |
no error — time.Time has no value meaning -infinity for pgx to refuse, so it silently produces a year-1 timestamp |
That silent half outlives the switch to 'epoch': a baseline the code never
read from the row still encodes as year 1, still equals no stored value, still
matches zero rows, and still drops a chunk's delta with nothing raised. The
spec now requires the baseline be the value read from the row, and Step 7's
acceptance asserts the delta applies for a never-reconciled scope, since
asserting no-error would pass against exactly that bug.
Database Review Evidence
Migration mode, re-run against the 'epoch' schema. goose 3.27.3 (CI pins
3.27.1), goose up → down → up across the db:migrate matrix, each on a
stock postgres:<v>-alpine at the production default
max_locks_per_transaction = 64 — the parity gate that job deliberately does
not raise.
| PG | Up | Down | Re-Up | Result |
|---|---|---|---|---|
| 16 | 5.91 ms / 9.44 ms | 32.85 ms / 9.58 ms | 13.79 ms / 31.71 ms | clean |
| 17 | 2.75 ms / 8.55 ms | 28.05 ms / 5.85 ms | 8.33 ms / 30.45 ms | clean |
| 18 | 4.92 ms / 38.98 ms | 34.63 ms / 7.11 ms | 11.46 ms / 47.49 ms | clean |
Per cell: create_namespace_statistics / add_repositories_last_reconciled_at,
in the order goose applied them (reversed under Down).
Caveat on what these timings do not measure. The round trip runs against an
empty database, so the seed copies zero rows and the column-add rewrites none.
The seed's real cost scales with namespace count, and
ADD COLUMN ... NOT NULL DEFAULT 'epoch' is metadata-only on PostgreSQL 11 and
later, so neither is expected to grow, but this evidence does not demonstrate
that at scale.
Query mode, re-measured against the index that ships. The figures this block
carried before measured index_namespace_statistics_on_last_reconciled_at, the
single-column form, and predate e8d1439c widening and renaming it — so the one
property the index comment argues for was the part they could not show.
PostgreSQL 18.4, 50,000 namespaces created through the trigger (so 50,000
namespace_statistics rows), 25,000 stamped and 25,000 still at 'epoch', page
size 1,000, EXPLAIN (ANALYZE, BUFFERS), warm second run.
| Query | (last_reconciled_at, namespace_id) |
(last_reconciled_at) |
|---|---|---|
| First candidate page | Index Only Scan, 9 buffers, 0.138 ms, no sort | Index Scan + Incremental Sort, 441 buffers, 4.043 ms |
| Keyset next page | Index Only Scan, 10 buffers, 0.140 ms | Index Scan + Incremental Sort, 441 buffers, 4.335 ms |
FindByNamespaceID |
Index Scan on pk_namespace_statistics, 4 buffers, 0.020 ms |
unchanged by this index |
The difference that matters is in Index Cond rather than in the timings. With
the composite index the keyset cursor is an index condition:
Index Only Scan using index_namespace_statistics_on_last_reconciled_at_and_ns_id
Index Cond: ((last_reconciled_at < (now() - '01:00:00'::interval))
AND (ROW(last_reconciled_at, namespace_id) > ROW($1, $2)))
Heap Fetches: 0
Buffers: shared hit=10
Execution Time: 0.140 msWith the single-column index it is demoted to a filter, and only the 'epoch'
lower bound reaches the index:
Index Scan using index_namespace_statistics_on_last_reconciled_at
Index Cond: ((last_reconciled_at < (now() - '01:00:00'::interval))
AND (last_reconciled_at >= '1970-01-01 00:00:00+00'::timestamptz))
Filter: (ROW(last_reconciled_at, namespace_id) > ROW($1, $2))
Rows Removed by Filter: 1000
rows=24001
Buffers: shared hit=441
Execution Time: 4.335 msSo each page re-reads the whole 'epoch' tie group and discards the rows it has
already returned — 24,001 rows scanned to return 1,000 — and that does not
improve as the cursor advances.
The same run corrected a claim in the index's own comment, which said both of
this index's consumers are index-only scans. The paged scan is, at any
staleness, because LIMIT bounds it. The backlog gauge's unbounded count(*)
is not: before the first reconciliation pass every row holds 'epoch' and
therefore qualifies, and the planner answers it with a sequential scan. That is
the cheaper plan for a whole-table aggregate, not a missing index.
| Gauge state | Plan | Buffers | Time |
|---|---|---|---|
Every row at 'epoch' (49,477 of 50,000 qualify) |
Seq Scan | 625 | 5.204 ms |
| 500 of 50,000 stale, vacuumed | Index Only Scan | 8 | 0.089 ms |
The same seeding run is also end-to-end evidence for the trigger: 50,000
namespaces inserts produced exactly 50,000 namespace_statistics rows.
lint:sql-format (pg_format --inplace then git diff --exit-code) and
squawk over internal/datastore/migrations/sql/*.sql are both clean. Each
migration carries its own squawk-ignore-file block with a rationale per
directive; the column-add's now follows
sql/*_add_npm_versions_size_bytes.sql, the precedent on main, rather than
resting on the table being empty.
Testing
Test-first: the test(datastore) commit lands the suites and a panic skeleton,
feat(datastore) the implementation, refactor a simplification pass. The
first used --no-verify under the documented test-first carve-out, because the
panic skeleton is meant to fail the go-test hook; every later commit ran the
full hook chain.
internal/datastore/migrations/namespace_statistics_schema_integration_test.go— columns, defaults, primary key, non-partitioning, thelast_reconciled_atindex, the absence of any second unique index, the foreign key and its cascade, negative counter values, the trigger, the seed, trigger-before-seed ordering, and apply/rollback for both migrations. It callsassertNoBatchingDoBlockover the whole file rather than theDownslice, soUpis covered too.internal/datastore/namespace_statistics_integration_test.go— the read, includingErrNotFoundon a missing row, scoping to the requested namespace, the'epoch'default reading back as a real instant rather than a zero time, and a stamped timestamp round-tripping.internal/datastore/namespace_statistics_test.go— untagged, so CI lint sees it and it runs in the unit job: the projection guard, its built-fresh-per-call half, the argument-guard sentinels, and the constructor's nil-client panic.
Both integration suites pass in full locally (./internal/datastore/ and
./internal/datastore/migrations/, -tags=integration).
e2e scenarios: none affected. The catalogs in docs/testing/e2e/ describe
client-visible flows, and this MR changes no request path and no response — the
counters those scenarios could observe do not move until a call site emits,
which no step in this plan adds.
Conformance: unaffected. No format protocol behavior here.
Spec coverage
Spec: docs/specs/S22-storage-accounting.md Plan: docs/plans/2026-08-04-s22-storage-accounting.md, Step 1
Acceptance criteria 14 (namespace_statistics and the column) and 16 are this
MR's. Criteria 30, 35 and error cases 13 and 17 land the columns and the row
their guards read, and are verified in Steps 8 and 14. Every other row below
names the step that owns it, so this is the whole S22 map rather than only this
MR's slice — it lives here rather than in a commit body because squash-on-merge
does not preserve one.
Acceptance criteria
| # | Criterion | Tests |
|---|---|---|
| AC-1 | Repo-scoped increment lands after the next drain tick | Step 10. |
| AC-2 | Namespace-scoped increment lands after the next drain tick | Step 10. |
| AC-3 | Concurrent increments to one scope sum exactly | Step 10. |
| AC-4 | Re-increment while a claimed batch is in flight | Steps 8, 10. |
| AC-5 | Chunk past drain_chunk_stale_timeout bails |
Step 8. |
| AC-6 | Chunk failing every attempt re-adds its scopes | Step 8. |
| AC-7 | Redis unavailable at increment time does not fail the write | Step 6. |
| AC-8 | Reconciliation clears the buffer before the scan | Step 14. |
| AC-9 | (GA) recompute soft-delete visibility | Step 14. |
| AC-10 | Positive hit per version-type table and (format, kind) |
Steps 11, 13. |
| AC-11 | Drift recorded on the unit-matched histogram | Step 14. |
| AC-12 | Crash between HINCRBY and SADD |
Step 6. |
| AC-13 | Sliding TTL refreshed on every write; dirty sets carry none | Step 5. |
| AC-14 | Migrations apply and roundtrip the ORM; 'epoch' on every row |
TestNamespaceStatisticsSchema_Columns, TestNamespaceStatisticsSchema_ColumnDefaults, TestNamespaceStatisticsSchema_PrimaryKeyIsNamespaceID, TestNamespaceStatisticsSchema_IsNotPartitioned, TestNamespaceStatisticsSchema_LastReconciledAtIndex, TestNamespaceStatisticsSchema_ForeignKey, TestNamespaceStatisticsSchema_CountersAdmitNegativeValues, TestRepositoriesSchema_LastReconciledAtColumn, TestNamespaceStatisticsMigrations_SeedRowsPresentBeforeTheMigration, TestNamespaceStatisticsMigrations_NoBatchingDoBlock, TestNamespaceStatisticsMigration_UpCreatesTriggerBeforeSeed, TestNamespaceStatisticsMigration_DownDropsTriggerFunctionAndTable, TestRepositoriesLastReconciledAtMigration_DownDropsTheColumn, TestNamespaceStatisticsStore_FindByNamespaceID, TestNamespaceStatisticsStore_FindByNamespaceID_ScopedToTheRequestedNamespace, TestNamespaceStatisticsSchema_NoSeparateUniqueIndex. The blob_storage_blobs_by_namespace clause is Step 2a's. |
| AC-15 | blob_storage_blobs triggers keep the shadow consistent |
Step 2b. |
| AC-16 | Every namespace has a zeroed row by construction | TestNamespaceStatisticsMigrations_SeedRowsPresentBeforeTheMigration (the seed, clause a), TestNamespaceStatisticsTrigger_CreatesZeroedRowOnNamespaceInsert (the trigger, clause b). |
| AC-17 | npm publish increment swapped onto the pipeline | S20-a plan (format call sites). |
| AC-18 | OCI emits increments at the actual sites | S20-a plan. |
| AC-19 | OCI emits decrements at the per-artifact deleters | S20-a plan. |
| AC-20 | Maven BumpRepoCounters swapped onto the pipeline |
S20-a plan. |
| AC-21 | (CB) repository cascade delete fires the decrements | S20-a plan, gated on #464 (closed). |
| AC-22 | One asynq task per namespace candidate | Step 15. |
| AC-23 | A namespace with no row gets one on its first pass | Steps 13, 14. |
| AC-24 | reconciliation_max_in_flight is never exceeded |
Step 14. |
| AC-25 | Source-first ordering at the emit sites | Contract half Step 6; verification travels to the S20-a plan. |
| AC-26 | last_reconciled_at stamped only after every repository |
Step 14. |
| AC-27 | The trigger selects only stale namespaces | Step 15. |
| AC-28 | UniqueByArgs caps a namespace at one outstanding task |
Step 15. |
| AC-29 | reconciliation_backlog has a single writer |
Step 16. |
| AC-30 | Drain chunk guard on last_reconciled_at |
Step 8. This MR lands the two columns that guard reads. |
| AC-31 | counter_dirty_set_size sampled once per tick before SPOP |
Step 10. |
| AC-32 | Config load rejects each invalid configuration | Step 3. |
| AC-33 | The five metrics are registered; the three alerts fire | Steps 8, 10, 14, 16 for the registration half; the alert half lands with #354. |
| AC-34 | Saturation policy re-enqueues rather than shedding | Step 14. |
| AC-35 | No-baseline chunk drops its delta and deletes :flushed |
Step 8. Its fixture deletes the row this MR's trigger creates. |
| AC-36 | A failing recovery SADD does not lose the delta |
Step 8. |
Error cases
| # | Condition | Tests |
|---|---|---|
| E-1 | Redis unavailable at increment time | Step 6. |
| E-2 | Redis unavailable at drain-trigger time | Step 10. |
| E-3 | Chunk job's Postgres UPDATE fails |
Step 8. |
| E-4 | :flushed DEL fails after the UPDATE succeeded |
Step 8. |
| E-5 | Chunk job exhausts all retry attempts | Step 8. |
| E-6 | Recovery SADD itself fails |
Step 8. |
| E-7 | Chunk dequeued later than drain_chunk_stale_timeout |
Step 8. |
| E-8 | Worker dies mid-chunk after merging into :flushed |
Step 8. |
| E-9 | Two chunks run the same scope concurrently | Steps 8, 10. |
| E-10 | Trigger's EnqueueTx fails while the process is alive |
Step 10. |
| E-11 | Crash between a trigger's SPOP and its EnqueueTx |
Accepted residual gap; no test (spec states it is crash-only and backstopped by reconciliation). |
| E-12 | Assigned repository or namespace row hard-deleted before drain | Step 8. |
| E-13 | Namespace-scoped chunk drains a namespace with no statistics row | Step 8. This MR's trigger and seed are what make the case unreachable on the normal path. |
| E-14 | Crash between HINCRBY and SADD |
Step 6. |
| E-15 | Crash between reconciliation's clear and its SET counter = V |
Step 14. |
| E-16 | Reconciliation scan races a concurrent increment | Step 14. |
| E-17 | A drain chunk and a reconciliation process the same scope | Step 8. This MR lands the last_reconciled_at columns the guard compares. |
| E-18 | Reconciliation finds a discrepancy | Step 14. |
| E-19 | Namespace has no statistics row when its task runs | Steps 13, 14. TestNamespaceStatisticsStore_FindByNamespaceID pins that the read surfaces ErrNotFound rather than a zero row. |
| E-20 | Reconciliation task fails before its final UPSERT | Step 14. |
| E-21 | A namespace can never be reconciled | Steps 15, 16. |
Security considerations
| # | Concern | Tests |
|---|---|---|
| S-1 | Redis keys carry internal UUIDs only, no user-controlled text | Step 5. This MR introduces no Redis surface. |
| S-2 | Counter values feed billing, so drift has financial impact | The discrepancy metrics and the standing reconciliation schedule are Steps 14 through 16. This MR lands the last_reconciled_at bookkeeping both read, covered by AC-14 and AC-16 above. |
| S-3 | No new credential surface | Structural. This MR adds no client, no configuration, and no credential; the migrations and the accessor reuse the existing datastore pool. |
Related to #515