From ecc55c739bfbf55f68c02b6c25ff37760589ee39 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:22:53 -0400 Subject: [PATCH] Count archived history in the analysis warm-up, and read v_ views everywhere (#1809) The 24h sufficiency check measured the span on raw wait_stats; the 512MB ArchiveAllAndResetAsync empties the hot store, so multi-server installs that reset sub-daily never finished warm-up. The check now reads v_wait_stats (hot + parquet). The broken gate was also shielding the same defect downstream: the fact collector read raw tables in 22 more places, including the on-load config snapshots that are EMPTY after a reset until the next app start. Gate-only would have analyzed a thin post-reset window - the exact distortion the floor exists to prevent. All 23 sites now read their v_ archive views, and a catalog-driven sweep guard (ArchivableTables) fails on any raw FROM in Lite/Analysis, naming the file. Verified: behavioral fixture proves the span counts hot + archived parquet across a simulated reset and still gates a young install; watched red by reverting the query (both guards fired); full Lite.Tests 1601/1601; zero warnings. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 6 + Lite.Tests/AnalysisDataSpanTests.cs | 181 ++++++++++++++++++ Lite/Analysis/AnalysisService.cs | 6 +- Lite/Analysis/DrillDownCollector.Plans.cs | 2 +- Lite/Analysis/DuckDbFactCollector.Activity.cs | 6 +- Lite/Analysis/DuckDbFactCollector.Config.cs | 10 +- .../Analysis/DuckDbFactCollector.QueryPerf.cs | 4 +- .../Analysis/DuckDbFactCollector.Resources.cs | 12 +- Lite/Analysis/DuckDbFactCollector.Storage.cs | 6 +- Lite/Analysis/DuckDbFactCollector.Waits.cs | 4 +- 10 files changed, 213 insertions(+), 24 deletions(-) create mode 100644 Lite.Tests/AnalysisDataSpanTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index ef4f629d59..5bd3ee693e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Recommendations warm-up now survives the 512 MB archive/reset, and analysis reads the archive tier everywhere** ([#1811], closes [#1809]) - the 24-hour sufficiency check measured MIN..MAX(collection_time) on the RAW hot wait_stats table, while `ArchiveAllAndResetAsync` copies everything to Parquet and empties the hot store. On a multi-server install that trips the size threshold more often than daily, the measured span restarted with every reset and warm-up never completed - even though the archived history was sitting in Parquet and every other tab could read it through the `v_` union views. The check now reads `v_wait_stats`, so archived plus hot history counts, and a genuinely young install still gates (both pinned, watched red by reverting the query to the raw table). + + The subtlety that made this a 24-file-line sweep rather than a one-word fix: the broken gate was accidentally SHIELDING a second instance of the same defect. The analysis fact collector read raw tables in 22 more places - the window reads (query stats, snapshots, memory, perfmon, file IO, blocking, deadlocks, storage) and the on-load config snapshots (server/database config, trace flags, server properties), which after a reset are EMPTY until the next app start. Fixing only the gate would have run analysis over a thin post-reset hot window, reintroducing the exact fraction-of-period distortion the 24-hour floor exists to prevent ("5 seconds of THREADPOOL looks alarming in a 16-minute window"). All 23 sites now read their `v_` archive views - column-identical by construction (only `config_alert_log` carries a view-only column, and analysis does not read it), dedup-safe where re-collection could duplicate rows (the QUALIFY views), and a catalog-driven sweep guard holds the whole pipeline there: for every table in `ArchiveService.ArchivableTables`, a raw `FROM` anywhere in Lite/Analysis fails the build's tests naming the file - so a new collector's table is guarded the day it exists. + - **A benign 1-second lock-timeout yield no longer paints a Critical health day** ([#1808], closes [#1805]) - exactly one collector carries `SET LOCK_TIMEOUT 1000` (query_snapshots, both variants), and yielding after 1 second instead of joining a blocking chain is the monitor keeping its never-be-a-blocker promise; the data cost is one skipped point-in-time sweep. But nothing anywhere handled SQL error 1222 specially, so the yield surfaced as a generic collection error - and the daily-health banding treats ONE collection error, alone, as a CRITICAL day, so each yield painted the calendar red over a non-event. The fix is classification only; the collection behavior was already correct. A new `YieldsOnLockTimeout` flag on the shared collector definition (true only for query_snapshots, so a 1222 from any other collector - which never sets the guard - stays an ERROR) routes both runners' catch sites to a new `YIELDED` collection_log status. The exclusion needed no reader changes at all: every error count that feeds collector health, the fleet roll-ups, and the daily band's collErrors input matches `status = 'ERROR'` exactly, audited across both apps - so the pins guard the predicate SHAPE (exact-match, in both daily summaries) against ever being broadened to a not-success form that would re-admit yields. In-memory health treats a yield as neither success nor failure: it does not reset a real error streak (a yield is not proof the collector works) and does not grow one (the guard worked as designed). The re-frame the issue asked for is kept visible: yields get their own counter - a Yields column on both Collection Health grids and a `yields` field on both `get_collection_health` MCP tools - because a cluster of them is evidence about the TARGET's lock contention, mislabeled until now as a monitoring failure. Deliberately unchanged: the timeout itself, no retries, no extension of the guard to other collectors, and the health-band classifier - a collector yielding for hours goes honestly stale with the Yields column sitting right beside the staleness explaining why. - **The .NET SDK band is now pinned three ways instead of agreeing by float** ([#1807], closes [#1758]) - the issue predicted CI would break when runner SDKs moved to the 10.0.3xx band, because the committed lock files carried the 2xx-band `Microsoft.NET.ILLink.Tasks` pin. Investigating found the fuse had already half-fired in reverse: the 2026-07-25 maintenance pass regenerated the locks under a 3xx SDK, and CI has been green only because `setup-dotnet`'s floating `dotnet-version: 10.0.x` happens to download the same band - while `global.json` still named `10.0.204`, a version nothing was actually using. Every piece agreed by coincidence, and the NEXT band (10.0.4xx) would have broken it again from either side: a runner image float re-arms NU1004 in CI, a local `latestFeature` roll-forward silently rewrites the locks. Now `global.json` names the band that wrote the locks (`10.0.302`, `rollForward: latestPatch` so a newer installed band is ignored rather than silently adopted), and all four `setup-dotnet` sites read `global-json-file: global.json` instead of floating - so the invariant the issue states (CI SDK band == `global.json` band == the band that wrote the locks) is enforced by one file, and moving bands becomes the deliberate single-PR act it should be. Verified: `--force-evaluate` under 10.0.302 changes zero lock files, `--locked-mode` restore passes on all nine, and the Installer chain builds without the lock-file churn the issue documented on exactly this machine. @@ -1895,6 +1899,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#1803]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1803 [#1805]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1805 [#1808]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1808 +[#1809]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1809 +[#1811]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1811 [#1787]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1787 [#1806]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1806 [#1758]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1758 diff --git a/Lite.Tests/AnalysisDataSpanTests.cs b/Lite.Tests/AnalysisDataSpanTests.cs new file mode 100644 index 0000000000..b3aef51746 --- /dev/null +++ b/Lite.Tests/AnalysisDataSpanTests.cs @@ -0,0 +1,181 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor Lite. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.IO; +using System.Linq; +using System.Text.RegularExpressions; +using System.Threading.Tasks; +using DuckDB.NET.Data; +using PerformanceMonitorLite.Analysis; +using PerformanceMonitorLite.Database; +using PerformanceMonitorLite.Services; +using Xunit; + +namespace PerformanceMonitorLite.Tests; + +/// +/// #1809: the Recommendations 24-hour warm-up must survive the 512 MB archive/reset. The sufficiency +/// check measured MIN..MAX(collection_time) on the RAW hot table, so every +/// ArchiveAllAndResetAsync restarted the measured span — on a multi-server install that resets +/// more often than daily, warm-up never completed, even though the archived history was sitting right +/// there in parquet and every other surface could read it through the v_ views. +/// +/// Two layers: the behavioral fixture proves the span reads hot + archived parquet through +/// v_wait_stats (and that a genuinely young install still gates), and the sweep guard holds the +/// WHOLE analysis pipeline to the view tier — the fact collector's window reads had the same defect, +/// masked until now by the broken gate blocking analysis entirely after each reset. Fixing the gate +/// without the reads would have analyzed a thin post-reset hot window, reintroducing the exact +/// fraction-of-period distortion the 24-hour floor exists to prevent. +/// +public sealed class AnalysisDataSpanTests : IDisposable +{ + private readonly string _tempDir; + private readonly string _dbPath; + private readonly string _archivePath; + + public AnalysisDataSpanTests() + { + _tempDir = Path.Combine(Path.GetTempPath(), "LiteTests_" + Guid.NewGuid().ToString("N")[..8]); + Directory.CreateDirectory(_tempDir); + _dbPath = Path.Combine(_tempDir, "test.duckdb"); + _archivePath = Path.Combine(_tempDir, "archive"); + Directory.CreateDirectory(_archivePath); + } + + public void Dispose() + { + try + { + if (Directory.Exists(_tempDir)) + Directory.Delete(_tempDir, recursive: true); + } + catch + { + /* Best-effort cleanup */ + } + } + + [Fact] + public async Task DataSpan_CountsArchivedParquetHistory_AcrossAnArchiveReset() + { + var initializer = new DuckDbInitializer(_dbPath); + await initializer.InitializeAsync(); + + using (var connection = new DuckDBConnection($"Data Source={_dbPath}")) + { + await connection.OpenAsync(TestContext.Current.CancellationToken); + + /* 28 hours of history collected BEFORE the reset... */ + await ExecuteAsync(connection, @" +INSERT INTO wait_stats (collection_id, collection_time, server_id, server_name, wait_type) +VALUES + (1, now() - INTERVAL 30 HOUR, 1, 'S1', 'SOS_SCHEDULER_YIELD'), + (2, now() - INTERVAL 3 HOUR, 1, 'S1', 'SOS_SCHEDULER_YIELD')"); + + /* ...archived to parquet and wiped from the hot store, the way ArchiveAllAndResetAsync + does it (COPY whole table, then reset)... */ + var parquetPath = Path.Combine(_archivePath, "20260101_0000_wait_stats.parquet").Replace("\\", "/"); + await ExecuteAsync(connection, $"COPY wait_stats TO '{parquetPath}' (FORMAT PARQUET)"); + await ExecuteAsync(connection, "DELETE FROM wait_stats"); + + /* ...and only 2 hours of post-reset hot data. The raw-table read this test guards against + sees 2 hours here and blocks analysis forever. */ + await ExecuteAsync(connection, @" +INSERT INTO wait_stats (collection_id, collection_time, server_id, server_name, wait_type) +VALUES + (3, now() - INTERVAL 2 HOUR, 1, 'S1', 'SOS_SCHEDULER_YIELD'), + (4, now(), 1, 'S1', 'SOS_SCHEDULER_YIELD')"); + } + + await initializer.CreateArchiveViewsAsync(); + + var analysis = new AnalysisService(initializer); + var spanHours = await analysis.GetTotalDataSpanHoursAsync(serverId: 1); + + Assert.True(spanHours >= 29, + $"span must count hot + archived history (~30h), got {spanHours:F1}h — the check is reading the raw table again"); + } + + [Fact] + public async Task DataSpan_StillGatesAGenuinelyYoungInstall() + { + var initializer = new DuckDbInitializer(_dbPath); + await initializer.InitializeAsync(); + + using (var connection = new DuckDBConnection($"Data Source={_dbPath}")) + { + await connection.OpenAsync(TestContext.Current.CancellationToken); + await ExecuteAsync(connection, @" +INSERT INTO wait_stats (collection_id, collection_time, server_id, server_name, wait_type) +VALUES + (1, now() - INTERVAL 2 HOUR, 1, 'S1', 'SOS_SCHEDULER_YIELD'), + (2, now(), 1, 'S1', 'SOS_SCHEDULER_YIELD')"); + } + + await initializer.CreateArchiveViewsAsync(); + + var analysis = new AnalysisService(initializer); + var spanHours = await analysis.GetTotalDataSpanHoursAsync(serverId: 1); + + Assert.InRange(spanHours, 1.5, 3); + } + + /// + /// The sweep guard: NO archivable table may be read raw anywhere in the analysis pipeline — every + /// read goes through its v_ archive view, or the #1809 class of defect returns one query at + /// a time. Driven by , the same catalog-derived list + /// the archiver itself uses, so a new collector's table is guarded the day it exists. + /// + [Fact] + public void AnalysisPipeline_NeverReadsAnArchivableTableRaw() + { + var analysisDir = Path.Combine(FindRepoDirectory(Path.Combine("Lite", "Analysis"))); + var sources = Directory.GetFiles(analysisDir, "*.cs", SearchOption.AllDirectories); + Assert.NotEmpty(sources); + + var offenders = new System.Collections.Generic.List(); + foreach (var (table, _) in ArchiveService.ArchivableTables) + { + var raw = new Regex($@"\bFROM\s+{Regex.Escape(table)}\b"); + foreach (var file in sources) + { + foreach (Match match in raw.Matches(File.ReadAllText(file))) + { + offenders.Add($"{Path.GetFileName(file)}: {match.Value}"); + } + } + } + + Assert.True(offenders.Count == 0, + "Analysis reads archivable tables raw (use the v_ archive view so history survives the 512 MB reset):\n" + + string.Join("\n", offenders.Distinct())); + } + + private static async Task ExecuteAsync(DuckDBConnection connection, string sql) + { + using var cmd = connection.CreateCommand(); + cmd.CommandText = sql; + await cmd.ExecuteNonQueryAsync(TestContext.Current.CancellationToken); + } + + private static string FindRepoDirectory(string relativePath) + { + var dir = AppContext.BaseDirectory; + for (int i = 0; i < 8 && dir is not null; i++) + { + var candidate = Path.Combine(dir, relativePath); + if (Directory.Exists(candidate)) + { + return candidate; + } + dir = Path.GetDirectoryName(dir); + } + throw new DirectoryNotFoundException($"Could not locate {relativePath} walking up from {AppContext.BaseDirectory}"); + } +} diff --git a/Lite/Analysis/AnalysisService.cs b/Lite/Analysis/AnalysisService.cs index 0636848524..3d14edd619 100644 --- a/Lite/Analysis/AnalysisService.cs +++ b/Lite/Analysis/AnalysisService.cs @@ -335,7 +335,9 @@ public async Task CleanupAsync(int retentionDays = 30) /// the analysis window. A server with 100 hours of total history can safely /// be analyzed over a 4-hour window without dilution. /// - private async Task GetTotalDataSpanHoursAsync(int serverId) + /* Internal for AnalysisDataSpanTests (#1809): the span must survive an archive/reset, which is + only observable with a real DuckDB + parquet fixture. */ + internal async Task GetTotalDataSpanHoursAsync(int serverId) { try { @@ -346,7 +348,7 @@ private async Task GetTotalDataSpanHoursAsync(int serverId) using var cmd = connection.CreateCommand(); cmd.CommandText = @" SELECT EXTRACT(EPOCH FROM (MAX(collection_time) - MIN(collection_time))) / 3600.0 -FROM wait_stats +FROM v_wait_stats WHERE server_id = $1"; cmd.Parameters.Add(new DuckDBParameter { Value = serverId }); diff --git a/Lite/Analysis/DrillDownCollector.Plans.cs b/Lite/Analysis/DrillDownCollector.Plans.cs index d2d788d6ca..23bcc5cd10 100644 --- a/Lite/Analysis/DrillDownCollector.Plans.cs +++ b/Lite/Analysis/DrillDownCollector.Plans.cs @@ -136,7 +136,7 @@ private async Task CollectPlanAdvisoryDetail(AnalysisFinding finding, AnalysisCo using var cmd = connection.CreateCommand(); cmd.CommandText = @" SELECT query_plan_xml -FROM query_stats +FROM v_query_stats WHERE server_id = $1 AND collection_time >= $2 AND collection_time <= $3 diff --git a/Lite/Analysis/DuckDbFactCollector.Activity.cs b/Lite/Analysis/DuckDbFactCollector.Activity.cs index bb14ad1217..b2fd66fc6e 100644 --- a/Lite/Analysis/DuckDbFactCollector.Activity.cs +++ b/Lite/Analysis/DuckDbFactCollector.Activity.cs @@ -124,7 +124,7 @@ private async Task CollectActiveQueryFactsAsync(AnalysisContext context, List 1 THEN 1 END) AS parallel_count, MAX(dop) AS max_dop, COUNT(DISTINCT session_id) AS distinct_sessions -FROM query_snapshots +FROM v_query_snapshots WHERE server_id = $1 AND collection_time >= $2 AND collection_time <= $3"; @@ -185,7 +185,7 @@ private async Task CollectRunningJobFactsAsync(AnalysisContext context, List= $2 AND collection_time <= $3"; @@ -238,7 +238,7 @@ private async Task CollectSessionFactsAsync(AnalysisContext context, List WITH latest AS ( SELECT program_name, connection_count, running_count, sleeping_count, dormant_count, ROW_NUMBER() OVER (PARTITION BY program_name ORDER BY collection_time DESC) AS rn - FROM session_stats + FROM v_session_stats WHERE server_id = $1 AND collection_time >= $2 AND collection_time <= $3 diff --git a/Lite/Analysis/DuckDbFactCollector.Config.cs b/Lite/Analysis/DuckDbFactCollector.Config.cs index b7c67d5743..aafe3908b9 100644 --- a/Lite/Analysis/DuckDbFactCollector.Config.cs +++ b/Lite/Analysis/DuckDbFactCollector.Config.cs @@ -34,7 +34,7 @@ WITH latest AS ( configuration_name, value_in_use, ROW_NUMBER() OVER (PARTITION BY configuration_name ORDER BY capture_time DESC) AS rn - FROM server_config + FROM v_server_config WHERE server_id = $1 AND configuration_name IN ( 'cost threshold for parallelism', @@ -118,7 +118,7 @@ private async Task CollectServerMetadataFactsAsync(AnalysisContext context, List cmd.CommandText = @" SELECT engine_edition, CAST(SPLIT_PART(product_version, '.', 1) AS INTEGER) AS major_version -FROM server_properties +FROM v_server_properties WHERE server_id = $1 ORDER BY collection_time DESC LIMIT 1"; @@ -159,7 +159,7 @@ WITH latest AS ( is_query_store_on, compatibility_level, page_verify_option, is_accelerated_database_recovery_on, ROW_NUMBER() OVER (PARTITION BY database_name ORDER BY capture_time DESC) AS rn - FROM database_config + FROM v_database_config WHERE server_id = $1 ) SELECT @@ -234,7 +234,7 @@ private async Task CollectTraceFlagFactsAsync(AnalysisContext context, List= $2 AND collection_time <= $3 @@ -461,7 +461,7 @@ private async Task CollectPlanAdvisoryFactsAsync(AnalysisContext context, List= $2 AND collection_time <= $3 diff --git a/Lite/Analysis/DuckDbFactCollector.Resources.cs b/Lite/Analysis/DuckDbFactCollector.Resources.cs index 0348f76f7b..7bd98fada7 100644 --- a/Lite/Analysis/DuckDbFactCollector.Resources.cs +++ b/Lite/Analysis/DuckDbFactCollector.Resources.cs @@ -27,7 +27,7 @@ private async Task CollectMemoryFactsAsync(AnalysisContext context, List f using var cmd = connection.CreateCommand(); cmd.CommandText = @" SELECT total_physical_memory_mb, buffer_pool_mb, target_server_memory_mb -FROM memory_stats +FROM v_memory_stats WHERE server_id = $1 AND collection_time <= $2 ORDER BY collection_time DESC @@ -77,7 +77,7 @@ private async Task CollectRunnableTaskFactsAsync(AnalysisContext context, List= $2 AND collection_time <= $3 @@ -186,7 +186,7 @@ private async Task CollectMemoryClerkFactsAsync(AnalysisContext context, List WITH latest AS ( SELECT counter_name, cntr_value, delta_cntr_value, ROW_NUMBER() OVER (PARTITION BY counter_name ORDER BY collection_time DESC) AS rn - FROM perfmon_stats + FROM v_perfmon_stats WHERE server_id = $1 AND collection_time >= $2 AND collection_time <= $3 @@ -397,7 +397,7 @@ private async Task CollectPlanCacheFactsAsync(AnalysisContext context, List= 2 OR memory_indicators_system >= 2 THEN 1 ELSE 0 END) AS pressure_event_count, MAX(memory_indicators_process) AS max_process, MAX(memory_indicators_system) AS max_system -FROM memory_pressure_events +FROM v_memory_pressure_events WHERE server_id = $1 AND collection_time >= $2 AND collection_time <= $3"; diff --git a/Lite/Analysis/DuckDbFactCollector.Storage.cs b/Lite/Analysis/DuckDbFactCollector.Storage.cs index 4151944eaa..f8411b1360 100644 --- a/Lite/Analysis/DuckDbFactCollector.Storage.cs +++ b/Lite/Analysis/DuckDbFactCollector.Storage.cs @@ -29,7 +29,7 @@ private async Task CollectDatabaseSizeFactAsync(AnalysisContext context, List 0 @@ -215,7 +215,7 @@ private async Task CollectFileAutogrowthFactsAsync(AnalysisContext context, List WITH latest AS ( SELECT database_name, file_id, total_size_mb, is_percent_growth, ROW_NUMBER() OVER (PARTITION BY database_name, file_id ORDER BY collection_time DESC) AS rn - FROM database_size_stats + FROM v_database_size_stats WHERE server_id = $1 ) SELECT @@ -269,7 +269,7 @@ private async Task CollectDiskSpaceFactsAsync(AnalysisContext context, List 0 diff --git a/Lite/Analysis/DuckDbFactCollector.Waits.cs b/Lite/Analysis/DuckDbFactCollector.Waits.cs index 980636e627..caa2f8c0a3 100644 --- a/Lite/Analysis/DuckDbFactCollector.Waits.cs +++ b/Lite/Analysis/DuckDbFactCollector.Waits.cs @@ -92,7 +92,7 @@ private async Task CollectBlockingFactsAsync(AnalysisContext context, List MAX(wait_time_ms) AS max_wait_time_ms, COUNT(DISTINCT blocking_spid) AS distinct_head_blockers, COUNT(CASE WHEN blocking_status = 'sleeping' THEN 1 END) AS sleeping_blocker_count -FROM blocked_process_reports +FROM v_blocked_process_reports WHERE server_id = $1 AND collection_time >= $2 AND collection_time <= $3"; @@ -148,7 +148,7 @@ private async Task CollectDeadlockFactsAsync(AnalysisContext context, List using var command = connection.CreateCommand(); command.CommandText = @" SELECT COUNT(*) AS deadlock_count -FROM deadlocks +FROM v_deadlocks WHERE server_id = $1 AND collection_time >= $2 AND collection_time <= $3";