Skip to content

Raw top-N reads sum zero-interval first-collection rows that the interval-honest rollups exclude #4394

Closed

Description

@erikdarlingdata

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 IntervalHonestSourceFilter into their CREATE (TimescaleSupport.IntervalHonestSourceFilter, sample_interval_seconds IS DISTINCT FROM 0). A query_stats or procedure_stats row with sample_interval_seconds = 0 never reaches them. That row is the first collection of a plan.
  • Raw's top-N read: DarlingDataReader.TopQueriesSql, and the Viewer's twin, sum delta_worker_time / delta_elapsed_time / delta_execution_count over 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. IntervalHonestSourceFilter exists for exactly this.

Measured

Fix

  • Census first: every raw SUM(delta_*) read over query_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.cs
    • Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.QueryStats.cs, ViewerDataService.ProcedureStats.cs, ViewerDataService.FinOps.Workload.cs, ViewerDataService.QueryTrends.cs, ViewerDataService.FinOps.Storage.cs
    • Darling/PerformanceMonitor.Darling.Storage/DurationTrendRouting.cs
    • Darling/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 uses NULLIF(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

Activity

  1. added 3 commits that reference this issue on Sep 26, 2026
  2. erikdarlingdata commented on Sep 26, 2026

    @erikdarlingdata
    OwnerAuthor

    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. CollectorDeltaCalculator returns (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:

    Store Table Zero-interval rows With any nonzero delta Σ delta_worker_time on them
    A query_stats 42,361 183 0
    A procedure_stats 21,281 0 0
    B query_stats 206,968 8,853 0
    B procedure_stats 16,122 0 0

    CPU 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_stats zero-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.

  3. erikdarlingdata commented on Sep 26, 2026

    @erikdarlingdata
    OwnerAuthor

    Claude posting for Erik Darling.

    Residual, traced (no change planned). QueryStatsCollector computes each counter's delta through its own cache key (query_stats_exec, query_stats_worker, query_stats_elapsed, …) and stores sample_interval_seconds from 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_stats rows 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". ProcedureStatsCollector stores 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.

  4. erikdarlingdata commented on Sep 26, 2026

    @erikdarlingdata
    OwnerAuthor

    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.

  5. erikdarlingdata commented on Sep 26, 2026

    @erikdarlingdata
    OwnerAuthor

    Claude posting for Erik Darling.

    #4423, #4414 and #4417 are merged. query_stats now 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.

  6. erikdarlingdata commented on Sep 26, 2026

    @erikdarlingdata
    OwnerAuthor

    Claude posting for Erik Darling.

    Closing: the work this issue asked for has shipped, and the remaining cause is tracked in #4428.

    What shipped

    What the field count found

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions