feat(accounting): reconciliation trigger and candidate selection (S22 plan: 15/21)

Pawel Rozlachrequested to merge
prozlach/s22-storage-accounting-step-15 into main

What this step delivers

This step lands the leader-elected half of reconciliation.

It registers two periodic River kinds rather than one. trigger:reconciliation walks the namespaces whose namespace_statistics row is stale beyond reconciliation_interval. trigger:reconciliation_orphan_sweep reaches the namespaces that walk cannot see, because they hold no statistics row at all. A River periodic job carries one interval, and the two cadences differ, so one kind cannot carry both walks.

ReconcileCandidateStore pages the two selections. The staleness walk reads namespace_statistics alone, keyset-wise over (last_reconciled_at, namespace_id) as a row-value comparison, so the bound rides the index. The orphan sweep is a NOT EXISTS anti-join over namespaces, keyset by id. Both walks apply their bound on every page, including the first, so one statement shape runs for a whole fire. Both open strictly below every row, at ('epoch', uuid.Nil) and at the nil UUID, so neither walk misses a never-reconciled namespace. Neither walk persists a cursor.

Each fire fans out one asynq task per namespace under UniqueByArgs, never one task per repository. Each trigger overrides Timeout with its own kind's interval, because River's one-minute default can cut a large fire short and leave the tail unfanned. Each fire reports its enqueued, outstanding and page counts on every exit path. A walk that finished logs accounting: reconciliation fan-out complete, and a walk that stopped early logs accounting: reconciliation fan-out ended early.

Each trigger also declares InsertOpts() with MaxAttempts: 3, in place of River's default of 25. Each kind's Timeout equals the interval its own schedule fires on. The attempts of a fire that keeps exhausting that budget therefore run beside the fresh fires the schedule keeps inserting, not after them. The count of one kind's concurrently executing fires converges on its attempt budget, against the single per-pod River queue that every registered kind shares.

Each candidate page read now runs under a 5 s context.WithTimeout derived from the fire's context. Before this, the fire's budget was the only bound on a page read. That budget is one hour for the staleness walk and three hours for the sweep, at the default intervals. An expiry fails the fire and spends one attempt of the narrowed budget.

RegisterAsynqHandlers is no longer empty. It registers the reconciliation:namespace pass that the two triggers enqueue.

The wiring needed an answer the tier order cannot give. The triggers fan out on the asynq client, which wireJobs builds after wireAccounting has already contributed its registrations. asynqClientHolder carries that client backwards, in the same shape as riverClientHolder. A deferred assembly thunk closes the mirror-image gap on the asynq side, and a nil thunk fails boot with a named error. The two new accounting dependency sets each assemble through their own guarded constructor, so an omitted field fails boot instead of the first leader-elected fire.

The configuration decision on this branch

ReconcileTriggerDeps.pageSize first clamped storage_accounting.reconciliation_batch_size into the candidate store's page maximum of 1000. A file that asked for 5000 rows a page loaded cleanly, paged at 1000, and printed 5000 back at boot. The spec makes that field the LIMIT of both selections, and it argues load-time rejection for the nearest sibling knob against this same failure shape.

The operator chose rejection at configuration load. StorageAccountingConfig.validate now refuses a value above datastore.MaxReconcileCandidatePageSize with a named sentinel, in the shape the drain-chunk payload cap already uses. A compile-time assertion pins the documented default of 500 under that maximum, so a raised default cannot make an omitted field unloadable. pageSize no longer applies a ceiling, and it passes the configured value through. It keeps the floor, which guards a hand-built dependency set that a loaded configuration cannot produce.

That decision is why this MR touches six paths the step card's Files entry does not list. internal/config/storageaccounting.go carries the new bound. docs/dev/configuration-reference.md and config.example.yaml move the count of startup rejections from seven to eight, and name the cap on the field's own row. proto/artifactregistry/config/v1/config.proto states the same rejection on the field, beside the four sibling fields that already state theirs. gen/artifactregistry/config/v1/config.pb.go is the regenerated stub that mirrors that comment. docs/specs/S22-storage-accounting.md carries the amendment.

