Skip to content

Rollup reads stop at the window end instead of adding the hour or day that starts there - #4859

Merged
erikdarlingdata merged 3 commits into
devfrom
fix/rollup-reads-exclusive-window-end
Sep 30, 2026
Merged

erikdarlingdata merged 3 commits into
devfrom
fix/rollup-reads-exclusive-window-end

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 30, 2026 •

Copy link
Copy Markdown
Owner

Why

A rollup bucket is stamped at its start. Nine reads over the hourly rollups (and, for Custom Views, the daily one) bounded the window end with bucket <= end. When the end falls exactly on a bucket start, the read also took the whole bucket that begins there, which lies after the window. One example: a Queries tab range older than raw retention whose To was 14:00 summed the 14:00-15:00 hour as well. Another: a Custom View on the daily tier ending at midnight summed the next day.

Behavior change

  • Only a window end that falls exactly on a bucket start changes anything. That means on the hour for the hourly reads and at midnight for the daily tier. Those totals and points no longer include the bucket that starts at the end.
  • A window end inside a bucket reads exactly as before. That bucket is still counted whole.
  • Raw reads are unchanged. A raw sample is stamped when it was taken, so a sample stamped exactly at the end still belongs to the window and keeps <=.
  • The served-span note that describes the hourly edges (HourlyWindowEdges) followed the old bound. For an end exactly on the hour it now says the span stops at the end and claims nothing counted past it. Without as_of the tools read up to now, so the note changes only for an as_of exactly on the hour. The top-queries and top-procedures tools divide their cpu_attribution share by that same span. Its denominator now covers the same hours as the ranked CPU.

What changes

bucket <= end becomes bucket < end in nine places. What each user sees:

  1. Queries tab, hourly tier (ViewerDataService.QueryStats.cs). A range older than raw retention stops before the hour that starts at To. The inline SQL moved into BuildTopQueriesHourlySql so a test can read it. The text is unchanged except the bound.
  2. Procedures tab, hourly tier (ViewerDataService.ProcedureStats.cs). Same change, BuildTopProceduresHourlySql.
  3. MCP top queries by CPU, hourly tier (DarlingDataReader.TopQueriesHourlySql). The materialization-ceiling slot stays right after the bound.
  4. MCP top procedures by CPU, hourly tier (DarlingDataReader.TopProceduresHourlySql). Same.
  5. Custom Views (ComposeCompiler). A rollup route (hourly or daily) bounds bucket with <. A raw route keeps <= on its raw time column. A daily-tier view ending at midnight no longer sums the next day.
  6. Hourly duration trend, query and procedure (DurationTrendRouting.BuildHourlyTrendSql, used by the viewer and the MCP trend tools). The point for the bucket that starts at the end is gone.
  7. Bucketed hourly duration trend (DurationTrendRouting.BuildBucketedHourlyTrendSql). Same.
  8. Query Store trend, rollup arm (QueryStoreTrendRouting.BuildRollupTrendSql). It now stops before both the window end and the raw boundary (bucket < $4 is kept).
  9. One query's hourly history (DarlingTrendReader.QueryHistoryHourlySqlFor, read by the MCP query history read).

The three first-bucket probes (HourlyFirstBucketSql and HourlyFirstBucketSingleRelationSql, in DarlingDataReader) are unchanged on purpose. They only locate the first bucket the window holds, to report where the served span starts. A bucket at the very end can be that first bucket only when the window holds no earlier one. In that case the read now returns no rows. RollupWindowEndBoundTests.FirstBucketProbes_AreUnchanged pins this.

Doc comments next to the edited bounds now say the end is exclusive on the rollup tiers.

Lite

Lite: checked, none. I ran git grep -nE '_hourly|_daily|rollup|bucket[a-z_]*[[:space:]]*<=' -- 'Lite/*.cs'. It returned 30 lines, all of them the word "rollup" in comments or the get_daily_summary tool names. Lite has no bucket ... <= read and no _hourly or _daily relation. Its query tables are raw-only.

Test plan

  • RollupWindowEndBoundTests (new, 18 tests). For each of the nine SQL texts, the bucket's end bound is bucket < $N for that text's end parameter, and no bucket <= is left. For Custom Views, an hourly route and a daily route bound bucket with <. A raw route keeps <= on collection_time.
  • RED. I put path 3's bound and path 5's rollup bound back to <=. The class then failed on exactly those reads, and it passes again after restoring both:
