Skip to content

Plan regression reads the table for the part of the window it covers and raw only for the rest (#5513) - #5531

Closed
erikdarlingdata wants to merge 1 commit into
devfrom
fix/5513-plan-regression-raw-fallback
Closed

erikdarlingdata wants to merge 1 commit into
devfrom
fix/5513-plan-regression-raw-fallback

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Oct 8, 2026 •

Copy link
Copy Markdown
Owner

Fixes #5513

What changes

QueryStoreIntervalLatest.UseTable is all or nothing. When the interval table covers most of the plan regression window but not all of it, the pass reads the whole raw query_store_stats slice and dedups it. This PR adds a third answer, Mixed: the table for the part of the window its coverage claim holds, raw rows for the rest.

  • QueryStoreIntervalLatest.DecideSource and ResolveReadSourceAsync (new) pick Table, Mixed or Raw. ReadsTableAsync stays and answers true only for Table, so the drill-down fallback and the older tests are unchanged.
  • PgFactCollector.PlanRegressionMixedSql (new) puts two sets into one GROUP BY using the raw read's own key and its own order, (collection_time DESC, execution_count DESC). The first set is the raw snapshots the table does not stand in for. The second is the table's own rows. A table row is its interval's newest snapshot, so it wins any group it shares with older raw snapshots (an interval that is open on one side of the edge and closed on the other). The plan_agg text and the suffix are the shared ones. The raw prefix was split in two with identical text so the two reads cannot drift.
  • A table row stands in only for snapshots with collection_time >= max(filled_since, raw's reading floor) of intervals with first_execution_time >= the table's floor. So the table never reaches below what raw holds, and the answer stays the raw read's answer (the table-only read's reach below raw's purge is not taken here).
  • The raw part is bounded above by max(coveredFrom, table floor + 1 day interval + 1 day skew margin), as a bare parameter, so a hypertable keeps only the chunks in between.
  • Pending batches, no coverage row, and an empty table for the server still read raw. Any decision fault reads raw.
  • The drill-down is told "raw" for Mixed (PlanRegressionReadsIntervalTable = false). It reads the raw SQL it always read, which returns the same rows, so there is no third drill-down shape.
  • The V168 daily path is unchanged and is used only when the source is Table.

Diagnosis (code reading; the large store was not available)

Conditions that make ReadsTableAsync return false, with whether a steady store hits them:

  1. A fault in the decision (logged at Warning). Rare.
  2. No coverage row. Only servers that never wrote Query Store rows. The raw read for those is cheap.
  3. A pending row. One collection cycle after a failed apply. Rare.
  4. filled_since newer than max(raw's oldest chunk, window bound). Coverage restarts at the clock whenever the gap check, a purged pending batch, or a Regular row with no first_execution_time resets it, and V143 has no backfill, so a newly added or newly upgraded server is the same. On a timescale store raw keeps about 4 days, so one restart costs about 4 days on raw for that server. A steady fleet with a restart per server every few days sits near half on raw. Inferred, not observed.
  5. The table's floor MIN(first_execution_time) above the window bound while raw holds more (clause 3 of the rule). On a plain store (no raw floor) this happens after every daily purge, because the worker's window starts 4 hours before now and the purge keeps 15 days from the purge time. Inferred, not observed.

Both 4 and 5 now read the table for what it covers. Reading raw for the remainder costs about the uncovered span (at worst a fresh restart reads the same raw span as before, plus a trivial table side). I cannot say which of 4 or 5 holds the field store's 46% without the coverage rows, so a read-only query set (coverage start against the window bound and raw's floor, pending rows, table floor, per server) is posted on #5508 (comment 6052170573, section 1). If the restarts themselves are the cause, they need their own look: this PR makes them cheap, not rare.

Measurements

Issue step 1 numbers (43-server store at V165, 13.2 days): the raw read ran 12,510 times and read 605 M blocks over 99,159 s (47,586 and 51,341 blocks per call). The table read ran 14,866 times, so about 46% of passes took the raw path.

Rig numbers (my rig, PostgreSQL 18 with TimescaleDB, a scratch database; no cost numbers were measured, the field store is the place for that):

  • PlanRegressionMixedSourceTests.TheMixedRead_ReturnsTheRawReadsRows_...: the mixed read returns the raw read's rows column for column at five coverage edges (below all snapshots, inside an interval open on one side and closed on the other, the middle, past the newest, and a later one), then again after the table's intervals older than day 3 were deleted (a purge). In that state the table-only read differs from raw (asserted), and the mixed read still equals raw.
  • ThePass_ReadsTheMixedSource_...: runs the real CollectFactsAsync with a restarted claim; the log line says Mixed, the drill-down flag is false, no collection failure is recorded, and the PLAN_REGRESSION fact (value, database, metadata, offenders) equals the fact from a second pass forced to raw by a pending row.

Test plan

  • PlanRegressionMixedSourceTests (8 tests, 3 live): 8 passed, 0 skipped, on a PG 18 TimescaleDB rig.
  • *PgFactCollector*, *PlanRegression*, *QueryStoreIntervalLatest*, *FactCollector*, *DrillDown*: 210 tests, 0 failed, 1 skipped (before the AllSql pin update).
  • Census, Ratchet, Hygiene, Pin, repo guards, scans and the fact-collector failure/timeout classes together: 1732 tests, 1 failed, 3 skipped. The one failure, StatementCollectionCensusTests.ABlockingRingBufferOfAboutFourMegabytes_IsJudgedWellUnderTheBudget, passes when run alone and does not read anything this PR touches; I did not investigate why it failed in the full run.
  • Build: 0 warnings, 0 errors.
  • Pins updated on purpose: PgFactCollectorTests.AllSql_CoversEveryQuery_... (one more query: the mixed twin) and the any_value check; PlanRegressionDailyBoundaryTests (the command-selection text now names the mixed read).
  • RED-plant not run. The new tests use the new API (ResolveReadSourceAsync, PlanRegressionMixedSql), so they cannot compile on the base commit, which is a weak RED. A behavior plant (make DecideSource return Raw and rerun ThePass_ReadsTheMixedSource_...) was not run for lack of time.
  • Full suite not run (the laptop does not run it; CI decides).
  • Cost on the large store (blocks per call before and after) not measured; needs the field store.

Changelog

SECTION: Fixed
ENTRY: Plan regression analysis no longer reads the whole raw Query Store history when its summary table is only partly caught up. The pass now reads the table for the part it covers and raw rows only for the rest, and returns the same rows as the raw read.
REF: this PR

Review notes

  • DecideSource computes firstExecutionFloor = rawBound when clause 3 holds, so the second disjunct of the raw part is first_execution_time < rawBound (rows from intervals older than the window needs; harmless and exact).
  • A race the existing table path already has: the purge can remove table rows between the floor read and the data read.
  • The mixed read has not been run under TimescaleDB chunk exclusion with real chunks; the live tests use plain tables (as the other table-versus-raw tests do).

…and raw rows only for the rest (#5513)

QueryStoreIntervalLatest.UseTable is all or nothing, so a coverage claim that restarted a day or two ago, or a table whose
purge had just moved its floor above the window bound, sent the whole 15-day slice through the raw dedup. A third answer,
Mixed, groups the raw snapshots the table does not stand in for together with the table's own rows under the raw read's
own order, so it returns the raw read's rows. The drill-down is told "raw" for it and reads the raw SQL it always read.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ZGNj3cTaf9E9jYjdAa9vA
@erikdarlingdata

Copy link
Copy Markdown
Owner Author

Claude posting for Erik Darling.

Closing this unmerged. The raw path this PR speeds up has not run on the big store since about 2026-09-30, and all 43 servers now read the table. The evidence is in #5513's closing comment and #5508 comment 6053411189, section 1.

The review found a gap in the table rule that is older than this PR. It is filed as #5541.

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