Each of the six is a consequence of the chosen option, not scope drift. Once internal/config/** changes, the configuration-reference edit is mandatory in the same MR. The three operator-facing descriptions of the field also now say it paces both walks, not the staleness walk alone. One value reaches both workers through ReconcileTriggerDeps.pageSize, and a page costs differently in each walk.

The spec amendment rides here rather than in a separate spec MR. Both edits exist only to make this code change honest. They are one bullet in the Go-side bound list, one case in the config-load acceptance criterion, and the two counts those lists carry. A separate MR puts the code and the sentence that justifies it behind different reviews.

The new bound is checked at configuration load, not where a page size is derived. No code on main reads this field today, so a value above 1000 changes nothing at runtime. Such a file still loads on main, and the service prints the value back at boot. On the binary this MR produces, the same file does not load at all. The upgrade is therefore configuration-breaking for any deployment that sets the field above 1000.

The direction is the safe one for a rolling deploy. A new pod stops at boot and the old pods keep serving. The error names the field, the value, and the limit: storage_accounting.reconciliation_batch_size (5000) would exceed the largest page the reconciliation candidate store accepts (at most 1000 rows per page).

Other paths outside the step card's Files entry

Path Why it is here
cmd/artifact-registry/wire.go, cmd/artifact-registry/wire_jobs.go The bootDeps holder field, the thunk field, and the boot-abort guard. The plan named wire_accounting.go alone for the cmd/ edit.
internal/datastore/query_names.go docs/dev/database-query-patterns.md requires a name per query. No S22 step lists this file, and steps 1, 7, 11, 12, 13 and 16 each added names to it.
.claude/skills/run-artifact-registry/SKILL.md The run recipe must stay current with boot changes. One Gotchas bullet, and a re-run of driver.sh smoke.

Spec coverage

The rows below map the step card's acceptance clauses to the tests on this branch. Three clauses carry a residual gap, and those gaps are stated in the table rather than left out of it.

Acceptance clause Covering test Residual gap
A River client carrying both periodic kinds and both workers starts cleanly, and either schedule registered without its worker fails Start with ErrPeriodicKindNoWorker TestRegisterRiverJobs_SchedulesBothReconciliationKindsWithTheirWorkers, TestRegisterRiverJobs_AScheduleWithoutItsWorkerFailsStart (table-driven over both kinds), TestWireAccounting_ContributesARegistrationPerRegisteredKind, TestWireAccounting_ContributedClosuresRegisterEveryKind None
Only namespaces stale beyond reconciliation_interval are fanned out, against a threshold fixed for the whole fire TestReconciliationTrigger_FansOutOneTaskPerStaleNamespace, TestReconcileCandidateStore_ListStaleNamespaces_SelectsOnlyRowsBehindTheThreshold, TestReconciliationTrigger_OpensAtEpochAndFixesItsThresholdForTheWholeFire None
A namespace already queued or in flight is not enqueued again TestReconciliationTrigger_DoesNotEnqueueANamespaceItAlreadyHasOutstanding Yes. The case asserts the in-flight arm alone (pending == 0 && active == 1). The already-queued arm has no case of its own, although one ErrTaskIDConflict mechanism serves both.
Fan-out is one task per namespace, never one per repository TestReconciliationTrigger_FansOutOneTaskPerStaleNamespace Yes. NamespaceReconcileArgs carries only NamespaceID, and UniqueByArgs dedupes on it, so a per-repository walk still delivers the task count the assertion admits. Only a repository-keyed-args variant fails the test. The live run under Runtime evidence adds what the test cannot: three namespaces produced three tasks while several repositories sat inside one namespace.
The reconciliation:namespace handler is registered before the asynq client starts, so a fanned-out task runs rather than dead-letters as an unknown kind TestRegisterAsynqHandlers_MakesTheReconciliationTaskDispatchable, with TestReconciliationTaskKind_IsUnknownWithoutTheRegistration as its negative control, and TestWireJobs_RegistersTheReconciliationHandlerOnTheAsynqClient at the composition root None. The composition-root case was added on this branch after a review pass found the registration line deletable with the package suite still green. That mutation was run: every other test in the package passed, and this case alone failed, on ErrUnknownKind.
The keyset walk covers every stale namespace exactly once across a fire's pages, with more stale namespaces than reconciliation_batch_size and a page boundary inside a run of rows sharing one timestamp TestReconciliationTrigger_WalksEveryStaleNamespaceAcrossItsPages, TestReconcileCandidateStore_ListStaleNamespaces_PagesEveryStaleNamespaceOnce None. The fixture seeds 5 rows at 'epoch' plus 2 aged rows against a page size of 2, so both page boundaries fall inside the tied run.
The first page returns the 'epoch'-holding rows, which pins the opening cursor TestReconcileCandidateStore_ListStaleNamespaces_OpensOnTheNeverReconciledRows, TestReconciliationTrigger_OpensAtEpochAndFixesItsThresholdForTheWholeFire None. The first case also asserts the complement: the same walk reopened one microsecond above 'epoch' returns empty, and a sentinel above the floor cannot pass that assertion.
A namespace with no namespace_statistics row is absent from the staleness selection and present in the orphan sweep, and swaps sides once the sweep's task creates its row, driven by a fire of the sweep kind TestReconciliationOrphanSweep_ReachesWhatTheStalenessWalkCannotSee, TestReconcileCandidateStore_ANamespaceWithNoStatisticsRow_IsSweptNotWalked Yes. The swap is driven by a raw INSERT of the statistics row, not by the sweep's task running its write-back UPSERT. Merged step 14's TestNamespaceReconcile_ANamespaceWithNoStatisticsRowSettlesAtSource covers that link, so the chain is complete across three tests and never end to end in one.
The sweep fires on reconciliation_orphan_sweep_interval rather than on reconciliation_interval TestRegisterRiverJobs_SchedulesBothReconciliationKindsWithTheirWorkers, in its per-kind fire-count assertion None
The sweep pages by namespace id from the nil UUID and reaches every orphan once TestReconciliationOrphanSweep_OpensAtTheNilUUIDAndAdvancesByID, TestReconcileCandidateStore_ListNamespacesMissingStatistics_PagesEveryNamespaceOnce None
Every page is read at reconciliation_batch_size TestReconcileTriggerDeps_PageSize (pass-through at the maximum and one row past it, floor at zero and at a negative value), TestLoad_StorageAccounting_ReconciliationBatchPageBound, and the per-page assertions in the two fan-out cases None, after the decision above. The configured value now reaches the walk unchanged, so the claim holds at every loadable value.

Not claimed here, and not this step's: acceptance criterion 27's namespaces_missing_statistics metric, which step 16 carries, and acceptance criterion 22's second half, the reconciliation task's own execution, which merged step 14 covers.

Runtime evidence

The run drove this step on a running service, not on tests alone. Every figure in this section was measured at commit 18b011e64 ("docs(config): say what this block gives an operator over reconciliation"), the tip of the review round that answered the findings below. The last commit on this branch that changes what a running service does is 970b1538b ("fix(accounting): bound one candidate page read of either trigger walk"); every production-Go change after it is comment-only, so the figures hold for the whole review round rather than for one commit.

GOFLAGS= .claude/skills/run-artifact-registry/driver.sh smoke exits 0 at 43 passed, 0 failed. The earlier recorded figure of 24 was measured against a smaller driver: the rebase onto main brought in an expanded driver.sh, which this branch does not modify, and the count of unconditional checks it asserts moved with it. The boot line river: applied job registrations names both new kinds once each, next to the two merged counter-drain kinds, confirmed by grepping ${AR_RUN_DIR}/ar.log after the run: periodic_kinds reads ["remote:health-sweep","counter_drain_chunk_repo","counter_drain_chunk_namespace","trigger:reconciliation","trigger:reconciliation_orphan_sweep"]. No reconciliation fan-out line appears in that log, which is the bullet's own claim that a driver run never fires either trigger.

Neither schedule sets RunOnStart, and the two intervals default to 1h and 3h, so a recipe run never meets a fire. The run booted a second Artifact Registry in an isolated run directory, at reconciliation_interval: "5s" and reconciliation_batch_size: 2. It ran against three namespaces all stamped at 1970-01-01. Two rows a page against three tied rows puts a page boundary inside a run of rows at one timestamp. A single-column cursor cannot page through that case.

Observed over ten completed trigger:reconciliation jobs in about 50 seconds:

  • reconciliation_namespaces_enqueued: 3 over reconciliation_candidate_pages: 2, so the walk crossed the boundary and still reached each namespace once.
  • Three jobsasynq: job complete lines for kind reconciliation:namespace at job_attempt: 0, with no unknown kind and no archived task. That is the end-to-end proof that the handler was registered before the client started.
  • Fan-out was one task per namespace while several repositories sat inside one namespace.
  • Alternating fires reported enqueued: 0, because each pass stamped last_reconciled_at and moved the namespaces out of the stale set until the 5s threshold expired again.
  • All three last_reconciled_at values moved off 1970-01-01, and reconciliation_namespaces_outstanding stayed at 0 throughout.

The orphan sweep was not driven live. Configuration load floors reconciliation_orphan_sweep_interval at 30m, so no valid configuration brings a fire inside a bounded run. The boot line names the kind, and the River client started. A schedule without its worker fails Start, so a clean start is proof of the worker. The integration suite alone covers the sweep's query and its fan-out.

The behaviour-changing commits this measurement covers, by their post-rebase SHAs: 0f214e8d7 adds the configuration rejection described above, and no smoke path meets it, because the driver generates a configuration with no storage_accounting block. b6c103106 moves the fan-out summary into a defer, so a fire that stops early logs the same three counts under its own message. 232a7ed12 narrows the attempt budget of both trigger kinds to 3. 970b1538b bounds one candidate page read at 5 s. The two fan-out messages are covered by tests rather than by this run, in 2cc1065e1: smoke fires neither trigger, so the driver cannot reach them.

E2E scenario catalogs

docs/testing/ gains no scenario, and no existing scenario is affected.

Each catalog under docs/testing/e2e/ holds the user-level journeys of one format: docker.md, oci.md, maven.md, and npm.md. A manual UAT session or the automated e2e harness executes each scenario. Both reach it through a format client, the management API, or the raw protocol API. This step adds two leader-elected background triggers, one candidate query, and one asynq handler registration. None of the three is reachable from a format client, from the management API, or from a raw protocol request, so no catalog row changes. This MR carries type::feature, and the branch carries feat and fix commits. The catalog rule therefore binds it, and the paragraph above is the statement the rule asks for.

Diff size

The diff is past 500 reviewable lines, so docs/dev/development-model.md asks for a split or a justification.

git diff --numstat origin/main...HEAD reports 24 files, +4066 −218. git rev-list --count origin/main..HEAD reports 40 commits. Both ranges are measured at head b8e364bce against main at d3231c18c. One file in that set is generated: gen/artifactregistry/config/v1/config.pb.go, at +8 −5, regenerated from the proto file beside it. The other 23 files are +4058 −213. No vendored or binary file is in the set.

Group Files Added Removed
New production Go internal/accounting/reconcile_trigger.go, internal/datastore/reconcile_candidates.go 733 0
Modified production Go cmd/artifact-registry/wire.go, cmd/artifact-registry/wire_accounting.go, cmd/artifact-registry/wire_jobs.go, internal/accounting/reconcile_task.go, internal/accounting/register.go, internal/config/storageaccounting.go, internal/datastore/query_names.go 473 110
Tests internal/accounting/reconcile_trigger_integration_test.go, internal/accounting/reconcile_trigger_internal_test.go, internal/accounting/reconcile_trigger_test.go, internal/datastore/reconcile_candidates_integration_test.go, internal/datastore/reconcile_candidates_test.go, internal/config/storageaccounting_test.go, cmd/artifact-registry/wire_accounting_test.go, cmd/artifact-registry/wire_jobs_test.go 2783 78
Documentation, example configuration, run recipe docs/dev/background-jobs.md, docs/dev/configuration-reference.md, docs/specs/S22-storage-accounting.md, config.example.yaml, .claude/skills/run-artifact-registry/SKILL.md 61 20
Configuration schema and generated stub proto/artifactregistry/config/v1/config.proto, gen/artifactregistry/config/v1/config.pb.go 16 10

Tests are 2783 of the 4066 added lines. Production Go is +1206 −110, documentation with the example configuration and the run recipe is +61 −20, and the schema with its generated stub is +16 −10.

A split does not help here, for four reasons.

  1. The candidate store and the triggers cannot land apart. The triggers are the only caller of the store, so a store-only MR lands unreachable code and a trigger-only MR does not compile.
  2. The two periodic kinds cannot land apart. They share one contribution seam, one dependency set, one guarded constructor, and one sync.Once. A split doubles the wiring diff and leaves the first half with count claims the second half falsifies.
  3. The wiring cell and the deferred thunk exist only so the triggers can reach the asynq client. They carry no meaning without the triggers.
  4. The configuration bound is the decision described above, and its code half is one file, internal/config/storageaccounting.go, at +39 −7. A separate MR for it splits the code from the sentence that justifies it, which the decision section above already names.

A reading order that keeps the review tractable: internal/datastore/reconcile_candidates.go first, then internal/accounting/reconcile_trigger.go, then internal/accounting/register.go with the three cmd/artifact-registry files, then internal/config/storageaccounting.go with the proto field it bounds, then the documentation and the tests.

Merge order and deferred work

Status row 15 is not edited here. CLAUDE.md gives the plan's Status table a single writer: step MRs do not edit the plan file, the Status table included. When this MR opens, row 15 goes to the S22 standing docs(plans) MR.

The step's Type card moves to feat in its own docs(plans) MR. This MR's title is feat(accounting), and the plan's Step 15 card said chore. !1850 (merged) moves the card, names the step in the plan's Type-prefixes enumeration, and corrects that paragraph's two counts, which were already off by one against the cards before this change. The card and the title are read together, so whichever lands second leaves no window where they disagree about a merged step.

Stale plan text goes to its own docs(plans) MR. Plan lines 59, 1004 and 1382 still say three trigger:* kinds and two registrations for step 15. !1837 (merged) corrects all three; line 1381 is an unrelated composition-root-rename row. docs/specs/S22-storage-accounting.md lines 768 to 773 tabulate four trigger:* kinds, and the plan's own step 15 entry says four registrations. The code follows the spec and the step entry, so this is stale plan text rather than a defect on this branch. A step MR cannot correct it.

!1751 (merged), step 10, is open and shares four edit sites with this branch. The two are plan siblings and the plan states no order between them. Whichever lands second rebases over the first across the kind list, the contributed register closures, and the boot-line assertions.

This step widens no validity predicate ahead of the code that serves it. The configuration change narrows what loads, and the two workers land with the two schedules in this same MR.

The fan-out loop carries no page-count bound, and that is a decision. fanOutNamespaces ends a walk on a short page, and nothing bounds the page count. A page-count cap is a test-only pattern in this repository: four integration suites carry one, and no non-test file under internal/ or cmd/ does. reconcileRepositories in internal/accounting/reconcile_task.go merged in step 14 with the same loop shape and no cap, closing the non-advancing cursor by construction. docs/specs/S22-storage-accounting.md assigns detection of a wedged trigger to a required paging alert rather than to code, and records that those rules land with #354. The residual is written up on work item #435.

Database Review Evidence

This branch adds no file under internal/datastore/migrations/sql/, so migration mode did not run. Query mode ran over the one changed file that dispatches a statement, internal/datastore/reconcile_candidates.go.

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17 container (matching GL_PG_CURR_VERSION from .gitlab-ci-other-versions.yml; image postgres:17-alpine, server 17.10), with synthesized seed data rolled back per query and the container torn down at the end of the run. Numbers reflect moderate cardinality 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 Time Buffers (hit / read) Partitions
datastore.ReconcileCandidateStore.ListNamespacesMissingStatistics Limit n/a 50 / 50 333.60 2.769ms 139 / 0 n/a
datastore.ReconcileCandidateStore.ListStaleNamespaces Limit index_namespace_statistics_on_last_reconciled_at_and_ns_id 500 / 500 77.58 0.112ms 11 / 0 n/a
datastore.ReconcileCandidateStore.ListNamespacesMissingStatistics

Summary: The plan matches the method's intent and the method's doc comment. PostgreSQL has no index for the correlated NOT EXISTS, so it reads both tables with sequential scans and joins them with a Hash Anti Join. A Sort above that join supplies the ORDER BY, so the LIMIT does not stop the scan early. One page therefore reads every row of both tables. Execution is 2.876 ms over 5000 namespaces rows and 4950 namespace_statistics rows.

Seed shape: namespaces=5000, namespace_statistics=4950

The 5000-row insert into namespaces gives each row a namespace_statistics row through trg_namespaces_create_statistics. The seed then removes 50 of those statistics rows. That is the residual state this walk exists to reach: a trigger that did not fire, or a namespaces row written with the trigger disabled.

Rendered SQL:

SELECT namespaces.id AS "missing_statistics_row.namespace_id"
FROM public.namespaces
WHERE (NOT (EXISTS (
           SELECT $1
           FROM public.namespace_statistics
           WHERE namespace_statistics.namespace_id = namespaces.id
      ))) AND (namespaces.id > $2::uuid)
ORDER BY namespaces.id ASC
LIMIT $3;

Bound args: [1, 00000000-0000-0000-0000-000000000000, 500]

$1 is the constant pg.Int(1) inside the EXISTS probe, which go-jet renders as a placeholder. $2 is the opening cursor, the nil UUID. $3 is 500, the default storage_accounting.reconciliation_batch_size.

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Limit  (cost=333.48..333.60 rows=50 width=16) (actual time=2.760..2.769 rows=50 loops=1)
   Buffers: shared hit=139
   ->  Sort  (cost=333.48..333.60 rows=50 width=16) (actual time=2.758..2.763 rows=50 loops=1)
         Sort Key: namespaces.id
         Sort Method: quicksort  Memory: 25kB
         Buffers: shared hit=139
         ->  Hash Anti Join  (cost=153.38..332.07 rows=50 width=16) (actual time=1.440..2.726 rows=50 loops=1)
               Hash Cond: (namespaces.id = namespace_statistics.namespace_id)
               Buffers: shared hit=139
               ->  Seq Scan on namespaces  (cost=0.00..159.50 rows=5000 width=16) (actual time=0.008..0.707 rows=5000 loops=1)
                     Filter: (id > '00000000-0000-0000-0000-000000000000'::uuid)
                     Buffers: shared hit=97
               ->  Hash  (cost=91.50..91.50 rows=4950 width=16) (actual time=1.340..1.341 rows=4950 loops=1)
                     Buckets: 8192  Batches: 1  Memory Usage: 297kB
                     Buffers: shared hit=42
                     ->  Seq Scan on namespace_statistics  (cost=0.00..91.50 rows=4950 width=16) (actual time=0.005..0.485 rows=4950 loops=1)
                           Buffers: shared hit=42
 Planning:
   Buffers: shared hit=98
 Planning Time: 1.036 ms
 Execution Time: 2.876 ms

Timings: planning 1.036ms, execution 2.876ms, total 3.912ms.

datastore.ReconcileCandidateStore.ListStaleNamespaces

Summary: The plan matches the method's intent. PostgreSQL takes an Index Only Scan over index_namespace_statistics_on_last_reconciled_at_and_ns_id. It applies the staleness bound and the row-value keyset bound as index conditions, not as a post-scan filter. The estimate matches the seeded reality at 500 rows against 500 rows. One 500-row page costs 11 buffer hits and no reads.

A second run measured the pre-first-pass state, where every row holds the epoch default. The node type and the index do not change there. The estimate is 1 row against 500 actual, and one page costs 975 buffer hits instead of 11. Execution stays under 0.5 ms, and the estimate recovers once the first pass spreads the timestamps.

Seed shape: namespaces=5000, namespace_statistics=5000

The 5000-row insert into namespaces gives each row a namespace_statistics row through trg_namespaces_create_statistics. The seed then spreads last_reconciled_at over 5000 minutes, so the walk's ORDER BY and keyset bound discriminate. The Heap Fetches count equals the returned rows because the seed runs inside the transaction that EXPLAIN rolls back, so the visibility map is not set.

Rendered SQL:

SELECT namespace_statistics.namespace_id AS "namespace_statistics.namespace_id",
     namespace_statistics.last_reconciled_at AS "namespace_statistics.last_reconciled_at"
FROM public.namespace_statistics
WHERE (namespace_statistics.last_reconciled_at < $1::timestamp with time zone) AND (ROW(namespace_statistics.last_reconciled_at, namespace_statistics.namespace_id) > ROW($2::timestamp with time zone, $3::uuid))
ORDER BY namespace_statistics.last_reconciled_at ASC, namespace_statistics.namespace_id ASC
LIMIT $4;

Bound args: [2026-08-20 23:03:16.745221+00, 2026-08-19 06:23:16.745221+00, 7e45a71f-c670-4d7a-b68f-4061b2d299ad, 500]

$1 is staleBefore, one hour back from the seed's clock. $2 and $3 are one cursor taken from the middle of the seeded range. $4 is 500, the default storage_accounting.reconciliation_batch_size.

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Limit  (cost=0.28..77.58 rows=500 width=24) (actual time=0.016..0.112 rows=500 loops=1)
   Buffers: shared hit=11
   ->  Index Only Scan using index_namespace_statistics_on_last_reconciled_at_and_ns_id on namespace_statistics  (cost=0.28..381.98 rows=2469 width=24) (actual time=0.016..0.088 rows=500 loops=1)
         Index Cond: ((last_reconciled_at < '2026-08-20 23:03:16.745221+00'::timestamp with time zone) AND (ROW(last_reconciled_at, namespace_id) > ROW('2026-08-19 06:23:16.745221+00'::timestamp with time zone, '7e45a71f-c670-4d7a-b68f-4061b2d299ad'::uuid)))
         Heap Fetches: 500
         Buffers: shared hit=11
 Planning:
   Buffers: shared hit=3
 Planning Time: 0.104 ms
 Execution Time: 0.166 ms

Timings: planning 0.104ms, execution 0.166ms, total 0.270ms.

Query notes:

  • datastore.ReconcileCandidateStore.ListNamespacesMissingStatistics: two Seq Scans, over 5000 namespaces rows and 4950 namespace_statistics rows, with no index used. The method's doc comment states this cost and states why the caller runs the sweep on a cadence of its own. No change to the query is proposed.
  • datastore.ReconcileCandidateStore.ListNamespacesMissingStatistics: the LIMIT does not bound the work. The Sort above the Hash Anti Join consumes the whole match set before the first row leaves it. The keyset cursor namespaces.id > $2 runs as a Seq Scan filter rather than an index condition, so a later page re-reads the rows the walk already passed. A measured fourth page over a 2000-row backlog reported Rows Removed by Filter: 1501. A full sweep therefore costs one pair of table scans for each page. Commit 67cd7c6a1 folded these facts into the method's doc comment, which now states the per-page cost, the Sort, and the re-read.
  • Plan shape for that method depends on how many rows are missing, not on the cursor alone. At 2000 missing rows of 5000, and at the opening cursor, PostgreSQL picked a Merge Anti Join over two Index Only Scans with no Sort, and the LIMIT did bound it at 0.693 ms. The Hash Anti Join with a Sort is what the near-zero density of the expected state produces.
  • datastore.ReconcileCandidateStore.ListStaleNamespaces: measured a second time in the pre-first-pass state, where every row holds the epoch default. The node type and the index hold. The estimate is 1 row against 500 actual, and one page costs 975 buffer hits instead of 11, at under 0.5 ms. The estimate recovers once the first reconciliation pass spreads the timestamps.
  • Neither statement is unbounded. Both carry a LIMIT, datastore.MaxReconcileCandidatePageSize caps it at 1000, and configuration load rejects a reconciliation_batch_size above that constant.
  • Neither statement reads a partitioned table, so the Partitions column is n/a for both rows. namespace_statistics is unpartitioned by the reason its migration records, and namespaces is unpartitioned too.

Review ledger

Findings this review raised and did not act on, recorded so a later round does not re-open them.

From Note Decision
GitLabDuo 3714953929 No-findings review digest, carrying no actionable claim. Declined by a standing operator ruling that such a note earns no round. No reply and no resolve.

Related to #515

This is a bot message 🤖 — /smurfit

Edited by Pawel Rozlach

Merge request reports