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_statisticsnamespace_id (PK, FK to namespaces), 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 is namespace_id, with no separate id column, 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, an AFTER INSERT trigger on namespaces, 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 CONFLICT had no test. Every case ran it against an empty table (a fresh migration, or a DownTo-then-Up cycle), so deleting the clause would have passed the whole suite. TestNamespaceStatisticsMigration_SeedIsIdempotent re-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 pinning DO NOTHING rather than a DO UPDATE that would zero a namespace's accounting. Falsified by deleting the clause: the test then fails on duplicate key value violates unique constraint "pk_namespace_statistics".
  • The ADD COLUMN migration gave two opposite premises. Line 1 said repositories is empty; the body said the change is safe because it is the first NOT 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.sql is already on main adding size_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:adrs has 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 updownup 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 ms

With 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 ms

So 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, the last_reconciled_at index, 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 calls assertNoBatchingDoBlock over the whole file rather than the Down slice, so Up is covered too.
  • internal/datastore/namespace_statistics_integration_test.go — the read, including ErrNotFound on 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

Edited by Pawel Rozlach

Merge request reports

Loading
Loading