Darling.Tests.RollupWindowEndBoundTests.McpTopQueriesHourlySql_StopsBeforeTheWindowEnd_AndKeepsTheCeilingSlot [FAIL]
Darling.Tests.RollupWindowEndBoundTests.ComposeHourlyRoute_StopsBeforeTheWindowEnd [FAIL]
Darling.Tests.RollupWindowEndBoundTests.ComposeDailyRoute_StopsBeforeTheWindowEnd [FAIL]
Darling.Tests  Total: 18, Errors: 0, Failed: 3, Skipped: 0, Not Run: 0
  • Existing pins updated on purpose. DarlingMcpTrendToolsTests and QueryStoreTrendRoutingTests asserted bucket <= $3 and now assert bucket < $3. HourlyWindowEdgesTests pinned "an end exactly on the hour counts the whole next hour". That case now asserts the span stops at the end. The unaligned end keeps its old assertion. ViewerTrendRoutingPortTests (RoutedQueryDurationTrend_ReadsTheRollupPastTheRawHorizon_WithTheDatabaseFilter) counted 7 x 24 + 1 hourly points for a window that ends at the top of the hour. Its seed stamps a row at that instant. The extra point was the bucket that starts at the window end, which lies after the window, so the test now counts 7 x 24.
  • HourlyAttributionSpanTests.Hourly_OnTheHourAsOf_DividesByExactlyTheHoursBeforeIt (new). An as_of of 14:00 with the first bucket at 10:00 gives a span of exactly 4 hours. At 25% of 8 cores that is 28,800 CPU-seconds, and 21,600 ranked seconds is a share of 0.75. An as_of of 14:20 still gives a span that ends at 15:00. With the served span put back to an hour past an on-the-hour end, it fails: Expected 2026-01-05T14:00:00Z, Actual 15:00:00Z.
  • Lite.Tests and Darling.Tests build with 0 warnings.
  • Every Darling.Tests class that names one of the nine members or ComposeCompiler passes. So do TsqlConventionGuardTests, DocCommentHygieneTests, LivePostgresCollectionHygieneTests and LiveCleanupConversionRatchetTests. That run was 620 tests, 0 failed, 31 skipped (live classes, no connection string locally).
  • Full Darling.Tests suite, once, without a live connection string:
Darling.Tests  Total: 18890, Errors: 0, Failed: 0, Skipped: 1190, Not Run: 1, Time: 91.102s
  • RollupWindowEndBoundLiveTests against PostgreSQL with TimescaleDB. It skips locally without DARLING_TEST_PG and runs in CI. It passed there at 51a9007, and all three PostgreSQL test shards had 0 failures. It seeds one query and one procedure in three hours of an aged window. H0 has 1 execution, H1 has 10 and H2 has 100, each stamped 10 minutes past its hour. It refreshes the hourly rollups and reads with three ends: exactly H2 (11), H2 plus 25 minutes (111) and H1 plus 30 minutes (11). The live pins cover:
    • Paths 3 and 4 (the MCP reads) and paths 1 and 2 (the viewer reads). Each runs on the raw tier first, then on the hourly tier after the raw rows are purged, with the tier asserted. The raw total for the window ending at H2 equals the hourly total for it.
    • Paths 6, 7 and 9, run over the seeded hourly relations. The ends give the H0 and H1 points, and the H2 point only when the end is inside H2.
    • Path 5 on the hourly route, as a Custom View sum over the same rows.
  • Text-level pins only, not seeded live. Path 8 is one, because the corrected Query Store rollup needs interval-level Query Store rows. Path 5 on the daily route is the other, where the compiler-level pin above stands in for a daily refresh.

CHANGELOG

SECTION: Fixed
ENTRY:

@erikdarlingdata
erikdarlingdata marked this pull request as ready for review September 30, 2026 10:50
@erikdarlingdata
erikdarlingdata marked this pull request as draft September 30, 2026 10:52
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review September 30, 2026 11:11
@erikdarlingdata
erikdarlingdata merged commit f12072c into dev Sep 30, 2026
19 of 20 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/rollup-reads-exclusive-window-end branch September 30, 2026 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant