Rollup reads stop at the window end instead of adding the hour or day that starts there - #4859
Merged
Merged
Conversation
…g the bucket that starts there
erikdarlingdata
marked this pull request as ready for review
September 30, 2026 10:50
erikdarlingdata
marked this pull request as draft
September 30, 2026 10:52
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
<=.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. Withoutas_ofthe tools read up to now, so the note changes only for anas_ofexactly on the hour. The top-queries and top-procedures tools divide theircpu_attributionshare by that same span. Its denominator now covers the same hours as the ranked CPU.What changes
bucket <= endbecomesbucket < endin nine places. What each user sees:ViewerDataService.QueryStats.cs). A range older than raw retention stops before the hour that starts at To. The inline SQL moved intoBuildTopQueriesHourlySqlso a test can read it. The text is unchanged except the bound.ViewerDataService.ProcedureStats.cs). Same change,BuildTopProceduresHourlySql.DarlingDataReader.TopQueriesHourlySql). The materialization-ceiling slot stays right after the bound.DarlingDataReader.TopProceduresHourlySql). Same.ComposeCompiler). A rollup route (hourly or daily) boundsbucketwith<. A raw route keeps<=on its raw time column. A daily-tier view ending at midnight no longer sums the next day.DurationTrendRouting.BuildHourlyTrendSql, used by the viewer and the MCP trend tools). The point for the bucket that starts at the end is gone.DurationTrendRouting.BuildBucketedHourlyTrendSql). Same.QueryStoreTrendRouting.BuildRollupTrendSql). It now stops before both the window end and the raw boundary (bucket < $4is kept).DarlingTrendReader.QueryHistoryHourlySqlFor, read by the MCP query history read).The three first-bucket probes (
HourlyFirstBucketSqlandHourlyFirstBucketSingleRelationSql, inDarlingDataReader) 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_AreUnchangedpins 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 theget_daily_summarytool names. Lite has nobucket ... <=read and no_hourlyor_dailyrelation. Its query tables are raw-only.Test plan
RollupWindowEndBoundTests(new, 18 tests). For each of the nine SQL texts, the bucket's end bound isbucket < $Nfor that text's end parameter, and nobucket <=is left. For Custom Views, an hourly route and a daily route boundbucketwith<. A raw route keeps<=oncollection_time.<=. The class then failed on exactly those reads, and it passes again after restoring both:DarlingMcpTrendToolsTestsandQueryStoreTrendRoutingTestsassertedbucket <= $3and now assertbucket < $3.HourlyWindowEdgesTestspinned "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). Anas_ofof 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. Anas_ofof 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.ComposeCompilerpasses. So doTsqlConventionGuardTests,DocCommentHygieneTests,LivePostgresCollectionHygieneTestsandLiveCleanupConversionRatchetTests. That run was 620 tests, 0 failed, 31 skipped (live classes, no connection string locally).RollupWindowEndBoundLiveTestsagainst PostgreSQL with TimescaleDB. It skips locally withoutDARLING_TEST_PGand 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:CHANGELOG
SECTION: Fixed
ENTRY:
REF:
[Rollup reads stop at the window end instead of adding the hour or day that starts there #4859]: Rollup reads stop at the window end instead of adding the hour or day that starts there #4859