Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- **Collection Health's Note and Last Error now show the NEWEST message, not the alphabetically greatest one** ([#1855]) - both columns were read as `MAX(error_message)` over the trailing window, which returns the greatest STRING, not the most recent one. That was harmless for as long as every message was a stable failure-mode sentence, and the entry below broke the assumption by shipping the first note that carries a NUMBER: text does not sort like a count, so `12 item(s) failed their enumeration probe` sorts BELOW `9 item(s)`, and a collector whose probe-failure count moved cycle to cycle displayed an arbitrary earlier run's number with nothing to mark it stale. Both columns now take the message from the newest run that left one, by ranking each class of message newest-first and reading the top row. **A later clean run does not blank the note**: the column still answers "the last thing this collector reported in the window", which is what makes the `(3 of 96 runs)` qualifier mean anything - ranking on time alone would empty it the moment one cycle came back quiet. **A note still cannot surface as a failure**: when no failing run in the window carried text, the error ranking falls through to the newest row of any class, so the read re-checks the status on that row - without it, a quiet enumeration's note would be reported as the collector's last error, the exact confusion the separate Note column exists to prevent. The `last error` TIME is unchanged and still names the newest failure outright, text or not, because "when did this last fail" is about the run. The fix needed no dialect split despite there being no portable "value at the greatest timestamp" aggregate (DuckDB has `arg_max`, PostgreSQL has no `max_by`): the ranking is one byte-identical query shape on both stores, verified by running the same string against a real DuckDB and a real PostgreSQL 18.4, and it settles an exact-timestamp tie the same way on each - which the old `MAX` did not, since DuckDB compares text by bytes and PostgreSQL by the database collation, so Lite's grid and the Darling Viewer's could disagree on identical data. Applies to Lite's Collection Health grid, the Darling Viewer's, and both `get_collection_health` MCP tools with the web dashboard's table behind them. **The Darling Viewer's FLEET rollup deliberately carries no exemplar text at all**, where it previously carried the wrong-run `MAX`: its only caller counts collectors and failing bands for the status bar, no surface renders a fleet row's message, and ranking one per group turns a parallel hash aggregate into a serial sort of every row in the window - measured at 0.84s to 13.9s over a 200-server, 4-million-row store. The note COUNT, which is order-independent and cheap, stays real on both reads.

