From 84715d96697c7ba120ee0a8da96162182d006456 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 8 Oct 2026 00:12:48 -0400 Subject: [PATCH] Plan regression reads the table for the part of the window it covers and raw rows only for the rest (#5513) QueryStoreIntervalLatest.UseTable is all or nothing, so a coverage claim that restarted a day or two ago, or a table whose purge had just moved its floor above the window bound, sent the whole 15-day slice through the raw dedup. A third answer, Mixed, groups the raw snapshots the table does not stand in for together with the table's own rows under the raw read's own order, so it returns the raw read's rows. The drill-down is told "raw" for it and reads the raw SQL it always read. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_015ZGNj3cTaf9E9jYjdAa9vA --- Darling/Darling.Tests/PgFactCollectorTests.cs | 13 +- .../PlanRegressionDailyBoundaryTests.cs | 2 +- .../PlanRegressionMixedSourceTests.cs | 421 ++++++++++++++++++ .../PgFactCollector.QueryPerf.cs | 133 +++++- .../PgFactCollector.cs | 1 + .../QueryStoreIntervalLatest.cs | 127 +++++- 6 files changed, 678 insertions(+), 19 deletions(-) create mode 100644 Darling/Darling.Tests/PlanRegressionMixedSourceTests.cs diff --git a/Darling/Darling.Tests/PgFactCollectorTests.cs b/Darling/Darling.Tests/PgFactCollectorTests.cs index 8c3fa0d4a7..fefabc1731 100644 --- a/Darling/Darling.Tests/PgFactCollectorTests.cs +++ b/Darling/Darling.Tests/PgFactCollectorTests.cs @@ -111,10 +111,11 @@ public void AllSql_CoversEveryQuery_OnePerCollectMethodPlusTheDmvFallback() { /* 32 collect methods (31 fact readers plus the #3538 coverage witness), one query each, plus the DMV-snapshot fallback the blocking-chain method appends through PgBlockingPairRowQuery, plus - PLAN_REGRESSION's #3953 table twin and #5448 daily-totals twin: the plan-regression method runs one of - three reads per server, and Lite has no store for the second or the third (its DuckDB keeps no - latest-snapshot interval table). */ - Assert.Equal(LiteCollectMethodSurface.Length + 3, PgFactCollector.AllSql.Count); + PLAN_REGRESSION's #3953 table twin, #5448 daily-totals twin and #5513 mixed twin: the plan-regression + method runs one of four reads per server, and Lite has no store for the second, third or fourth (its + DuckDB keeps no latest-snapshot interval table). */ + Assert.Equal(LiteCollectMethodSurface.Length + 4, PgFactCollector.AllSql.Count); + Assert.Contains(PgFactCollector.PlanRegressionMixedSql, PgFactCollector.AllSql); Assert.Contains(PgFactCollector.PlanRegressionTableSql, PgFactCollector.AllSql); Assert.Contains(PgFactCollector.PlanRegressionDailySql, PgFactCollector.AllSql); Assert.Contains(PgBlockingPairRowQuery.DmvSnapshotSql, PgFactCollector.AllSql); @@ -254,11 +255,11 @@ It is deliberate in the plan-regression aggregation and nowhere else. */ { if (sql.Contains("any_value", StringComparison.OrdinalIgnoreCase)) { - /* The three PLAN_REGRESSION reads (#3953: raw, and its interval-table twin; #5448: the daily-totals twin), + /* The four PLAN_REGRESSION reads (#3953: raw, and its interval-table twin; #5448: the daily-totals twin; #5513: the mixed twin), and nothing else. */ Assert.True( sql == PgFactCollector.PlanRegressionSql || sql == PgFactCollector.PlanRegressionTableSql - || sql == PgFactCollector.PlanRegressionDailySql, + || sql == PgFactCollector.PlanRegressionDailySql || sql == PgFactCollector.PlanRegressionMixedSql, "any_value() outside the PLAN_REGRESSION reads:\n" + sql); } } diff --git a/Darling/Darling.Tests/PlanRegressionDailyBoundaryTests.cs b/Darling/Darling.Tests/PlanRegressionDailyBoundaryTests.cs index 939d160207..7722e78a2e 100644 --- a/Darling/Darling.Tests/PlanRegressionDailyBoundaryTests.cs +++ b/Darling/Darling.Tests/PlanRegressionDailyBoundaryTests.cs @@ -106,7 +106,7 @@ public void TheFact_RunsBothReadsInOneRepeatableReadReadOnlyTransaction_WithTheL Assert.Contains("BeginTransactionAsync(System.Data.IsolationLevel.RepeatableRead", fact, StringComparison.Ordinal); Assert.Contains("SET TRANSACTION READ ONLY", fact, StringComparison.Ordinal); Assert.Contains("connection, snapshot, context.ServerId, windowFloor", fact, StringComparison.Ordinal); - Assert.Contains("readsDays ? PlanRegressionDailySql : readsTable ? PlanRegressionTableSql : PlanRegressionSql, connection, snapshot)", fact, StringComparison.Ordinal); + Assert.Contains("readsDays ? PlanRegressionDailySql : readsTable ? PlanRegressionTableSql : readsMixed ? PlanRegressionMixedSql : PlanRegressionSql, connection, snapshot)", fact, StringComparison.Ordinal); var sql = PgFactCollector.PlanRegressionDailySql; Assert.Contains("l.first_execution_time >= $4::timestamp", sql, StringComparison.Ordinal); diff --git a/Darling/Darling.Tests/PlanRegressionMixedSourceTests.cs b/Darling/Darling.Tests/PlanRegressionMixedSourceTests.cs new file mode 100644 index 0000000000..265cdd8c1d --- /dev/null +++ b/Darling/Darling.Tests/PlanRegressionMixedSourceTests.cs @@ -0,0 +1,421 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Npgsql; +using NpgsqlTypes; +using PerformanceMonitor.Analysis; +using PerformanceMonitor.Collectors; +using PerformanceMonitor.Darling.Analysis; +using PerformanceMonitor.Darling.Service; +using PerformanceMonitor.Darling.Storage; +using Xunit; + +namespace Darling.Tests; + +/// +/// #5513: PLAN_REGRESSION fell back to the raw query_store_stats read on 46% of a 43-server store's passes, because +/// is all or nothing: a coverage claim that restarted a day ago, or a table +/// whose purge had just moved its floor above the window's bound, sent the whole 15-day slice through the raw dedup, for a +/// window the table held all but a day or two of. The third answer, , +/// reads the table where its claim holds and raw rows for the rest, grouped together. +/// +/// The merge gate is the same one #3953 had: the mixed read returns the raw read's rows, column for column, at every edge +/// of the claim, including an interval that is open on one side of the edge and closed on the other, and a table whose +/// oldest intervals were purged while raw still holds them (where the table alone reads fewer intervals than raw, so the +/// comparison is not vacuous). +/// +/* #1776 own-store: deliberately NOT [Collection("live-postgres")]. Every live test here reaches DARLING_TEST_PG only to + CREATE and DROP its own database through ScratchPostgres and then works entirely inside it. */ +public sealed class PlanRegressionMixedSourceTests +{ + private const int ServerId = -5513001; + + /// The analysis window's start: the 14-day comparison window reaches back to T0 - 4 days. + private static readonly DateTime TimeRangeStart = new(2026, 9, 11, 0, 0, 0, DateTimeKind.Unspecified); + + private static readonly DateTime T0 = TimeRangeStart.AddDays(-10); + + private static string? BaseConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + + /* ---- the pure rule ----------------------------------------------------------------------------------- */ + + [Fact] + public void DecideSource_IsMixed_WhereTheOldRuleSaidRaw_AndTheTableCoversPartOfTheWindow() + { + var now = new DateTime(2026, 10, 7, 0, 0, 0, DateTimeKind.Unspecified); + var rawBound = now.AddDays(-15); + + /* A timescale store: raw keeps about 4 days, the table keeps 15. A claim that restarted 2 days ago is above raw's + floor, so the table alone is refused (the old rule: raw, 4 days of dedup). The table covers the newest 2 days and + raw is read for the 2 before. */ + var rawFloor = now.AddDays(-4); + var tableFloor = now.AddDays(-15).AddHours(2); + var filledSince = now.AddDays(-2); + Assert.False(QueryStoreIntervalLatest.UseTable(filledSince, hasPending: false, rawFloor, rawBound, tableFloor)); + var source = QueryStoreIntervalLatest.DecideSource(filledSince, hasPending: false, rawFloor, rawBound, tableFloor); + Assert.Equal(QueryStoreIntervalLatest.PlanRegressionReadKind.Mixed, source.Kind); + Assert.Equal(filledSince, source.CoveredFrom); + + /* The claim is older than raw's floor: nothing is uncovered, so the table alone (unchanged). */ + Assert.Equal( + QueryStoreIntervalLatest.PlanRegressionReadKind.Table, + QueryStoreIntervalLatest.DecideSource(now.AddDays(-6), hasPending: false, rawFloor, rawBound, tableFloor).Kind); + } + + [Fact] + public void DecideSource_TheTableNeverReachesBelowRawsFloor_SoTheAnswerStaysRaws() + { + var now = new DateTime(2026, 10, 7, 0, 0, 0, DateTimeKind.Unspecified); + var rawBound = now.AddDays(-15); + + /* A plain store, no raw floor: the claim starts 3 days ago, so the table covers from there. */ + var plain = QueryStoreIntervalLatest.DecideSource(now.AddDays(-3), false, rawFloor: null, rawBound, now.AddDays(-15).AddHours(3)); + Assert.Equal(QueryStoreIntervalLatest.PlanRegressionReadKind.Mixed, plain.Kind); + Assert.Equal(now.AddDays(-3), plain.CoveredFrom); + + /* A claim older than the bound, but a table whose purge has moved its floor above the bound while raw still holds + everything: the old rule refused the table (raw holds history it dropped). The table now speaks only for + intervals at or above its floor; raw is read for the older ones, in a span bounded above. */ + var tableFloor = now.AddDays(-15).AddHours(3); + Assert.False(QueryStoreIntervalLatest.UseTable(now.AddDays(-30), false, rawFloor: null, rawBound, tableFloor)); + var purged = QueryStoreIntervalLatest.DecideSource(now.AddDays(-30), false, rawFloor: null, rawBound, tableFloor); + Assert.Equal(QueryStoreIntervalLatest.PlanRegressionReadKind.Mixed, purged.Kind); + Assert.Equal(rawBound, purged.CoveredFrom); + Assert.Equal(tableFloor, purged.FirstExecutionFloor); + Assert.Equal(tableFloor.AddDays(2), purged.RawUpperBound); + } + + [Theory] + [InlineData(null, false, true)] // no coverage row: nothing is covered + [InlineData(-3.0, true, true)] // pending batches: a batch the table missed is inside the claim + [InlineData(-3.0, false, false)] // an empty table for the server + public void DecideSource_StaysRaw_WhenNothingIsCovered(double? filledSinceDays, bool hasPending, bool tableHasRows) + { + var now = new DateTime(2026, 10, 7, 0, 0, 0, DateTimeKind.Unspecified); + DateTime? filledSince = filledSinceDays is double f ? now.AddDays(f) : null; + DateTime? tableFloor = tableHasRows ? now.AddDays(-14) : null; + + Assert.Equal( + QueryStoreIntervalLatest.PlanRegressionReadKind.Raw, + QueryStoreIntervalLatest.DecideSource(filledSince, hasPending, rawFloor: now.AddDays(-4), now.AddDays(-15), tableFloor).Kind); + } + + /* ---- the SQL ----------------------------------------------------------------------------------------- */ + + [Fact] + public void TheMixedSql_UsesTheRawReadsOrderAndKeepsBareBoundsOnThePartitioningColumn() + { + var sql = PgFactCollector.PlanRegressionMixedSql; + + /* The same seven aggregates, in the same order, as the raw read's deduped CTE: the table row of an interval is its + newest snapshot and wins the group on collection_time. */ + var orderBy = "ORDER BY collection_time DESC, execution_count DESC))[1]"; + Assert.Equal(7, CountOf(sql, orderBy)); + Assert.Equal(7, CountOf(PgFactCollector.PlanRegressionSql, orderBy)); + + /* Bare parameters on collection_time (chunk exclusion), both bounds, and the two sides of the edge. */ + Assert.Contains("AND collection_time >= $3", sql, StringComparison.Ordinal); + Assert.Contains("AND collection_time < $6", sql, StringComparison.Ordinal); + Assert.Contains("(collection_time < $4 OR first_execution_time IS NULL OR first_execution_time < $5)", sql, StringComparison.Ordinal); + Assert.Contains("AND collection_time >= $4", sql, StringComparison.Ordinal); + Assert.Contains("AND first_execution_time >= $5", sql, StringComparison.Ordinal); + + /* The raw side keeps the raw read's own filters; the plan_agg and the suffix are the shared ones. */ + Assert.Contains("execution_type_desc = 'Regular'", sql, StringComparison.Ordinal); + Assert.EndsWith(PgFactCollectorSuffixTail, sql, StringComparison.Ordinal); + Assert.EndsWith(PgFactCollectorSuffixTail, PgFactCollector.PlanRegressionSql, StringComparison.Ordinal); + } + + private const string PgFactCollectorSuffixTail = "LIMIT 20"; + + private static int CountOf(string text, string needle) + { + var count = 0; + for (var i = text.IndexOf(needle, StringComparison.Ordinal); i >= 0; i = text.IndexOf(needle, i + needle.Length, StringComparison.Ordinal)) + { + count++; + } + + return count; + } + + /* ---- live: the mixed read is the raw read's answer ---------------------------------------------------- */ + + [Fact] + public async Task TheMixedRead_ReturnsTheRawReadsRows_AtEveryEdgeOfTheClaim_AndWhereTheTablesOldIntervalsWerePurged() + { + var baseCs = BaseConnectionString; + Assert.SkipWhen(string.IsNullOrEmpty(baseCs), "Set DARLING_TEST_PG to a Postgres connection string to run the #5513 parity test."); + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(baseCs!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + await using var postgres = NpgsqlDataSource.Create(scratch.ConnectionString); + + await SeedAsync(new DarlingCollectorRunner(postgres, new CollectorDeltaCalculator()), ct); + + var raw = await FactRowsAsync(connection, PgFactCollector.PlanRegressionSql, null, ct); + Assert.True(raw.Count >= 2, $"the seed produced {raw.Count} PLAN_REGRESSION row(s); the comparison needs real regressions"); + + var floor = (await ScalarAsync(connection, "SELECT MIN(first_execution_time) FROM collect.query_store_interval_latest WHERE server_id = " + ServerId, ct)) as DateTime?; + Assert.NotNull(floor); + var bound = TimeRangeStart.AddDays(-15); + + /* Below every snapshot (the table alone), one inside each interval that is open on one side and closed on the + other (the open snapshot is 30 minutes in, the closed one 65), the middle, and past the newest snapshot (all raw). */ + var edges = new[] + { + T0.AddDays(-1), + T0.AddDays(2).AddHours(12).AddMinutes(45), + T0.AddDays(5), + T0.AddDays(9).AddHours(12).AddMinutes(45), + T0.AddDays(20), + }; + foreach (var coveredFrom in edges) + { + var mixed = await FactRowsAsync(connection, PgFactCollector.PlanRegressionMixedSql, Mixed(coveredFrom, floor!.Value), ct); + Assert.Equal(raw, mixed); + } + + /* The table's purge ran: the intervals first seen before day 3 are gone from it, raw still has them. The table alone + now reads fewer intervals than raw (the old rule's reason to refuse it); the mixed read still returns raw's. */ + await ExecAsync(connection, "DELETE FROM collect.query_store_interval_latest WHERE server_id = " + ServerId + " AND first_execution_time < @cutoff", T0.AddDays(3), ct); + var purgedFloor = (await ScalarAsync(connection, "SELECT MIN(first_execution_time) FROM collect.query_store_interval_latest WHERE server_id = " + ServerId, ct)) as DateTime?; + Assert.NotNull(purgedFloor); + Assert.True(purgedFloor > floor); + + var tableAlone = await FactRowsAsync(connection, PgFactCollector.PlanRegressionTableSql, null, ct); + Assert.NotEqual(raw, tableAlone); + foreach (var coveredFrom in edges) + { + var mixed = await FactRowsAsync(connection, PgFactCollector.PlanRegressionMixedSql, Mixed(coveredFrom, purgedFloor!.Value), ct); + Assert.Equal(raw, mixed); + } + + /* The decision lands on these edges by itself: a claim that restarted at day 5, a plain store. */ + await ExecAsync(connection, "UPDATE collect.query_store_interval_latest_coverage SET filled_since = @cutoff WHERE server_id = " + ServerId, T0.AddDays(5), ct); + var source = await QueryStoreIntervalLatest.ResolveReadSourceAsync(connection, ServerId, bound, 60, null, ct); + Assert.Equal(QueryStoreIntervalLatest.PlanRegressionReadKind.Mixed, source.Kind); + Assert.Equal(T0.AddDays(5), source.CoveredFrom); + Assert.Equal(raw, await FactRowsAsync(connection, PgFactCollector.PlanRegressionMixedSql, source, ct)); + } + + [Fact] + public async Task ThePass_ReadsTheMixedSource_WhenTheClaimRestartedMidWindow_AndReportsTheRawReadsFact() + { + var baseCs = BaseConnectionString; + Assert.SkipWhen(string.IsNullOrEmpty(baseCs), "Set DARLING_TEST_PG to a Postgres connection string to run the #5513 pass test."); + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(baseCs!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + await using var seedSource = NpgsqlDataSource.Create(scratch.ConnectionString); + await SeedAsync(new DarlingCollectorRunner(seedSource, new CollectorDeltaCalculator()), ct); + + await using var postgres = NpgsqlDataSource.Create( + new NpgsqlConnectionStringBuilder(scratch.ConnectionString) { SearchPath = "collect,config,public" }.ConnectionString); + + /* A coverage claim that restarted at day 5 (a gap or a purged pending batch did it): above the window's bound, so the + table alone is refused, as the old rule did, and the pass used to read raw for all of it. */ + await ExecAsync(connection, "UPDATE collect.query_store_interval_latest_coverage SET filled_since = @cutoff WHERE server_id = " + ServerId, T0.AddDays(5), ct); + + var mixedLog = new CapturingLogger(); + var mixedContext = NewContext(); + var mixedFact = (await new PgFactCollector(postgres, mixedLog).CollectFactsAsync(mixedContext)).Single(f => f.Key == "PLAN_REGRESSION"); + Assert.Contains(mixedLog.Lines, l => l.Contains("PLAN_REGRESSION source for server", StringComparison.Ordinal) && l.Contains("Mixed", StringComparison.Ordinal)); + Assert.False(mixedContext.PlanRegressionReadsIntervalTable, "the drill-down reads the raw SQL for the mixed source, which has the same rows"); + Assert.DoesNotContain(mixedContext.CollectionFailures, f => f.Family == "plan_regression"); + + /* A pending batch forces the raw read. The two passes must report the same fact. */ + await ExecAsync(connection, + "INSERT INTO collect.query_store_interval_latest_pending (server_id, collection_time, database_name, recorded_at) VALUES (" + ServerId + ", @cutoff, 'qsA', @cutoff)", + T0, ct); + var rawLog = new CapturingLogger(); + var rawContext = NewContext(); + var rawFact = (await new PgFactCollector(postgres, rawLog).CollectFactsAsync(rawContext)).Single(f => f.Key == "PLAN_REGRESSION"); + Assert.DoesNotContain(rawLog.Lines, l => l.Contains("Mixed", StringComparison.Ordinal)); + Assert.Contains(rawLog.Lines, l => l.Contains("pending batches", StringComparison.Ordinal)); + + Assert.Equal(rawFact.Value, mixedFact.Value); + Assert.Equal(rawFact.DatabaseName, mixedFact.DatabaseName); + Assert.Equal(rawFact.Metadata.OrderBy(kv => kv.Key).ToList(), mixedFact.Metadata.OrderBy(kv => kv.Key).ToList()); + Assert.Equal( + rawContext.PlanRegressionOffenders?.Select(o => (o.DatabaseName, o.QueryId)).ToList(), + mixedContext.PlanRegressionOffenders?.Select(o => (o.DatabaseName, o.QueryId)).ToList()); + } + + /* ---- helpers ----------------------------------------------------------------------------------------- */ + + private static QueryStoreIntervalLatest.PlanRegressionSource Mixed(DateTime coveredFrom, DateTime firstExecutionFloor) + { + var spanEnd = firstExecutionFloor.AddDays(2); + return new QueryStoreIntervalLatest.PlanRegressionSource( + QueryStoreIntervalLatest.PlanRegressionReadKind.Mixed, coveredFrom, firstExecutionFloor, spanEnd > coveredFrom ? spanEnd : coveredFrom); + } + + private static AnalysisContext NewContext() => new() + { + ServerId = ServerId, + ServerName = "qsil-mixed-host", + TimeRangeStart = TimeRangeStart, + TimeRangeEnd = TimeRangeStart.AddHours(4), + ServerUtcOffset = TimeSpan.Zero, + }; + + private static QueryStoreCollector.Row Row( + string database, long queryId, long planId, long intervalId, DateTime first, DateTime last, long executions, + long cpuUs, string? role = null) => new() + { + DatabaseName = database, + QueryId = queryId, + PlanId = planId, + ExecutionTypeDesc = "Regular", + FirstExecutionTime = first, + LastExecutionTime = last, + QueryHash = "0xQ" + queryId.ToString(CultureInfo.InvariantCulture), + QueryPlanHash = "0xP" + planId.ToString(CultureInfo.InvariantCulture), + ExecutionCount = executions, + AvgCpuTimeUs = cpuUs, + AvgDurationUs = cpuUs * 3, + IsForcedPlan = false, + ForceFailureCount = 0, + ReplicaRole = role, + RuntimeStatsIntervalId = intervalId, + }; + + /// + /// Ten daily intervals, each shipped as an open snapshot (30 minutes in) and then its closed one (65 minutes in), through + /// the real write path so the table holds exactly what the writer keeps. Query 100 regresses on the NULL role (cheap plan + /// days 1-3, expensive days 8-9); query 200 on a named role; query 300 has one plan. + /// + private static async Task SeedAsync(DarlingCollectorRunner runner, CancellationToken ct) + { + var context = new CollectorContext + { + ServerId = ServerId, + ServerName = "qsil-mixed-host", + CollectionTime = DateTime.UtcNow, + Deltas = new CollectorDeltaCalculator(), + }; + var server = new ServerRuntime + { + Config = new MonitoredServer { Name = "qsil-mixed", Host = "qsil-mixed-host" }, + ConnectionString = "Server=qsil-mixed-host", + Target = new CollectorTargetInfo { SqlMajorVersion = 16 }, + StorageName = "qsil-mixed-host", + ServerId = ServerId, + EngineEdition = 3, + }; + + var interval = 1000L; + for (var day = 0; day < 10; day++) + { + var start = T0.AddDays(day).AddHours(12); + interval++; + + var rows = new List<(double Fraction, QueryStoreCollector.Row Row)>(); + void Both(string db, long query, long plan, long execs, long cpu, string? role = null) + { + rows.Add((0.5, Row(db, query, plan, interval, start, start.AddMinutes(25), execs / 2, cpu, role))); + rows.Add((1.0, Row(db, query, plan, interval, start, start.AddMinutes(55), execs, cpu, role))); + } + + if (day is >= 1 and <= 3) Both("qsA", 100, 1001, 300, 100); + if (day is 8 or 9) Both("qsA", 100, 1002, 20000, 1000); + if (day == 2) Both("qsB", 200, 2001, 500, 200, role: "secondary1"); + if (day == 9) Both("qsB", 200, 2002, 30000, 900, role: "secondary1"); + Both("qsB", 300, 3001, 1000, 50); + + foreach (var (fraction, batch) in new[] { 0.5, 1.0 }.Select(f => (f, rows.Where(r => r.Fraction == f).Select(r => r.Row).ToList()))) + { + var collectionTime = start.AddMinutes(fraction == 0.5 ? 30 : 65); + foreach (var perDatabase in batch.GroupBy(r => r.DatabaseName)) + { + await runner.WriteBackfillBatchAsync(QueryStoreCollector.Instance, perDatabase.ToList(), server, collectionTime, context, ct); + } + } + } + } + + /// One read, bound as the collector binds it: the raw and table shapes carry $1-$3, the mixed one $1-$6. + private static async Task> FactRowsAsync( + NpgsqlConnection connection, string sql, QueryStoreIntervalLatest.PlanRegressionSource? source, CancellationToken ct) + { + var windowStart = TimeRangeStart.AddDays(-14); + var collectionBound = TimeRangeStart.AddDays(-15); + await using var command = new NpgsqlCommand(sql, connection); + command.Parameters.AddWithValue(ServerId); + command.Parameters.AddWithValue(NpgsqlDbType.Timestamp, windowStart); + command.Parameters.AddWithValue(NpgsqlDbType.Timestamp, collectionBound); + if (sql == PgFactCollector.PlanRegressionTableSql) + { + command.Parameters.AddWithValue(NpgsqlDbType.Timestamp, collectionBound); + } + else if (sql == PgFactCollector.PlanRegressionMixedSql) + { + Assert.NotNull(source); + command.Parameters.AddWithValue(NpgsqlDbType.Timestamp, source!.Value.CoveredFrom); + command.Parameters.AddWithValue(NpgsqlDbType.Timestamp, source.Value.FirstExecutionFloor); + command.Parameters.AddWithValue(NpgsqlDbType.Timestamp, source.Value.RawUpperBound); + } + + var rows = new List(); + await using var reader = await command.ExecuteReaderAsync(ct); + while (await reader.ReadAsync(ct)) + { + var values = new string[reader.FieldCount]; + for (var i = 0; i < reader.FieldCount; i++) + { + values[i] = reader.IsDBNull(i) + ? "" + : Convert.ToString(reader.GetValue(i), CultureInfo.InvariantCulture) ?? ""; + } + + rows.Add(string.Join("|", values)); + } + + return rows; + } + + private static async Task ScalarAsync(NpgsqlConnection connection, string sql, CancellationToken ct) + { + await using var command = new NpgsqlCommand(sql, connection); + var value = await command.ExecuteScalarAsync(ct); + return value is DBNull ? null : value; + } + + private static async Task ExecAsync(NpgsqlConnection connection, string sql, DateTime cutoff, CancellationToken ct) + { + await using var command = new NpgsqlCommand(sql, connection); + command.Parameters.AddWithValue("cutoff", NpgsqlDbType.Timestamp, cutoff); + await command.ExecuteNonQueryAsync(ct); + } + + private sealed class CapturingLogger : ILogger + { + public List Lines { get; } = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) => + Lines.Add(formatter(state, exception)); + } +} diff --git a/Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.QueryPerf.cs b/Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.QueryPerf.cs index e65c2ffa26..3f6a26d624 100644 --- a/Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.QueryPerf.cs +++ b/Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.QueryPerf.cs @@ -308,7 +308,7 @@ private async Task CollectParameterSensitivityFactsAsync(AnalysisContext context /// slice, then plan_agg over it. is shared with /// , so the two reads cannot drift above plan_agg. /// - private const string PlanRegressionRawPrefix = @" + private const string PlanRegressionRawDedupedCte = @" WITH deduped AS ( -- Collapse incremental re-collections of the same open runtime-stats interval: @@ -379,7 +379,13 @@ FROM v_query_store_stats AND collection_time >= $3 GROUP BY database_name, query_id, plan_id, replica_role, runtime_stats_interval_id, first_execution_time ), -plan_agg AS +"; + + /// + /// plan_agg over deduped (the raw prefix's second half, byte-identical, split off for #5513): the raw + /// read and both end in it, so the two cannot drift. + /// + private const string PlanRegressionPlanAggOverDeduped = @"plan_agg AS ( -- Execution-weighted per-exec cost per plan_id, per replica. Keeping replica_role in the grain all -- the way down is what makes the wider dedup key an improvement rather than a blend: were it dropped @@ -400,6 +406,110 @@ plan_agg AS FROM deduped GROUP BY database_name, query_id, plan_id, replica_role ), +"; + + private const string PlanRegressionRawPrefix = PlanRegressionRawDedupedCte + PlanRegressionPlanAggOverDeduped; + + /// + /// PLAN_REGRESSION over the table for the part of the window its coverage claim holds and raw rows for the rest + /// (#5513), chosen by when the table alone is refused + /// () but the table still speaks for most of the window. It was 46% of + /// a 43-server store's passes: each of them dedups the whole 15-day raw slice, 48,000 blocks and 8 seconds a call, + /// for a window the table already holds all but the last day or two of. + /// + /// The same function of the same snapshots as . deduped groups two + /// sets of snapshots TOGETHER by the raw read's own key and picks the newest of each group by the raw read's own order, + /// (collection_time DESC, execution_count DESC): the raw snapshots the table does not stand in for, and the table + /// rows that do. A table row stands in for the raw snapshots of its interval collected at or after $4 + /// (filled_since, no lower than raw holds, so the table's reach below raw's purge is not taken) whose interval is + /// not one the table purged (first_execution_time >= $5). The table row IS the interval's newest snapshot, so + /// it outranks any older raw snapshot of the same interval that falls in the group (an interval open across the edge); + /// an interval with no table row (closed before the claim, or older than the purge) has only its raw snapshots. + /// A raw snapshot with no first_execution_time is never one the table represents, so it is always read from + /// raw. + /// + /// Parameters: $1 server_id, $2 the raw read's last_execution_time bound, $3 its collection_time bound + /// and the first-execution bound for the table's chunks, $4 where the table's coverage starts, $5 the table's + /// first-execution floor, $6 the raw part's upper bound: no snapshot the raw part has to read is collected after it + /// (the larger of $4 and an interval plus the skew margin above $5), bare so a hypertable excludes the newer chunks. + /// The table side carries execution_type_desc only by construction (it holds Regular rows only). + /// + public const string PlanRegressionMixedSql = PlanRegressionMixedPrefix + PlanRegressionPlanAggOverDeduped + PlanRegressionSuffix; + + private const string PlanRegressionMixedPrefix = @" +WITH deduped AS +( + -- #5513: the raw read's deduped CTE over the union of the raw snapshots the table does not stand in for and the + -- table's own rows. The aggregates are the raw read's, line for line: the table row of an interval is its newest + -- snapshot, so it wins the group on collection_time; an interval with no table row keeps only its raw snapshots. + SELECT + database_name, + query_id, + plan_id, + replica_role, + runtime_stats_interval_id, + first_execution_time, + (array_agg(query_plan_hash ORDER BY collection_time DESC, execution_count DESC))[1] AS query_plan_hash, + (array_agg(execution_count ORDER BY collection_time DESC, execution_count DESC))[1] AS execution_count, + (array_agg(avg_cpu_time_us ORDER BY collection_time DESC, execution_count DESC))[1] AS avg_cpu_time_us, + (array_agg(avg_duration_us ORDER BY collection_time DESC, execution_count DESC))[1] AS avg_duration_us, + (array_agg(last_execution_time ORDER BY collection_time DESC, execution_count DESC))[1] AS last_execution_time, + (array_agg(is_forced_plan ORDER BY collection_time DESC, execution_count DESC))[1] AS is_forced_plan, + (array_agg(force_failure_count ORDER BY collection_time DESC, execution_count DESC))[1] AS force_failure_count + FROM + ( + -- Raw snapshots the table does not stand in for: collected before the table's coverage starts, or of an interval + -- the table purged (or one it cannot hold: no first_execution_time). $3 and $6 are bare parameters on the + -- partitioning column, so a hypertable keeps only the chunks between them. + SELECT + database_name, + query_id, + plan_id, + replica_role, + runtime_stats_interval_id, + first_execution_time, + collection_time, + query_plan_hash, + execution_count, + avg_cpu_time_us, + avg_duration_us, + last_execution_time, + is_forced_plan, + force_failure_count + FROM v_query_store_stats + WHERE server_id = $1 + AND execution_type_desc = 'Regular' + AND last_execution_time >= $2 + AND collection_time >= $3 + AND collection_time < $6 + AND (collection_time < $4 OR first_execution_time IS NULL OR first_execution_time < $5) + + UNION ALL + + -- The table: each row is its interval's newest snapshot, for the snapshots the claim holds. + SELECT + database_name, + query_id, + plan_id, + replica_role, + runtime_stats_interval_id, + first_execution_time, + collection_time, + query_plan_hash, + execution_count, + avg_cpu_time_us, + avg_duration_us, + last_execution_time, + is_forced_plan, + force_failure_count + FROM query_store_interval_latest + WHERE server_id = $1 + AND last_execution_time >= $2 + AND collection_time >= $4 + AND first_execution_time >= $5 + ) AS snapshots + GROUP BY database_name, query_id, plan_id, replica_role, runtime_stats_interval_id, first_execution_time +), "; /// @@ -675,9 +785,14 @@ its own parameter rather than written as "$2 - INTERVAL '1 day'" so the planner context.TimeRangeStart.AddDays(-(PlanRegressionWindowDays + PlanRegressionSkewMarginDays))); /* #3953: the latest-snapshot interval table where its coverage holds everything this read would read, - raw otherwise. Decided once here and recorded, so the drill-down reads the same source. */ - var readsTable = await QueryStoreIntervalLatest.ReadsTableAsync( + raw otherwise. Decided once here and recorded, so the drill-down reads the same source. + #5513: a third answer, the table for the part of the window its claim holds and raw rows for the rest. + It returns raw's own answer, so the drill-down is told "raw" for it and reads the raw SQL it always read: + the same rows, and no third drill-down shape. */ + var source = await QueryStoreIntervalLatest.ResolveReadSourceAsync( connection, context.ServerId, collectionBound, FactCommandTimeoutSeconds, _logger, context.CancellationToken); + var readsTable = source.Kind == QueryStoreIntervalLatest.PlanRegressionReadKind.Table; + var readsMixed = source.Kind == QueryStoreIntervalLatest.PlanRegressionReadKind.Mixed; context.PlanRegressionReadsIntervalTable = readsTable; /* #5448: the closed days that are fully built, only when the table is the source. Any failure is an empty @@ -717,7 +832,7 @@ not swallowed. */ var readsDays = builtDays.Count > 0; using var cmd = new NpgsqlCommand( - readsDays ? PlanRegressionDailySql : readsTable ? PlanRegressionTableSql : PlanRegressionSql, connection, snapshot) + readsDays ? PlanRegressionDailySql : readsTable ? PlanRegressionTableSql : readsMixed ? PlanRegressionMixedSql : PlanRegressionSql, connection, snapshot) { CommandTimeout = FactCommandTimeoutSeconds }; cmd.Parameters.AddWithValue(context.ServerId); if (readsDays) @@ -741,6 +856,14 @@ not swallowed. */ /* $4, the table's partitioning column: the same value, bare, for the same reason. */ cmd.Parameters.AddWithValue(collectionBound); } + else if (readsMixed) + { + /* #5513: $4 where the table's coverage starts, $5 its first-execution floor, $6 the raw part's upper + bound. Each one a bare parameter, so a hypertable excludes chunks while it plans. */ + cmd.Parameters.AddWithValue(NpgsqlTypes.NpgsqlDbType.Timestamp, source.CoveredFrom); + cmd.Parameters.AddWithValue(NpgsqlTypes.NpgsqlDbType.Timestamp, source.FirstExecutionFloor); + cmd.Parameters.AddWithValue(NpgsqlTypes.NpgsqlDbType.Timestamp, source.RawUpperBound); + } } var offenderCount = 0; diff --git a/Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.cs b/Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.cs index 2e878d6da5..e017e84312 100644 --- a/Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.cs +++ b/Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.cs @@ -315,6 +315,7 @@ public async Task> CollectConfigAuditFactsAsync(AnalysisContext conte PlanRegressionSql, PlanRegressionTableSql, PlanRegressionDailySql, + PlanRegressionMixedSql, BadActorSql, PerfmonSql, MemoryClerkSql, diff --git a/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs b/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs index 917534882a..91ea735339 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/QueryStoreIntervalLatest.cs @@ -400,10 +400,104 @@ public static bool UseTable(DateTime? filledSince, bool hasPending, DateTime? ra return (rawFloor is DateTime floor && floor >= h) || rawBound >= h; } + /// Which read a PLAN_REGRESSION pass runs (#5513). + public enum PlanRegressionReadKind + { + /// The shipped raw read. + Raw, + + /// The table alone: holds. + Table, + + /// + /// The table for the part of the window its claim covers and raw rows for the rest (#5513). It returns the raw + /// read's own answer over the same snapshots (see ). + /// + Mixed, + } + + /// + /// The decision and, for , the edges of the part the table speaks for. + /// + /// The read to run. + /// + /// Mixed only: the table's rows stand in for raw snapshots with collection_time at or after this. It is + /// max(filled_since, raw's reading floor), so the table never reaches below what raw holds and the answer stays raw's. + /// + /// + /// Mixed only: the table's rows stand in only for intervals whose first_execution_time is at or after this (its + /// purge removes older intervals, which raw may still hold). + /// + /// + /// Mixed only: no raw snapshot the raw part has to read is collected at or after this. It is the larger of + /// and the table's first-execution floor plus an interval (a day) and the skew margin + /// (a day), so the raw part stays inside the chunks that matter. + /// + public readonly record struct PlanRegressionSource( + PlanRegressionReadKind Kind, + DateTime CoveredFrom, + DateTime FirstExecutionFloor, + DateTime RawUpperBound) + { + /// The shipped raw read. + public static PlanRegressionSource Raw => new(PlanRegressionReadKind.Raw, default, default, default); + + /// The table alone. + public static PlanRegressionSource Table => new(PlanRegressionReadKind.Table, default, default, default); + } + + /// + /// How far above the table's first-execution floor a raw snapshot of an older interval can be collected (#5513): the + /// interval itself spans at most a day, and the monitored clock can run a day ahead of the store's (the same margin + /// gives the chunk bound). + /// + internal const int MixedRawSpanDays = 1 + PlanRegressionDaily.PlanRegressionSkewMarginDays; + + /// + /// (#5513) says "all or nothing", and a store in steady state fails it for reasons that leave + /// most of the window covered: a coverage claim that restarted a day or two ago (a gap, a purged pending batch, a + /// server added or upgraded recently), or a table whose purge has just moved its floor above the window's bound. Each + /// of those used to send the whole 15-day slice through the raw dedup. This picks + /// there: the table where the claim holds, raw rows for the remainder, grouped TOGETHER so an interval that straddles + /// the edge is deduplicated by the raw read's own order and keeps its newest snapshot. + /// + /// Why it returns the raw read's answer. A table row stands in only for snapshots it represents: those + /// with collection_time >= CoveredFrom (the claim, and no lower than raw holds, so the table's extension below + /// raw's purge is not taken) of intervals the table has not purged (first_execution_time >= FirstExecutionFloor). + /// Every other raw snapshot is read from raw. For an interval with a table row, that row IS its newest snapshot, so it + /// outranks the older raw snapshots in the same group; for an interval without one, raw is read as before. + /// + /// Pending batches, no coverage row, and an empty table for the server still choose raw: nothing is covered. + /// + public static PlanRegressionSource DecideSource(DateTime? filledSince, bool hasPending, DateTime? rawFloor, DateTime rawBound, DateTime? tableFloor) + { + if (UseTable(filledSince, hasPending, rawFloor, rawBound, tableFloor)) + { + return PlanRegressionSource.Table; + } + + if (filledSince is not DateTime f || hasPending || tableFloor is not DateTime h) + { + return PlanRegressionSource.Raw; + } + + var rawReadsFrom = rawFloor is DateTime r && r > rawBound ? r : rawBound; + var coveredFrom = f > rawReadsFrom ? f : rawReadsFrom; + + /* The table has purged nothing raw still holds when clause 3 of the rule holds, and then no interval is raw-only. */ + var tablePurgedNothingRawHolds = (rawFloor is DateTime floor && floor >= h) || rawBound >= h; + var firstExecutionFloor = tablePurgedNothingRawHolds ? rawBound : h; + + var spanEnd = firstExecutionFloor.AddDays(MixedRawSpanDays); + return new PlanRegressionSource( + PlanRegressionReadKind.Mixed, coveredFrom, firstExecutionFloor, spanEnd > coveredFrom ? spanEnd : coveredFrom); + } + /// /// Decides the PLAN_REGRESSION source for one server and pass. is the raw read's own /// collection_time bound ($3). Any fault picks raw: the raw SQL is the shipped read, so the worst a - /// broken decision can cost is today's behaviour. + /// broken decision can cost is today's behaviour. True only for the table alone; + /// has the mixed answer too (#5513). /// public static async Task ReadsTableAsync( NpgsqlConnection connection, @@ -412,6 +506,22 @@ public static async Task ReadsTableAsync( int commandTimeoutSeconds, ILogger? logger, CancellationToken cancellationToken) + { + var source = await ResolveReadSourceAsync(connection, serverId, rawBound, commandTimeoutSeconds, logger, cancellationToken); + return source.Kind == PlanRegressionReadKind.Table; + } + + /// + /// 's decision with the third answer (#5513): , + /// the table where its claim holds and raw rows for the rest. Same inputs, same fault rule (any fault picks raw). + /// + public static async Task ResolveReadSourceAsync( + NpgsqlConnection connection, + int serverId, + DateTime rawBound, + int commandTimeoutSeconds, + ILogger? logger, + CancellationToken cancellationToken) { try { @@ -430,7 +540,10 @@ public static async Task ReadsTableAsync( if (filledSince is null || hasPending) { - return false; + logger?.LogDebug( + "PLAN_REGRESSION source for server {ServerId}: raw ({Why})", + serverId, filledSince is null ? "no coverage row" : "pending batches"); + return PlanRegressionSource.Raw; } DateTime? rawFloor = null; @@ -453,16 +566,16 @@ public static async Task ReadsTableAsync( tableFloor = await plain.ExecuteScalarAsync(cancellationToken) as DateTime?; } - var useTable = UseTable(filledSince, hasPending, rawFloor, rawBound, tableFloor); + var source = DecideSource(filledSince, hasPending, rawFloor, rawBound, tableFloor); logger?.LogDebug( - "PLAN_REGRESSION source for server {ServerId}: {Source} (coverage since {FilledSince:o}; raw floor {RawFloor:o}; raw bound {RawBound:o}; table floor {TableFloor:o})", - serverId, useTable ? "interval table" : "raw", filledSince, rawFloor, rawBound, tableFloor); - return useTable; + "PLAN_REGRESSION source for server {ServerId}: {Source} (coverage since {FilledSince:o}; raw floor {RawFloor:o}; raw bound {RawBound:o}; table floor {TableFloor:o}; table covers from {CoveredFrom:o})", + serverId, source.Kind, filledSince, rawFloor, rawBound, tableFloor, source.CoveredFrom); + return source; } catch (Exception ex) when (ex is not OperationCanceledException) { logger?.LogWarning(ex, "PLAN_REGRESSION source decision failed for server {ServerId}; reading raw", serverId); - return false; + return PlanRegressionSource.Raw; } }