Skip to content

feat(remote): re-dump production statistics on a daily floor - #198

Merged
veksen merged 2 commits into
mainfrom
feat-stats-daily-floor
Jul 27, 2026
Merged

feat(remote): re-dump production statistics on a daily floor#198
veksen merged 2 commits into
mainfrom
feat-stats-daily-floor

Conversation

@veksen

@veksen veksen commented Jul 27, 2026

Copy link
Copy Markdown
Member

Closes Query-Doctor/Site#3671.

Goal

Bound how stale a snapshot's column statistics can get when neither drift signal fires.

What

Before, a snapshot refreshed only on Shape Drift or Size Drift. A database whose tables and sizes hold steady never re-dumped, however old its statistics got.

After, a connected analyzer re-dumps at least once every 24 hours.

How

isPastRefreshFloor in stats-drift.ts is the decision, kept pure and beside the drift signals. Remote records when it last pushed, and the existing schema-poll check now fires on drift or the floor. No new schedule.

Two properties worth noting:

  • A push from either trigger updates the timestamp, so drift and the floor can't dump twice in quick succession.
  • The floor never fires before a first push. With no baseline there is nothing to compare against, and dumping on a timer for an analyzer that has never pushed would publish statistics for a database the server may not expect.

Departure from the ADR

ADR 0007 §2 specifies this backstop as a cron in Site. It sits in the analyzer here. The server currently has no way to ask an analyzer to refresh, so the Site-side version needs a new ClientApi method, an analyzer implementation, and a cron, where this needs one timestamp. If the Site placement is preferred, amend the ADR rather than leaving the two out of step.

Tests

Five cases in stats-drift.test.ts: never before a first push, not while recent, true at exactly a day, true well past, and a custom floor honoured in both directions.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Query Doctor Analysis

3 queries analyzed

0 regressed · 0 improved · 0 new · 0 removed

2 pre-existing issues

Using assumed statistics (10000000 rows/table). For better results, sync production stats.

More detail → get_ci_run({ runId: "019fa558-4f0f-749a-ba4c-c3b66a738f07" }) · view run · docs

Closes #3671.

Shape Drift and Size Drift watch structure and row counts. Neither sees a
database whose tables and sizes hold steady while the distribution of its data
moves: histograms, most-common-value lists and correlation all age with the
values rather than the count. A floor bounds how wrong those can get.

The check rides the same schema poll as drift, so no new schedule. A push from
either trigger updates the timestamp, so drift and the floor can't dump twice
in quick succession.

Never fires before a first push. With no baseline there is nothing to compare
against, and dumping on a timer for an analyzer that has never pushed would
publish statistics for a database the server may not expect.

ADR 0007 §2 places this backstop as a cron in Site. It sits in the analyzer
instead: the server has no way to ask an analyzer to refresh, so the Site-side
version needs a new ClientApi method, an implementation, and a cron, where this
needs a timestamp. Worth amending the ADR if the Site placement is preferred.

Co-Authored-By: Claude <[email protected]>
@veksen
veksen force-pushed the feat-stats-daily-floor branch from 820944a to cffc60f Compare July 27, 2026 20:45
Review of the daily floor found it never fires for the analyzers it targets.
`lastStatsPushAt` is written only by `applyStatistics`, and the sync path
reaches the optimizer directly instead, so a seeded analyzer leaves it
undefined and `isPastRefreshFloor` short-circuits forever. Only a
drift-triggered dump ever armed it, which is the case the floor exists to
cover when drift does not fire. It also re-armed nothing across restarts.

Seeding now sets the timestamp. It is dated from the seed rather than the
snapshot's capture time, which the RPC doesn't carry, so the floor fires 24h
after connect rather than immediately.

A failing dump also retried on every 60s poll, because the timestamp only
advances on success. A statement_timeout on a large pg_statistic read would
run DUMP_STATS_SQL ~1440 times a day. Failures now back off for 15 minutes.

Renamed `refreshStatsIfDrifted` to `refreshStatsIfStale`, since it has had two
triggers since the floor landed.

Co-Authored-By: Claude <[email protected]>
@veksen

veksen commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

Review found the floor never fires for the analyzers it targets. Fixed in 072ebe8.

lastStatsPushAt is written only by applyStatistics, and the sync path (onSuccessfulSync) calls optimizer.start directly instead. So a seeded analyzer left it undefined and isPastRefreshFloor short-circuited forever — only a drift-triggered dump ever armed it, which is exactly the case the floor is meant to cover when drift doesn't fire. It also reset on every process restart.

Seeding now arms it, dated from the seed rather than the snapshot's capture time, which getProductionStats doesn't carry. The floor fires 24h after connect rather than immediately.

Also fixed: a failing dump retried on every 60s poll, since the timestamp only advances on success. A statement_timeout on a large pg_statistic read would have run DUMP_STATS_SQL about 1440 times a day. Failures now back off 15 minutes.

Two findings left for a follow-up, both pre-existing and wider than this PR:

  • applyStatistics keys on statsMode.kind === "fromStatisticsExport" to decide whether stats are source-derived, but statsModeFromExport returns that kind for imports too. So a recurring import advances the floor timer without a source dump, and resetStats (which is fromAssumption) clears neither the baseline nor the timer. The comment above that check already describes the intent the discriminator doesn't implement.
  • No integration coverage for the refresh path in remote.test.ts. The findings above were all invisible to the suite.

@veksen
veksen merged commit a1f7ac7 into main Jul 27, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant