Repository navigation
Raw top-N reads sum zero-interval first-collection rows that the interval-honest rollups exclude #4394
Copy link
Copy link
Closed
Description
Claude posting for Erik Darling
Problem
The interval-honest hourly rollups exclude zero-interval rows, but raw top-N reads sum them. So the same window can rank differently depending on the tier that answers it.
- The rollups' filter: they bake
IntervalHonestSourceFilterinto their CREATE (TimescaleSupport.IntervalHonestSourceFilter,sample_interval_seconds IS DISTINCT FROM 0). Aquery_statsorprocedure_statsrow withsample_interval_seconds = 0never reaches them. That row is the first collection of a plan. - Raw's top-N read:
DarlingDataReader.TopQueriesSql, and the Viewer's twin, sumdelta_worker_time/delta_elapsed_time/delta_execution_countover every raw row in the window, zero-interval rows included. - Why that's a correctness gap: a first-collection row's "delta" can be cumulative since the plan was created, which may be days before the window. So raw top-N can over-rank a newly observed plan by an unbounded amount.
IntervalHonestSourceFilterexists for exactly this.
Measured
- Where: At "Last 7 days" the WPF Queries grids silently show ~4 days: Top Queries, Top Procedures and Query Store read raw tables dropped at 4 days, with no floor disclosure (get_query_store_top got one in #2364; the viewer and the query-stats MCP tools didn't) #4231's stage-3a live pin, on a seeded TimescaleDB rig (not a production store), comparing the same window answered from raw and from the hourly rollup.
- What differed: one query_hash, by 900,000 µs of CPU and 1 execution, which was the zero-interval row's share. Every other group matched exactly.
- That's one case, not a bound. A real plan's first row can carry days of history.
Fix
- Census first: every raw
SUM(delta_*)read overquery_stats/procedure_stats, across the grids, MCP, the web reads, compose measures and analysis. Mark each one filtered or unfiltered, with file:line. - Candidate files (a grep for
SUM(delta_…); not yet classified):Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingDataReader.csDarling/PerformanceMonitor.Darling.Viewer/ViewerDataService.QueryStats.cs,ViewerDataService.ProcedureStats.cs,ViewerDataService.FinOps.Workload.cs,ViewerDataService.QueryTrends.cs,ViewerDataService.FinOps.Storage.csDarling/PerformanceMonitor.Darling.Storage/DurationTrendRouting.csDarling/PerformanceMonitor.Darling.Service/Compose/MeasureCatalog.cs- Lite's twin reads, for parity.
- Some of these may already exclude the row. For example,
DarlingDataReader's rate math usesNULLIF(sample_interval_seconds, 0).
- Apply the filter to each unfiltered raw sum, through the one shared constant, never a copied literal.
Pins
- Raw equals hourly for the same window, on a seed that includes zero-interval rows. Both sides go through the product's tier routing.
- The PR body shows the ranking change on that seed, before and after.
Notes
- Sequencing: it follows At "Last 7 days" the WPF Queries grids silently show ~4 days: Top Queries, Top Procedures and Query Store read raw tables dropped at 4 days, with no floor disclosure (get_query_store_top got one in #2364; the viewer and the query-stats MCP tools didn't) #4231 stage 3a, because both touch
DarlingDataReader's top-N reads. Until then, At "Last 7 days" the WPF Queries grids silently show ~4 days: Top Queries, Top Procedures and Query Store read raw tables dropped at 4 days, with no floor disclosure (get_query_store_top got one in #2364; the viewer and the query-stats MCP tools didn't) #4231'sprecision_noteand PR body disclose the difference. - Priority: normal.
Metadata
Metadata
Assignees
Labels
No labels
Activity
Plain comments in the top-N routing change (#4394)
Correct #4394's comments: zero-interval rows carry zero deltas (#4394)
Correct #4394's comments: zero-interval rows carry zero deltas (#4394)
erikdarlingdata commentedon Sep 26, 2026
Claude posting for Erik Darling.
Closing: the over-count this issue describes doesn't occur in real data.
Why. The shared collector writes a zero-interval row with zero deltas.
CollectorDeltaCalculatorreturns (delta 0, interval 0) for a first sighting, a counter reset, and a gap past the delta policy. A first sighting that did real work (#2235) carries its real interval, so the interval-honest filter keeps it. The 900,000 µs row under "Measured" was planted on a test rig; the collector does not write that shape.Field count, 24 hours, two production SQL Server stores:
delta_worker_timeon themquery_statsprocedure_statsquery_statsprocedure_statsCPU totals and top-N by CPU are unaffected.
What shipped anyway. #4414 (MCP top-N) and #4417 (the Viewer's FinOps totals) apply the filter so that raw and hourly reads agree by construction. They move no CPU figure. #4417 also fixes idle-database detection: a database whose only recent samples were first sightings is no longer reported idle when its last execution falls inside the window. The remaining parts (the Viewer's query grids and Lite) are dropped.
Residual. A small share of
query_statszero-interval rows (0.002% and 0.08% above) carry a nonzero execution-count or elapsed delta with zero CPU, which breaks the collector's "interval 0 means no delta" rule. It's being traced separately; a follow-up issue will be filed if the cause is clear.erikdarlingdata commentedon Sep 26, 2026
Claude posting for Erik Darling.
Residual, traced (no change planned).
QueryStatsCollectorcomputes each counter's delta through its own cache key (query_stats_exec,query_stats_worker,query_stats_elapsed, …) and storessample_interval_secondsfrom the worker-time call alone (PerformanceMonitor.Collectors/QueryStatsCollector.cs, ~508–517). When the worker-time cache takes a no-delta path (first sighting, reset or gap) while the execution-count or elapsed cache does not, the row stores interval 0 and zero CPU beside a real execution or elapsed delta. That matches the field shape exactly: zero CPU on every such row.Effect: the interval-honest rollups skip those rows, so their execution-count and elapsed deltas are missing from the hourly and daily tiers (0.002% and 0.08% of
query_statsrows in the count above). Raw reads without the filter still count them.Why no small fix: one interval per row can't say "CPU unknown, executions known".
ProcedureStatsCollectorstores the minimum interval across its counters, but applying that here would mark more rows unknowable, not fewer. A per-counter validity flag would be the real fix, and that's out of proportion for this share of rows. Left as is.erikdarlingdata commentedon Sep 26, 2026
Claude posting for Erik Darling.
Reopening: the residual above loses real executions and duration for some rare queries (up to all of a query's day), so the collector fix goes ahead. When only the CPU counter's delta is unknowable, the row will record NULL CPU with a real interval instead of a false zero. #4414 and #4417 follow it.
Stop query_stats from recording a false zero CPU when only the CPU de…
Top-N raw reads apply the hourly rollups' zero-interval filter (#4414)
FinOps raw totals apply the zero-interval filter; idle detection coun…
erikdarlingdata commentedon Sep 26, 2026
Claude posting for Erik Darling.
#4423, #4414 and #4417 are merged.
query_statsnow records a NULL CPU with a real interval when only the CPU delta is unknowable, so those rows keep their executions and duration. The raw top-N and FinOps reads then apply the same zero-interval filter as the rollups. FinOps idle detection also counts a recent execution.erikdarlingdata commentedon Sep 26, 2026
Claude posting for Erik Darling.
Closing: the work this issue asked for has shipped, and the remaining cause is tracked in #4428.
What shipped
query_statsno longer records a false zero CPU when only the CPU delta is unknowable. It records NULL CPU over a borrowed interval instead.What the field count found