- **The Query Store hourly and daily rollups stopped baking in the pre-dedup inflation - a Custom Views panel reaching past the raw tier no longer steps by two orders of magnitude at the boundary** ([#1849]) - [#1841] tier 1 and tier 2 corrected every RAW-tier read, and said plainly what they could not reach: `query_store_stats_hourly` had materialized `sum(execution_count)` from un-deduped rows and `query_store_stats_daily` inherited it, so a panel whose window stayed inside the raw tier read corrected numbers and a window past it read inflated ones, with a visible step where the router crossed over. No read-side edit could undo it - the duplicates are gone once materialized, and the rollup output carries no interval identity to key on. Three NEW continuous aggregates now carry the corrected numbers: `query_store_stats_interval_hourly` collapses each Query Store interval to its final snapshot with `last(execution_count, collection_time)`, and `query_store_stats_corrected_hourly` / `_corrected_daily` sum those deduped values into the SAME column names the old pair carries, so every composed panel reads them with no change. Measured on a seeded store: one interval re-collected 496 times inside an hour reports **506** where the original rollup reports **123,311**.

**The old rollups are KEPT, and that is why a boundary still exists.** A continuous aggregate's columns cannot be ALTERed, so reshaping means DROP and recreate - and with retention active the rebuild would re-materialize from 4 days of raw and permanently destroy the 21-day hourly and indefinite daily history ([#1759]/[#1793]: materialized history is never destroyed). So the corrected rollups start EMPTY and deepen from deploy while the old pair keeps everything it already holds, and reads prefer the corrected one wherever it has actually materialized the window. **Past that point a window still reads the old inflated numbers.** The step has not been removed, it has been moved outward - and it shrinks every day as the corrected rollups deepen and the old rows age out. `--backfill-rollups` closes it outright, and it already covers the new rollups with the same disk preflight, in dependency order.

**The raw purge cannot outrun any of it.** `query_store_stats` now has TWO rollup families reading it, so the [#1680] arming gate became an AND over both: raw's retention policy 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 that checked either one alone would have dropped 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 on purpose: `GREATEST` skips NULLs, so an empty new rollup would have vanished from the comparison and the gate would have passed on the old one alone.

**Sizing was settled before shipping, not after** ([#1849] flags capacity as a real input; [#1581] is why). The interval layer keys on query_id/plan_id/interval, so its cardinality is near-raw: on a 600-query store at the default 5-minute `query_store` cadence, 24 hours of collection costs raw 40 MB, the interval layer 11 MB (28% of raw), and each composer-grain rollup 4.4 MB. It therefore gets its own SHORT 7-day horizon rather than inheriting the 21-day one - 79 MB against 238 MB projected, which would have made the store's intermediate dedup layer larger than its whole raw tier. Seven days rather than four because it has to outlive raw with margin or the arming gate races itself.

**No schema migration and no viewer version gate**, deliberately: continuous aggregates are runtime TimescaleDB setup, not schema objects, so the existing `to_regclass` availability probe answers "does this store have them" and a viewer pointed at a store whose service predates this build routes to the old pair exactly as before. Bumping the store schema version for objects no migration creates would have locked viewers out of stores that are functionally identical.

Five CAGG shapes were live-probed against PostgreSQL 18.4 + TimescaleDB 2.28.1 to settle the design, and one result is worth recording because it is in no documentation and it inverts the shape every other daily rollup in this codebase uses: **an identity-width hierarchical continuous aggregate is a LEAF**. A child whose bucket equals its parent's width 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`. Depth is not the blocker (a plain 1h -> 1d -> 7d chain is accepted), the equal width is. So `_corrected_daily` is a SIBLING of `_corrected_hourly` sourced from the interval layer directly, where every other daily here is built on its hourly. One residual is left and filed rather than hidden: an interval whose snapshots straddle an hour boundary is counted once per collection HOUR (~2) instead of once per COLLECTION (up to 496), which is irreducible at the hourly grain and removable at the daily grain only by adding a fourth near-raw-cardinality aggregate - a costed capacity decision, written up with the live probe results and the measurements at [#1869].

- **Query Store charts now put an interval's work in the hour it RAN, and the duration trend stopped overstating** ([#1841], tier 2) - the collector stores the real interval identity at last: `runtime_stats_interval_id` and `interval_start_time_utc`, read straight from `sys.query_store_runtime_stats_interval` and converted to UTC at collection. Two things follow. **Every dedup now keys on the real interval** instead of the `first_execution_time` proxy tier 1 had to settle for. The proxy stays in the key beside it, because it is the only identity rows collected before this version have - but where the real id exists it also fixes what the proxy could not: a row Query Store never attributed a first execution to had NO identity under tier 1 and collapsed with every other such interval of the same plan, which under-counts. **And the bars stopped lagging.** A Query Store interval was previously drawn in the hour it was last COLLECTED, which on the default 60-minute interval is reliably one bar late, because the closing fetch lands in the cycle after the interval ends. Slicer bars are now placed at the interval's own start. The **duration-trend chart is corrected rather than excluded**, reversing tier 1's deliberate hold: each interval contributes its true final total once, at the hour it ran, so the chart neither triple-counts a growing interval nor collapses to a single zero-valued point. The premise that blocked this turned out to be false - Query Store's interval start and `first_execution_time` are both `datetimeoffset` (verified live on a server reading +00:00 while sitting at UTC-4), and the collector already normalized them to UTC before storing, so the interval clock was never the monitored server's local wall time. **Nothing needs re-collecting, and a store holding older rows does not lie**: reads split on whether a row carries the identity, with no overlap and no gap, so pre-upgrade rows keep exactly the behavior they always had and the mixture resolves itself as they age out of retention. Lite schema v49, Darling store V41, both additive and nullable. Watched failing first against real DuckDB and a real PostgreSQL 18 + TimescaleDB store - including the live proof of the one-bucket lag (a bar at 16:00 for work that ran at 15:00) and of the tier-1 under-count (5 executions reported where 11 ran). Two things remain open and are filed rather than hidden: the hourly/daily rollups still bake in the pre-dedup inflation, so a Custom Views panel reaching past the raw tier still steps at the routing boundary ([#1849], with the corrected-rollup design and the live TimescaleDB probe results that constrain it), and four analysis-layer dedups still drop a replica's rows on an Availability Group primary ([#1850]).

- **Query Store totals no longer count the same interval once per collection - live stores were overstating hot queries by one to two orders of magnitude** ([#1841]) - Query Store's runtime-stats rows are CUMULATIVE per-interval snapshots, and the collector is incremental on `last_execution_time`, so while a query keeps running its OPEN interval is re-fetched every cycle and stored again with a growing `execution_count`. The schema has no interval-id column, and every aggregate read simply summed the raw rows - so one interval's work was counted once per collection. Measured on a real store: the same (server, database, query_id, plan_id) appeared **496 times inside a single hour bucket** with `execution_count` of 1 each time (one interval, 496 collections, its CPU and duration billed 496 times), alongside cases growing 1 -> 23 cumulatively where every re-collection re-charged the accumulated total as well. Execution-count weighting did not rescue the averages: the repeated snapshots carry DIFFERENT growing weights AND different `avg_*` values, so an open interval was weighted by the triangular sum of its own growth. Every affected read now collapses each interval to its LATEST snapshot before aggregating - keyed on `first_execution_time`, the interval identity the schema does expose - which is the same `ROW_NUMBER() ... WHERE rn = 1` shape the plan-regression analysis collectors have always used. **This corrects the numbers for data already collected**; no re-collection or migration is needed. Fixed in both SKUs: Lite's Query Store time-slice bars, top-queries grid and current-vs-baseline comparison, the Darling Viewer's equivalents plus its regressions grid and slicer overlay-on-select, the service's MCP/REST `get_query_store_top`, and the Custom Views raw route. Three things this does NOT change, each stated at the source so none of them reads as an oversight. **Bar placement still lags**: a bar is keyed on when an interval was last COLLECTED, not when it ran, which on Query Store's default 60-minute interval is reliably one bucket late - window totals are right, placement is not, and correcting it needs `first_execution_time`, which is the monitored server's LOCAL wall clock against a UTC axis. **The duration-trend chart is deliberately left un-deduped and still overstates**: deduping is right for totals and wrong for that chart, because it keeps one row per interval at the collection where the interval closed, and a 60-minute interval against a 5-minute cadence collapses twelve snapshots onto one timestamp - a 1-hour window would render a single point valued zero. Better an honestly-labelled inflated line than an empty chart, until tier 2 can place the work at the time it ran. **The Custom Views rollup is not repaired**: `query_store_stats_hourly` materialized its sums from un-deduped rows, so a panel whose window reaches past the raw tier still reads the old inflated numbers, and no read-side change can undo that - a continuous aggregate cannot contain a window function, so it needs a rebuild rather than an edit. All three are tracked on [#1841]. Two further reads are correct as they stand: the per-collection history drilldown (a raw projection with no aggregate - the "show me every snapshot" surface) and the stored-plan lookups (already latest-row-only). New round-trips execute the corrected reads against real DuckDB and real Postgres and assert the interval is counted once, each watched failing against the old queries first; the composed panel gained the first test that ever RUNS the compiler's output against Postgres.
Expand Down Expand Up @@ -2036,3 +2048,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#1859]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1859
[#1863]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1863
[#1867]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1867
[#1869]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1869
Loading
Loading