Repository navigation
Correct the Query Store rollups: dedup cumulative interval snapshots in the CAGG chain (#1849) - #1870
Conversation
…in the CAGG chain (#1849) The hourly/daily Query Store rollups materialized sum(execution_count) from un-deduped raw rows, so one interval's work was baked in once per COLLECTION. Tier 2 (#1853) fixed every raw-tier read and could not reach these: the duplicates are gone once materialized and the rollup output carries no interval identity to key on. A Custom Views panel inside the raw tier read corrected numbers; past it, inflated ones, with a visible step at the routing boundary. Three NEW aggregates carry the corrected numbers, alongside the old pair rather than replacing it (a CAGG cannot be reshaped in place, and rebuilding would destroy 21 days of hourly and all daily history - #1759/#1793): query_store_stats_interval_hourly L1, interval grain, last(x, collection_time) query_store_stats_corrected_hourly composer grain, identity-width off L1 query_store_stats_corrected_daily composer grain, 1-day bucket off L1 Column names are identical to the old pair, so ComposeCaggValueMapper and every composed panel read them unchanged. Measured on a seeded store: one interval re-collected 496 times reports 506 where the old rollup reports 123,311. The shape is forced by what TimescaleDB accepts, live-probed on PG 18.4 / TimescaleDB 2.28.1. An identity-width hierarchical CAGG is a LEAF - nothing can be built on a child whose bucket equals its parent's width, though a plain 1h -> 1d -> 7d chain is fine - so the corrected daily is a SIBLING of the corrected hourly sourced from L1, where every other daily here is built on its hourly. That constraint is not in #1849; it was found building this. Arming: query_store_stats now has two rollup families, so RawTierCoverage's coverage became a LIST and the #1680 gate an AND over all of them. Raw stays paused until both reach back over everything it holds. Evaluated in the reader, not folded into GREATEST, which skips NULLs and would let an empty new rollup vanish from the comparison. Routing: ComposeCaggInfo gains the superseded pair. Corrected wins wherever its coverage reaches; beyond it reads fall back to the old pair, comparatively (only where it measurably reaches further back), so a backfilled store never reads inflated. Availability is settled before coverage - a store whose service predates this routes to the old pair exactly as before, which is why this needs no migration and no viewer version gate. Backfill: RollupViews now carries BucketWidth explicitly. "Hierarchical" and "daily" used to be the same fact and the backfill inferred grain from the source column; the corrected hourly is the first hierarchical rollup that is not a daily, and inferring would have given it a 24x-too-wide bucket - an under-estimate, the one direction the disk preflight exists to prevent. Capacity settled before shipping (#1581): the interval layer is near-raw cardinality, so it takes a short 7-day horizon rather than the 21-day one - 79 MB against 238 MB projected on a 600-query store at the default cadence. Verified against live PostgreSQL 18.4 + TimescaleDB 2.28.1, watched red first: removing L1 from the arming gate arms a purge over history nothing holds; last() -> sum() in L1 reports 123,256; dropping or inverting the legacy fallback each fails a named routing test. Darling 3848 passed / 0 failed, Lite 1728 / 0, full-solution rebuild 0 warnings. Filed rather than guessed: #1869, the daily tier's ~2x hour-straddle residual and the fourth near-raw-cardinality aggregate that would remove it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ReviewOverview. This adds a corrected Query Store CAGG chain ( Correctness — traced in detail, no defects found:
Lite/Darling parity. No Lite files are touched, and that's correct here rather than a drift risk: continuous aggregates are a TimescaleDB/Postgres-only concept, and grepping the Lite tree turns up zero references to Security. All new SQL is built from hardcoded internal view-name constants ( Performance. Capacity was measured up front (600-query seeded store, 24h) rather than guessed, and the interval-grain L1 layer intentionally gets a short 7-day retention instead of inheriting the 21-day hourly horizon specifically because its cardinality is near-raw — good call given it would otherwise outgrow the raw tier it's supposed to be smaller than. Known, disclosed limitation. The hour-boundary-straddle residual (an interval collected once per collection HOUR instead of once per COLLECTION at the hourly grain) is explicitly surfaced in the doc comments and filed as #1869 rather than silently left in — appropriate scope cut given the capacity cost of fixing it (a 4th near-raw-cardinality aggregate). No blocking issues found. This is a large, unusually well-substantiated change (live-probed engine constraints, red-then-green tests, capacity math) and the code matches what the PR description claims. |
1 participant
Closes #1849.
query_store_stats_hourlymaterializedsum(execution_count)from un-deduped raw rows andquery_store_stats_dailyinherited it hierarchically. Query Store rows are cumulative per-interval snapshots that the collector re-fetches every cycle, so one interval's work was baked in once per COLLECTION. #1853 corrected every RAW-tier read and stated plainly that it could not reach these: the duplicates are gone once materialized, and the rollup output carries no interval identity to key on. A Custom Views panel whose window stayed inside the raw tier read corrected numbers; a window past it read inflated ones, with a visible step at the routing boundary.What shipped
Three NEW continuous aggregates, alongside the old pair rather than replacing it:
query_store_stats_interval_hourly(L1)last(x, collection_time)query_store_stats_corrected_hourlyquery_store_stats_corrected_dailyColumn names are identical to the old pair, so
ComposeCaggValueMapperand every composed panel read them with no change — only which relation is named differs.The old pair is kept because a continuous aggregate's columns cannot be ALTERed: reshaping means DROP + recreate, and with retention active the rebuild re-materializes from 4 days of raw and permanently destroys the 21-day hourly and indefinite daily history (#1759/#1793). Their data, retention and jobs are byte-unchanged — the diff contains zero removed lines touching either definition.
The shape was forced by the engine, not chosen
Live-probed on PostgreSQL 18.4 / TimescaleDB 2.28.1. #1849 carried five probe results; these were re-probed and one more was found:
An identity-width hierarchical CAGG is a LEAF. A child whose bucket equals its parent's width (a 1-hour bucket over a 1-hour parent) creates and refreshes normally, but nothing can be built on top of it — a further aggregate fails with
time bucket function must reference the primary hypertable dimension column. Verified that depth is not the blocker: a plain 1h to 1d to 7d chain is accepted.So
_corrected_dailyis a sibling of_corrected_hourlysourced from L1 directly, where every other daily in this codebase is built on its hourly. That asymmetry is load-bearing and documented at the definition so it does not get "made consistent" later. It is also the better shape: each corrected rollup is one hop from the dedup, so the daily does not compound the hourly's residual.Legacy rows are included, not excluded
runtime_stats_interval_idis NULL on exactly the pre-V41 generation and nothing can backfill it. L1 groups by both the real id and the tier-1first_execution_timeproxy — #1853's argument applied to the rollup: the two generations can never collide, so a post-V41 row keys on its real id (the proxy is functionally dependent and adds no groups) and a legacy row degrades to precisely tier 1's key. Excluding them would have been the easier claim to make true and would have silently dropped every pre-upgrade hour out of the corrected rollup while the store still held the rows. A COALESCE into one key is expressible in a CAGG GROUP BY (probed: accepted) and was rejected for fusing two identity domains into one text column.Arming (#1784 / #1790 territory)
query_store_statsnow has two rollup families reading it, soRawTierCoverage's coverage became a list and the #1680 gate an AND over all of them. Raw's purge stays PAUSED until the corrected interval layer and the original hourly each reach back over everything raw holds, and arms itself on the next start once they do. A gate reading either one alone would drop raw history the corrected rollups had never seen — which is the state every existing store is in the moment it takes this build.The plurality is evaluated in the reader rather than folded into SQL:
GREATESTskips NULLs, so an empty new rollup would vanish from the comparison and the gate would pass on the old one alone. Both purge paths (EnsureRetentionPoliciesAsyncand the catalog sweep'sIsRawTierDropSafeAsync) read the one extended map, so they cannot disagree.L1 itself gets a policy too, coverage-gated on both corrected views (it has two children), on a deliberately short horizon. The corrected daily is kept indefinitely like every other daily and gets no policy.
Routing, and the boundary
ComposeCaggInfogains the superseded pair. Corrected wins wherever its coverage reaches; beyond it, reads fall back to the old pair — comparatively, only where it measurably reaches further back, so a backfilled store never silently reads inflated numbers. Availability is settled before coverage: a store whose service predates this build has no corrected rollups, every floor is null, and the comparative rule would correctly refuse to move — stranding every Query Store window on raw. Settling existence first restores exactly the pre-#1849 routing.The step is not removed, it is moved outward, and it shrinks daily as the corrected rollups deepen and the old rows age out. Documented in
ComposeCompiler.BuildFactRelation, at the catalog entry, at the router, and in the CHANGELOG.No migration, deliberately
Continuous aggregates are runtime TimescaleDB setup, not schema objects — the existing
to_regclassavailability probe already answers "does this store have them", and a viewer pointed at an older store degrades per tier (#1664/#1665). BumpingStorageVersion.SchemaVersionfor objects no migration creates would lock viewers out of stores that are functionally identical at V41. So: no V42, no probe sentinel, noMapProbedSchemaVersionarm.Backfill (#1788 / #1799)
RollupBackfill.Targetsderives fromRollupViews, so the three new rollups are covered automatically with the same disk preflight, in dependency order (L1 is raw-sourced and sorts first).One real defect was fixed to make that true:
RollupViewsnow carriesBucketWidthexplicitly. "Hierarchical" and "daily" used to be the same fact, so the backfill inferred grain from the source column — and_corrected_hourlyis the first hierarchical rollup that is not a daily. Inferring would have given it a 24x-too-wide bucket, making every bucket count, slice count and disk estimate for it an under-estimate: the one direction the preflight exists to prevent. The ordering test was rewritten to assert on each target's actualSourceedge instead of rewriting_dailyto_hourlyin the view name, a guess that only held while every hierarchical rollup was named after its parent.Capacity, settled before shipping (#1581)
Measured on a seeded 600-query store at the default 5-minute
query_storecadence, 24h of collection:query_store_statsinterval_hourlycorrected_hourlyold_hourly(existing)L1 keys on query_id/plan_id/interval, so its cardinality is near-raw. It therefore takes its own short 7-day horizon instead of inheriting the 21-day one: 79 MB against 238 MB projected, which would have made the store's intermediate dedup layer larger than its entire raw tier. Seven rather than four days because it must outlive raw with margin or the arming gate races itself — pinned as a test invariant.
Verification
Live PostgreSQL 18.4 + TimescaleDB 2.28.1 on a dedicated port, watched red first:
RawTierCoveragelast()becomessum()BeyondCorrectedCoverage_FallsBackToTheLegacyPairNeverRoutesToTheInflatedPair, with the diagnostic naming the windowThe new live tests prove, against a real store: the 496x interval collapses to one L1 row holding 496; the corrected hourly reports 506 where the original reports 123,311 side by side in the same test; legacy (NULL-id) rows dedup at tier-1 fidelity; the weighted mean composes to a hand-computed value; the corrected daily equals the sum of its corrected hours; and the arming gate holds a raw purge while L1 is short, then releases by itself once L1 covers raw.
Gating verified honestly — the pair SKIPS without
DARLING_TEST_PGand PASSES with it. One flake was found and fixed rather than retried: the arming test asserted L1 was empty, which a refresh policy's immediate first run (#1564) can falsify under suite load; it now strips refresh policies and asserts the real precondition (shallow-or-empty). Three consecutive full-suite runs clean.-t:Rebuild: 0 warnings, 0 errorsFiled, not deferred silently
#1869 — an interval straddling an hour boundary is counted once per collection HOUR (~2) instead of once per COLLECTION (up to 496). Irreducible at the hourly grain; removable at the daily grain by an interval-grain daily re-dedup, whose both levels were live-probed ACCEPTED here, at the cost of a fourth near-raw-cardinality aggregate. Written up with the probe SQL, the leaf constraint that makes it legal, the measurements, and the wiring it would need — a costed capacity decision rather than a guess.
🤖 Generated with Claude Code