Repository navigation
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
Closed
erikdarlingdata wants to merge 1 commit into
erikdarlingdata wants to merge 1 commit into
Conversation
…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
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5513
What changes
QueryStoreIntervalLatest.UseTableis all or nothing. When the interval table covers most of the plan regression window but not all of it, the pass reads the whole rawquery_store_statsslice 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.DecideSourceandResolveReadSourceAsync(new) pick Table, Mixed or Raw.ReadsTableAsyncstays and answers true only for Table, so the drill-down fallback and the older tests are unchanged.PgFactCollector.PlanRegressionMixedSql(new) puts two sets into oneGROUP BYusing 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). Theplan_aggtext and the suffix are the shared ones. The raw prefix was split in two with identical text so the two reads cannot drift.collection_time >= max(filled_since, raw's reading floor)of intervals withfirst_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).max(coveredFrom, table floor + 1 day interval + 1 day skew margin), as a bare parameter, so a hypertable keeps only the chunks in between.PlanRegressionReadsIntervalTable = false). It reads the raw SQL it always read, which returns the same rows, so there is no third drill-down shape.Diagnosis (code reading; the large store was not available)
Conditions that make
ReadsTableAsyncreturn false, with whether a steady store hits them:filled_sincenewer thanmax(raw's oldest chunk, window bound). Coverage restarts at the clock whenever the gap check, a purged pending batch, or a Regular row with nofirst_execution_timeresets 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.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 realCollectFactsAsyncwith a restarted claim; the log line saysMixed, 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).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.PgFactCollectorTests.AllSql_CoversEveryQuery_...(one more query: the mixed twin) and the any_value check;PlanRegressionDailyBoundaryTests(the command-selection text now names the mixed read).ResolveReadSourceAsync,PlanRegressionMixedSql), so they cannot compile on the base commit, which is a weak RED. A behavior plant (makeDecideSourcereturn Raw and rerunThePass_ReadsTheMixedSource_...) was not run for lack of time.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
DecideSourcecomputesfirstExecutionFloor = rawBoundwhen clause 3 holds, so the second disjunct of the raw part isfirst_execution_time < rawBound(rows from intervals older than the window needs; harmless and exact).