From 1c10e9bd60f07bccb5459f2ed54d89e489352e96 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 8 Oct 2026 02:32:35 -0400 Subject: [PATCH 1/2] Query Store interval-table gates clear raw's floor by the purge-edge margin before reading the table (#5541) --- ...RegressionIntervalTableEquivalenceTests.cs | 22 +++++++++++++++++-- .../QueryStoreIntervalWideGateTests.cs | 19 ++++++++++++++-- .../QueryStoreIntervalLatest.cs | 15 ++++++++----- .../QueryStoreIntervalWide.cs | 12 ++++++---- 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/Darling/Darling.Tests/PlanRegressionIntervalTableEquivalenceTests.cs b/Darling/Darling.Tests/PlanRegressionIntervalTableEquivalenceTests.cs index 543dc78c19..d2d1b5957e 100644 --- a/Darling/Darling.Tests/PlanRegressionIntervalTableEquivalenceTests.cs +++ b/Darling/Darling.Tests/PlanRegressionIntervalTableEquivalenceTests.cs @@ -151,8 +151,26 @@ public void UseTable_RefusesAWindowReachingBelowTheTablesFloor_WhenRawStillHolds /* A held store: raw keeps 30 days, the table's floor is 16 days back, and an anchored pass reaches 20. */ Assert.False(QueryStoreIntervalLatest.UseTable(now.AddDays(-25), false, now.AddDays(-30), rawBound, now.AddDays(-16))); - /* The same window where raw holds nothing older than the table: the table. */ - Assert.True(QueryStoreIntervalLatest.UseTable(now.AddDays(-25), false, now.AddDays(-15), rawBound, now.AddDays(-16))); + /* The same window where raw holds nothing older than the table, a full margin clear of its floor: the table. */ + Assert.True(QueryStoreIntervalLatest.UseTable(now.AddDays(-25), false, now.AddDays(-14), rawBound, now.AddDays(-16))); + } + + [Fact] + public void UseTable_RawFloorWithinTheMarginOfTheTableFloor_ReadsRaw() + { + /* #5541: raw drops a snapshot by collection_time, the table drops an interval by first_execution_time. A raw + floor at or just above the table floor can still hold a snapshot of an interval the table dropped, so the + table is only safe once raw's floor clears the table's by the purge-edge margin. */ + var now = new DateTime(2026, 9, 23, 0, 0, 0, DateTimeKind.Unspecified); + var rawBound = now.AddDays(-20); + var h = now.AddDays(-16); + var margin = QueryStoreIntervalWide.PurgeEdgeMargin; + var minute = TimeSpan.FromMinutes(1); + + Assert.False(QueryStoreIntervalLatest.UseTable(now.AddDays(-25), false, h, rawBound, h)); + Assert.False(QueryStoreIntervalLatest.UseTable(now.AddDays(-25), false, h + margin - minute, rawBound, h)); + Assert.True(QueryStoreIntervalLatest.UseTable(now.AddDays(-25), false, h + margin, rawBound, h)); + Assert.True(QueryStoreIntervalLatest.UseTable(now.AddDays(-25), false, h + margin + minute, rawBound, h)); } /* ---- the seed ---------------------------------------------------------------------------------------- */ diff --git a/Darling/Darling.Tests/QueryStoreIntervalWideGateTests.cs b/Darling/Darling.Tests/QueryStoreIntervalWideGateTests.cs index d8bcede618..9cecffb5aa 100644 --- a/Darling/Darling.Tests/QueryStoreIntervalWideGateTests.cs +++ b/Darling/Darling.Tests/QueryStoreIntervalWideGateTests.cs @@ -65,8 +65,23 @@ public void Clause3_TableFloorAtOrBeyondTheOneDayMargin_ReadsTheTable() => Assert.True(Rule(tableFloor: WindowStart.AddDays(-1))); [Fact] - public void Clause3_RawFloorAtOrAboveTableFloor_ReadsTheTable_EvenOutsideTheMargin() => - Assert.True(Rule(rawFloor: WindowStart.AddDays(-3), tableFloor: WindowStart.AddDays(-3))); + public void Clause3_RawFloorAtOrAboveTableFloorPlusMargin_ReadsTheTable_EvenOutsideTheMargin() + { + /* The table floor sits 12h under the window start, short of the one-day skew margin, so only 3a can pass. */ + var h = WindowStart.AddHours(-12); + Assert.True(Rule(rawFloor: h + QueryStoreIntervalWide.PurgeEdgeMargin, tableFloor: h)); + Assert.True(Rule(rawFloor: h + QueryStoreIntervalWide.PurgeEdgeMargin + TimeSpan.FromMinutes(1), tableFloor: h)); + } + + [Fact] + public void Clause3_RawFloorWithinTheMarginOfTheTableFloor_ReadsRaw() + { + /* #5541: raw drops a snapshot by collection_time, the table drops an interval by first_execution_time, so + raw at or just above the table floor can still hold a snapshot of an interval the table dropped. */ + var h = WindowStart.AddHours(-12); + Assert.False(Rule(rawFloor: h, tableFloor: h)); + Assert.False(Rule(rawFloor: h + QueryStoreIntervalWide.PurgeEdgeMargin - TimeSpan.FromMinutes(1), tableFloor: h)); + } [Fact] public void Clause4_OpenEnd_SkipsTheAppliedThroughComparison() => diff --git a/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs b/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs index 917534882a..c67a3eed87 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs @@ -370,10 +370,15 @@ FROM collect.query_store_interval_latest AS t /// F exists: this build has claimed coverage for the server. /// F <= max(R, B): the table holds every snapshot the raw read would read (it reads /// collection_time >= max(R, B)), and may hold more, which is the ruled window extension. - /// R >= H or B >= H: raw holds no history the table has dropped, or every interval the - /// window needs starts inside the table (B is the window minus a day, and an interval spans at most a - /// day). A NULL H means the table holds nothing for the server, and then clause 2 already says raw holds - /// nothing in the window either. + /// R >= H + or B >= H: raw holds no + /// snapshot of an interval the table has dropped, or every interval the window needs starts inside the table + /// (B is the window minus a day, and an interval spans at most a day). The margin is there because the two + /// purge on different columns (#5541): raw drops a snapshot by collection_time, the table drops an interval + /// by first_execution_time, and a snapshot can be collected up to one interval length plus the collector's + /// lag after its interval began. So with R only at or above H, raw can still hold a snapshot of an + /// interval that began below H and that the table dropped; R a full margin above H rules that + /// out (same bound as the Wide gate's below-floor edge). A NULL H means the table holds nothing for the + /// server, and then clause 2 already says raw holds nothing in the window either. /// No pending batch: a batch that stored raw but missed the table is not in the claim. /// /// A NULL R is raw with no chunk floor (a plain store, whose raw keeps 30 days): minus infinity, which can @@ -397,7 +402,7 @@ public static bool UseTable(DateTime? filledSince, bool hasPending, DateTime? ra return true; } - return (rawFloor is DateTime floor && floor >= h) || rawBound >= h; + return (rawFloor is DateTime floor && floor >= h + QueryStoreIntervalWide.PurgeEdgeMargin) || rawBound >= h; } /// diff --git a/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalWide.cs b/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalWide.cs index 59bfcdcd83..d1683438af 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalWide.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalWide.cs @@ -622,9 +622,13 @@ FROM collect.collection_log AS l /// Coverage exists ( is not null) and there is no pending batch. /// filledSince <= max(R, S) (, ): /// the table holds every snapshot raw's own read would, and may hold more (the ruled window extension). - /// R >= H or S - 1 day >= H (, - /// ): raw holds no history the table has dropped, or every interval the - /// window needs starts inside the table. A NULL means the table holds nothing + /// R >= H + or S - 1 day >= H (, + /// ): raw holds no snapshot of an interval the table has dropped, or every + /// interval the window needs starts inside the table. The margin is there because the two purge on different + /// columns (#5541): raw drops a snapshot by collection_time, the table drops an interval by + /// first_execution_time, and a snapshot lands in raw up to after its interval + /// began. With R only at or above H, raw can still hold a snapshot of an interval that began below + /// H and that the table dropped. A NULL means the table holds nothing /// for the server, and clause 2 already refuses that case. /// A literal (a custom range, MCP as_of) must be at or /// after . NULL means an open end (a preset), which skips this clause @@ -662,7 +666,7 @@ public static bool UseTable( if (tableFloor is DateTime h) { var skewFloor = windowStart - IntervalSpanMargin; - if (!((rawFloor is DateTime floor && floor >= h) || skewFloor >= h)) + if (!((rawFloor is DateTime floor && floor >= h + PurgeEdgeMargin) || skewFloor >= h)) { return false; } From 8c286ca85af33fdad3830370934a2a04baa5b07a Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 8 Oct 2026 03:00:45 -0400 Subject: [PATCH 2/2] Interval-table gate: Wide clause 3b uses the full purge-edge margin, and the table floor always comes from the per-server minimum (#5541) The Wide raw read counts a snapshot by collection_time alone, so a snapshot collected at or after the window start can belong to an interval that began down to S - 26 h. Clause 3b used S - 1 day and left a one-hour hole; it now uses S - PurgeEdgeMargin. Neither interval table is a hypertable in production (retention purges both with a sliced DELETE), and a catalog chunk floor would sit far below the real one because emptied chunks stay listed. Both rules now always read H as the per-server MIN(first_execution_time); the chunk-catalog read keeps only raw's floor. Stale one-day wording in the docs now says 26 h. --- ...ComposeQueryStoreWideExactnessLiveTests.cs | 2 +- .../PlanRegressionDailyReadShapeLiveTests.cs | 4 +- .../QueryStoreIntervalWideGateTests.cs | 31 ++++-- .../QueryStoreIntervalWideGridLiveTests.cs | 2 +- ...ueryStoreTopLiteralEndStraddleLiveTests.cs | 6 +- ...QueryStoreWideFleetEligibilityLiveTests.cs | 4 +- .../DarlingRetention.cs | 6 +- .../QueryStoreIntervalLatest.cs | 65 +++++-------- .../QueryStoreIntervalWide.cs | 96 +++++++------------ 9 files changed, 100 insertions(+), 116 deletions(-) diff --git a/Darling/Darling.Tests/ComposeQueryStoreWideExactnessLiveTests.cs b/Darling/Darling.Tests/ComposeQueryStoreWideExactnessLiveTests.cs index 2a37a0a0b6..b168c4cf18 100644 --- a/Darling/Darling.Tests/ComposeQueryStoreWideExactnessLiveTests.cs +++ b/Darling/Darling.Tests/ComposeQueryStoreWideExactnessLiveTests.cs @@ -346,7 +346,7 @@ QueryStoreCollector.Row Row( }; /* An anchor interval 45 days before windowStart (mirrors QueryStoreIntervalWideGridLiveTests' - SeedGridAsync): pushes this server's table floor comfortably past clause 3's IntervalSpanMargin + SeedGridAsync): pushes this server's table floor comfortably past clause 3's PurgeEdgeMargin check, so windowStart itself can sit exactly on the seeded span without the gate refusing on table floor alone. Outside the compared window, so it never appears in either side's rows. */ var anchor = windowStart.AddDays(-45); diff --git a/Darling/Darling.Tests/PlanRegressionDailyReadShapeLiveTests.cs b/Darling/Darling.Tests/PlanRegressionDailyReadShapeLiveTests.cs index 0f02f37f9a..80a5b06e9b 100644 --- a/Darling/Darling.Tests/PlanRegressionDailyReadShapeLiveTests.cs +++ b/Darling/Darling.Tests/PlanRegressionDailyReadShapeLiveTests.cs @@ -36,8 +36,8 @@ namespace Darling.Tests; /// server's; the test asserts what holds with that index (it does not scan the whole table, it keeps rows read under a /// bound set by the open days, and the day half reads plan_regression_daily through its unique index) and writes the /// rows read, the rows returned and the blocks touched to DARLING_5448_SHAPE_OUT when that is set, which is where the -/// numbers in the PR come from. On a hypertable (the table is converted by hand: the product does not partition it yet, -/// though QueryStoreIntervalLatest.ChunkFloorsSql already has the arm for it) the live half scans only the chunks +/// numbers in the PR come from. On a hypertable (the table is converted by hand: the product does not partition it, and the read gate +/// takes the table floor from the per-server minimum either way, #5541) the live half scans only the chunks /// at or above the live floor. /// /* #1776 own-store: deliberately NOT [Collection("live-postgres")]. Every test here reaches DARLING_TEST_PG only to diff --git a/Darling/Darling.Tests/QueryStoreIntervalWideGateTests.cs b/Darling/Darling.Tests/QueryStoreIntervalWideGateTests.cs index 9cecffb5aa..1dd21b9cf3 100644 --- a/Darling/Darling.Tests/QueryStoreIntervalWideGateTests.cs +++ b/Darling/Darling.Tests/QueryStoreIntervalWideGateTests.cs @@ -55,19 +55,38 @@ public void Clause2_FilledSinceAfterMaxOfRawFloorAndWindowStart_ReadsRaw() => Assert.False(Rule(filledSince: WindowEnd)); [Fact] - public void Clause3_TableFloorShortOfTheOneDayMargin_ReadsRaw() => - // table floor only reaches 23h before window start — short of the full one-day margin clause 3 needs — - // and raw has no floor (a plain store): the table cannot answer for the earliest part of the window. + public void Clause3_TableFloorShortOfThePurgeEdgeMargin_ReadsRaw() + { + /* #5541: the raw read counts a snapshot by collection_time alone, so a snapshot collected at or after the window + start can belong to an interval that began down to S - PurgeEdgeMargin (a 1440-minute interval collected up to + an hour plus the catch-up cap after it ended). A table floor between S - 26 h and S - 24 h used to pass the + old S - 1 day arm and then miss that interval. Raw has no floor here (a plain store), so only 3b can pass. */ + Assert.False(Rule(tableFloor: WindowStart.AddHours(-24))); + Assert.False(Rule(tableFloor: WindowStart - QueryStoreIntervalWide.PurgeEdgeMargin + TimeSpan.FromMinutes(1))); Assert.False(Rule(tableFloor: WindowStart.AddHours(-23))); + } [Fact] - public void Clause3_TableFloorAtOrBeyondTheOneDayMargin_ReadsTheTable() => - Assert.True(Rule(tableFloor: WindowStart.AddDays(-1))); + public void Clause3_ReviewFailingInput_AOneDayIntervalCollectedJustAfterTheWindowStart_ReadsRaw() + { + /* The round-1 review's pure form: default retention (raw keeps 30 days, so R is far below H and 3a fails), + H = S - 24 h, a 12 h minimum window. It returned true before the fix. */ + var s = WindowStart; + Assert.False(QueryStoreIntervalWide.UseTable( + s.AddDays(-1), false, s.AddDays(-10), s, s.AddDays(1), null, s.AddDays(1), s.AddHours(-24), TimeSpan.FromHours(12))); + } + + [Fact] + public void Clause3_TableFloorAtOrBeyondThePurgeEdgeMargin_ReadsTheTable() + { + Assert.True(Rule(tableFloor: WindowStart - QueryStoreIntervalWide.PurgeEdgeMargin)); + Assert.True(Rule(tableFloor: WindowStart - QueryStoreIntervalWide.PurgeEdgeMargin - TimeSpan.FromMinutes(1))); + } [Fact] public void Clause3_RawFloorAtOrAboveTableFloorPlusMargin_ReadsTheTable_EvenOutsideTheMargin() { - /* The table floor sits 12h under the window start, short of the one-day skew margin, so only 3a can pass. */ + /* The table floor sits 12h under the window start, short of the purge-edge margin, so only 3a can pass. */ var h = WindowStart.AddHours(-12); Assert.True(Rule(rawFloor: h + QueryStoreIntervalWide.PurgeEdgeMargin, tableFloor: h)); Assert.True(Rule(rawFloor: h + QueryStoreIntervalWide.PurgeEdgeMargin + TimeSpan.FromMinutes(1), tableFloor: h)); diff --git a/Darling/Darling.Tests/QueryStoreIntervalWideGridLiveTests.cs b/Darling/Darling.Tests/QueryStoreIntervalWideGridLiveTests.cs index 2b7adfa195..d059709a79 100644 --- a/Darling/Darling.Tests/QueryStoreIntervalWideGridLiveTests.cs +++ b/Darling/Darling.Tests/QueryStoreIntervalWideGridLiveTests.cs @@ -124,7 +124,7 @@ await ExecAsync(connection, /// /// Review D4R H1: a window under must issue ZERO /// round trips against collect.query_store_interval_wide — no ReadSourceInputsSql, no - /// ChunkFloorsSql, and above all no PlainTableFloorSql (the unindexed + /// RawChunkFloorSql, and above all no PlainTableFloorSql (the unindexed /// MIN(first_execution_time) WHERE server_id = $1 scan) — counted directly off /// pg_stat_user_tables.seq_scan/idx_scan before and after the call, since PostgreSQL's own /// catalog is the only seam that tells "no statement ran" apart from "a statement ran and returned diff --git a/Darling/Darling.Tests/QueryStoreTopLiteralEndStraddleLiveTests.cs b/Darling/Darling.Tests/QueryStoreTopLiteralEndStraddleLiveTests.cs index 41e72d733d..793e819e81 100644 --- a/Darling/Darling.Tests/QueryStoreTopLiteralEndStraddleLiveTests.cs +++ b/Darling/Darling.Tests/QueryStoreTopLiteralEndStraddleLiveTests.cs @@ -200,10 +200,10 @@ private static async Task SeedStraddleAsync(DarlingCollectorRunner runner, Cance /* Clause 3's own floor control: UseTable's tableFloor is MIN(first_execution_time) across the WHOLE server, not just this identity, and clause 3 refuses unless raw's chunk floor or WindowStart minus - IntervalSpanMargin ("skewFloor") reaches at or past it. Left with only the 7001 identity below, the - table's floor sits at day0 (WindowStart + 1h) - AFTER skewFloor (WindowStart - 1 day) - so clause 3 + PurgeEdgeMargin ("edgeFloor") reaches at or past it. Left with only the 7001 identity below, the + table's floor sits at day0 (WindowStart + 1h) - AFTER edgeFloor (WindowStart - 26 h) - so clause 3 alone refuses even the open-end positive control, before clause 4 is ever reached. A second, - unrelated identity anchored at or before skewFloor pulls the server's table floor down there, so + unrelated identity anchored at or before edgeFloor pulls the server's table floor down there, so clause 3 passes and only clause 4 (LiteralEnd vs applied_through) can still refuse the straddle read. */ var floorAnchorFirst = WindowStart.AddDays(-2); var floorAnchor = new QueryStoreCollector.Row diff --git a/Darling/Darling.Tests/QueryStoreWideFleetEligibilityLiveTests.cs b/Darling/Darling.Tests/QueryStoreWideFleetEligibilityLiveTests.cs index 31e5be41a6..309e536652 100644 --- a/Darling/Darling.Tests/QueryStoreWideFleetEligibilityLiveTests.cs +++ b/Darling/Darling.Tests/QueryStoreWideFleetEligibilityLiveTests.cs @@ -157,7 +157,7 @@ public async Task FleetCheck_ReadsTheStoreWideFloorsOnce_AndEqualsTheSerialAnswe /* Scoped to this scratch database by name, so a class running in parallel against another database is invisible to the count. */ - using var floorReads = new NpgsqlCommandCounter(scratch.DatabaseName, "table_is_hypertable", "raw_floor"); + using var floorReads = new NpgsqlCommandCounter(scratch.DatabaseName, "raw_floor"); using var controlReads = new NpgsqlCommandCounter(scratch.DatabaseName, "fleet_floor_count_control"); await using (var control = postgres.CreateCommand("SELECT 1 AS fleet_floor_count_control")) { @@ -191,7 +191,7 @@ public async Task FleetCheck_WhenTheFirstServerRefusesBeforeTheFloors_ReadsNoFlo /* A literal end before applied_through refuses at the step ahead of the floors read. */ var literalEnd = S.AddHours(13); - using var floorReads = new NpgsqlCommandCounter(scratch.DatabaseName, "table_is_hypertable", "raw_floor"); + using var floorReads = new NpgsqlCommandCounter(scratch.DatabaseName, "raw_floor"); using var controlReads = new NpgsqlCommandCounter(scratch.DatabaseName, "fleet_floor_count_control"); await using (var control = postgres.CreateCommand("SELECT 1 AS fleet_floor_count_control")) { diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingRetention.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingRetention.cs index 56b0e22123..0ed0099947 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingRetention.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingRetention.cs @@ -185,7 +185,7 @@ internal static int EffectivePurgeRetentionDays(string collectorName, int resolv /// different windows. The table keeps 9 days and deletes row by row on first_execution_time. A Query /// Store interval can span a day, so 8 days is exactly the edge for an interval that starts just past the /// horizon and closes inside a 7-day window; 9 days gives a day of margin. Reads use the table below raw's - /// chunk floor, down to L = max(window start, filled_since, table floor + 1 day), where the table floor is + /// chunk floor, down to L = max(window start, filled_since, table floor + 26 h (the purge-edge margin)), where the table floor is /// the oldest first_execution_time the purge has left. Like /// this is a named constant, not a knob, and in no collector schedule. /// @@ -803,8 +803,8 @@ would otherwise stay forever. Failure-isolated like every sibling. */ (QueryStoreIntervalWideRetentionDays) on first_execution_time — a shorter horizon than V143's 15 days. Same batched-DELETE shape, same pending-replay horizon reasoning, failure-isolated like every sibling. The purge deletes row by row on first_execution_time, so the table floor the read gate - checks (MIN(first_execution_time)) advances as it runs, and reads below raw's floor stop one day - above it (L = max(window start, filled_since, table floor + 1 day)). */ + checks (MIN(first_execution_time)) advances as it runs, and reads below raw's floor stop 26 h + above it (L = max(window start, filled_since, table floor + 26 h, the purge-edge margin)). */ var intervalWideDeleted = await PurgeOneAsync( postgres, QueryStoreIntervalWide.TableName, TimeSlicedDeleteSql("collect." + QueryStoreIntervalWide.TableName, "first_execution_time"), diff --git a/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs b/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs index c67a3eed87..30ceeb41a9 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs @@ -327,37 +327,26 @@ LEFT JOIN collect.query_store_interval_latest_coverage AS c ON c.server_id = $1;"; /// - /// The floors, from TimescaleDB's catalog (metadata, never a scan): raw's oldest chunk (NULL when raw is not a - /// hypertable, which the rule reads as "raw holds everything"), and the table's, when the table is one. A - /// chunk's range_start is at or below its oldest row, so both err toward raw. AT TIME ZONE 'UTC' - /// collapses the catalog's timestamptz to the store's naive-UTC discipline, as DarlingRetention does. + /// Raw's floor, from TimescaleDB's catalog (metadata, never a scan): its oldest chunk (NULL when raw is not a + /// hypertable, which the rule reads as "raw holds everything"). A chunk's range_start is at or below its + /// oldest row, so a low R errs toward raw. AT TIME ZONE 'UTC' collapses the catalog's + /// timestamptz to the store's naive-UTC discipline, as DarlingRetention does. The TABLE's floor is never + /// read from the catalog (#5541): neither interval table is a hypertable in production (retention purges both with + /// a sliced DELETE), and a chunk's range_start would sit far below the real floor anyway, because a DELETE + /// leaves emptied chunks listed, so a catalog H would move the rule toward the table, not toward raw. /// - public const string ChunkFloorsSql = @" + public const string RawChunkFloorSql = @" SELECT - ( - SELECT - MIN(ch.range_start) AT TIME ZONE 'UTC' - FROM timescaledb_information.chunks AS ch - WHERE ch.hypertable_schema = 'collect' - AND ch.hypertable_name = 'query_store_stats' - ) AS raw_floor, - EXISTS - ( - SELECT - 1 - FROM timescaledb_information.hypertables AS h - WHERE h.hypertable_schema = 'collect' - AND h.hypertable_name = 'query_store_interval_latest' - ) AS table_is_hypertable, - ( - SELECT - MIN(ch.range_start) AT TIME ZONE 'UTC' - FROM timescaledb_information.chunks AS ch - WHERE ch.hypertable_schema = 'collect' - AND ch.hypertable_name = 'query_store_interval_latest' - ) AS table_floor;"; + MIN(ch.range_start) AT TIME ZONE 'UTC' AS raw_floor +FROM timescaledb_information.chunks AS ch +WHERE ch.hypertable_schema = 'collect' +AND ch.hypertable_name = 'query_store_stats';"; - /// The table's floor for one server where the table is a plain heap: its oldest interval. + /// + /// The table's floor H for one server: its oldest interval. Always this read (#5541), whether or not the + /// table is ever converted to a hypertable, because it is exact: the purge deletes oldest first on + /// first_execution_time, so every deleted row is below the remaining minimum. + /// public const string PlainTableFloorSql = @" SELECT MIN(t.first_execution_time) @@ -377,7 +366,11 @@ FROM collect.query_store_interval_latest AS t /// by first_execution_time, and a snapshot can be collected up to one interval length plus the collector's /// lag after its interval began. So with R only at or above H, raw can still hold a snapshot of an /// interval that began below H and that the table dropped; R a full margin above H rules that - /// out (same bound as the Wide gate's below-floor edge). A NULL H means the table holds nothing for the + /// out (same bound as the Wide gate's below-floor edge). The B >= H arm needs no margin here, unlike the + /// Wide gate's twin: this rule's raw read also filters last_execution_time >= $2, which keeps every + /// interval it counts at first_execution_time >= B (an interval spans at most a day), so an interval + /// the table purged (first execution below H) is below B and raw never counts it either. The Wide raw + /// read filters collection_time only. A NULL H means the table holds nothing for the /// server, and then clause 2 already says raw holds nothing in the window either. /// No pending batch: a batch that stored raw but missed the table is not in the claim. /// @@ -439,21 +432,15 @@ public static async Task ReadsTableAsync( } DateTime? rawFloor = null; - DateTime? tableFloor = null; - var tableIsHypertable = false; if (hasTimescale) { - await using var floors = new NpgsqlCommand(ChunkFloorsSql, connection) { CommandTimeout = commandTimeoutSeconds }; - await using var reader = await floors.ExecuteReaderAsync(cancellationToken); - await reader.ReadAsync(cancellationToken); - rawFloor = reader.IsDBNull(0) ? null : reader.GetDateTime(0); - tableIsHypertable = reader.GetBoolean(1); - tableFloor = reader.IsDBNull(2) ? null : reader.GetDateTime(2); + await using var floors = new NpgsqlCommand(RawChunkFloorSql, connection) { CommandTimeout = commandTimeoutSeconds }; + rawFloor = await floors.ExecuteScalarAsync(cancellationToken) as DateTime?; } - if (!tableIsHypertable) + DateTime? tableFloor; + await using (var plain = new NpgsqlCommand(PlainTableFloorSql, connection) { CommandTimeout = commandTimeoutSeconds }) { - await using var plain = new NpgsqlCommand(PlainTableFloorSql, connection) { CommandTimeout = commandTimeoutSeconds }; plain.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Integer, Value = serverId }); tableFloor = await plain.ExecuteScalarAsync(cancellationToken) as DateTime?; } diff --git a/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalWide.cs b/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalWide.cs index d1683438af..e34644bf6d 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalWide.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalWide.cs @@ -409,7 +409,7 @@ RETURNING 1 by offset, before touching this. */ /* The clamp (ClampedStart) is the read's lower bound AT and ABOVE raw's chunk floor. Below that floor the table is the only record left, and ResolveReadAsync extends the bound down to - ExactBelowFloorStart = max(window start, filled_since, table floor + one day) and no further: those are the + ExactBelowFloorStart = max(window start, filled_since, table floor + PurgeEdgeMargin (26 h)) and no further: those are the three bounds under which the table provably holds what raw held before its purge. There is no per-server probe on this table: of its two secondary btrees, idx_query_store_interval_wide_first_exec leads with first_execution_time, and the wide btree on (server_id, first_execution_time) (#4952, @@ -441,38 +441,21 @@ LEFT JOIN collect.query_store_interval_wide_coverage AS c ON c.server_id = $1;"; /// - /// The floors, from TimescaleDB's catalog (metadata, never a scan), exactly as - /// : raw's oldest chunk, and this table's when - /// TimescaleDB's catalog carries it as a hypertable. It does not today — this table is engine-plain, per its - /// migration's own comment — so the table_is_hypertable arm is defensive, matching V143's. + /// Raw's floor, from TimescaleDB's catalog (metadata, never a scan), exactly as + /// : raw's oldest chunk (NULL when raw is not a + /// hypertable). This table's floor is never read from the catalog (#5541): the table is engine-plain, per its + /// migration's own comment, and retention purges it with a sliced DELETE that leaves emptied chunks listed, so a + /// catalog floor would sit far below the real one and move the rule toward the table. /// - public const string ChunkFloorsSql = @" + public const string RawChunkFloorSql = @" SELECT - ( - SELECT - MIN(ch.range_start) AT TIME ZONE 'UTC' - FROM timescaledb_information.chunks AS ch - WHERE ch.hypertable_schema = 'collect' - AND ch.hypertable_name = 'query_store_stats' - ) AS raw_floor, - EXISTS - ( - SELECT - 1 - FROM timescaledb_information.hypertables AS h - WHERE h.hypertable_schema = 'collect' - AND h.hypertable_name = 'query_store_interval_wide' - ) AS table_is_hypertable, - ( - SELECT - MIN(ch.range_start) AT TIME ZONE 'UTC' - FROM timescaledb_information.chunks AS ch - WHERE ch.hypertable_schema = 'collect' - AND ch.hypertable_name = 'query_store_interval_wide' - ) AS table_floor;"; - - /// The table's floor for one server where the table is a plain heap (today, always): its oldest - /// interval. + MIN(ch.range_start) AT TIME ZONE 'UTC' AS raw_floor +FROM timescaledb_information.chunks AS ch +WHERE ch.hypertable_schema = 'collect' +AND ch.hypertable_name = 'query_store_stats';"; + + /// The table's floor H for one server: its oldest interval. Always this read (#5541): the purge + /// deletes oldest first on first_execution_time, so every deleted row is below the remaining minimum. public const string PlainTableFloorSql = @" SELECT MIN(t.first_execution_time) @@ -524,10 +507,10 @@ AND s.interval_start_time_utc IS NULL public static readonly TimeSpan GridWideMinWindow = TimeSpan.FromHours(12); /// - /// How far below the window start clause 3 still allows the table (V143's clause 3 restated: "B is the - /// window minus a day, and an interval spans at most a day"). One Query Store runtime-stats interval can - /// span up to a day, so an interval that STARTED up to a day before the window can still have executions - /// inside it. Restated here (rather than referencing PgFactCollector.QueryPerf.PlanRegressionSkewMarginDays) + /// How long one Query Store runtime-stats interval can span (V143's clause 3 restated: "B is the window minus + /// a day, and an interval spans at most a day"), so an interval that STARTED up to a day before the window can + /// still have executions inside it. Clause 3 of no longer uses it alone (#5541): it is + /// one term of . Restated here (rather than referencing PgFactCollector.QueryPerf.PlanRegressionSkewMarginDays) /// because the viewer does not reference the service assembly (#1661 / #2530). /// public static readonly TimeSpan IntervalSpanMargin = TimeSpan.FromDays(1); @@ -622,9 +605,12 @@ FROM collect.collection_log AS l /// Coverage exists ( is not null) and there is no pending batch. /// filledSince <= max(R, S) (, ): /// the table holds every snapshot raw's own read would, and may hold more (the ruled window extension). - /// R >= H + or S - 1 day >= H (, - /// ): raw holds no snapshot of an interval the table has dropped, or every - /// interval the window needs starts inside the table. The margin is there because the two purge on different + /// R >= H + or S - >= H + /// (): raw holds no snapshot of an interval the table has dropped, or every + /// interval the raw read can count for the window starts inside the table. This read counts a snapshot by + /// collection_time alone (no last_execution_time filter, unlike the Latest rule), so a snapshot + /// collected at or after S can belong to an interval that began down to S - PurgeEdgeMargin, which + /// is why the second arm uses the whole margin and not one interval length (#5541). The margin is there because the two purge on different /// columns (#5541): raw drops a snapshot by collection_time, the table drops an interval by /// first_execution_time, and a snapshot lands in raw up to after its interval /// began. With R only at or above H, raw can still hold a snapshot of an interval that began below @@ -665,8 +651,8 @@ public static bool UseTable( if (tableFloor is DateTime h) { - var skewFloor = windowStart - IntervalSpanMargin; - if (!((rawFloor is DateTime floor && floor >= h + PurgeEdgeMargin) || skewFloor >= h)) + var edgeFloor = windowStart - PurgeEdgeMargin; + if (!((rawFloor is DateTime floor && floor >= h + PurgeEdgeMargin) || edgeFloor >= h)) { return false; } @@ -935,7 +921,7 @@ private static async Task CadenceAllowsAsync( /// /// : the coverage claim starts there and is never backdated, and a gap /// check restarts it above a hole (). - /// + : the 9-day purge deletes row by + /// + : the 9-day purge deletes row by /// row on first_execution_time, so a row whose interval began before the table floor is gone even when /// its collection_time is inside the window. /// : never read before the window asked for. @@ -991,13 +977,13 @@ public static (DateTime Start, WideStartBound Bound)? ExactBelowFloorStart( } /// - /// The inputs reads that do not depend on the server: the three floors of - /// , which takes no parameter and reads TimescaleDB's catalog for the whole store. + /// The inputs reads that do not depend on the server: the raw floor of + /// , which takes no parameter and reads TimescaleDB's catalog for the whole store. /// ('s coverage and pending columns, , /// and the cadence probe all filter on the server, so they stay per server.) /// A caller that resolves many servers for one window passes one to each call. /// - public sealed record StoreWideInputs(DateTime? RawFloor, bool TableIsHypertable, DateTime? TableFloor); + public sealed record StoreWideInputs(DateTime? RawFloor); /// /// A per-check holder for . fills it the first @@ -1055,7 +1041,7 @@ resolves its window from DateTime.UtcNow (the MCP tool) would otherwise fail eve } /* Review D4R H1: clauses 4 and 5 need no table floor at all, so check them before the two - remaining round trips (ChunkFloorsSql's metadata read and, worse, PlainTableFloorSql's + remaining round trips (RawChunkFloorSql's metadata read and, worse, PlainTableFloorSql's unindexed server-wide scan). A short window or a literal end before appliedThrough can only ever land on "raw" (UseTable's own tail), so failing here saves both queries. */ if (windowEnd - windowStart < minWindow || (literalWindowEnd is DateTime e && e < appliedThrough)) @@ -1064,30 +1050,22 @@ remaining round trips (ChunkFloorsSql's metadata read and, worse, PlainTableFloo } DateTime? rawFloor = null; - DateTime? tableFloor = null; - var tableIsHypertable = false; if (hasTimescale && storeWide?.Value is StoreWideInputs cached) { rawFloor = cached.RawFloor; - tableIsHypertable = cached.TableIsHypertable; - tableFloor = cached.TableFloor; } else if (hasTimescale) { - await using var floors = new NpgsqlCommand(ChunkFloorsSql, connection) { CommandTimeout = commandTimeoutSeconds }; - await using var reader = await floors.ExecuteReaderAsync(cancellationToken); - await reader.ReadAsync(cancellationToken); - rawFloor = reader.IsDBNull(0) ? null : reader.GetDateTime(0); - tableIsHypertable = reader.GetBoolean(1); - tableFloor = reader.IsDBNull(2) ? null : reader.GetDateTime(2); + await using var floors = new NpgsqlCommand(RawChunkFloorSql, connection) { CommandTimeout = commandTimeoutSeconds }; + rawFloor = await floors.ExecuteScalarAsync(cancellationToken) as DateTime?; if (storeWide is not null) { - storeWide.Value = new StoreWideInputs(rawFloor, tableIsHypertable, tableFloor); + storeWide.Value = new StoreWideInputs(rawFloor); } } /* Clause 2 (filledSince <= max(rawFloor, windowStart)) needs only rawFloor, already in hand from - ChunkFloorsSql's metadata read (or never set, on a non-Timescale store) — never the table floor. + RawChunkFloorSql's metadata read (or never set, on a non-Timescale store) — never the table floor. An upgraded store whose claim doesn't cover the window yet fails here, before PlainTableFloorSql's scan, instead of after it. */ if (filledSince > ClampedStart(rawFloor, windowStart)) @@ -1095,9 +1073,9 @@ remaining round trips (ChunkFloorsSql's metadata read and, worse, PlainTableFloo return new WideReadPlan(false, windowStart, windowStart, null); } - if (!tableIsHypertable) + DateTime? tableFloor; + await using (var plain = new NpgsqlCommand(PlainTableFloorSql, connection) { CommandTimeout = commandTimeoutSeconds }) { - await using var plain = new NpgsqlCommand(PlainTableFloorSql, connection) { CommandTimeout = commandTimeoutSeconds }; plain.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Integer, Value = serverId }); tableFloor = await plain.ExecuteScalarAsync(cancellationToken) as DateTime?; }