diff --git a/CHANGELOG.md b/CHANGELOG.md index 08bc7fed..647e806b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **The Job History tab speaks display names** ([#2126], asked by ghauan) - both the Server filter dropdown and the Server column showed the raw collected server name while every other tab shows the operator's alias, so a fleet navigated by aliases turned into a memory quiz on exactly the tab an operator visits during an incident. Both readers (job history and the Agent status header) now resolve through the servers registry - the alias when one exists, the raw name otherwise - so the filter, the column, the per-column filter popup, and the CSV export all speak the same names as the rest of the viewer, and the Agent roll-up sorts by them. Lite's Job History tab had the same gap through a different mechanism (review catch): Lite's display-name concept lives at the CONFIG layer, not in DuckDB (the stored servers.display_name column is unpopulated by design), so the shell now passes a server_id-to-alias snapshot into the tab Overview-style and rows swap in the alias on every refresh - a server no longer in config keeps its raw collected name, the durable-record case. - **The long-query completion XE session actually gets created now** ([#2129], from ghauan's field report on #2061 - they enabled the collector on two servers and the Long Queries tab stayed empty forever) - the session DDL SET a customizable attribute `collect_object_name` on `sqlserver.rpc_completed`, and no such attribute exists on that event on ANY version (it belongs to `sp_statement_completed`) - `object_name` is one of rpc_completed's DEFAULT data fields, collected with no SET at all. So the CREATE failed on every server, the session never existed, and the reconcile's follow-up START surfaced as the confusing second error ('Cannot alter the event session... does not exist'). Never caught in dogfood because the collector ships OFF by design, and the DDL test pin asserted the wrong claim, so CI enforced the bug. The SET is gone (the reader already shreds the default field generically - no reader or table change), and the pin now asserts the attribute is ABSENT, with the story attached. Anyone who flipped the collector on before this fix: it starts working on the next reconcile tick after upgrading, no re-toggle needed. - **`--collapse-legacy-slices` narrows its slice instead of dying when a day does not fit the statement timeout** ([#2105] round three, ghauan once more - with the decompression rail lifted, the run made it ~15 minutes in and died at the NEW wall: a day-wide stage aggregation on a store carrying 60k split intervals blows through the 15-minute per-statement timeout, and the operator got the same bare stream exception) - the verb's fixed day-per-slice loop now runs the same adaptive schedule the Query Store backfill worker shipped this week (`AdaptiveSpan`, 24h base): a failed slice halves the window and retries the SAME start (announced with a [RETRY] line naming the error, so narrowing reads as progress rather than a hang), a completed slice resets to full width, and only a slice that fails at the ~22-minute floor gives up to the existing idempotent re-run message. Healthy stores still repair in a handful of day-wide slices - the narrowing costs nothing until a slice actually fails. +- **Query Store collection no longer has a fixed cost that big catalogs cannot pay** ([#2133], the actual root cause under the whole catch-up saga - #2102's death spiral, #2111's yield, and #2125's adaptive shrink were all mitigating it) - the collector joined its slice aggregate straight into the query_store_plan/query/text catalog TVFs, handing the optimizer nothing but fixed-guess cardinalities, and the plan it picked re-materialized a TVF per probe: on an 82k-plan catalog that was a fixed 30-second-plus cost that NO catch-up window width could reduce - which is exactly why the fleet's big databases (echo, oak, Surge, spruce, insa...) pinned at the 15-minute shrink floor and never converged while their smaller neighbors on the same servers stayed current. Bisected live: the aggregate alone ran in 81 ms and each TVF scanned bare in ~300 ms, yet aggregate-JOIN-plan could not finish in 30 s, hinted or not. The payload now STAGES the aggregate in a temp table and joins FROM it - real row counts instead of guesses, each TVF scanned exactly once, sp_QuickieStore's architecture for the same reason - and the old LOOP JOIN hint is gone for good (looping from the temp into the TVFs is the same per-probe re-materialization by another name). The interval pre-filter also resolves ids from the tiny interval catalog now instead of scanning runtime_stats itself (20 ms vs 426 ms, same id set). Measured end to end on the wedged field store: the full 55-column batch with plan capture completed a one-hour backlog in 21.3 s where the old shape never finished inside 60; the staged core is 524 ms. Same batch = one result set, TOP WITH TIES / derived-watermark / byte-budget semantics unchanged, both SKUs, both engine arms, live and backfill. ## [3.4.0] - 2026-08-06 @@ -2635,3 +2636,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#2119]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2119 [#2126]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2126 [#2129]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2129 +[#2133]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2133 diff --git a/Lite.Tests/QueryStoreCollectorDefinitionTests.cs b/Lite.Tests/QueryStoreCollectorDefinitionTests.cs index a24d3aec..1772a3de 100644 --- a/Lite.Tests/QueryStoreCollectorDefinitionTests.cs +++ b/Lite.Tests/QueryStoreCollectorDefinitionTests.cs @@ -192,10 +192,11 @@ ordinals on both paths. */ /* Interval-grain incremental filter since #1907, on this path too — the Azure body IS the shared body, so the WHERE→HAVING move lands here by construction rather than by a second edit. */ - Assert.Contains("HAVING\n MAX(qsrs.last_execution_time) > @cutoff_time", Lf(plan.Text), StringComparison.Ordinal); + Assert.Contains("HAVING\n MAX(qsrs.last_execution_time) > @cutoff_time", Lf(plan.Text), StringComparison.Ordinal); Assert.DoesNotContain("WHERE qsrs.last_execution_time > @cutoff_time", plan.Text, StringComparison.Ordinal); Assert.Contains("ORDER BY qsrs.last_execution_time ASC", plan.Text, StringComparison.Ordinal); - Assert.Contains("OPTION(RECOMPILE, LOOP JOIN);", plan.Text, StringComparison.Ordinal); + Assert.Contains("OPTION(RECOMPILE);", plan.Text, StringComparison.Ordinal); + Assert.DoesNotContain("LOOP JOIN", plan.Text, StringComparison.Ordinal); var parameter = Assert.Single(plan.Parameters); Assert.Equal("@cutoff_time", parameter.Name); @@ -464,7 +465,7 @@ public void BuildPerItemQuery_CombinesTheSlicesOfOneInterval_OnTheViewsNaturalKe Anything coarser would merge work that is genuinely distinct; anything finer would leave the slices split, which is the bug. */ Assert.Contains( - "GROUP BY\n qsrs.plan_id,\n qsrs.runtime_stats_interval_id,\n qsrs.execution_type_desc,\n qsrs.replica_group_id", + "GROUP BY\n qsrs.plan_id,\n qsrs.runtime_stats_interval_id,\n qsrs.execution_type_desc,\n qsrs.replica_group_id", text, StringComparison.Ordinal); @@ -481,20 +482,34 @@ public void BuildPerItemQuery_CombinesTheSlicesOfOneInterval_OnTheViewsNaturalKe Assert.Contains("max_dop = MAX(qsrs.max_dop)", text, StringComparison.Ordinal); /* The pre-filter is a prune, not a semantic: its interval list is a superset of what the HAVING - keeps, so it can never subtract a row. Without it the aggregate runs over the database's entire - retained Query Store every cycle — measured 1203ms against 375ms on a real 212k-row store. */ + keeps, so it can never subtract a row. #2133: the ids resolve from the INTERVAL CATALOG — + hundreds of rows — never by scanning runtime_stats itself (measured 20 ms vs 426 ms for the + identical id set on the field store that wedged). */ Assert.Contains("WHERE qsrs.runtime_stats_interval_id IN", text, StringComparison.Ordinal); - Assert.Contains("WHERE f.last_execution_time > @cutoff_time", text, StringComparison.Ordinal); - - /* The row SHAPE must not move: 55 selected columns, and the TOP/ORDER BY stay OUTSIDE the - aggregate so the cap counts intervals and can never truncate one interval's slices into a + Assert.Contains("FROM sys.query_store_runtime_stats_interval AS i", text, StringComparison.Ordinal); + Assert.Contains("WHERE i.end_time > @cutoff_time", text, StringComparison.Ordinal); + Assert.DoesNotContain("FROM sys.query_store_runtime_stats AS f", text, StringComparison.Ordinal); + + /* #2133 STAGING: the aggregate lands in a temp table and the plan/query/text joins run FROM it, + so the optimizer joins with real cardinalities instead of TVF fixed guesses — the monolithic + join re-materialized a TVF per probe, a fixed ≥30s cost on an 82k-plan catalog that no + catch-up width could reduce (staged: 524 ms, same store, same window). SELECT INTO emits no + result set, so the batch still returns exactly one; the leading DROP covers Azure's pooled + direct connections (on-prem the sp_executesql scope self-cleans). */ + Assert.Contains("DROP TABLE IF EXISTS #pm_qs_slice;", text, StringComparison.Ordinal); + Assert.Contains("INTO #pm_qs_slice", text, StringComparison.Ordinal); + Assert.Contains("FROM #pm_qs_slice AS qsrs\nJOIN sys.query_store_plan AS qsp", Lf(text), StringComparison.Ordinal); + + /* The row SHAPE must not move: 55 selected columns, and the TOP/ORDER BY stay on the final + SELECT so the cap counts intervals and can never truncate one interval's slices into a partial sum. WITH TIES + ASC are the #1960 never-a-hole pair: oldest-first shipping keeps the derived watermark at the shipped boundary, and WITH TIES stops a bare TOP from splitting a group of rows tied at that boundary — the strict `> @cutoff_time` would strand the - unshipped half forever. */ + unshipped half forever. The LOOP JOIN hint must never return to this query: looping from the + temp into the TVFs is the per-probe re-materialization #2133 removed. */ Assert.Contains($"TOP ({QueryStoreCollector.MaxRowsPerDatabase}) WITH TIES", text, StringComparison.Ordinal); - Assert.Contains(") AS qsrs\nJOIN sys.query_store_plan AS qsp", text, StringComparison.Ordinal); - Assert.Contains("ORDER BY qsrs.last_execution_time ASC\nOPTION(RECOMPILE, LOOP JOIN);", text, StringComparison.Ordinal); + Assert.Contains("ORDER BY qsrs.last_execution_time ASC\nOPTION(RECOMPILE);", Lf(text), StringComparison.Ordinal); + Assert.DoesNotContain("LOOP JOIN", text, StringComparison.Ordinal); } /// @@ -517,11 +532,13 @@ public void Payload_EveryAverageColumn_IsTheCountWeightedMean() { var text = PayloadSql(MakeContext(probeResult: 16)); - /* Only the aggregating derived table — the outer projection references the same names as plain - columns, which is correct there and must not be mistaken for an un-weighted aggregate. */ - var open = text.IndexOf("FROM\n(", StringComparison.Ordinal); - var close = text.IndexOf(") AS qsrs", StringComparison.Ordinal); - Assert.True(open > 0 && close > open, "could not locate the slice-aggregating derived table"); + /* Only the aggregating STAGING statement (#2133: the aggregate lands in #pm_qs_slice and the + joins run from it) — the final projection references the same names as plain columns, which + is correct there and must not be mistaken for an un-weighted aggregate. The staging SELECT + is the marker's first occurrence; the final SELECT carries TOP on the marker line. */ + var open = text.IndexOf("SELECT /* PerformanceMonitorLite */\n", StringComparison.Ordinal); + var close = text.IndexOf("INTO #pm_qs_slice", StringComparison.Ordinal); + Assert.True(open > 0 && close > open, "could not locate the slice-aggregating staging statement"); var aggregate = text[open..close]; var averages = System.Text.RegularExpressions.Regex @@ -566,18 +583,18 @@ public void BuildPerItemQuery_ReplicaGroupIdEntersTheGroupingKey_OnlyWhereItBind foreach (var probe in new object[] { 16, 17 }) { var attributed = PayloadSql(MakeContext(probeResult: probe)); - Assert.Contains("qsrs.execution_type_desc,\n qsrs.replica_group_id", attributed, StringComparison.Ordinal); + Assert.Contains("qsrs.execution_type_desc,\n qsrs.replica_group_id", attributed, StringComparison.Ordinal); } var azure = AzurePayloadSql(MakeContext(isAzureSqlDb: true, probeResult: 12)); - Assert.Contains("qsrs.execution_type_desc,\n qsrs.replica_group_id", azure, StringComparison.Ordinal); + Assert.Contains("qsrs.execution_type_desc,\n qsrs.replica_group_id", azure, StringComparison.Ordinal); /* Pre-2022 box and Managed Instance: the column must not be named anywhere, GROUP BY included. */ foreach (var probe in new object?[] { 13, 14, 15, null }) { var ungated = PayloadSql(MakeContext(probeResult: probe)); Assert.DoesNotContain("replica_group_id", ungated, StringComparison.Ordinal); - Assert.Contains("GROUP BY\n qsrs.plan_id,\n qsrs.runtime_stats_interval_id,\n qsrs.execution_type_desc\n", ungated, StringComparison.Ordinal); + Assert.Contains("GROUP BY\n qsrs.plan_id,\n qsrs.runtime_stats_interval_id,\n qsrs.execution_type_desc\n", ungated, StringComparison.Ordinal); } } @@ -604,13 +621,14 @@ public void BuildPerItemQuery_PreSql2017_GatedFamiliesLeaveTheAggregate_ButKeepT Assert.Contains("avg_log_bytes_used = NULL, min_log_bytes_used = NULL, max_log_bytes_used = NULL,", old, StringComparison.Ordinal); Assert.Contains("avg_tempdb_space_used = NULL, min_tempdb_space_used = NULL, max_tempdb_space_used = NULL,", old, StringComparison.Ordinal); - /* The inner list must end cleanly on the last ungated column when all three are absent. */ - Assert.Contains("max_rowcount = MAX(qsrs.max_rowcount)\n FROM sys.query_store_runtime_stats AS qsrs", old, StringComparison.Ordinal); + /* The staging list must end cleanly on the last ungated column when all three are absent — + #2133: the aggregate lands in #pm_qs_slice, so INTO sits between the list and FROM. */ + Assert.Contains("max_rowcount = MAX(qsrs.max_rowcount)\nINTO #pm_qs_slice\nFROM sys.query_store_runtime_stats AS qsrs", old, StringComparison.Ordinal); /* On 2017+ they are present, aggregated, and the list ends with the last gated family instead. */ var newer = PayloadSql(MakeContext(probeResult: 14)); Assert.Contains("max_rowcount = MAX(qsrs.max_rowcount),\n", newer, StringComparison.Ordinal); - Assert.Contains("max_tempdb_space_used = MAX(qsrs.max_tempdb_space_used)\n FROM sys.query_store_runtime_stats AS qsrs", newer, StringComparison.Ordinal); + Assert.Contains("max_tempdb_space_used = MAX(qsrs.max_tempdb_space_used)\nINTO #pm_qs_slice\nFROM sys.query_store_runtime_stats AS qsrs", newer, StringComparison.Ordinal); } [Fact] @@ -763,7 +781,7 @@ that would have pinned the bug. A per-slice WHERE cannot survive slice aggregati original defect with an aggregate bolted on. HAVING MAX(...) asks whether the INTERVAL saw new activity and then takes all of it. */ var normalized = plan.Text.Replace("\r\n", "\n", StringComparison.Ordinal); - Assert.Contains("HAVING\n MAX(qsrs.last_execution_time) > @cutoff_time", normalized, StringComparison.Ordinal); + Assert.Contains("HAVING\n MAX(qsrs.last_execution_time) > @cutoff_time", normalized, StringComparison.Ordinal); Assert.DoesNotContain("WHERE qsrs.last_execution_time > @cutoff_time", normalized, StringComparison.Ordinal); /* #1565: NO SQL-side self-exclusion — the old NOT LIKE was 75% of the read's elapsed time (full nvarchar(max) scan per row on a column no index can serve; field A/B: 4.3x without it), and no @@ -771,7 +789,8 @@ activity and then takes all of it. */ where the text is already materialized (pinned below). The query still CONTAINS the marker — in its own leading comment. */ Assert.DoesNotContain("NOT LIKE", plan.Text, StringComparison.Ordinal); - Assert.Contains("OPTION(RECOMPILE, LOOP JOIN);", plan.Text, StringComparison.Ordinal); + Assert.Contains("OPTION(RECOMPILE);", plan.Text, StringComparison.Ordinal); + Assert.DoesNotContain("LOOP JOIN", plan.Text, StringComparison.Ordinal); Assert.Contains("N'@cutoff_time datetime2(7)',", plan.Text, StringComparison.Ordinal); var parameter = Assert.Single(plan.Parameters); @@ -823,8 +842,11 @@ TOP from splitting a group of rows tied at that boundary (the strict `> @cutoff_ { Assert.Contains($"TOP ({QueryStoreCollector.MaxRowsPerDatabase}) WITH TIES", plan.Text, StringComparison.Ordinal); Assert.Contains("ORDER BY qsrs.last_execution_time ASC", plan.Text, StringComparison.Ordinal); - /* The row-bounding ORDER BY sits before the existing query hint, which the OPTION pin still checks. */ - Assert.Contains("OPTION(RECOMPILE, LOOP JOIN);", plan.Text, StringComparison.Ordinal); + /* The row-bounding ORDER BY sits before the existing query hint, which the OPTION pin still + checks. RECOMPILE only — the old LOOP JOIN hint is the #2133 pathology (per-probe TVF + re-materialization) and must never return. */ + Assert.Contains("OPTION(RECOMPILE);", plan.Text, StringComparison.Ordinal); + Assert.DoesNotContain("LOOP JOIN", plan.Text, StringComparison.Ordinal); } Assert.Equal(50_000, QueryStoreCollector.MaxRowsPerDatabase); @@ -1082,8 +1104,11 @@ live window instead of the backlog. */ Assert.Contains("EXECUTE [StackOverflow].sys.sp_executesql", plan.Text, StringComparison.Ordinal); Assert.Contains("N'@floor_time datetime2(7), @ceiling_time datetime2(7)'", plan.Text, StringComparison.Ordinal); - Assert.Contains("f.last_execution_time > @floor_time", plan.Text, StringComparison.Ordinal); - Assert.Contains("f.last_execution_time < @ceiling_time", plan.Text, StringComparison.Ordinal); + /* #2133: the pre-filter's two-sided window asks the INTERVAL CATALOG which intervals OVERLAP + (floor, ceiling) — end after the floor AND start before the ceiling — a superset the exact + HAVING below then narrows, exactly like the live path's one-sided form. */ + Assert.Contains("i.end_time > @floor_time", plan.Text, StringComparison.Ordinal); + Assert.Contains("i.start_time < @ceiling_time", plan.Text, StringComparison.Ordinal); Assert.Contains("MAX(qsrs.last_execution_time) > @floor_time", plan.Text, StringComparison.Ordinal); Assert.Contains("MAX(qsrs.last_execution_time) < @ceiling_time", plan.Text, StringComparison.Ordinal); Assert.Contains("ORDER BY qsrs.last_execution_time DESC", plan.Text, StringComparison.Ordinal); @@ -1109,8 +1134,8 @@ own catalog would be worse than a loud wrong-path error. */ var plan = QueryStoreCollector.Instance.BuildBackfillQuery(MakeContext(isAzureSqlDb: true), floor, ceiling); Assert.DoesNotContain("sp_executesql", plan.Text, StringComparison.Ordinal); - Assert.Contains("f.last_execution_time > @floor_time", plan.Text, StringComparison.Ordinal); - Assert.Contains("f.last_execution_time < @ceiling_time", plan.Text, StringComparison.Ordinal); + Assert.Contains("i.end_time > @floor_time", plan.Text, StringComparison.Ordinal); + Assert.Contains("i.start_time < @ceiling_time", plan.Text, StringComparison.Ordinal); Assert.Contains("ORDER BY qsrs.last_execution_time DESC", plan.Text, StringComparison.Ordinal); Assert.DoesNotContain("@cutoff_time", plan.Text, StringComparison.Ordinal); /* The same eligibility gate the live Azure query leads with. */ @@ -1132,7 +1157,7 @@ public void BuildBackfillPerItemQuery_LiveBodyStaysUntouched() one-sided cutoff and ASC order byte-for-byte, or phase 1's watermark-exact resume breaks in the same PR that builds on it. */ var live = QueryStoreCollector.Instance.BuildPerItemQuery("StackOverflow", MakeContext()); - Assert.Contains("f.last_execution_time > @cutoff_time", live.Text, StringComparison.Ordinal); + Assert.Contains("i.end_time > @cutoff_time", live.Text, StringComparison.Ordinal); Assert.Contains("MAX(qsrs.last_execution_time) > @cutoff_time", live.Text, StringComparison.Ordinal); Assert.Contains("ORDER BY qsrs.last_execution_time ASC", live.Text, StringComparison.Ordinal); Assert.DoesNotContain("@floor_time", live.Text, StringComparison.Ordinal); diff --git a/PerformanceMonitor.Collectors/QueryStoreCollector.cs b/PerformanceMonitor.Collectors/QueryStoreCollector.cs index dd5d1cef..3e1d37b9 100644 --- a/PerformanceMonitor.Collectors/QueryStoreCollector.cs +++ b/PerformanceMonitor.Collectors/QueryStoreCollector.cs @@ -605,15 +605,15 @@ ordinal so the 55-column reader contract never moves. The inner fragments carry comma and sit at the END of the inner select list precisely because they can be empty; the outer ones keep their original trailing-comma form because they are never empty. */ string numPhysIoReadsAgg = isNew - ? $",\n {WeightedAverage("avg_num_physical_io_reads")},\n min_num_physical_io_reads = MIN(qsrs.min_num_physical_io_reads),\n max_num_physical_io_reads = MAX(qsrs.max_num_physical_io_reads)" + ? $",\n {WeightedAverage("avg_num_physical_io_reads")},\n min_num_physical_io_reads = MIN(qsrs.min_num_physical_io_reads),\n max_num_physical_io_reads = MAX(qsrs.max_num_physical_io_reads)" : ""; string logBytesAgg = isNew - ? $",\n {WeightedAverage("avg_log_bytes_used")},\n min_log_bytes_used = MIN(qsrs.min_log_bytes_used),\n max_log_bytes_used = MAX(qsrs.max_log_bytes_used)" + ? $",\n {WeightedAverage("avg_log_bytes_used")},\n min_log_bytes_used = MIN(qsrs.min_log_bytes_used),\n max_log_bytes_used = MAX(qsrs.max_log_bytes_used)" : ""; string tempdbAgg = isNew - ? $",\n {WeightedAverage("avg_tempdb_space_used")},\n min_tempdb_space_used = MIN(qsrs.min_tempdb_space_used),\n max_tempdb_space_used = MAX(qsrs.max_tempdb_space_used)" + ? $",\n {WeightedAverage("avg_tempdb_space_used")},\n min_tempdb_space_used = MIN(qsrs.min_tempdb_space_used),\n max_tempdb_space_used = MAX(qsrs.max_tempdb_space_used)" : ""; string numPhysIoReadsCols = isNew @@ -681,7 +681,7 @@ fails the whole SELECT just as naming it in a select list would. When the gate i Leading comma: it splices into both the inner select list and the GROUP BY, and is empty on targets without the column. */ string replicaGroupKey = hasReplicaAttribution - ? ",\n qsrs.replica_group_id" + ? ",\n qsrs.replica_group_id" : ""; /* There is deliberately NO self-exclusion predicate in this query (#1565, actual-plan evidence @@ -786,18 +786,100 @@ boundary groups the same way (see ReadRowsAsync). */ its oldest shipped row, and the next slice's strict `< @ceiling_time` resumes with no hole or re-ship. Same TIES, same budget, same tie-group completion; only the window and the direction differ. */ + /* The interval pre-filter resolves candidate interval ids from the INTERVAL CATALOG + (sys.query_store_runtime_stats_interval, ~one row per interval of retained history — hundreds + of rows) rather than from runtime_stats itself (#2133; measured on the field store: 20 ms vs + 426 ms for the identical id set). end_time/start_time are datetimeoffset; the datetime2 + parameters promote with a zero offset, i.e. as the UTC instants they are — the same implicit + promotion the HAVING's last_execution_time comparison has always relied on. The catalog bound + is a SUPERSET (an interval can end after the cutoff while all its rows are older); the HAVING + below stays the exact row-level filter, so shipped semantics are unchanged. */ var intervalPreFilter = backfill - ? @"f.last_execution_time > @floor_time - AND f.last_execution_time < @ceiling_time" - : "f.last_execution_time > @cutoff_time"; + ? @"i.end_time > @floor_time + AND i.start_time < @ceiling_time" + : "i.end_time > @cutoff_time"; var intervalHaving = backfill ? @"MAX(qsrs.last_execution_time) > @floor_time - AND MAX(qsrs.last_execution_time) < @ceiling_time" + AND MAX(qsrs.last_execution_time) < @ceiling_time" : "MAX(qsrs.last_execution_time) > @cutoff_time"; var shipOrder = backfill ? "DESC" : "ASC"; + /* STAGED, not monolithic (#2133). Joining the slice aggregate straight into the + query_store_plan/query/text TVFs handed the optimizer nothing but fixed-guess cardinalities, + and the shape it picked re-materialized a TVF per probe — a fixed cost no window width could + reduce. Field bisection on an 82k-plan catalog (echo, SQL 2022): the aggregate alone ran in + 81 ms and each TVF scanned bare in ~300 ms, yet aggregate-JOIN-qsp could not finish in 30 s, + hinted or not; staged through the temp the same work totaled 524 ms (56 stage + 409 join). + That fixed cost is what wedged the big-catalog databases at EVERY catch-up width and made + #2125's shrink floor-pin instead of converge. The temp gives the final join REAL row counts — + and for that reason the old LOOP JOIN hint must NOT return: looping from the temp into the + TVFs is the same per-probe re-materialization by another name; the 524 ms join is unhinted, + chosen by the optimizer from true cardinalities. sp_QuickieStore stages for the same reason. + + Batch mechanics: SELECT INTO emits no result set, so the batch still returns exactly ONE + result set (the reader/byte-budget contract). Inside the on-prem [db].sys.sp_executesql + nesting the temp's scope dies with the invocation; on Azure's direct per-database path the + leading DROP TABLE IF EXISTS covers pooled-connection reuse. TOP ... WITH TIES, the ship + order, and the derived-watermark semantics live on the final SELECT, unchanged. + + BOTH statements carry OPTION(RECOMPILE) (review catch): split out on its own, the staging + statement would otherwise be cached via sp_executesql's parameterized text and sniffed + across live vs backfill windows of wildly different selectivity — the same fixed-guess + failure mode this rewrite removes, reintroduced one statement earlier. */ return $@"SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED; +DROP TABLE IF EXISTS #pm_qs_slice; + +SELECT /* PerformanceMonitorLite */ + qsrs.plan_id, + qsrs.runtime_stats_interval_id, + qsrs.execution_type_desc{replicaGroupKey}, + first_execution_time = MIN(qsrs.first_execution_time), + last_execution_time = MAX(qsrs.last_execution_time), + count_executions = SUM(qsrs.count_executions), + {WeightedAverage("avg_duration")}, + min_duration = MIN(qsrs.min_duration), + max_duration = MAX(qsrs.max_duration), + {WeightedAverage("avg_cpu_time")}, + min_cpu_time = MIN(qsrs.min_cpu_time), + max_cpu_time = MAX(qsrs.max_cpu_time), + {WeightedAverage("avg_logical_io_reads")}, + min_logical_io_reads = MIN(qsrs.min_logical_io_reads), + max_logical_io_reads = MAX(qsrs.max_logical_io_reads), + {WeightedAverage("avg_logical_io_writes")}, + min_logical_io_writes = MIN(qsrs.min_logical_io_writes), + max_logical_io_writes = MAX(qsrs.max_logical_io_writes), + {WeightedAverage("avg_physical_io_reads")}, + min_physical_io_reads = MIN(qsrs.min_physical_io_reads), + max_physical_io_reads = MAX(qsrs.max_physical_io_reads), + {WeightedAverage("avg_clr_time")}, + min_clr_time = MIN(qsrs.min_clr_time), + max_clr_time = MAX(qsrs.max_clr_time), + min_dop = MIN(qsrs.min_dop), + max_dop = MAX(qsrs.max_dop), + {WeightedAverage("avg_query_max_used_memory")}, + min_query_max_used_memory = MIN(qsrs.min_query_max_used_memory), + max_query_max_used_memory = MAX(qsrs.max_query_max_used_memory), + {WeightedAverage("avg_rowcount")}, + min_rowcount = MIN(qsrs.min_rowcount), + max_rowcount = MAX(qsrs.max_rowcount){numPhysIoReadsAgg}{logBytesAgg}{tempdbAgg} +INTO #pm_qs_slice +FROM sys.query_store_runtime_stats AS qsrs +WHERE qsrs.runtime_stats_interval_id IN +( + SELECT + i.runtime_stats_interval_id + FROM sys.query_store_runtime_stats_interval AS i + WHERE {intervalPreFilter} +) +GROUP BY + qsrs.plan_id, + qsrs.runtime_stats_interval_id, + qsrs.execution_type_desc{replicaGroupKey} +HAVING + {intervalHaving} +OPTION(RECOMPILE); + SELECT /* PerformanceMonitorLite */ TOP ({MaxRowsPerDatabase}) WITH TIES query_id = qsq.query_id, plan_id = qsp.plan_id, @@ -855,56 +937,7 @@ ELSE COALESCE( {replicaRoleCol}, runtime_stats_interval_id = qsrs.runtime_stats_interval_id, interval_start_time_utc = CONVERT(datetime2, qsrsi.start_time AT TIME ZONE 'UTC') -FROM -( - SELECT - qsrs.plan_id, - qsrs.runtime_stats_interval_id, - qsrs.execution_type_desc{replicaGroupKey}, - first_execution_time = MIN(qsrs.first_execution_time), - last_execution_time = MAX(qsrs.last_execution_time), - count_executions = SUM(qsrs.count_executions), - {WeightedAverage("avg_duration")}, - min_duration = MIN(qsrs.min_duration), - max_duration = MAX(qsrs.max_duration), - {WeightedAverage("avg_cpu_time")}, - min_cpu_time = MIN(qsrs.min_cpu_time), - max_cpu_time = MAX(qsrs.max_cpu_time), - {WeightedAverage("avg_logical_io_reads")}, - min_logical_io_reads = MIN(qsrs.min_logical_io_reads), - max_logical_io_reads = MAX(qsrs.max_logical_io_reads), - {WeightedAverage("avg_logical_io_writes")}, - min_logical_io_writes = MIN(qsrs.min_logical_io_writes), - max_logical_io_writes = MAX(qsrs.max_logical_io_writes), - {WeightedAverage("avg_physical_io_reads")}, - min_physical_io_reads = MIN(qsrs.min_physical_io_reads), - max_physical_io_reads = MAX(qsrs.max_physical_io_reads), - {WeightedAverage("avg_clr_time")}, - min_clr_time = MIN(qsrs.min_clr_time), - max_clr_time = MAX(qsrs.max_clr_time), - min_dop = MIN(qsrs.min_dop), - max_dop = MAX(qsrs.max_dop), - {WeightedAverage("avg_query_max_used_memory")}, - min_query_max_used_memory = MIN(qsrs.min_query_max_used_memory), - max_query_max_used_memory = MAX(qsrs.max_query_max_used_memory), - {WeightedAverage("avg_rowcount")}, - min_rowcount = MIN(qsrs.min_rowcount), - max_rowcount = MAX(qsrs.max_rowcount){numPhysIoReadsAgg}{logBytesAgg}{tempdbAgg} - FROM sys.query_store_runtime_stats AS qsrs - WHERE qsrs.runtime_stats_interval_id IN - ( - SELECT - f.runtime_stats_interval_id - FROM sys.query_store_runtime_stats AS f - WHERE {intervalPreFilter} - ) - GROUP BY - qsrs.plan_id, - qsrs.runtime_stats_interval_id, - qsrs.execution_type_desc{replicaGroupKey} - HAVING - {intervalHaving} -) AS qsrs +FROM #pm_qs_slice AS qsrs JOIN sys.query_store_plan AS qsp ON qsp.plan_id = qsrs.plan_id JOIN sys.query_store_query AS qsq @@ -915,7 +948,7 @@ LEFT JOIN sys.query_store_runtime_stats_interval AS qsrsi ON qsrsi.runtime_stats_interval_id = qsrs.runtime_stats_interval_id {replicaJoin} ORDER BY qsrs.last_execution_time {shipOrder} -OPTION(RECOMPILE, LOOP JOIN);"; +OPTION(RECOMPILE);"; } ///