diff --git a/Darling/Darling.Tests/AgGroupIdRungTests.cs b/Darling/Darling.Tests/AgGroupIdRungTests.cs index 0f3e501ef2..c9d323f000 100644 --- a/Darling/Darling.Tests/AgGroupIdRungTests.cs +++ b/Darling/Darling.Tests/AgGroupIdRungTests.cs @@ -27,8 +27,9 @@ namespace Darling.Tests; /// group_id IS NULL rather than erroring, and an INSERT in the collector's own column order round /// trips a GUID string end to end. /// -/// This file's "I am the top rung" claim takes over from -/// CollectionLogWatermarkAndJobHistoryIndexesRungTests (V150) now that V151 has landed. +/// This file's "I am the top rung" claim moved to CaggGroupIndexDropRungTests (V152) now +/// that V152 has landed; this file's own rung/probe facts below keep asserting what stays true forever +/// (present, in-order, gated behind the arm above it) rather than "is exactly the top". /// /* #1776 own-store: each fact mints its own scratch database through ScratchPostgres and never touches the shared store's tables, so it cannot race the live collection and serializing it would be pure slowdown. */ @@ -37,34 +38,36 @@ public sealed class AgGroupIdRungTests private const int RungVersion = 151; private const int PreviousVersion = 150; - /// This rung's sentinel ordinal in the viewer probe — the newest, so the last argument. + /// This rung's sentinel ordinal in the viewer probe — no longer the newest, since V152 + /// landed above it. private const int ProbeOrdinal = 126; private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); /// - /// The rung is registered and is the new top of the ladder — the claim this class takes over from - /// CollectionLogWatermarkAndJobHistoryIndexesRungTests (V150) now that V151 has landed. + /// The rung is registered, and the ladder stays dense above the historical gap — the claim this class + /// took over from CollectionLogWatermarkAndJobHistoryIndexesRungTests (V150) moved on again to + /// CaggGroupIndexDropRungTests (V152) now that V152 has landed. /// [Fact] - public void TheRungIsRegisteredAtTheTopOfADenseLadder() + public void TheRungIsRegistered_AndTheLadderIsDenseAboveTheHistoricalGap() { var versions = PgMigrations.Scripts.Select(s => s.Version).ToList(); Assert.Equal("ag-group-id", PgMigrations.Scripts.Single(s => s.Version == RungVersion).Name); - Assert.Equal(StorageVersion.SchemaVersion, PgMigrations.Scripts[^1].Version); - Assert.Equal(StorageVersion.SchemaVersion, versions.Max()); - Assert.Equal(RungVersion, StorageVersion.SchemaVersion); Assert.Equal(versions.Distinct().OrderBy(v => v), versions); + + var above = versions.Where(v => v > 45).OrderBy(v => v).ToList(); + Assert.Equal(Enumerable.Range(above[0], above.Count), above); } /// - /// The viewer probe's sentinel carries this rung, and the map treats it as the TOP arm: a missing top arm - /// maps a fully-migrated store one rung short, permanently, because + /// The viewer probe's sentinel carries this rung, and the map treats it as an arm gated below the + /// current top's arm — a missing arm maps a fully-migrated store one rung short, permanently, because /// is . /// [Fact] - public void TheProbeMapsAFullyMigratedStoreToThisTopRung() + public void TheProbeCarriesThisRungsSentinel_AndTheArmSitsBelowTheCurrentTop() { var probe = ViewerDataService.StoreSchemaProbeSql.Replace("\r\n", "\n", StringComparison.Ordinal); Assert.Contains("table_name = 'ag_replica_states'", probe, StringComparison.Ordinal); @@ -72,29 +75,31 @@ public void TheProbeMapsAFullyMigratedStoreToThisTopRung() var viewer = RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Viewer", "ViewerDataService.cs"); Assert.Contains($"reader.GetBoolean({ProbeOrdinal})", viewer, StringComparison.Ordinal); - Assert.DoesNotContain($"reader.GetBoolean({ProbeOrdinal + 1})", viewer, StringComparison.Ordinal); - - Assert.Equal(StorageVersion.SchemaVersion, ViewerDataService.RequiredStoreSchemaVersion); var method = typeof(ViewerDataService).GetMethod("MapProbedSchemaVersion", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)!; var arity = method.GetParameters().Length; - Assert.Equal(ProbeOrdinal, arity - 1); Assert.Equal("hasAgGroupId", method.GetParameters()[ProbeOrdinal].Name); + /* Every rung above this one (V152's hasCaggGroupIndexDrop) must also be false, or the map finds + the newer arm first and this assertion is checking the wrong rung's fallthrough. */ var all = Enumerable.Repeat((object)true, arity).ToArray(); - Assert.Equal(StorageVersion.SchemaVersion, (int)method.Invoke(null, all)!); - var behind = (object[])all.Clone(); - behind[ProbeOrdinal] = false; + for (var i = ProbeOrdinal; i < arity; i++) + { + behind[i] = false; + } Assert.Equal(PreviousVersion, (int)method.Invoke(null, behind)!); + /* V152 (#4503) is now the top rung, so this arm no longer needs to be the LAST one — it only has to + sit below the current top's arm, which is what the ladder-dense invariant above already + guarantees is registered ahead of it. */ var thisArm = viewer.IndexOf("if (hasAgGroupId)", StringComparison.Ordinal); - var previousArm = viewer.IndexOf("if (hasCollectionLogWatermarkAndJobHistoryIndexes)", StringComparison.Ordinal); + var topArm = viewer.IndexOf("if (hasCaggGroupIndexDrop)", StringComparison.Ordinal); Assert.True(thisArm >= 0, "the viewer has no V151 sentinel arm — a fully-migrated store would map one rung short"); - Assert.True(thisArm < previousArm, "the V151 arm sits below V150's, so a current store maps one rung short"); + Assert.True(topArm >= 0 && topArm < thisArm, "the current top rung's arm must sit above the V151 arm"); Assert.Contains( - "return " + StorageVersion.SchemaVersion.ToString(CultureInfo.InvariantCulture) + ";", - viewer[thisArm..previousArm], StringComparison.Ordinal); + "return " + RungVersion.ToString(CultureInfo.InvariantCulture) + ";", + viewer[thisArm..viewer.IndexOf("if (hasCollectionLogWatermarkAndJobHistoryIndexes)", StringComparison.Ordinal)], StringComparison.Ordinal); } /// diff --git a/Darling/Darling.Tests/CaggGroupIndexDropLiveTests.cs b/Darling/Darling.Tests/CaggGroupIndexDropLiveTests.cs new file mode 100644 index 0000000000..32d9fb2d71 --- /dev/null +++ b/Darling/Darling.Tests/CaggGroupIndexDropLiveTests.cs @@ -0,0 +1,383 @@ +/* + * 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.Linq; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; +using Npgsql; +using PerformanceMonitor.Darling.Storage; +using PerformanceMonitor.Darling.Viewer; +using Xunit; + +namespace Darling.Tests; + +/// +/// #4503: on a FRESH migrate, the six Query Store rollups' materializations carry EXACTLY the group +/// indexes an UPGRADED store keeps after V152's DO block drops the unread ones — the same key +/// columns, the same bucket DESC order, and the same PostgreSQL-assigned name — and nothing else. +/// +/// #1776 own-store — mints its own scratch database () so the +/// index catalog it reads is not shared with any other live class, the same reason +/// does. +/// +public sealed class CaggGroupIndexDropLiveTests +{ + /// The six views #4503 touches, with the key column(s) each KEEPS. + private static readonly (string View, string[] KeptColumns)[] Views = + { + (TimescaleSupport.QueryStoreStatsHourlyView, new[] { "server_id", "server_name" }), + (TimescaleSupport.QueryStoreStatsCorrectedHourlyView, new[] { "server_id", "server_name" }), + (TimescaleSupport.QueryStoreStatsDailyView, new[] { "server_id" }), + (TimescaleSupport.QueryStoreStatsCorrectedDailyView, new[] { "server_id" }), + (TimescaleSupport.QueryStoreStatsIntervalDailyView, new[] { "server_id" }), + (TimescaleSupport.QueryStoreStatsDayGrainDailyView, new[] { "server_id" }), + }; + + /// Every column any of the six views' default create_group_indexes would have built, + /// beyond what keeps — the DROPPED set this pin proves absent. + private static readonly IReadOnlyDictionary DroppedColumns = new Dictionary(StringComparer.Ordinal) + { + [TimescaleSupport.QueryStoreStatsHourlyView] = new[] { "database_name", "module_name", "query_hash" }, + [TimescaleSupport.QueryStoreStatsCorrectedHourlyView] = new[] { "database_name", "module_name", "query_hash" }, + [TimescaleSupport.QueryStoreStatsDailyView] = new[] { "database_name", "module_name", "query_hash", "server_name" }, + [TimescaleSupport.QueryStoreStatsCorrectedDailyView] = new[] { "database_name", "module_name", "query_hash", "server_name" }, + [TimescaleSupport.QueryStoreStatsIntervalDailyView] = new[] + { + "database_name", "execution_type_desc", "first_execution_time", "module_name", "plan_id", + "query_hash", "query_id", "replica_role", "runtime_stats_interval_id", "server_name", + }, + [TimescaleSupport.QueryStoreStatsDayGrainDailyView] = new[] { "database_name", "module_name", "query_hash", "server_name" }, + }; + + /// + /// (a) A fresh migrate to the top, then the aggregates ensured: on each of the six, every DROPPED + /// column's group index is absent, every KEPT column's is present, and the plain bucket_idx + /// survives too. RED on origin/dev — a fresh store there still carries the dropped indexes, + /// because create_group_indexes = false and the kept-index follow-up did not exist before #4503. + /// + [Fact] + public async Task FreshMigrate_KeepsOnlyTheSurvivingGroupIndexes_OnAllSixRollups() + { + var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), + "Set DARLING_TEST_PG to a Postgres connection string (with TimescaleDB installed) to run the live #4503 fresh-vs-upgraded index test (it mints its own scratch database)."); + + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + + var bodySucceeded = false; + try + { + await PgMigrations.MigrateAsync(connection, ct); + + Assert.True(await TimescaleSupport.TryEnableAsync(connection, null, ct), + "the dev fixture is expected to have TimescaleDB installed"); + await TimescaleSupport.ConvertToHypertablesAsync(connection, null, ct); + await TimescaleSupport.EnsureContinuousAggregatesAsync(connection, null, ct); + + foreach (var (view, keptColumns) in Views) + { + var (matSchema, matName) = await MaterializationOfAsync(connection, view, ct); + var indexes = await GroupIndexColumnsAsync(connection, matSchema, matName, ct); + + foreach (var dropped in DroppedColumns[view]) + { + Assert.DoesNotContain(dropped, indexes); + } + + foreach (var kept in keptColumns) + { + Assert.Contains(kept, indexes); + } + + /* The plain bucket index — never a group index, never touched by #4503 either side. */ + var plainBucket = await ScalarBoolAsync(connection, + $"SELECT EXISTS (SELECT 1 FROM pg_indexes WHERE schemaname = '{matSchema}' AND tablename = '{matName}' AND indexdef ~ 'USING btree \\(bucket DESC\\)$')", + ct); + Assert.True(plainBucket, $"{view}'s materialization lost its plain bucket index"); + } + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, async (_, _) => { }); + } + } + + /// + /// (c) The kept-index build is idempotent: running + /// a second time issues no error and leaves the SAME pg_indexes rows on every one of the six + /// materializations — the follow-up's own catalog read finds each kept index already present and + /// creates nothing more. + /// + [Fact] + public async Task EnsureContinuousAggregates_RunTwice_IsIdempotent_OnTheKeptIndexes() + { + var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), + "Set DARLING_TEST_PG to a Postgres connection string (with TimescaleDB installed) to run the live #4503 idempotency test (it mints its own scratch database)."); + + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + + var bodySucceeded = false; + try + { + await PgMigrations.MigrateAsync(connection, ct); + Assert.True(await TimescaleSupport.TryEnableAsync(connection, null, ct)); + await TimescaleSupport.ConvertToHypertablesAsync(connection, null, ct); + await TimescaleSupport.EnsureContinuousAggregatesAsync(connection, null, ct); + + var before = new Dictionary>(StringComparer.Ordinal); + foreach (var (view, _) in Views) + { + var (matSchema, matName) = await MaterializationOfAsync(connection, view, ct); + before[view] = await IndexDefinitionsAsync(connection, matSchema, matName, ct); + } + + /* Second pass — no error. */ + await TimescaleSupport.EnsureContinuousAggregatesAsync(connection, null, ct); + + foreach (var (view, _) in Views) + { + var (matSchema, matName) = await MaterializationOfAsync(connection, view, ct); + var after = await IndexDefinitionsAsync(connection, matSchema, matName, ct); + Assert.Equal(before[view], after); + } + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, async (_, _) => { }); + } + } + + /// + /// V152 on a store WITHOUT TimescaleDB is a no-op: a plain migrate (never calling + /// ) reaches the top with no exception, and the + /// recorded schema version is 152. RED at bab1c4114 (pre-guard): the DO block reaches + /// timescaledb_information unconditionally and raises 42P01, relation "timescaledb_information. + /// continuous_aggregates" does not exist. + /// + [Fact] + public async Task V152_OnAStoreWithoutTimescaleDb_CompletesAsANoOp() + { + var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), + "Set DARLING_TEST_PG to a Postgres connection string to run the V152 no-TimescaleDB no-op pin (it mints its own scratch database and never enables TimescaleDB on it)."); + + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + + var bodySucceeded = false; + try + { + /* Deliberately never calls LiveTimescaleProbe.TryEnableAsync — this is the plain-PostgreSQL + case the migration guard must no-op on. */ + await PgMigrations.MigrateAsync(connection, ct); + + using var version = new NpgsqlCommand("SELECT MAX(version) FROM darling_schema_version", connection); + var recorded = Convert.ToInt32(await version.ExecuteScalarAsync(ct)); + Assert.Equal(StorageVersion.SchemaVersion, recorded); + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, async (_, _) => { }); + } + } + + /// + /// The reviewer's false-positive case: a store at V150 with no rollup materialized at all does NOT + /// map to 152 — V151's own sentinel (the ag_replica_states.group_id column) is false at V150, + /// so the V152 arm's AND fails and the version falls through correctly. + /// + [Fact] + public async Task AStoreAtV150WithNoRollup_DoesNotMapToV152() + { + var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), + "Set DARLING_TEST_PG to a Postgres connection string to run the V150 false-positive pin (it mints its own scratch database)."); + + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + + var bodySucceeded = false; + try + { + await PgMigrations.MigrateAsync(connection, ct); + + /* Roll the version stamp back to V150 — no rollup, no group_id column, plain PostgreSQL, + exactly the reviewer's false-positive shape. */ + await using (var rollback = new NpgsqlCommand("DELETE FROM darling_schema_version WHERE version >= 151", connection)) + { + await rollback.ExecuteNonQueryAsync(ct); + } + + await using (var dropColumn = new NpgsqlCommand( + "ALTER TABLE collect.ag_replica_states DROP COLUMN IF EXISTS group_id", connection)) + { + await dropColumn.ExecuteNonQueryAsync(ct); + } + + var version = await ProbedVersionAsync(connection, ct); + Assert.Equal(150, version); + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, async (_, _) => { }); + } + } + + /// + /// A TimescaleDB store fully migrated to V151 (the index present, before V152's drop rung has run) + /// maps to exactly 151 — the group index is still there, so the negative NOT EXISTS half of the V152 + /// sentinel is false. + /// + [Fact] + public async Task ATimescaleDbStoreAtV151WithTheGroupIndexPresent_MapsToV151() + { + var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), + "Set DARLING_TEST_PG to a Postgres connection string (with TimescaleDB installed) to run the V151 probe pin (it mints its own scratch database)."); + + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + + var bodySucceeded = false; + try + { + await PgMigrations.MigrateAsync(connection, ct); + + var timescaleEnabled = await LiveTimescaleProbe.TryEnableAsync(scratch.ConnectionString, ct); + Assert.True(timescaleEnabled, "TimescaleDB must be available on CI for the V151 probe pin"); + + await TimescaleSupport.ConvertToHypertablesAsync(connection, null, ct); + + /* Roll back to V151, then create the rollups the V151-era way — default create_group_indexes, + the group index still present — the same fixture shape CaggGroupIndexDropUpgradeLiveTests uses. */ + await using (var rollback = new NpgsqlCommand("DELETE FROM darling_schema_version WHERE version >= 152", connection)) + { + await rollback.ExecuteNonQueryAsync(ct); + } + + var v151Shape = TimescaleSupport.CreateQueryStoreStatsHourlySql.Replace( + ", timescaledb.create_group_indexes = false", string.Empty, StringComparison.Ordinal); + await using (var create = new NpgsqlCommand(v151Shape, connection)) + { + await create.ExecuteNonQueryAsync(ct); + } + + var version = await ProbedVersionAsync(connection, ct); + Assert.Equal(151, version); + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, async (_, _) => { }); + } + } + + private static async Task ProbedVersionAsync(NpgsqlConnection connection, CancellationToken ct) + { + var method = typeof(ViewerDataService).GetMethod("MapProbedSchemaVersion", BindingFlags.NonPublic | BindingFlags.Static)!; + var arity = method.GetParameters().Length; + + await using var command = new NpgsqlCommand(ViewerDataService.StoreSchemaProbeSql, connection); + await using var reader = await command.ExecuteReaderAsync(ct); + Assert.True(await reader.ReadAsync(ct)); + + var sentinels = new object[arity]; + for (var i = 0; i < arity; i++) + { + sentinels[i] = reader.GetBoolean(i); + } + + return (int)method.Invoke(null, sentinels)!; + } + + private static async Task<(string Schema, string Name)> MaterializationOfAsync(NpgsqlConnection connection, string view, CancellationToken ct) + { + await using var command = new NpgsqlCommand( + "SELECT materialization_hypertable_schema, materialization_hypertable_name FROM timescaledb_information.continuous_aggregates WHERE view_schema = 'collect' AND view_name = $1", connection); + command.Parameters.AddWithValue(view); + await using var reader = await command.ExecuteReaderAsync(ct); + Assert.True(await reader.ReadAsync(ct), $"{view} was not created"); + return (reader.GetString(0), reader.GetString(1)); + } + + private static async Task> IndexDefinitionsAsync(NpgsqlConnection connection, string schema, string table, CancellationToken ct) + { + var definitions = new List(); + await using var command = new NpgsqlCommand( + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 ORDER BY indexname", connection); + command.Parameters.AddWithValue(schema); + command.Parameters.AddWithValue(table); + await using var reader = await command.ExecuteReaderAsync(ct); + while (await reader.ReadAsync(ct)) + { + definitions.Add(reader.GetString(0)); + } + + return definitions; + } + + /// The set of columns carrying a two-key (column, bucket) group index on the given materialization. + private static async Task> GroupIndexColumnsAsync(NpgsqlConnection connection, string schema, string table, CancellationToken ct) + { + var columns = new HashSet(StringComparer.Ordinal); + await using var command = new NpgsqlCommand(@" +SELECT a1.attname +FROM pg_index i +JOIN pg_attribute a1 ON a1.attrelid = i.indrelid AND a1.attnum = i.indkey[0] +JOIN pg_attribute a2 ON a2.attrelid = i.indrelid AND a2.attnum = i.indkey[1] +WHERE i.indrelid = format('%I.%I', $1, $2)::regclass +AND i.indnatts = 2 +AND a2.attname = 'bucket'", connection); + command.Parameters.AddWithValue(schema); + command.Parameters.AddWithValue(table); + await using var reader = await command.ExecuteReaderAsync(ct); + while (await reader.ReadAsync(ct)) + { + columns.Add(reader.GetString(0)); + } + + return columns; + } + + private static async Task ScalarBoolAsync(NpgsqlConnection connection, string sql, CancellationToken ct) + { + await using var command = new NpgsqlCommand(sql, connection); + var result = await command.ExecuteScalarAsync(ct); + return result is bool value && value; + } +} diff --git a/Darling/Darling.Tests/CaggGroupIndexDropRungTests.cs b/Darling/Darling.Tests/CaggGroupIndexDropRungTests.cs new file mode 100644 index 0000000000..02c7e8ec2d --- /dev/null +++ b/Darling/Darling.Tests/CaggGroupIndexDropRungTests.cs @@ -0,0 +1,91 @@ +/* + * 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.Globalization; +using System.Linq; +using Xunit; +using PerformanceMonitor.Darling.Storage; +using PerformanceMonitor.Darling.Viewer; + +namespace Darling.Tests; + +/// +/// Pins Darling rung V152 (#4503): the six Query Store rollups' auto-created two-key group index (first +/// key one of the rung's drop-list columns, second key bucket) is dropped by catalog shape rather +/// than by a built name, and the six rollups now carry create_group_indexes = false so a fresh +/// store never grows the index in the first place. This file is the RUNG (ladder, viewer probe) only — +/// the live schema-after-migrate proof (both the drop on an already-migrated store and a fresh store +/// never creating it) is CaggGroupIndexDropLiveTests. +/// +/// This file's "I am the top rung" claim takes over from AgGroupIdRungTests (V151) now that +/// V152 has landed. +/// +public sealed class CaggGroupIndexDropRungTests +{ + private const int RungVersion = 152; + private const int PreviousVersion = 151; + + /// This rung's sentinel ordinal in the viewer probe — the newest, so the last argument. + private const int ProbeOrdinal = 127; + + /// + /// The rung is registered and is the new top of the ladder — the claim this class takes over from + /// AgGroupIdRungTests (V151) now that V152 has landed. + /// + [Fact] + public void TheRungIsRegisteredAtTheTopOfADenseLadder() + { + var versions = PgMigrations.Scripts.Select(s => s.Version).ToList(); + + Assert.Equal("drop-unread-cagg-group-indexes", PgMigrations.Scripts.Single(s => s.Version == RungVersion).Name); + Assert.Equal(StorageVersion.SchemaVersion, PgMigrations.Scripts[^1].Version); + Assert.Equal(StorageVersion.SchemaVersion, versions.Max()); + Assert.Equal(RungVersion, StorageVersion.SchemaVersion); + Assert.Equal(versions.Distinct().OrderBy(v => v), versions); + } + + /// + /// The viewer probe's sentinel carries this rung, and the map treats it as the TOP arm: a missing top arm + /// maps a fully-migrated store one rung short, permanently, because + /// is . + /// + [Fact] + public void TheProbeMapsAFullyMigratedStoreToThisTopRung() + { + var probe = ViewerDataService.StoreSchemaProbeSql.Replace("\r\n", "\n", StringComparison.Ordinal); + Assert.Contains("query_hash", probe, StringComparison.Ordinal); + Assert.Contains("collect.query_store_stats_hourly", probe, StringComparison.Ordinal); + + var viewer = RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Viewer", "ViewerDataService.cs"); + Assert.Contains($"reader.GetBoolean({ProbeOrdinal})", viewer, StringComparison.Ordinal); + Assert.DoesNotContain($"reader.GetBoolean({ProbeOrdinal + 1})", viewer, StringComparison.Ordinal); + + Assert.Equal(StorageVersion.SchemaVersion, ViewerDataService.RequiredStoreSchemaVersion); + + var method = typeof(ViewerDataService).GetMethod("MapProbedSchemaVersion", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)!; + var arity = method.GetParameters().Length; + Assert.Equal(ProbeOrdinal, arity - 1); + Assert.Equal("hasCaggGroupIndexDrop", method.GetParameters()[ProbeOrdinal].Name); + + var all = Enumerable.Repeat((object)true, arity).ToArray(); + Assert.Equal(StorageVersion.SchemaVersion, (int)method.Invoke(null, all)!); + + var behind = (object[])all.Clone(); + behind[ProbeOrdinal] = false; + Assert.Equal(PreviousVersion, (int)method.Invoke(null, behind)!); + + var thisArm = viewer.IndexOf("if (hasCaggGroupIndexDrop)", StringComparison.Ordinal); + var previousArm = viewer.IndexOf("if (hasAgGroupId)", StringComparison.Ordinal); + Assert.True(thisArm >= 0, "the viewer has no V152 sentinel arm — a fully-migrated store would map one rung short"); + Assert.True(thisArm < previousArm, "the V152 arm sits below V151's, so a current store maps one rung short"); + Assert.Contains( + "return " + StorageVersion.SchemaVersion.ToString(CultureInfo.InvariantCulture) + ";", + viewer[thisArm..previousArm], StringComparison.Ordinal); + } +} diff --git a/Darling/Darling.Tests/CaggGroupIndexDropUpgradeLiveTests.cs b/Darling/Darling.Tests/CaggGroupIndexDropUpgradeLiveTests.cs new file mode 100644 index 0000000000..79dc0f10ea --- /dev/null +++ b/Darling/Darling.Tests/CaggGroupIndexDropUpgradeLiveTests.cs @@ -0,0 +1,546 @@ +/* + * 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 Npgsql; +using PerformanceMonitor.Darling.Storage; +using Xunit; + +namespace Darling.Tests; + +/// +/// #4503's upgrade-equivalence pin: a store already at V151, with its six Query Store rollups +/// materialized and compressed, migrates to V152 and every reader query returns the SAME rows before and +/// after. The rung only drops indexes — it must never change what a reader sees. +/// +/// #1776 own-store — mints its own scratch database (it creates the continuous aggregates the +/// shared fixture deliberately leaves to the tests, compresses their materialization chunks, and rolls the +/// version stamp back to plant a pre-upgrade store), so it cannot race the shared store and is not +/// serialized against the live-postgres collection. Same shape as +/// IntervalDedupMaterializationIndexesTests and QueryStoreCorrectedRollupLiveTests. +/// +public sealed class CaggGroupIndexDropUpgradeLiveTests +{ + private const int RungVersion = 152; + private const int PreviousVersion = 151; + + /// Distinctive fake ids — a real server_id is a storage-name hash, never these. + private const int ServerA = -415203; + private const int ServerB = -415204; + + private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + + /// + /// The six rollups this rung touches, in the shape the migration's own DO block enumerates them. + /// + private static readonly string[] SixRollups = + { + TimescaleSupport.QueryStoreStatsHourlyView, + TimescaleSupport.QueryStoreStatsCorrectedHourlyView, + TimescaleSupport.QueryStoreStatsDailyView, + TimescaleSupport.QueryStoreStatsCorrectedDailyView, + TimescaleSupport.QueryStoreStatsIntervalDailyView, + TimescaleSupport.QueryStoreStatsDayGrainDailyView, + }; + + /// + /// Pin (b): a V151 store with materialized, COMPRESSED Query Store rollups migrates to V152, and every + /// reader query returns identical rows before and after — proving the rung's index drop changes no + /// answer, only how it is served. + /// + /// RED on origin/dev (pre-#4503): the index-shape assertions at the end fail at + /// runtime — the group indexes this rung drops are still present, and the kept-index names this test + /// checks for do not exist yet (dev never creates them), so Assert.Empty on the leftover set and + /// Assert.Contains on the kept names both fail. See the PR body for the captured line. + /// + [Fact] + public async Task AStoreAtV151WithMaterializedCompressedRollups_MigratesToV152_AndEveryReaderReturnsTheSameRows() + { + var baseConnectionString = ConnectionString; + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), + "Set DARLING_TEST_PG to a Postgres connection string (with TimescaleDB installed) to run the V152 upgrade-equivalence pin."); + + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + + var bodySucceeded = false; + try + { + /* Climb to the top first (as every rung test does) — the CAGGs and their rollups need the rest + of the schema (servers, config) present. */ + await PgMigrations.MigrateAsync(connection, ct); + + var timescaleEnabled = await LiveTimescaleProbe.TryEnableAsync(scratch.ConnectionString, ct); + Assert.True(timescaleEnabled, "TimescaleDB must be available on CI for the V152 upgrade-equivalence pin"); + + await TimescaleSupport.ConvertToHypertablesAsync(connection, null, ct); + + /* With the group indexes: roll the version stamp back to V151 first, then create the rollups the + way a store on THAT build did — the default create_group_indexes shape, no create_group_indexes + option and no kept-index statements — rather than through this build's ensure sweep (which + already withholds them). This is what makes the fixture a genuine "already at V151" store + instead of a fresh migrate that happens to look like one. */ + await ExecAsync(connection, ct, "DELETE FROM darling_schema_version WHERE version >= " + RungVersion); + using (var version = new NpgsqlCommand("SELECT MAX(version) FROM darling_schema_version", connection)) + { + Assert.Equal(PreviousVersion, Convert.ToInt32(await version.ExecuteScalarAsync(ct))); + } + + await CreateV151ShapeRollupsAsync(connection, ct); + + /* Plant two servers x two days x a few databases/queries of query_store_stats. */ + var day1 = new DateTime(2026, 4, 6, 0, 0, 0, DateTimeKind.Unspecified); + var day2 = day1.AddDays(1); + await SeedQueryStoreStatsAsync(connection, ServerA, day1, ct); + await SeedQueryStoreStatsAsync(connection, ServerA, day2, ct); + await SeedQueryStoreStatsAsync(connection, ServerB, day1, ct); + await SeedQueryStoreStatsAsync(connection, ServerB, day2, ct); + + /* Refresh every one of the six views over both seeded days. */ + var from = day1; + var to = day2.AddDays(1); + foreach (var view in SixRollups) + { + await RefreshAsync(connection, view, from, to, ct); + } + + /* Compress the materialization chunks, the product's own way (ALTER MATERIALIZED VIEW ... SET + (timescaledb.compress ...) then compress_chunk over every chunk). */ + foreach (var view in SixRollups) + { + await CompressMaterializationAsync(connection, view, ct); + } + + /* Capture every reader's rows BEFORE the V152 step. */ + var before = await CaptureReaderRowsAsync(connection, ct); + + /* The upgrade: apply every rung above V151, which is exactly V152 on this build. */ + var applied = await PgMigrations.MigrateAsync(connection, ct); + Assert.Equal(PgMigrations.Scripts.Count(m => m.Version >= RungVersion), applied); + + /* Capture every reader's rows AFTER. */ + var after = await CaptureReaderRowsAsync(connection, ct); + + Assert.Equal(before.Count, after.Count); + for (var i = 0; i < before.Count; i++) + { + Assert.Equal(before[i].Name, after[i].Name); + Assert.Equal(before[i].Rows, after[i].Rows); + } + + /* The index-shape assertions: dropped indexes gone, kept indexes present, matched by key + columns — this half is RED on origin/dev (see the doc comment above). */ + foreach (var view in SixRollups) + { + var (matSchema, matTable) = await MaterializationOfAsync(connection, view, ct); + var indexes = await IndexColumnsAsync(connection, matSchema, matTable, ct); + + var keptColumns = KeptFirstColumnsFor(view); + var droppedColumns = DroppedFirstColumnsFor(view); + + foreach (var kept in keptColumns) + { + Assert.Contains(indexes, idx => idx.Count == 2 && idx[0] == kept && idx[1] == "bucket"); + } + + foreach (var dropped in droppedColumns) + { + Assert.DoesNotContain(indexes, idx => idx.Count == 2 && idx[0] == dropped && idx[1] == "bucket"); + } + } + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, async (cleanup, cleanupCt) => + { + /* Own-store scratch database is dropped by ScratchPostgres.DisposeAsync; nothing else to + restore on a database nobody else shares. */ + await Task.CompletedTask; + }); + } + } + + /// + /// #4503: V152's SET LOCAL lock_timeout is derived from + /// so the rung WAITS OUT a running rollup + /// refresh's AccessExclusiveLock on a materialization hypertable rather than failing on it. + /// A second connection locks one rollup's materialization for about 8 s; the migration, started on + /// another task, must still finish once that lock is released. + /// + /// RED at the pre-#4503 head: that build's lock_timeout is a flat '5s', shorter + /// than the 8 s hold, so the migration's DROP INDEX statement fails with 55P03 + /// ("canceling statement due to lock timeout") before the lock is ever released. See the PR body for + /// the captured exception. + /// + [Fact] + public async Task ARunningRollupRefreshHoldingTheMaterializationLockForEightSeconds_IsWaitedOut_AndTheMigrationReaches152() + { + var baseConnectionString = ConnectionString; + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), + "Set DARLING_TEST_PG to a Postgres connection string (with TimescaleDB installed) to run the V152 lock-wait pin."); + + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + + var bodySucceeded = false; + try + { + await PgMigrations.MigrateAsync(connection, ct); + + var timescaleEnabled = await LiveTimescaleProbe.TryEnableAsync(scratch.ConnectionString, ct); + Assert.True(timescaleEnabled, "TimescaleDB must be available on CI for the V152 lock-wait pin"); + + await TimescaleSupport.ConvertToHypertablesAsync(connection, null, ct); + + /* Stop this scratch database's TimescaleDB scheduler (the CollectionHealthAggregateTests + precedent): the refresh policy CreateV151ShapeRollupsAsync installs below would otherwise + launch its own first run and contend with this test's own ACCESS SHARE lock, which measured + as a 40P01 deadlock rather than the clean queue-and-wait this pin means to exercise. */ + await ExecAsync(connection, ct, "SELECT _timescaledb_functions.stop_background_workers()"); + + /* Roll the version stamp back and create the rollups the V151-era way (the default + create_group_indexes shape) — the same fixture CreateV151ShapeRollupsAsync builds for the + upgrade-equivalence pin above. Only that shape has a group index on the hourly view's + materialization for V152's DROP INDEX to actually take an AccessExclusiveLock on; the + current build's already-pruned kept-index shape has nothing there for a lock to contend + over, so the wait this pin measures would never happen against it. */ + await ExecAsync(connection, ct, "DELETE FROM darling_schema_version WHERE version >= " + RungVersion); + using (var version = new NpgsqlCommand("SELECT MAX(version) FROM darling_schema_version", connection)) + { + Assert.Equal(PreviousVersion, Convert.ToInt32(await version.ExecuteScalarAsync(ct))); + } + + await CreateV151ShapeRollupsAsync(connection, ct); + + var (matSchema, matTable) = await MaterializationOfAsync(connection, TimescaleSupport.QueryStoreStatsHourlyView, ct); + + await using var lockConnection = new NpgsqlConnection(scratch.ConnectionString); + await lockConnection.OpenAsync(ct); + await using var lockTransaction = await lockConnection.BeginTransactionAsync(ct); + await using (var lockCommand = new NpgsqlCommand( + $"LOCK TABLE {matSchema}.{matTable} IN ACCESS SHARE MODE", lockConnection, lockTransaction)) + { + await lockCommand.ExecuteNonQueryAsync(ct); + } + + var releaseAfter = Task.Delay(TimeSpan.FromSeconds(8), ct); + + var started = DateTime.UtcNow; + var migrateTask = Task.Run(async () => + { + await using var migrateConnection = new NpgsqlConnection(scratch.ConnectionString); + await migrateConnection.OpenAsync(ct); + return await PgMigrations.MigrateAsync(migrateConnection, ct); + }, ct); + + await releaseAfter; + await lockTransaction.CommitAsync(ct); + + var applied = await migrateTask; + var elapsed = DateTime.UtcNow - started; + + Assert.Equal(PgMigrations.Scripts.Count(m => m.Version >= RungVersion), applied); + Assert.True(elapsed >= TimeSpan.FromSeconds(7), $"expected the migration to have waited out the lock (~8s hold); it finished in {elapsed}"); + + using (var version = new NpgsqlCommand("SELECT MAX(version) FROM darling_schema_version", connection)) + { + Assert.Equal(RungVersion, Convert.ToInt32(await version.ExecuteScalarAsync(ct))); + } + + var indexes = await IndexColumnsAsync(connection, matSchema, matTable, ct); + foreach (var dropped in DroppedFirstColumnsFor(TimescaleSupport.QueryStoreStatsHourlyView)) + { + Assert.DoesNotContain(indexes, idx => idx.Count == 2 && idx[0] == dropped && idx[1] == "bucket"); + } + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, async (cleanup, cleanupCt) => + { + await Task.CompletedTask; + }); + } + } + + /// + /// The keep/drop decision (#4503, restated for this pin): bucket_idx and + /// (server_id, bucket) on all six, plus (server_name, bucket) on the two hourly views. + /// + private static string[] KeptFirstColumnsFor(string view) => + view == TimescaleSupport.QueryStoreStatsHourlyView || view == TimescaleSupport.QueryStoreStatsCorrectedHourlyView + ? new[] { "server_id", "server_name" } + : new[] { "server_id" }; + + private static string[] DroppedFirstColumnsFor(string view) => view switch + { + _ when view == TimescaleSupport.QueryStoreStatsHourlyView || view == TimescaleSupport.QueryStoreStatsCorrectedHourlyView => + new[] { "database_name", "module_name", "query_hash" }, + _ when view == TimescaleSupport.QueryStoreStatsIntervalDailyView => + new[] { "database_name", "execution_type_desc", "first_execution_time", "module_name", "plan_id", "query_hash", "query_id", "replica_role", "runtime_stats_interval_id", "server_name" }, + _ => new[] { "database_name", "module_name", "query_hash", "server_name" }, + }; + + /// + /// Creates the six rollups the way a V151-era build did: TimescaleDB's DEFAULT + /// create_group_indexes (no option at all — every group column gets its own + /// (column, bucket DESC) index), and none of this build's explicit kept-index statements. Built + /// by stripping , timescaledb.create_group_indexes = false out of this build's own CREATE text + /// rather than hand-copying the SELECT, so the fixture's columns can never drift from the real view. + /// + private static async Task CreateV151ShapeRollupsAsync(NpgsqlConnection connection, CancellationToken ct) + { + var createSqls = new[] + { + TimescaleSupport.CreateQueryStoreStatsHourlySql, + TimescaleSupport.CreateQueryStoreStatsIntervalHourlySql, + TimescaleSupport.CreateQueryStoreStatsCorrectedHourlySql, + TimescaleSupport.CreateQueryStoreStatsDailySql, + TimescaleSupport.CreateQueryStoreStatsCorrectedDailySql, + TimescaleSupport.CreateQueryStoreStatsIntervalDailySql, + TimescaleSupport.CreateQueryStoreStatsDayGrainDailySql, + }; + + foreach (var createSql in createSqls) + { + var v151Shape = createSql.Replace(", timescaledb.create_group_indexes = false", string.Empty, StringComparison.Ordinal); + Assert.DoesNotContain("create_group_indexes", v151Shape, StringComparison.Ordinal); + + await using var create = new NpgsqlCommand(v151Shape, connection); + await create.ExecuteNonQueryAsync(ct); + } + + foreach (var view in SixRollups) + { + await using var policy = new NpgsqlCommand( + view.EndsWith("_daily", StringComparison.Ordinal) + ? TimescaleSupport.AddDailyRefreshPolicySql(view) + : TimescaleSupport.AddHourlyRefreshPolicySql(view), connection); + await policy.ExecuteNonQueryAsync(ct); + } + } + + private static async Task SeedQueryStoreStatsAsync(NpgsqlConnection connection, int serverId, DateTime day, CancellationToken ct) + { + await using var insert = new NpgsqlCommand(@" +INSERT INTO collect.query_store_stats + (collection_id, collection_time, server_id, server_name, database_name, module_name, query_hash, + query_id, plan_id, execution_type_desc, replica_role, + runtime_stats_interval_id, interval_start_time_utc, first_execution_time, + execution_count, avg_duration_us, avg_cpu_time_us, max_duration_us, max_cpu_time_us) +SELECT + (extract(epoch FROM $1)::bigint * 1000) + (d.n * 100) + q.n, + $1 + (d.n * interval '1 hour'), + $2, 'upgrade-eq-server-' || $2::text, 'db' || d.n::text, 'dbo.proc' || q.n::text, md5('q' || d.n::text || q.n::text), + 100 + q.n, 1000 + q.n, 'Regular', 'PRIMARY', + 77, $1, $1, + 10 * (q.n + 1), 500, 300, 900, 700 +FROM generate_series(0, 5) AS d(n) CROSS JOIN generate_series(0, 3) AS q(n)", connection); + insert.Parameters.AddWithValue(day); + insert.Parameters.AddWithValue(serverId); + await insert.ExecuteNonQueryAsync(ct); + } + + /// + /// Captures the rows every named reader query returns, ordered so the before/after comparison is + /// stable. Named readers: with and without the + /// database filter, , and one composer-shaped SELECT + /// per view filtering on server_id + a bucket range and on database_name. + /// + private static async Task Rows)>> CaptureReaderRowsAsync(NpgsqlConnection connection, CancellationToken ct) + { + var results = new List<(string Name, List Rows)>(); + + var from = new DateTime(2026, 4, 6, 0, 0, 0, DateTimeKind.Unspecified); + var to = new DateTime(2026, 4, 8, 0, 0, 0, DateTimeKind.Unspecified); + var future = to.AddYears(1); + + foreach (var serverId in new[] { ServerA, ServerB }) + { + results.Add(($"trend-no-db-filter:{serverId}", await CaptureRowsAsync(connection, + QueryStoreTrendRouting.BuildRollupTrendSql(withDatabaseFilter: false), + cmd => + { + cmd.Parameters.AddWithValue(serverId); + cmd.Parameters.AddWithValue(from); + cmd.Parameters.AddWithValue(to); + cmd.Parameters.AddWithValue(future); + }, ct))); + + results.Add(($"trend-with-db-filter:{serverId}", await CaptureRowsAsync(connection, + QueryStoreTrendRouting.BuildRollupTrendSql(withDatabaseFilter: true), + cmd => + { + cmd.Parameters.AddWithValue(serverId); + cmd.Parameters.AddWithValue(from); + cmd.Parameters.AddWithValue(to); + cmd.Parameters.AddWithValue(future); + cmd.Parameters.AddWithValue(new[] { "db0", "db1" }); + }, ct))); + } + + results.Add(("rollup-bounds", await CaptureRowsAsync(connection, QueryStoreTrendRouting.RollupBoundsSql, _ => { }, ct))); + + foreach (var view in SixRollups) + { + foreach (var serverId in new[] { ServerA, ServerB }) + { + results.Add(($"composer-server:{view}:{serverId}", await CaptureRowsAsync(connection, + $"SELECT * FROM collect.{view} WHERE server_id = $1 AND bucket >= $2 AND bucket <= $3 ORDER BY bucket", + cmd => + { + cmd.Parameters.AddWithValue(serverId); + cmd.Parameters.AddWithValue(from); + cmd.Parameters.AddWithValue(to); + }, ct))); + + results.Add(($"composer-database:{view}:{serverId}", await CaptureRowsAsync(connection, + $"SELECT * FROM collect.{view} WHERE server_id = $1 AND database_name = 'db0' AND bucket >= $2 AND bucket <= $3 ORDER BY bucket", + cmd => + { + cmd.Parameters.AddWithValue(serverId); + cmd.Parameters.AddWithValue(from); + cmd.Parameters.AddWithValue(to); + }, ct))); + } + } + + return results; + } + + private static async Task> CaptureRowsAsync( + NpgsqlConnection connection, string sql, Action bind, CancellationToken ct) + { + var rows = new List(); + await using var command = new NpgsqlCommand(sql, connection); + bind(command); + await using var reader = await command.ExecuteReaderAsync(ct); + while (await reader.ReadAsync(ct)) + { + var cells = new string[reader.FieldCount]; + for (var i = 0; i < reader.FieldCount; i++) + { + cells[i] = reader.IsDBNull(i) ? "" : Convert.ToString(reader.GetValue(i), CultureInfo.InvariantCulture) ?? ""; + } + rows.Add(string.Join("|", cells)); + } + return rows; + } + + private static async Task<(string Schema, string Name)> MaterializationOfAsync(NpgsqlConnection connection, string view, CancellationToken ct) + { + await using var command = new NpgsqlCommand( + "SELECT materialization_hypertable_schema, materialization_hypertable_name FROM timescaledb_information.continuous_aggregates WHERE view_schema = 'collect' AND view_name = $1", connection); + command.Parameters.AddWithValue(view); + await using var reader = await command.ExecuteReaderAsync(ct); + Assert.True(await reader.ReadAsync(ct), $"{view} was not created"); + return (reader.GetString(0), reader.GetString(1)); + } + + /// The first two key columns of every btree on the materialization, one list per index — + /// a single-column bucket index reads as a one-element list, never matching the 2-column checks above. + private static async Task>> IndexColumnsAsync(NpgsqlConnection connection, string schema, string table, CancellationToken ct) + { + await using var command = new NpgsqlCommand(@" +SELECT array_agg(a.attname ORDER BY k.ord) +FROM pg_index i +JOIN pg_class c ON c.oid = i.indrelid +JOIN pg_namespace n ON n.oid = c.relnamespace +CROSS JOIN LATERAL unnest(i.indkey) WITH ORDINALITY AS k(attnum, ord) +JOIN pg_attribute a ON a.attrelid = i.indrelid AND a.attnum = k.attnum +WHERE n.nspname = $1 AND c.relname = $2 +GROUP BY i.indexrelid", connection); + command.Parameters.AddWithValue(schema); + command.Parameters.AddWithValue(table); + + var indexes = new List>(); + await using var reader = await command.ExecuteReaderAsync(ct); + while (await reader.ReadAsync(ct)) + { + indexes.Add(((string[])reader.GetValue(0)).ToList()); + } + return indexes; + } + + private static async Task CompressMaterializationAsync(NpgsqlConnection connection, string view, CancellationToken ct) + { + await using (var alter = new NpgsqlCommand( + $"ALTER MATERIALIZED VIEW collect.{view} SET (timescaledb.compress, timescaledb.compress_segmentby = 'server_id')", connection)) + { + await alter.ExecuteNonQueryAsync(ct); + } + + var chunks = await ReadMaterializationChunksAsync(connection, view, ct); + foreach (var chunk in chunks) + { + await using var compress = new NpgsqlCommand($"SELECT compress_chunk('{chunk}'::regclass, if_not_compressed => true)", connection); + await compress.ExecuteNonQueryAsync(ct); + } + } + + private static async Task> ReadMaterializationChunksAsync(NpgsqlConnection connection, string view, CancellationToken ct) + { + await using var read = new NpgsqlCommand($@" +SELECT format('%I.%I', c.chunk_schema, c.chunk_name) +FROM timescaledb_information.chunks AS c +JOIN timescaledb_information.continuous_aggregates AS ca + ON c.hypertable_schema = ca.materialization_hypertable_schema + AND c.hypertable_name = ca.materialization_hypertable_name +WHERE ca.view_schema = 'collect' AND ca.view_name = '{view}' +ORDER BY c.range_start", connection); + var chunks = new List(); + await using var reader = await read.ExecuteReaderAsync(ct); + while (await reader.ReadAsync(ct)) + { + chunks.Add(reader.GetString(0)); + } + return chunks; + } + + /// Bounded retry on 55P03, the same reason IntervalDedupMaterializationIndexesTests retries: a + /// policy's creation-time first run can still be finishing when the manual refresh lands. + private static async Task RefreshAsync(NpgsqlConnection connection, string view, DateTime from, DateTime to, CancellationToken ct) + { + for (var attempt = 1; ; attempt++) + { + try + { + await using var refresh = new NpgsqlCommand( + $"CALL refresh_continuous_aggregate('collect.{view}', $1::timestamp, $2::timestamp)", connection); + refresh.Parameters.AddWithValue(from); + refresh.Parameters.AddWithValue(to); + await refresh.ExecuteNonQueryAsync(ct); + return; + } + catch (PostgresException ex) when (ex.SqlState == "55P03" && attempt < 12) + { + await Task.Delay(TimeSpan.FromSeconds(1), ct); + } + } + } + + private static async Task ExecAsync(NpgsqlConnection connection, CancellationToken ct, string sql) + { + await using var command = new NpgsqlCommand(sql, connection); + await command.ExecuteNonQueryAsync(ct); + } +} diff --git a/Darling/Darling.Tests/IntervalDedupMaterializationIndexesTests.cs b/Darling/Darling.Tests/IntervalDedupMaterializationIndexesTests.cs index ad7dcbe81d..899d039ec9 100644 --- a/Darling/Darling.Tests/IntervalDedupMaterializationIndexesTests.cs +++ b/Darling/Darling.Tests/IntervalDedupMaterializationIndexesTests.cs @@ -63,14 +63,29 @@ public void L1_IsCreatedWithoutGroupIndexes() /// /// The scope pin: every OTHER registered aggregate keeps TimescaleDB's default. Enumerated from the three /// registries the ensure sweep builds from, so a new aggregate is covered the day it is registered. + /// + /// Widened by #4503: a production catalog read found the same measured, zero-idx_scan shape on + /// six more rollups, so they earned the same option L1 has. An aggregate not on that measured list still + /// keeps TimescaleDB's default until it is measured too. /// [Fact] public void EveryOtherAggregate_KeepsTheDefaultGroupIndexes_UntilMeasured() { + var measured = new[] + { + TimescaleSupport.QueryStoreStatsIntervalHourlyView, + TimescaleSupport.QueryStoreStatsHourlyView, + TimescaleSupport.QueryStoreStatsCorrectedHourlyView, + TimescaleSupport.QueryStoreStatsDailyView, + TimescaleSupport.QueryStoreStatsCorrectedDailyView, + TimescaleSupport.QueryStoreStatsIntervalDailyView, + TimescaleSupport.QueryStoreStatsDayGrainDailyView, + }; + var others = TimescaleSupport.HourlyAggregates .Concat(TimescaleSupport.DailyAggregates) .Concat(TimescaleSupport.BaselineAggregates) - .Where(a => !string.Equals(a.View, TimescaleSupport.QueryStoreStatsIntervalHourlyView, StringComparison.Ordinal)) + .Where(a => !measured.Contains(a.View, StringComparer.Ordinal)) .ToList(); Assert.NotEmpty(others); @@ -82,6 +97,20 @@ public void EveryOtherAggregate_KeepsTheDefaultGroupIndexes_UntilMeasured() /* And L1 is in the hourly registry, so the converge below finds a materialization to act on. */ Assert.Contains(TimescaleSupport.HourlyAggregates, a => string.Equals(a.View, TimescaleSupport.QueryStoreStatsIntervalHourlyView, StringComparison.Ordinal)); + + /* The six measured rollups DO carry the option (#4503) — not silently exempted from this pin, asserted + the other way. */ + var measuredAggregates = TimescaleSupport.HourlyAggregates + .Concat(TimescaleSupport.DailyAggregates) + .Concat(TimescaleSupport.BaselineAggregates) + .Where(a => measured.Contains(a.View, StringComparer.Ordinal) && !string.Equals(a.View, TimescaleSupport.QueryStoreStatsIntervalHourlyView, StringComparison.Ordinal)) + .ToList(); + + Assert.Equal(6, measuredAggregates.Count); + foreach (var (createSql, _) in measuredAggregates) + { + Assert.Contains("timescaledb.create_group_indexes = false", createSql, StringComparison.Ordinal); + } } /// diff --git a/Darling/Darling.Tests/MigrationDataMovingRungCensusPins.cs b/Darling/Darling.Tests/MigrationDataMovingRungCensusPins.cs index e3d894d7a0..e7af5297e3 100644 --- a/Darling/Darling.Tests/MigrationDataMovingRungCensusPins.cs +++ b/Darling/Darling.Tests/MigrationDataMovingRungCensusPins.cs @@ -459,17 +459,35 @@ public void ProseAndTriggerClausesNamingTheseShapes_AreNotFindings() ])); } + /// + /// The one rung today whose dynamic SQL is an accepted, hand-costed exception — the scan cannot read + /// past EXECUTE format(...) or a parameterised EXECUTE '...' USING ..., so this name has + /// to be kept in sync by hand rather than derived. V152's DO $$ ... $$ (#4503) uses both forms + /// against the catalog only: EXECUTE '...' INTO ... USING v_view reads a continuous aggregate's + /// materialization name from timescaledb_information, and EXECUTE format('DROP INDEX IF + /// EXISTS %s', ...) then drops a catalog index resolved via pg_index/pg_attribute + /// (metadata, not collected rows) — the opposite direction from every shape + /// tracks (which are all index BUILDS or DML over pre-existing rows), so it gets no + /// entry at all: there is no data-moving cost to declare, only a scan blind + /// spot to name. + /// + private static readonly int[] s_dynamicSqlExemptedRungs = [152]; + /// /// The scan is textual, so dynamic SQL would hide a data-moving statement from it completely. The - /// ladder has none — 109 rungs, and the only EXECUTE is EXECUTE FUNCTION in V17's - /// trigger definitions. Pinned so the blind spot stays theoretical: a rung that builds DDL with - /// format() needs to be costed by hand, and this is where that gets said. + /// ladder has one exempted rung today (V152, named in because + /// its DO $$ ... EXECUTE format(...) drops a catalog index rather than moving data) and + /// otherwise none — the only other EXECUTE is EXECUTE FUNCTION in V17's trigger + /// definitions. Pinned so the blind spot stays theoretical for every other rung: one that builds DDL + /// with format() needs to be costed by hand, added either to this exemption list (if it moves + /// no data) or to (if it does), and this is where that gets said. /// [Fact] public void TheLadderStillContainsNoDynamicSql() { var offenders = PgMigrations.Scripts .Where(m => s_dynamicSql.IsMatch(StripComments(m.Sql))) + .Where(m => !s_dynamicSqlExemptedRungs.Contains(m.Version)) .Select(m => $"V{m.Version.ToString(CultureInfo.InvariantCulture)} ({m.Name})") .ToList(); diff --git a/Darling/Darling.Tests/PgSchemaGeneratorTests.cs b/Darling/Darling.Tests/PgSchemaGeneratorTests.cs index 9542d58be0..31ea91d9da 100644 --- a/Darling/Darling.Tests/PgSchemaGeneratorTests.cs +++ b/Darling/Darling.Tests/PgSchemaGeneratorTests.cs @@ -802,4 +802,74 @@ public void SearchPath_ListsCollectConfigPublicInOrder() { Assert.Equal("collect, config, public", PgSchemaGenerator.SearchPath); } + + /// + /// #4503: the six Query Store rollups' KEPT group indexes must land IDENTICAL on a fresh migrate and on + /// an upgraded store — not merely the same key columns, but the same ORDER and the same NAME, because a + /// store's physical shape should never reveal whether it was installed fresh or upgraded (the same rule + /// pins for the collector + /// tables above). Both sides are compared here by TEXT, since neither side's SQL can be executed + /// offline: (the fresh side, run right + /// after each view's CREATE) must build its CREATE INDEX UNNAMED and end each kept column's + /// statement in , bucket DESC) — an explicit name or an ASC order would make the fresh side's + /// index a DIFFERENT object from an upgraded store's surviving auto-created one, which + /// proves live. 's V152 (the + /// upgraded side) must resolve the SAME kept columns — read from the same per-view drop lists this + /// rung's PR body documents — by catalog SHAPE rather than by a hand-built name, for the reason that + /// migration's own remarks give (a truncated auto-created name is not guessable from the view name + /// alone). + /// + [Fact] + public void QueryStoreRollupKeptIndexes_FreshSql_MatchesTheUpgradedShape_ForAllSixViews() + { + var v152 = PgMigrations.Scripts.Single(m => m.Version == 152).Sql; + + var keptColumnsByView = new (string View, string[] KeptColumns)[] + { + (TimescaleSupport.QueryStoreStatsHourlyView, new[] { "server_id", "server_name" }), + (TimescaleSupport.QueryStoreStatsCorrectedHourlyView, new[] { "server_id", "server_name" }), + (TimescaleSupport.QueryStoreStatsDailyView, new[] { "server_id" }), + (TimescaleSupport.QueryStoreStatsCorrectedDailyView, new[] { "server_id" }), + (TimescaleSupport.QueryStoreStatsIntervalDailyView, new[] { "server_id" }), + (TimescaleSupport.QueryStoreStatsDayGrainDailyView, new[] { "server_id" }), + }; + + var freshSqlByView = new (string View, string Sql)[] + { + (TimescaleSupport.QueryStoreStatsHourlyView, TimescaleSupport.CreateQueryStoreStatsHourlyKeptIndexesSql), + (TimescaleSupport.QueryStoreStatsCorrectedHourlyView, TimescaleSupport.CreateQueryStoreStatsCorrectedHourlyKeptIndexesSql), + (TimescaleSupport.QueryStoreStatsDailyView, TimescaleSupport.CreateQueryStoreStatsDailyKeptIndexesSql), + (TimescaleSupport.QueryStoreStatsCorrectedDailyView, TimescaleSupport.CreateQueryStoreStatsCorrectedDailyKeptIndexesSql), + (TimescaleSupport.QueryStoreStatsIntervalDailyView, TimescaleSupport.CreateQueryStoreStatsIntervalDailyKeptIndexesSql), + (TimescaleSupport.QueryStoreStatsDayGrainDailyView, TimescaleSupport.CreateQueryStoreStatsDayGrainDailyKeptIndexesSql), + }; + + foreach (var (view, keptColumns) in keptColumnsByView) + { + var freshSql = freshSqlByView.Single(f => f.View == view).Sql; + + /* The fresh side names the aggregate it's building the DO block for, and every kept column + gets its own unnamed, DESC-ordered CREATE INDEX — never a hand-built name, never ASC. */ + Assert.Contains($"view_name = '{view}'", freshSql, StringComparison.Ordinal); + + foreach (var column in keptColumns) + { + Assert.Contains($"CREATE INDEX ON %s ({column}, bucket DESC)", freshSql, StringComparison.Ordinal); + Assert.DoesNotContain($"CREATE INDEX IF NOT EXISTS {view}_{column}_bucket_idx", freshSql, StringComparison.Ordinal); + + /* The upgraded side (V152) drops this same view+column pair by SHAPE, never by a + hand-built _materialized_hypertable_N name — the exact bug this rung's PR body traces. */ + Assert.Contains($"'{view}'", v152, StringComparison.Ordinal); + Assert.DoesNotContain("_materialized_hypertable_", v152, StringComparison.Ordinal); + } + } + + /* Both sides are driven by resolving the materialization from timescaledb_information.continuous_aggregates, + never a hard-coded internal name — the one property that makes either side portable across stores. */ + Assert.Contains("timescaledb_information.continuous_aggregates", v152, StringComparison.Ordinal); + foreach (var (_, freshSql) in freshSqlByView) + { + Assert.Contains("timescaledb_information.continuous_aggregates", freshSql, StringComparison.Ordinal); + } + } } diff --git a/Darling/Darling.Tests/TimescaleContinuousAggregateTests.cs b/Darling/Darling.Tests/TimescaleContinuousAggregateTests.cs index 12cc2a1dc0..e2973b7306 100644 --- a/Darling/Darling.Tests/TimescaleContinuousAggregateTests.cs +++ b/Darling/Darling.Tests/TimescaleContinuousAggregateTests.cs @@ -888,7 +888,9 @@ public void QueryStoreStatsHourly_GroupedByComposerDims_CarriesWeightedSums() var sql = TimescaleSupport.CreateQueryStoreStatsHourlySql; Assert.Contains("CREATE MATERIALIZED VIEW IF NOT EXISTS collect.query_store_stats_hourly", sql, StringComparison.Ordinal); - Assert.Contains("WITH (timescaledb.continuous)", sql, StringComparison.Ordinal); + /* #4503: a production catalog read found zero lifetime idx_scan on this rollup's group indexes, so it + earned the same timescaledb.create_group_indexes = false option #3597 gave the interval-hourly L1. */ + Assert.Contains("WITH (timescaledb.continuous, timescaledb.create_group_indexes = false)", sql, StringComparison.Ordinal); Assert.Contains("time_bucket('1 hour', collection_time) AS bucket", sql, StringComparison.Ordinal); Assert.Contains("FROM collect.query_store_stats", sql, StringComparison.Ordinal); /* The COMPOSER's QS dimensions (module_name / query_hash) so a composed QS panel can route here — NOT @@ -1431,7 +1433,9 @@ public void QueryStoreStatsIntervalDaily_RededupsTheIntervalAcrossTheDay_FromL1( var sql = TimescaleSupport.CreateQueryStoreStatsIntervalDailySql; Assert.Contains("CREATE MATERIALIZED VIEW IF NOT EXISTS collect.query_store_stats_interval_daily", sql, StringComparison.Ordinal); - Assert.Contains("WITH (timescaledb.continuous)", sql, StringComparison.Ordinal); + /* #4503: a production catalog read found zero lifetime idx_scan on this rollup's group indexes (eleven + of them on this view alone), so it earned the same option #3597 gave the interval-hourly L1. */ + Assert.Contains("WITH (timescaledb.continuous, timescaledb.create_group_indexes = false)", sql, StringComparison.Ordinal); Assert.Contains($"FROM collect.{TimescaleSupport.QueryStoreStatsIntervalHourlyView}", sql, StringComparison.Ordinal); /* WIDENING, and that is what makes the level legal at all: an identity-width hierarchical CAGG is a diff --git a/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs b/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs index 467d295df9..591a3c71b3 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs @@ -42,6 +42,116 @@ public Migration(int version, string name, string sql) public string Sql { get; } } + /// + /// V152 (#4503) — drops the auto-created per-column GROUP BY index TimescaleDB builds on six Query Store + /// rollups' materialization hypertables, on every store that upgrades through this rung, the same way + /// #3597 did for 's materialization at + /// CREATE time. A production catalog read found the same shape repeated on + /// , , + /// , , + /// and + /// : five group indexes apiece + /// (database_name, module_name, query_hash, server_id, server_name, each + /// paired with bucket DESC) on the first five, eleven on the interval-daily view, and lifetime + /// idx_scan of ZERO on every one of them except server_id (kept) and server_name (kept + /// on the two hourly views only, where reads measurably use it) — the reader check this rung's PR body + /// carries in full. + /// + /// Resolved by view name and by the indexed COLUMN, never by a string-built index name + /// (#4503). A continuous aggregate's materialization hypertable id is assigned at CREATE time and + /// differs per store — the production catalog this rung read from happened to number them + /// 103/106/109/110/112/113, but a fresh or differently-ordered store would not; that part was already + /// handled by resolving materialization_hypertable_name from + /// timescaledb_information.continuous_aggregates. What was NOT safe is building the auto-created + /// index's NAME as a string and passing it to DROP INDEX IF EXISTS: PostgreSQL truncates an + /// identifier at 63 bytes with no hash suffix, and + /// _materialized_hypertable_112_runtime_stats_interval_id_bucket_idx is 68 characters, so the name + /// actually on disk is the 63-byte truncation, not the string this rung would have built — the DROP + /// would have silently no-op'd against a name nothing wears, leaving the real index in place. The + /// DO block below never builds that name: for each view it resolves the materialization's OID, + /// then reads pg_index/pg_attribute directly for a two-key btree whose FIRST key column is + /// one of that view's drop-list columns and whose SECOND key column is bucket — the exact shape + /// create_group_indexes's default builds — and drops whatever index actually carries that shape, + /// by its real (possibly-truncated) name via ::regclass. A single-column index (the kept + /// bucket_idx) and a two-key index whose kept column ISN'T in the drop list both fail the match + /// and are never touched. A plain-PostgreSQL store, which has never created the extension, has no + /// timescaledb_information catalog to read at all, so the whole block returns before the loop — + /// a no-op there, not a per-view skip. A TimescaleDB store missing one of the six CAGGs (one that never + /// enabled a given rollup) skips just that view rather than erroring, and re-running the block after the + /// indexes are already gone is a no-op — the catalog read finds nothing to drop. + /// + /// SET LOCAL lock_timeout, derived from . + /// DROP INDEX takes AccessExclusiveLock on the materialization hypertable and on every one + /// of its chunks, across six hypertables in one transaction, while a background refresh policy can hold + /// the same lock for as long as its own run takes — measured around 264 s on the hourly rollups, well + /// past a flat 5 s. Rather than fail the rung on the very refresh it is racing, the lock_timeout here is + /// set to minus a 20 s margin (280 s): long enough to wait + /// out one whole refresh cycle, but still short enough that the server's own clean, retryable + /// 55P03 always fires before the client-side command + /// timeout would cancel the statement out from under it — if a refresh somehow outlasts even that, the + /// failure is still retryable (lock-not-available is already in the retryable set the service's startup + /// triage carries), so the next start retries the same rung, still at V151, rather than blocking + /// collection. This is a per-STATEMENT wait inside one rung, smaller by construction than + /// , the whole-SESSION budget a sibling migrator polls + /// against; V152 spending up to 280 s of its own command timeout still leaves that budget's other + /// multiples for the rest of the ladder. + /// + private static readonly string V152Sql = @" +SET LOCAL lock_timeout = '" + (MigrationCommandTimeoutSeconds - 20) + @"s'; +DO $$ +DECLARE + v_view text; + v_drop_cols text[]; + v_mat_schema text; + v_mat_table text; + v_mat_oid regclass; + r record; +BEGIN + /* timescaledb_information only exists once the extension has been created, and this runs on stores + where it never was -- reaching it unconditionally raises 42P01. Probed with to_regclass (NULL rather + than an error when absent); no extension means there are no continuous aggregates and so no rollup + group indexes to drop, which makes this rung a no-op there. The per-view read below goes through + EXECUTE so the timescaledb_information reference is parsed only when it runs. */ + IF to_regclass('timescaledb_information.continuous_aggregates') IS NULL THEN + RETURN; + END IF; + + FOR v_view, v_drop_cols IN VALUES + ('query_store_stats_hourly', ARRAY['database_name', 'module_name', 'query_hash']), + ('query_store_stats_corrected_hourly', ARRAY['database_name', 'module_name', 'query_hash']), + ('query_store_stats_daily', ARRAY['database_name', 'module_name', 'query_hash', 'server_name']), + ('query_store_stats_corrected_daily', ARRAY['database_name', 'module_name', 'query_hash', 'server_name']), + ('query_store_stats_interval_daily', ARRAY['database_name', 'execution_type_desc', 'first_execution_time', 'module_name', 'plan_id', 'query_hash', 'query_id', 'replica_role', 'runtime_stats_interval_id', 'server_name']), + ('query_store_stats_daygrain_daily', ARRAY['database_name', 'module_name', 'query_hash', 'server_name']) + LOOP + EXECUTE 'SELECT materialization_hypertable_schema, materialization_hypertable_name + FROM timescaledb_information.continuous_aggregates + WHERE view_schema = ''collect'' AND view_name = $1' + INTO v_mat_schema, v_mat_table + USING v_view; + + IF v_mat_table IS NULL THEN + CONTINUE; + END IF; + + v_mat_oid := format('%I.%I', v_mat_schema, v_mat_table)::regclass; + + FOR r IN + SELECT i.indexrelid::regclass AS idx + FROM pg_index i + JOIN pg_attribute a1 ON a1.attrelid = i.indrelid AND a1.attnum = i.indkey[0] + JOIN pg_attribute a2 ON a2.attrelid = i.indrelid AND a2.attnum = i.indkey[1] + WHERE i.indrelid = v_mat_oid + AND i.indnkeyatts = 2 + AND NOT i.indisunique + AND a1.attname = ANY (v_drop_cols) + AND a2.attname = 'bucket' + LOOP + EXECUTE format('DROP INDEX IF EXISTS %s', r.idx); + END LOOP; + END LOOP; +END $$;"; + public static IReadOnlyList Scripts { get; } = new[] { new Migration(1, "collector-tables", PgSchemaGenerator.GenerateFullSchema()), @@ -231,6 +341,7 @@ holds the ordering. */ new Migration(149, "query-store-liveness-hot-touch", V149Sql), new Migration(150, "collection-log-watermark-and-job-history-indexes", V150Sql), new Migration(151, "ag-group-id", V151Sql), + new Migration(152, "drop-unread-cagg-group-indexes", V152Sql), }; /// diff --git a/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs b/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs index ffab2543d2..53ea15be9c 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs @@ -16,5 +16,5 @@ namespace PerformanceMonitor.Darling.Storage; /// public static class StorageVersion { - public const int SchemaVersion = 151; + public const int SchemaVersion = 152; } diff --git a/Darling/PerformanceMonitor.Darling.Storage/TimescaleSupport.cs b/Darling/PerformanceMonitor.Darling.Storage/TimescaleSupport.cs index 6433efdd88..8dbf7315fb 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/TimescaleSupport.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/TimescaleSupport.cs @@ -2142,7 +2142,7 @@ WHERE sample_interval_seconds IS DISTINCT FROM 0 /// WITH NO DATA + IF NOT EXISTS, one statement. /// public const string CreateQueryStoreStatsHourlySql = @"CREATE MATERIALIZED VIEW IF NOT EXISTS collect.query_store_stats_hourly -WITH (timescaledb.continuous) AS +WITH (timescaledb.continuous, timescaledb.create_group_indexes = false) AS SELECT server_id, server_name, @@ -2160,6 +2160,21 @@ FROM collect.query_store_stats GROUP BY server_id, server_name, database_name, module_name, query_hash, bucket WITH NO DATA"; + /// + /// The two group indexes #4503 KEEPS on 's materialization + /// now that create_group_indexes = false stops TimescaleDB building all five automatically: + /// (server_id, bucket) (every reader filters server + a time range) and (server_name, bucket) + /// (measured 131 lifetime scans on the production catalog this rung read from — the only non-server_id + /// group index either hourly view showed any read against). Built by so a + /// fresh migrate lands the EXACT shape TimescaleDB's own create_group_indexes would have given an + /// upgraded store's survivors: same key order, bucket DESC (not the ASC a hand-typed name would + /// carry), and PostgreSQL's own default name for that index, not a hand-built one — see + /// 's remarks for why an unnamed CREATE INDEX on the view is what makes + /// the two identical. + /// + public static readonly string CreateQueryStoreStatsHourlyKeptIndexesSql = + KeptIndexesSql(QueryStoreStatsHourlyView, "server_id", "server_name"); + /// /// The per-DATABASE query_stats rollup (#1661). Added rather than folded into /// deliberately: TimescaleDB cannot ALTER columns into a continuous @@ -2403,7 +2418,7 @@ FROM collect.procedure_stats_hourly /// duration_us_weighted_sum / execution_count_sum across days) and MAX the peaks. Same column NAMES as the /// hourly, so ComposeCaggValueMapper reads both with no change. Explicit-time_bucket GROUP BY. public const string CreateQueryStoreStatsDailySql = @"CREATE MATERIALIZED VIEW IF NOT EXISTS collect.query_store_stats_daily -WITH (timescaledb.continuous) AS +WITH (timescaledb.continuous, timescaledb.create_group_indexes = false) AS SELECT server_id, server_name, @@ -2421,6 +2436,15 @@ FROM collect.query_store_stats_hourly GROUP BY server_id, server_name, database_name, module_name, query_hash, time_bucket('1 day', bucket) WITH NO DATA"; + /// + /// The one group index #4503 KEEPS on 's materialization: + /// (server_id, bucket). Unlike the hourly pair, the production catalog showed no read against + /// server_name on the daily tier, so only the universal server dimension survives here. Built by + /// , same reason as the hourly pair above. + /// + public static readonly string CreateQueryStoreStatsDailyKeptIndexesSql = + KeptIndexesSql(QueryStoreStatsDailyView, "server_id"); + /* ═══════════ the CORRECTED Query Store rollups (#1849) ═══════════ WHY THREE NEW OBJECTS INSTEAD OF FIXING THE TWO ABOVE. A continuous aggregate's columns cannot be @@ -2563,7 +2587,7 @@ FROM collect.query_store_stats /// reads L1 rather than this view. /// public const string CreateQueryStoreStatsCorrectedHourlySql = @"CREATE MATERIALIZED VIEW IF NOT EXISTS collect.query_store_stats_corrected_hourly -WITH (timescaledb.continuous) AS +WITH (timescaledb.continuous, timescaledb.create_group_indexes = false) AS SELECT server_id, server_name, @@ -2581,6 +2605,23 @@ FROM collect.query_store_stats_interval_hourly GROUP BY server_id, server_name, database_name, module_name, query_hash, time_bucket('1 hour', bucket) WITH NO DATA"; + /// + /// The two group indexes #4503 KEEPS on 's + /// materialization: (server_id, bucket) for every reader's server + time-range filter, and + /// (server_name, bucket) for a scoped composer read — not the same measured-131-scan reason as + /// the legacy hourly view above; this materialization's own server_name index showed 0 lifetime + /// scans in the production catalog read, and is kept for the composer's scoped-panel reads instead. + /// database_name's group index is DROPPED even though + /// can filter on it (rollupFilter): that + /// same read also filters server_id = $1 plus the bucket range, so it is served by the + /// (server_id, bucket) index kept here (a composed panel filtering on database_name gets + /// the same rows via that index, just without a secondary key on the dropped column) — and the + /// production catalog showed ZERO lifetime scans of the real database_name index on this view's + /// chunks. See the PR body's reader table. + /// + public static readonly string CreateQueryStoreStatsCorrectedHourlyKeptIndexesSql = + KeptIndexesSql(QueryStoreStatsCorrectedHourlyView, "server_id", "server_name"); + /// /// The corrected composer-grain DAILY rollup (#1849) — the same columns as /// at a 1-day bucket, sourced from L1 DIRECTLY. @@ -2594,7 +2635,7 @@ FROM collect.query_store_stats_interval_hourly /// Kept indefinitely (no retention policy), like the other daily rollups. /// public const string CreateQueryStoreStatsCorrectedDailySql = @"CREATE MATERIALIZED VIEW IF NOT EXISTS collect.query_store_stats_corrected_daily -WITH (timescaledb.continuous) AS +WITH (timescaledb.continuous, timescaledb.create_group_indexes = false) AS SELECT server_id, server_name, @@ -2612,6 +2653,14 @@ FROM collect.query_store_stats_interval_hourly GROUP BY server_id, server_name, database_name, module_name, query_hash, time_bucket('1 day', bucket) WITH NO DATA"; + /// + /// The one group index #4503 KEEPS on 's + /// materialization: (server_id, bucket). No server_name pair here — the daily tier showed + /// no reads against it on the production catalog, unlike the two hourly views. + /// + public static readonly string CreateQueryStoreStatsCorrectedDailyKeptIndexesSql = + KeptIndexesSql(QueryStoreStatsCorrectedDailyView, "server_id"); + /// /// L2 of the corrected Query Store rollups (#1869): L1 re-deduped at the DAY grain — one row per interval /// identity per DAY, projecting that interval's LAST snapshot of the day. @@ -2644,7 +2693,7 @@ FROM collect.query_store_stats_interval_hourly /// (). /// public const string CreateQueryStoreStatsIntervalDailySql = @"CREATE MATERIALIZED VIEW IF NOT EXISTS collect.query_store_stats_interval_daily -WITH (timescaledb.continuous) AS +WITH (timescaledb.continuous, timescaledb.create_group_indexes = false) AS SELECT server_id, server_name, @@ -2671,6 +2720,17 @@ FROM collect.query_store_stats_interval_hourly time_bucket('1 day', bucket) WITH NO DATA"; + /// + /// The one group index #4503 KEEPS on 's + /// materialization: (server_id, bucket). Every OTHER group index this view's default would have + /// built is dropped — this is L2, "not a read target" by its own doc comment above, and the production + /// catalog showed zero reads on every one of its eleven group indexes including this one, but + /// server_id is kept anyway for the same universal-dimension reason every other view here keeps + /// it, not because a specific reader was found. + /// + public static readonly string CreateQueryStoreStatsIntervalDailyKeptIndexesSql = + KeptIndexesSql(QueryStoreStatsIntervalDailyView, "server_id"); + /// /// The composer-grain DAILY rollup computed from the DAY-grain dedup (#1869) — /// 's column set to the byte, sourced from L2 instead @@ -2689,7 +2749,7 @@ FROM collect.query_store_stats_interval_hourly /// worth its cost: the daily tier is the one whose numbers persist and get compared year over year. /// public const string CreateQueryStoreStatsDayGrainDailySql = @"CREATE MATERIALIZED VIEW IF NOT EXISTS collect.query_store_stats_daygrain_daily -WITH (timescaledb.continuous) AS +WITH (timescaledb.continuous, timescaledb.create_group_indexes = false) AS SELECT server_id, server_name, @@ -2707,6 +2767,13 @@ FROM collect.query_store_stats_interval_daily GROUP BY server_id, server_name, database_name, module_name, query_hash, time_bucket('1 day', bucket) WITH NO DATA"; + /// + /// The one group index #4503 KEEPS on 's + /// materialization: (server_id, bucket), same reasoning as the other daily rollups above. + /// + public static readonly string CreateQueryStoreStatsDayGrainDailyKeptIndexesSql = + KeptIndexesSql(QueryStoreStatsDayGrainDailyView, "server_id"); + /// /// The HOURLY refresh window: each hourly continuous aggregate re-materializes [now - 1 day, /// now - 1 hour] on every run. @@ -5485,6 +5552,16 @@ calls nothing. See SetOffGridMaterializationChunkIntervalSql. */ await width.ExecuteNonQueryAsync(cancellationToken); } + /* #4503: a fresh migrate gets the SAME kept-index shape an upgraded store reaches through + V152's DO block — the CREATE INDEX statements ride right after this view's own CREATE, + idempotent by a catalog read rather than IF NOT EXISTS (an unnamed CREATE INDEX has no + name to guard with one), so every start after the first is a read with no write. */ + if (QueryStoreRollupKeptIndexesSql.TryGetValue(view, out var keptIndexesSql)) + { + using var keptIndexes = new NpgsqlCommand(keptIndexesSql, connection) { CommandTimeout = SetupTimeoutSeconds }; + await keptIndexes.ExecuteNonQueryAsync(cancellationToken); + } + if (policyFor is null) { /* #3653 LC: FrozenRollupAggregates carries no policy builder for this view. Detach @@ -9849,6 +9926,90 @@ public static readonly (string CreateSql, string View, Func PolicySql)[] (CreateCollectionHealthHourlySql, CollectionHealthHourlyView, AddCollectionHealthRefreshPolicySql), }; + /// + /// Builds the DO block that gives a FRESH migrate the EXACT index a store upgrading through + /// V152's own DO block reaches by keeping (rather than dropping) its surviving auto-created group + /// index, for one kept on — called once per kept + /// column, so a view keeping two (the hourly pair's server_id and server_name) gets two + /// separate statements concatenated by . + /// + /// Why an unnamed CREATE INDEX resolved onto the materialization from the catalog, not + /// a hand-typed name. The DO block below issues its CREATE INDEX directly against the + /// materialization OID it just resolved (v_mat_oid), not against the view — TimescaleDB's own + /// create_group_indexes is what issues an unnamed index against the continuous aggregate's VIEW + /// and lets PostgreSQL route it to the materialization, which names it by ITS OWN default-naming rule + /// (<table>_<column>_bucket_idx, truncated to 63 bytes with PostgreSQL's own + /// de-duplicating suffix if that truncation collides) — the same result this code reaches, never a string this + /// code builds. A hand-built name matching that pattern for five of the six views would still be WRONG + /// for the sixth: query_store_stats_interval_daily's materialization name is itself a generated + /// _materialized_hypertable_N, wider than the fixed views' own table names, so a name built from + /// the VIEW's name would drift from what PostgreSQL actually assigns the materialization's copy — the + /// exact bug #4503's PR body traces on the upgrade side ('s V152 remarks) and + /// the reason this resolves the materialization from the catalog and lets PostgreSQL name the index, + /// the same as an upgraded store's survivor. bucket DESC, never ASC, for the same reason: an + /// upgraded store's survivor was built DESC, and an index with a different sort order is a DIFFERENT + /// index to 's fresh-vs-upgraded comparison. + /// + /// Idempotent by a catalog read, not by IF NOT EXISTS (an unnamed CREATE INDEX + /// has no name to guard with one — a second unnamed CREATE INDEX on the same columns would create + /// a SECOND index under PostgreSQL's own collision-suffixed name). The DO block instead reads + /// pg_index/pg_attribute for a two-key btree already carrying + /// then bucket — the same shape-matching rule 's V152 drop uses on the + /// upgrade side — and issues the CREATE INDEX only when that read finds nothing, so every start + /// after the first is a read with no write. A missing view (v_mat_oid IS NULL, a plain-PostgreSQL + /// store or one where this aggregate never enabled) is skipped rather than erroring. + /// + private static string KeptIndexesSql(string view, string column) => $@" +DO $$ +DECLARE + v_mat_oid regclass; +BEGIN + SELECT format('%I.%I', materialization_hypertable_schema, materialization_hypertable_name)::regclass + INTO v_mat_oid + FROM timescaledb_information.continuous_aggregates + WHERE view_schema = 'collect' AND view_name = '{view}'; + + IF v_mat_oid IS NULL THEN + RETURN; + END IF; + + IF NOT EXISTS ( + SELECT 1 + FROM pg_index i + JOIN pg_attribute a1 ON a1.attrelid = i.indrelid AND a1.attnum = i.indkey[0] + JOIN pg_attribute a2 ON a2.attrelid = i.indrelid AND a2.attnum = i.indkey[1] + WHERE i.indrelid = v_mat_oid + AND i.indnatts = 2 + AND a1.attname = '{column}' + AND a2.attname = 'bucket' + ) THEN + EXECUTE format('CREATE INDEX ON %s ({column}, bucket DESC)', v_mat_oid); + END IF; +END $$;"; + + /// Concatenates one block per kept column on . + private static string KeptIndexesSql(string view, params string[] columns) => + string.Join("\n", columns.Select(column => KeptIndexesSql(view, column))); + + /// + /// #4503: the six Query Store rollups whose Create...Sql now carries create_group_indexes = + /// false, mapped to the follow-on KeptIndexesSql DO block(s) that give a FRESH + /// store the same kept-index shape a store that upgrades through V152 reaches by resolving and keeping + /// (rather than dropping) its own two-key group indexes. Read by + /// right after each view's CREATE, so a fresh migrate and an upgraded store land the identical index — + /// same key order, bucket DESC, and the same PostgreSQL-assigned name + /// ('s fresh-vs-upgraded comparison checks both). + /// + public static readonly IReadOnlyDictionary QueryStoreRollupKeptIndexesSql = new Dictionary(StringComparer.Ordinal) + { + [QueryStoreStatsHourlyView] = CreateQueryStoreStatsHourlyKeptIndexesSql, + [QueryStoreStatsCorrectedHourlyView] = CreateQueryStoreStatsCorrectedHourlyKeptIndexesSql, + [QueryStoreStatsDailyView] = CreateQueryStoreStatsDailyKeptIndexesSql, + [QueryStoreStatsCorrectedDailyView] = CreateQueryStoreStatsCorrectedDailyKeptIndexesSql, + [QueryStoreStatsIntervalDailyView] = CreateQueryStoreStatsIntervalDailyKeptIndexesSql, + [QueryStoreStatsDayGrainDailyView] = CreateQueryStoreStatsDayGrainDailyKeptIndexesSql, + }; + /// Is (bare or collect.-qualified) one of ? public static bool IsOffGridAggregate(string? view) { diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs index 2c22265a1a..b01d47799a 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs @@ -994,7 +994,31 @@ AND EXISTS (SELECT 1 FROM pg_indexes WHERE schemaname = 'collect' AND indexname table existence cannot separate the rungs. It is not yet read by any viewer surface, so this gate rests on the standing invariant alone. Named only in this probe line, never in prose, per the V71 finding. */ - EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'ag_replica_states' AND column_name = 'group_id')"; + EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'ag_replica_states' AND column_name = 'group_id'), + /* V152 (#4503) probes NEGATIVELY, the V149 shape: the six Query Store rollups' auto-created two-key + group index (first key one of the rung's drop-list columns, second key bucket) is ABSENT. This walks + query_store_stats_hourly's own pg_rewrite/pg_depend dependency to its materialization hypertable + rather than the TimescaleDB information views, so it needs no pg_extension guard. ANDed with V151's + own sentinel, copied verbatim, rather than a to_regclass(...) IS NOT NULL existence check on the + view: a plain-PostgreSQL store has no rollup at all, so to_regclass(...) is NULL there at every + version, the dependency walk finds no rows, and the bare NOT EXISTS reads that absence as ""index + dropped"" — misreporting a plain-PostgreSQL store at V151 as V152. That store is harmless to leave + unresolved here: V152 is a no-op without TimescaleDB, so a plain-PostgreSQL store's schema at V151 + and V152 is identical, and ANDing with V151's own sentinel makes this arm require V151 first, so a + store below V151 fails that sentinel and falls through to the next arm instead. It is not yet read + by any viewer surface, so this gate rests on the standing invariant alone. Named only in this probe + line, never in prose, per the V71 finding. */ + (NOT EXISTS ( + SELECT 1 + FROM pg_rewrite r + JOIN pg_depend d ON d.objid = r.oid + JOIN pg_class matc ON matc.oid = d.refobjid AND matc.oid <> r.ev_class + JOIN pg_index i ON i.indrelid = matc.oid AND i.indnkeyatts = 2 + JOIN pg_attribute a1 ON a1.attrelid = matc.oid AND a1.attnum = i.indkey[0] AND a1.attname = 'query_hash' + JOIN pg_attribute a2 ON a2.attrelid = matc.oid AND a2.attnum = i.indkey[1] AND a2.attname = 'bucket' + WHERE r.ev_class = to_regclass('collect.query_store_stats_hourly') + ) + AND EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'ag_replica_states' AND column_name = 'group_id'))"; /// The store schema version this viewer build requires — the highest migration it knows /// (). The connect-time gate blocks a store below this. @@ -1016,7 +1040,7 @@ AND EXISTS (SELECT 1 FROM pg_indexes WHERE schemaname = 'collect' AND indexname await using var reader = await command.ExecuteReaderAsync(cancellationToken); if (await reader.ReadAsync(cancellationToken)) { - return MapProbedSchemaVersion(reader.GetBoolean(0), reader.GetBoolean(1), reader.GetBoolean(2), reader.GetBoolean(3), reader.GetBoolean(4), reader.GetBoolean(5), reader.GetBoolean(6), reader.GetBoolean(7), reader.GetBoolean(8), reader.GetBoolean(9), reader.GetBoolean(10), reader.GetBoolean(11), reader.GetBoolean(12), reader.GetBoolean(13), reader.GetBoolean(14), reader.GetBoolean(15), reader.GetBoolean(16), reader.GetBoolean(17), reader.GetBoolean(18), reader.GetBoolean(19), reader.GetBoolean(20), reader.GetBoolean(21), reader.GetBoolean(22), reader.GetBoolean(23), reader.GetBoolean(24), reader.GetBoolean(25), reader.GetBoolean(26), reader.GetBoolean(27), reader.GetBoolean(28), reader.GetBoolean(29), reader.GetBoolean(30), reader.GetBoolean(31), reader.GetBoolean(32), reader.GetBoolean(33), reader.GetBoolean(34), reader.GetBoolean(35), reader.GetBoolean(36), reader.GetBoolean(37), reader.GetBoolean(38), reader.GetBoolean(39), reader.GetBoolean(40), reader.GetBoolean(41), reader.GetBoolean(42), reader.GetBoolean(43), reader.GetBoolean(44), reader.GetBoolean(45), reader.GetBoolean(46), reader.GetBoolean(47), reader.GetBoolean(48), reader.GetBoolean(49), reader.GetBoolean(50), reader.GetBoolean(51), reader.GetBoolean(52), reader.GetBoolean(53), reader.GetBoolean(54), reader.GetBoolean(55), reader.GetBoolean(56), reader.GetBoolean(57), reader.GetBoolean(58), reader.GetBoolean(59), reader.GetBoolean(60), reader.GetBoolean(61), reader.GetBoolean(62), reader.GetBoolean(63), reader.GetBoolean(64), reader.GetBoolean(65), reader.GetBoolean(66), reader.GetBoolean(67), reader.GetBoolean(68), reader.GetBoolean(69), reader.GetBoolean(70), reader.GetBoolean(71), reader.GetBoolean(72), reader.GetBoolean(73), reader.GetBoolean(74), reader.GetBoolean(75), reader.GetBoolean(76), reader.GetBoolean(77), reader.GetBoolean(78), reader.GetBoolean(79), reader.GetBoolean(80), reader.GetBoolean(81), reader.GetBoolean(82), reader.GetBoolean(83), reader.GetBoolean(84), reader.GetBoolean(85), reader.GetBoolean(86), reader.GetBoolean(87), reader.GetBoolean(88), reader.GetBoolean(89), reader.GetBoolean(90), reader.GetBoolean(91), reader.GetBoolean(92), reader.GetBoolean(93), reader.GetBoolean(94), reader.GetBoolean(95), reader.GetBoolean(96), reader.GetBoolean(97), reader.GetBoolean(98), reader.GetBoolean(99), reader.GetBoolean(100), reader.GetBoolean(101), reader.GetBoolean(102), reader.GetBoolean(103), reader.GetBoolean(104), reader.GetBoolean(105), reader.GetBoolean(106), reader.GetBoolean(107), reader.GetBoolean(108), reader.GetBoolean(109), reader.GetBoolean(110), reader.GetBoolean(111), reader.GetBoolean(112), reader.GetBoolean(113), reader.GetBoolean(114), reader.GetBoolean(115), reader.GetBoolean(116), reader.GetBoolean(117), reader.GetBoolean(118), reader.GetBoolean(119), reader.GetBoolean(120), reader.GetBoolean(121), reader.GetBoolean(122), reader.GetBoolean(123), reader.GetBoolean(124), reader.GetBoolean(125), reader.GetBoolean(126)); + return MapProbedSchemaVersion(reader.GetBoolean(0), reader.GetBoolean(1), reader.GetBoolean(2), reader.GetBoolean(3), reader.GetBoolean(4), reader.GetBoolean(5), reader.GetBoolean(6), reader.GetBoolean(7), reader.GetBoolean(8), reader.GetBoolean(9), reader.GetBoolean(10), reader.GetBoolean(11), reader.GetBoolean(12), reader.GetBoolean(13), reader.GetBoolean(14), reader.GetBoolean(15), reader.GetBoolean(16), reader.GetBoolean(17), reader.GetBoolean(18), reader.GetBoolean(19), reader.GetBoolean(20), reader.GetBoolean(21), reader.GetBoolean(22), reader.GetBoolean(23), reader.GetBoolean(24), reader.GetBoolean(25), reader.GetBoolean(26), reader.GetBoolean(27), reader.GetBoolean(28), reader.GetBoolean(29), reader.GetBoolean(30), reader.GetBoolean(31), reader.GetBoolean(32), reader.GetBoolean(33), reader.GetBoolean(34), reader.GetBoolean(35), reader.GetBoolean(36), reader.GetBoolean(37), reader.GetBoolean(38), reader.GetBoolean(39), reader.GetBoolean(40), reader.GetBoolean(41), reader.GetBoolean(42), reader.GetBoolean(43), reader.GetBoolean(44), reader.GetBoolean(45), reader.GetBoolean(46), reader.GetBoolean(47), reader.GetBoolean(48), reader.GetBoolean(49), reader.GetBoolean(50), reader.GetBoolean(51), reader.GetBoolean(52), reader.GetBoolean(53), reader.GetBoolean(54), reader.GetBoolean(55), reader.GetBoolean(56), reader.GetBoolean(57), reader.GetBoolean(58), reader.GetBoolean(59), reader.GetBoolean(60), reader.GetBoolean(61), reader.GetBoolean(62), reader.GetBoolean(63), reader.GetBoolean(64), reader.GetBoolean(65), reader.GetBoolean(66), reader.GetBoolean(67), reader.GetBoolean(68), reader.GetBoolean(69), reader.GetBoolean(70), reader.GetBoolean(71), reader.GetBoolean(72), reader.GetBoolean(73), reader.GetBoolean(74), reader.GetBoolean(75), reader.GetBoolean(76), reader.GetBoolean(77), reader.GetBoolean(78), reader.GetBoolean(79), reader.GetBoolean(80), reader.GetBoolean(81), reader.GetBoolean(82), reader.GetBoolean(83), reader.GetBoolean(84), reader.GetBoolean(85), reader.GetBoolean(86), reader.GetBoolean(87), reader.GetBoolean(88), reader.GetBoolean(89), reader.GetBoolean(90), reader.GetBoolean(91), reader.GetBoolean(92), reader.GetBoolean(93), reader.GetBoolean(94), reader.GetBoolean(95), reader.GetBoolean(96), reader.GetBoolean(97), reader.GetBoolean(98), reader.GetBoolean(99), reader.GetBoolean(100), reader.GetBoolean(101), reader.GetBoolean(102), reader.GetBoolean(103), reader.GetBoolean(104), reader.GetBoolean(105), reader.GetBoolean(106), reader.GetBoolean(107), reader.GetBoolean(108), reader.GetBoolean(109), reader.GetBoolean(110), reader.GetBoolean(111), reader.GetBoolean(112), reader.GetBoolean(113), reader.GetBoolean(114), reader.GetBoolean(115), reader.GetBoolean(116), reader.GetBoolean(117), reader.GetBoolean(118), reader.GetBoolean(119), reader.GetBoolean(120), reader.GetBoolean(121), reader.GetBoolean(122), reader.GetBoolean(123), reader.GetBoolean(124), reader.GetBoolean(125), reader.GetBoolean(126), reader.GetBoolean(127)); } return null; @@ -1041,7 +1065,7 @@ AND EXISTS (SELECT 1 FROM pg_indexes WHERE schemaname = 'collect' AND indexname /// is unit-tested without a live store; any schema bump past the newest arm trips the pinning test that keeps /// this in step with . /// - internal static int MapProbedSchemaVersion(bool hasConfigControlPlane, bool hasAlertDeliveryOverride, bool hasAnalysisState, bool hasAlertTuningKnobs, bool hasDefaultTraceEvents, bool hasIndexObjectStatsLatestIndex, bool hasCollectionLogHypertableOrPlainPg, bool hasJobHistory, bool hasAgentStatus, bool hasGenericWebhook, bool hasDeadlocksDatabaseName, bool hasQueryStoreReplicaRole, bool hasLongQueryCompletions, bool hasWebDashboardConfig, bool hasCustomViews, bool hasServerTags, bool hasConnectionRefireKnobs = false, bool hasAgCollectors = false, bool hasAgAlertKnobs = false, bool hasAgLatencyColumns = false, bool hasAgDisconnectRefire = false, bool hasPayloadDimensions = false, bool hasDimFloorIndexes = false, bool hasBlockingWaitThreshold = false, bool hasQueryStoreIntervalIdentity = false, bool hasPagerDutyWebhook = false, bool hasPagerDutyProxy = false, bool hasCollectorState = false, bool hasPlanCorrection = false, bool hasPvsStats = false, bool hasPvsPressureKnobs = false, bool hasDatabaseStateAlert = false, bool hasServerTagColour = false, bool hasQueryStatsHostObject = false, bool hasFindingDrillDown = false, bool hasStoreMetrics = false, bool hasPlanDimGzip = false, bool hasSelfAlertKnobs = false, bool hasJobMetricsColumns = false, bool hasJobCadenceKnob = false, bool hasBackfillSwitch = false, bool hasCollectorMemoryKnobs = false, bool hasDatabaseStateEdgeMemory = false, bool hasIncidentOccurrences = false, bool hasPlanXmlCompressionKnob = false, bool hasMonitoredServerEngine = false, bool hasPgBlockingEdges = false, bool hasQueryStorePlanMap = false, bool hasPgStatementText = false, bool hasQueryStoreText = false, bool hasPlanContentRetentionKnob = false, bool hasQueryStoreHealth = false, bool hasQueryStoreTextHash = false, bool hasComposeTimeoutKnob = false, bool hasFileGrowthAlert = false, bool hasCollectionLogFanoutRollup = false, bool hasTempDbMaxSize = false, bool hasServerEngineKind = false, bool hasPgDatabaseStats = false, bool hasPgIndexUsageStats = false, bool hasPgTableBloatStats = false, bool hasPgSessionStates = false, bool hasPgPlanCaptureReadiness = false, bool hasPgWriteStats = false, bool hasPgExtensionAvailability = false, bool hasPgLockStats = false, bool hasPgColumnStats = false, bool hasPgReplicationStats = false, bool hasPgBufferUsage = false, bool hasPgIndexBloat = false, bool hasPgPerDatabaseAttribution = false, bool hasPgWaitSampling = false, bool hasPgKernelStats = false, bool hasPgPredicateStats = false, bool hasPgPlanCapture = false, bool hasPgMajorVersion = false, bool hasPg18IoBytes = false, bool hasPgServerConfig = false, bool hasPgDeadlocks = false, bool hasPgDeadlockIdentity = false, bool hasCollectorCost = false, bool hasPgCpuUtilization = false, bool hasPlanForceActions = false, bool hasCollectionLogPhaseSplit = false, bool hasCollectionLogDrainForensics = false, bool hasCollectionLogFetchPhaseSums = false, bool hasStoreLogSelfMonitoring = false, bool hasCollectorStallProbes = false, bool hasRemediationCredentialAndActor = false, bool hasPgIndexBloatEstimate = false, bool hasPgCpuCapacityHeadroom = false, bool hasCustomAlertCore = false, bool hasMuteRuleReloadBeacon = false, bool hasBuiltinAlertPersistence = false, bool hasRetentionHoldRatioKnobs = false, bool hasDeadlockRateBandKnobs = false, bool hasOversizedPlanBacklog = false, bool hasPgAlertCountKnobs = false, bool hasFleetSweepState = false, bool hasFleetSweepCadenceKnobs = false, bool hasCollectorScheduleDatabases = false, bool hasSelfDiskWarnGbFloor = false, bool hasDeltaFamilyIntervalColumns = false, bool hasDeltaFamilyIntervalCompletion = false, bool hasPgLogEvents = false, bool hasPgLogEventMetrics = false, bool hasNotificationRoutes = false, bool hasPerfmonCounterType = false, bool hasPgNumbackendsAndSampledMs = false, bool hasTimeHonesty = false, bool hasLrqExclusionKnob = false, bool hasPgDatabaseSizeStatsAndHostMemory = false, bool hasQsCaptureModeRouteKnobToast = false, bool hasPgServerConfigDatabaseRoleOverrides = false, bool hasPostmasterStartTime = false, bool hasCheckpointsTimed = false, bool hasCollectionCaveats = false, bool hasIndexObjectStatsServerTimeIndex = false, bool hasQueryStoreIntervalLatest = false, bool hasRawChunkIntervalRungHistory = false, bool hasQueryStoreIntervalWide = false, bool hasManagedConfVerdicts = false, bool hasComposeTimeoutSixty = false, bool hasReadLatency = false, bool hasHotLivenessTouch = false, bool hasCollectionLogWatermarkAndJobHistoryIndexes = false, bool hasAgGroupId = false) + internal static int MapProbedSchemaVersion(bool hasConfigControlPlane, bool hasAlertDeliveryOverride, bool hasAnalysisState, bool hasAlertTuningKnobs, bool hasDefaultTraceEvents, bool hasIndexObjectStatsLatestIndex, bool hasCollectionLogHypertableOrPlainPg, bool hasJobHistory, bool hasAgentStatus, bool hasGenericWebhook, bool hasDeadlocksDatabaseName, bool hasQueryStoreReplicaRole, bool hasLongQueryCompletions, bool hasWebDashboardConfig, bool hasCustomViews, bool hasServerTags, bool hasConnectionRefireKnobs = false, bool hasAgCollectors = false, bool hasAgAlertKnobs = false, bool hasAgLatencyColumns = false, bool hasAgDisconnectRefire = false, bool hasPayloadDimensions = false, bool hasDimFloorIndexes = false, bool hasBlockingWaitThreshold = false, bool hasQueryStoreIntervalIdentity = false, bool hasPagerDutyWebhook = false, bool hasPagerDutyProxy = false, bool hasCollectorState = false, bool hasPlanCorrection = false, bool hasPvsStats = false, bool hasPvsPressureKnobs = false, bool hasDatabaseStateAlert = false, bool hasServerTagColour = false, bool hasQueryStatsHostObject = false, bool hasFindingDrillDown = false, bool hasStoreMetrics = false, bool hasPlanDimGzip = false, bool hasSelfAlertKnobs = false, bool hasJobMetricsColumns = false, bool hasJobCadenceKnob = false, bool hasBackfillSwitch = false, bool hasCollectorMemoryKnobs = false, bool hasDatabaseStateEdgeMemory = false, bool hasIncidentOccurrences = false, bool hasPlanXmlCompressionKnob = false, bool hasMonitoredServerEngine = false, bool hasPgBlockingEdges = false, bool hasQueryStorePlanMap = false, bool hasPgStatementText = false, bool hasQueryStoreText = false, bool hasPlanContentRetentionKnob = false, bool hasQueryStoreHealth = false, bool hasQueryStoreTextHash = false, bool hasComposeTimeoutKnob = false, bool hasFileGrowthAlert = false, bool hasCollectionLogFanoutRollup = false, bool hasTempDbMaxSize = false, bool hasServerEngineKind = false, bool hasPgDatabaseStats = false, bool hasPgIndexUsageStats = false, bool hasPgTableBloatStats = false, bool hasPgSessionStates = false, bool hasPgPlanCaptureReadiness = false, bool hasPgWriteStats = false, bool hasPgExtensionAvailability = false, bool hasPgLockStats = false, bool hasPgColumnStats = false, bool hasPgReplicationStats = false, bool hasPgBufferUsage = false, bool hasPgIndexBloat = false, bool hasPgPerDatabaseAttribution = false, bool hasPgWaitSampling = false, bool hasPgKernelStats = false, bool hasPgPredicateStats = false, bool hasPgPlanCapture = false, bool hasPgMajorVersion = false, bool hasPg18IoBytes = false, bool hasPgServerConfig = false, bool hasPgDeadlocks = false, bool hasPgDeadlockIdentity = false, bool hasCollectorCost = false, bool hasPgCpuUtilization = false, bool hasPlanForceActions = false, bool hasCollectionLogPhaseSplit = false, bool hasCollectionLogDrainForensics = false, bool hasCollectionLogFetchPhaseSums = false, bool hasStoreLogSelfMonitoring = false, bool hasCollectorStallProbes = false, bool hasRemediationCredentialAndActor = false, bool hasPgIndexBloatEstimate = false, bool hasPgCpuCapacityHeadroom = false, bool hasCustomAlertCore = false, bool hasMuteRuleReloadBeacon = false, bool hasBuiltinAlertPersistence = false, bool hasRetentionHoldRatioKnobs = false, bool hasDeadlockRateBandKnobs = false, bool hasOversizedPlanBacklog = false, bool hasPgAlertCountKnobs = false, bool hasFleetSweepState = false, bool hasFleetSweepCadenceKnobs = false, bool hasCollectorScheduleDatabases = false, bool hasSelfDiskWarnGbFloor = false, bool hasDeltaFamilyIntervalColumns = false, bool hasDeltaFamilyIntervalCompletion = false, bool hasPgLogEvents = false, bool hasPgLogEventMetrics = false, bool hasNotificationRoutes = false, bool hasPerfmonCounterType = false, bool hasPgNumbackendsAndSampledMs = false, bool hasTimeHonesty = false, bool hasLrqExclusionKnob = false, bool hasPgDatabaseSizeStatsAndHostMemory = false, bool hasQsCaptureModeRouteKnobToast = false, bool hasPgServerConfigDatabaseRoleOverrides = false, bool hasPostmasterStartTime = false, bool hasCheckpointsTimed = false, bool hasCollectionCaveats = false, bool hasIndexObjectStatsServerTimeIndex = false, bool hasQueryStoreIntervalLatest = false, bool hasRawChunkIntervalRungHistory = false, bool hasQueryStoreIntervalWide = false, bool hasManagedConfVerdicts = false, bool hasComposeTimeoutSixty = false, bool hasReadLatency = false, bool hasHotLivenessTouch = false, bool hasCollectionLogWatermarkAndJobHistoryIndexes = false, bool hasAgGroupId = false, bool hasCaggGroupIndexDrop = false) { /* V71 (the PostgreSQL blocking-edges rung): a table-existence sentinel and now the newest-first arm. A collector table would ordinarily get no arm at all — see the V63-V69 note below — but the TOP @@ -1221,10 +1245,22 @@ the rung below and showing a spurious upgrade banner on a store that is current. The WPF viewer runs no analysis, so no viewer read names the new table; this arm exists so the version banner stays truthful, which is the only effect the rung has on the viewer. Named only in the probe line, not this prose, per the V71 finding. */ + /* V152 (#4503): the six Query Store rollups' auto-created two-key group index is now ABSENT, and + now the TOP rung, so a fully-migrated store maps to EXACTLY StorageVersion.SchemaVersion rather + than falling through to the rung below and showing a spurious upgrade banner on a store that is + current. + + The WPF viewer runs no analysis, so no viewer read names any of the six rollups or their + indexes; this arm exists so the version banner stays truthful, which is the only effect the rung + has on the viewer. Named only in the probe line, not this prose, per the V71 finding. */ + if (hasCaggGroupIndexDrop) + { + return 152; + } + /* V151 (#4475): ag_replica_states.group_id, a column that identifies which physical AG a replica - row belongs to, and now the TOP rung, so a fully-migrated store maps to EXACTLY - StorageVersion.SchemaVersion rather than falling through to the rung below and showing a - spurious upgrade banner on a store that is current. + row belongs to. Formerly the TOP rung — RequiredStoreSchemaVersion is StorageVersion.SchemaVersion + and a store below this arm now falls through to V150 instead of stopping here. The WPF viewer runs no analysis, so no viewer read names the new column; this arm exists so the version banner stays truthful, which is the only effect the rung has on the viewer. Named only