From 0d71985e12035cac2b8de975e75cfb63560fbbeb Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:01:09 -0400 Subject: [PATCH 01/12] Bucket Lite's Wait Stats trend and picker reads (#4234) GetWaitStatsTrendsByTypesAsync now buckets to TrendBudget.Chart's per-series point budget instead of returning every collection (196,800 rows at 7 days for 20 types, per #4234's measured number), following the #4304 ruling: a rated CTE sums wait and signal wait over rated seconds per bucket, avg_ms_per_wait is the summed wait over summed waiting tasks, and a point is stamped at its bucket's raw first_collection_time only when every bucket in the call holds exactly one physical collection. GetDistinctWaitTypesAsync is memoized through the new LiteNameListCache (per server and window length, 15-minute TTL, a hit also requires the cached window's end to stay within the TTL of the requested end) so the picker's full-window DISTINCT no longer reruns on every 1-minute auto-refresh. The cache is a standalone file so the Perfmon picker (and a later memory-clerk picker) can reuse it. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- Lite/Services/LiteNameListCache.cs | 76 ++++++++++++++ Lite/Services/LocalDataService.WaitStats.cs | 104 ++++++++++++++++---- 2 files changed, 162 insertions(+), 18 deletions(-) create mode 100644 Lite/Services/LiteNameListCache.cs diff --git a/Lite/Services/LiteNameListCache.cs b/Lite/Services/LiteNameListCache.cs new file mode 100644 index 0000000000..396e3058de --- /dev/null +++ b/Lite/Services/LiteNameListCache.cs @@ -0,0 +1,76 @@ +/* + * 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.Collections.Concurrent; +using System.Collections.Generic; + +namespace PerformanceMonitorLite.Services; + +/// +/// #4234: TTL memoization for the Wait Stats and Perfmon picker's name-list reads +/// (, ), +/// each a full-window DISTINCT that used to run on every 1-minute auto-refresh. A server's set of +/// collected names barely moves inside its own window, so this caches the list per (server, window length) +/// for and lets a refresh inside that window reuse it — window LENGTH, not bounds, because a +/// sliding preset's start/end both move every tick while the length stays constant, and keying on the moving +/// bounds would never hit. The Darling twin is ViewerNameListCache (#4234, PR #4304); this port keeps +/// its rules so the two SKUs cache the same way. +/// +/// The key is (server, length) alone, but a hit ALSO needs the cached window's END to still be +/// recent. Keying on length alone would let a stale entry answer for a window it never fetched: a user +/// viewing this week's 7-day range, then switching to a custom 7-day range from last month, would get THIS +/// WEEK's list under last month's window, because both windows share one length-keyed slot. A hit therefore +/// also requires the cached to sit within of the requested +/// end — the same freshness distance the fetch timestamp already checks, applied to the window itself rather +/// than the wall clock, and checked BOTH directions (Duration()) since a custom range can move the end +/// either way, not just forward like a sliding preset's auto-refresh does. +/// +/// Meant to be reused beside any other Lite picker that reads a name list over a window the same way +/// (one instance per , matching that service's own per-tab lifetime). +/// +internal sealed class LiteNameListCache +{ + /// How long a fetched list is trusted before the next call re-runs the DISTINCT (the #4234 ruling's + /// number). + public static readonly TimeSpan Ttl = TimeSpan.FromMinutes(15); + + private readonly record struct Key(int ServerId, int WindowLengthMinutes); + private readonly record struct Entry(List Names, DateTime WindowEndUtc, DateTime FetchedAtUtc); + + private readonly ConcurrentDictionary _entries = new(); + + /// True (with populated) when a list for this server and window length was + /// fetched within of AND that fetch's window end sits within + /// of — two independent freshness checks, because a same-length + /// window from a different point in time is a different window, not a refresh of this one. + public bool TryGet(int serverId, TimeSpan windowLength, DateTime endUtc, DateTime nowUtc, out List names) + { + if (_entries.TryGetValue(KeyFor(serverId, windowLength), out var entry) + && nowUtc - entry.FetchedAtUtc < Ttl + && (endUtc - entry.WindowEndUtc).Duration() < Ttl) + { + names = entry.Names; + return true; + } + + names = new List(); + return false; + } + + /// Records a freshly fetched list for the window ending , stamped + /// . + public void Set(int serverId, TimeSpan windowLength, DateTime endUtc, List names, DateTime nowUtc) => + _entries[KeyFor(serverId, windowLength)] = new Entry(names, endUtc, nowUtc); + + /// Rounded to the whole minute: a preset window's length is already a whole number of hours and a + /// custom range is quarter-hour granular, so rounding only absorbs floating-point noise in the subtraction, + /// never two genuinely different windows into one key. + private static Key KeyFor(int serverId, TimeSpan windowLength) => + new(serverId, (int)Math.Round(windowLength.TotalMinutes)); +} diff --git a/Lite/Services/LocalDataService.WaitStats.cs b/Lite/Services/LocalDataService.WaitStats.cs index 98a7e19453..7cd6a2af3d 100644 --- a/Lite/Services/LocalDataService.WaitStats.cs +++ b/Lite/Services/LocalDataService.WaitStats.cs @@ -11,12 +11,16 @@ using System.Threading.Tasks; using DuckDB.NET.Data; using PerformanceMonitor.Alerting; +using PerformanceMonitor.Common; using PerformanceMonitor.Ui; namespace PerformanceMonitorLite.Services; public partial class LocalDataService { + /// #4234: TTL memoization for — see . + private readonly LiteNameListCache _distinctWaitTypesCache = new(); + /* #1240: the wait-stats tab shares the collector's ignored-wait list and excludes those types at DISPLAY time, so benign waits already in the DuckDB (collected before the filter was active) don't surface in the tab/picker — copying the JSON only stops new collection, not existing rows. */ @@ -110,14 +114,26 @@ FROM v_wait_stats /// /// Gets the distinct wait types that have been collected for a server. + /// #4234: memoized through — keyed on (server, window length) + /// for , so the full-window read behind this runs at most once per 15 + /// minutes rather than on every 1-minute auto-refresh. is the cache's clock + /// seam — null uses the wall clock; a test passes an explicit time to fast-forward past the TTL without + /// sleeping. /// - public async Task> GetDistinctWaitTypesAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null) + public async Task> GetDistinctWaitTypesAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null, DateTime? nowUtc = null) { + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); + + var effectiveNow = nowUtc ?? DateTime.UtcNow; + var windowLength = endTime - startTime; + if (_distinctWaitTypesCache.TryGet(serverId, windowLength, endTime, effectiveNow, out var cached)) + { + return cached; + } + using var connection = await OpenConnectionAsync(); using var command = connection.CreateCommand(); - var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); - var exclude = IgnoredWaitTypes.BuildExclusionClause(_ignoredWaitTypes.Value); command.CommandText = $@" SELECT @@ -141,6 +157,8 @@ GROUP BY wait_type { items.Add(reader.GetString(0)); } + + _distinctWaitTypesCache.Set(serverId, windowLength, endTime, items, effectiveNow); return items; } @@ -216,10 +234,21 @@ FROM raw /// Batched sibling of : fetches the per-second trend for /// ALL selected wait types in ONE query (replacing an N+1 query-per-type loop), grouped by type. /// The LAG window is partitioned by wait_type so each type's per-second rate is computed independently. + /// #4234: buckets to 's point budget PER SERIES (the ruling's own + /// wording, not the MCP convention of dividing one shared budget across every line a call draws), so the + /// pin is rows <= budget * series count; seriesCount is therefore always 1 into + /// . A bucket's rate is its summed wait over its summed rated + /// seconds (time-weighted, never an average of per-collection rates), and a bucket with no rated + /// collection is dropped (HAVING), same as the per-collection read always dropped that collection. + /// When EVERY bucket the whole call returned holds exactly one physical collection, each point is stamped + /// at its bucket's first_collection_time (that one collection's own raw time) instead of + /// bucket_start, a time_bucket grid line; a single merged bucket anywhere (any wait type) + /// keeps bucket_start throughout. Darling's twin is ViewerDataService.WaitTrendsSql / + /// GetWaitStatsTrendsByTypesAsync (#4234, PR #4304). /// public async Task>> GetWaitStatsTrendsByTypesAsync(int serverId, List waitTypes, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) { - using var _q = TimeQuery("GetWaitStatsTrendsByTypesAsync", "v_wait_stats trends batched by type"); + using var _q = TimeQuery("GetWaitStatsTrendsByTypesAsync", "v_wait_stats trends batched by type, bucketed"); var result = new Dictionary>(); if (waitTypes.Count == 0) return result; @@ -228,6 +257,10 @@ public async Task>> GetWaitStatsTre var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); var typeParams = string.Join(", ", waitTypes.Select((_, i) => "$" + (i + 4))); + var widthParam = "$" + (waitTypes.Count + 4); + + var windowMinutes = Math.Max(1, (int)Math.Ceiling((endTime - startTime).TotalMinutes)); + var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); command.CommandText = $@" WITH raw AS @@ -248,43 +281,78 @@ FROM v_wait_stats AND collection_time >= $2 AND collection_time <= $3 AND wait_type IN ({typeParams}) +), +rated AS +( + SELECT + wait_type, + collection_time, + CASE WHEN interval_seconds > 0 THEN CAST(delta_wait_time_ms AS DOUBLE PRECISION) END AS rated_wait_ms, + CASE WHEN interval_seconds > 0 THEN CAST(delta_signal_wait_time_ms AS DOUBLE PRECISION) END AS rated_signal_ms, + CASE WHEN interval_seconds > 0 THEN interval_seconds END AS rated_seconds, + CASE WHEN interval_seconds > 0 THEN delta_waiting_tasks END AS rated_tasks + FROM raw ) SELECT wait_type, - collection_time, - CASE WHEN interval_seconds > 0 THEN CAST(delta_wait_time_ms AS DOUBLE PRECISION) / interval_seconds END AS wait_time_ms_per_second, - CASE WHEN interval_seconds > 0 THEN CAST(delta_signal_wait_time_ms AS DOUBLE PRECISION) / interval_seconds END AS signal_wait_time_ms_per_second, - CASE WHEN interval_seconds > 0 AND delta_waiting_tasks > 0 THEN CAST(delta_wait_time_ms AS DOUBLE PRECISION) / delta_waiting_tasks WHEN interval_seconds > 0 THEN 0 END AS avg_ms_per_wait -FROM raw -ORDER BY wait_type, collection_time"; + GREATEST(time_bucket(to_minutes(CAST({widthParam} AS INTEGER)), collection_time, {TrendBuckets.OriginSql}), $2) AS bucket_start, + SUM(rated_wait_ms) / SUM(rated_seconds) AS wait_time_ms_per_second, + SUM(rated_signal_ms) / SUM(rated_seconds) AS signal_wait_time_ms_per_second, + CASE WHEN SUM(rated_tasks) > 0 THEN SUM(rated_wait_ms) / SUM(rated_tasks) ELSE 0 END AS avg_ms_per_wait, + MIN(collection_time) AS first_collection_time, + COUNT(*) AS collection_count +FROM rated +GROUP BY wait_type, 2 +HAVING COUNT(rated_seconds) > 0 +ORDER BY wait_type, 2"; command.Parameters.Add(new DuckDBParameter { Value = serverId }); command.Parameters.Add(new DuckDBParameter { Value = startTime }); command.Parameters.Add(new DuckDBParameter { Value = endTime }); foreach (var wt in waitTypes) command.Parameters.Add(new DuckDBParameter { Value = wt }); + command.Parameters.Add(new DuckDBParameter { Value = bucketMinutes }); + + var rows = new List<(string WaitType, DateTime BucketStart, DateTime FirstCollectionTime, double Rate, double Signal, double Avg)>(); + var everyBucketSingleton = true; using var reader = await command.ExecuteReaderAsync(); while (await reader.ReadAsync()) { - /* A NULL rate is an unknowable interval (#3540): the row is dropped, not read as 0. */ + /* A NULL rate is a bucket with no rated collection; HAVING already excludes it. */ if (reader.IsDBNull(2)) { continue; } - var wt = reader.GetString(0); - if (!result.TryGetValue(wt, out var list)) + if (reader.GetInt64(6) != 1) + { + everyBucketSingleton = false; + } + + rows.Add(( + reader.GetString(0), + reader.GetDateTime(1), + reader.GetDateTime(5), + reader.GetDouble(2), + reader.IsDBNull(3) ? 0 : reader.GetDouble(3), + reader.IsDBNull(4) ? 0 : reader.GetDouble(4))); + } + + foreach (var row in rows) + { + if (!result.TryGetValue(row.WaitType, out var list)) { list = new List(); - result[wt] = list; + result[row.WaitType] = list; } + list.Add(new WaitStatsTrendPoint { - CollectionTime = reader.GetDateTime(1), - WaitTimeMsPerSecond = reader.GetDouble(2), - SignalWaitTimeMsPerSecond = reader.IsDBNull(3) ? 0 : reader.GetDouble(3), - AvgMsPerWait = reader.IsDBNull(4) ? 0 : reader.GetDouble(4) + CollectionTime = everyBucketSingleton ? row.FirstCollectionTime : row.BucketStart, + WaitTimeMsPerSecond = row.Rate, + SignalWaitTimeMsPerSecond = row.Signal, + AvgMsPerWait = row.Avg }); } From 02fad7b660da82af941c70078c04d4f95068938b Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:02:26 -0400 Subject: [PATCH 02/12] Bucket Lite's Perfmon trend and picker reads (#4234) GetPerfmonTrendsByCountersAsync now wraps the unchanged per-collection SUM as a subquery and re-aggregates into TrendBudget.Chart-sized buckets instead of returning every collection (118,080 rows at 7 days for 12 counters, per #4234's measured number), following the #4304 ruling: Value is the bucket's rounded gauge average, DeltaValue/SampleIntervalSeconds are summed only over rated collections so their ratio downstream stays the ruling's summed-deltas-over-summed-intervals rate, and a point is stamped at its bucket's raw first_collection_time only when every bucket in the call holds exactly one physical collection. GetDistinctPerfmonCountersAsync is memoized through the LiteNameListCache T3b added alongside the Wait Stats picker, so this picker's full-window DISTINCT also stops rerunning on every 1-minute auto-refresh. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- Lite/Services/LocalDataService.Perfmon.cs | 117 +++++++++++++++++----- 1 file changed, 93 insertions(+), 24 deletions(-) diff --git a/Lite/Services/LocalDataService.Perfmon.cs b/Lite/Services/LocalDataService.Perfmon.cs index c62e5b9b55..55724b4584 100644 --- a/Lite/Services/LocalDataService.Perfmon.cs +++ b/Lite/Services/LocalDataService.Perfmon.cs @@ -10,11 +10,15 @@ using System.Collections.Generic; using System.Threading.Tasks; using DuckDB.NET.Data; +using PerformanceMonitor.Common; namespace PerformanceMonitorLite.Services; public partial class LocalDataService { + /// #4234: TTL memoization for — see . + private readonly LiteNameListCache _distinctPerfmonCountersCache = new(); + /// /// Gets the latest perfmon counters for a server. /// @@ -58,14 +62,26 @@ FROM v_perfmon_stats /// /// Gets the distinct perfmon counter names for a server. + /// #4234: memoized through — keyed on (server, window + /// length) for , so the full-window DISTINCT behind this runs at + /// most once per 15 minutes rather than on every 1-minute auto-refresh. is the + /// cache's clock seam — null uses the wall clock; a test passes an explicit time to fast-forward past the + /// TTL without sleeping. /// - public async Task> GetDistinctPerfmonCountersAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null) + public async Task> GetDistinctPerfmonCountersAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null, DateTime? nowUtc = null) { + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); + + var effectiveNow = nowUtc ?? DateTime.UtcNow; + var windowLength = endTime - startTime; + if (_distinctPerfmonCountersCache.TryGet(serverId, windowLength, endTime, effectiveNow, out var cached)) + { + return cached; + } + using var connection = await OpenConnectionAsync(); using var command = connection.CreateCommand(); - var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); - command.CommandText = @" SELECT DISTINCT counter_name FROM v_perfmon_stats @@ -84,6 +100,8 @@ FROM v_perfmon_stats { items.Add(reader.GetString(0)); } + + _distinctPerfmonCountersCache.Set(serverId, windowLength, endTime, items, effectiveNow); return items; } @@ -139,10 +157,23 @@ GROUP BY collection_time /// /// Batched sibling of : fetches the trend for ALL selected /// counters in ONE query (replacing an N+1 query-per-counter loop), grouped by counter name. + /// #4234: wraps the unchanged per-collection SUM as a subquery and re-aggregates into buckets sized + /// to 's point budget PER SERIES (seriesCount always 1 into + /// , the ruling's own wording). 's + /// fields keep their per-collection MEANING: Value is the bucket's rounded GAUGE average (the + /// ruling's rule for a gauge); DeltaValue/SampleIntervalSeconds are summed only over rated + /// collections (a stored interval > 0), so DeltaValue / SampleIntervalSeconds downstream is + /// still the ruling's rate (summed deltas over summed intervals), and both are NULL together for a bucket + /// with no rated collection — the same "no delta" state an unrated per-collection row always carried; + /// CntrType keeps the per-collection MIN=MAX-agreement rule, now double-aggregated. When EVERY + /// bucket the whole call returned holds exactly one physical collection, each point is stamped at its + /// bucket's raw first_collection_time instead of the time_bucket grid line; a single merged + /// bucket anywhere (any counter) keeps bucket_start throughout. Darling's twin is + /// ViewerDataService.PerfmonTrendsSql / GetPerfmonTrendsByCountersAsync (#4234, PR #4304). /// public async Task>> GetPerfmonTrendsByCountersAsync(int serverId, List counterNames, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) { - using var _q = TimeQuery("GetPerfmonTrendsByCountersAsync", "v_perfmon_stats trends batched by counter"); + using var _q = TimeQuery("GetPerfmonTrendsByCountersAsync", "v_perfmon_stats trends batched by counter, bucketed"); var result = new Dictionary>(); if (counterNames.Count == 0) return result; @@ -151,45 +182,83 @@ public async Task>> GetPerfmonTrendsB var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); var nameParams = string.Join(", ", counterNames.Select((_, i) => "$" + (i + 4))); + var widthParam = "$" + (counterNames.Count + 4); + + var windowMinutes = Math.Max(1, (int)Math.Ceiling((endTime - startTime).TotalMinutes)); + var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); command.CommandText = $@" SELECT counter_name, - collection_time, - SUM(cntr_value) AS cntr_value, - SUM(delta_cntr_value) AS delta_cntr_value, - MAX(sample_interval_seconds) AS sample_interval_seconds, - CASE WHEN MIN(cntr_type) = MAX(cntr_type) THEN MAX(cntr_type) END AS cntr_type -FROM v_perfmon_stats -WHERE server_id = $1 -AND collection_time >= $2 -AND collection_time <= $3 -AND counter_name IN ({nameParams}) -GROUP BY counter_name, collection_time -ORDER BY counter_name, collection_time"; + GREATEST(time_bucket(to_minutes(CAST({widthParam} AS INTEGER)), collection_time, {TrendBuckets.OriginSql}), $2) AS bucket_start, + CAST(ROUND(AVG(cntr_value)) AS BIGINT) AS cntr_value, + SUM(delta_cntr_value) FILTER (WHERE sample_interval_seconds > 0) AS delta_cntr_value, + SUM(sample_interval_seconds) FILTER (WHERE sample_interval_seconds > 0) AS sample_interval_seconds, + CASE WHEN MIN(cntr_type) = MAX(cntr_type) THEN MAX(cntr_type) END AS cntr_type, + MIN(collection_time) AS first_collection_time, + COUNT(*) AS collection_count +FROM ( + SELECT + counter_name, + collection_time, + SUM(cntr_value) AS cntr_value, + SUM(delta_cntr_value) AS delta_cntr_value, + MAX(sample_interval_seconds) AS sample_interval_seconds, + CASE WHEN MIN(cntr_type) = MAX(cntr_type) THEN MAX(cntr_type) END AS cntr_type + FROM v_perfmon_stats + WHERE server_id = $1 + AND collection_time >= $2 + AND collection_time <= $3 + AND counter_name IN ({nameParams}) + GROUP BY counter_name, collection_time +) AS collections +GROUP BY counter_name, 2 +ORDER BY counter_name, 2"; command.Parameters.Add(new DuckDBParameter { Value = serverId }); command.Parameters.Add(new DuckDBParameter { Value = startTime }); command.Parameters.Add(new DuckDBParameter { Value = endTime }); foreach (var cn in counterNames) command.Parameters.Add(new DuckDBParameter { Value = cn }); + command.Parameters.Add(new DuckDBParameter { Value = bucketMinutes }); + + var rows = new List<(string CounterName, DateTime BucketStart, DateTime FirstCollectionTime, long Value, long? DeltaValue, long? SampleIntervalSeconds, int? CntrType)>(); + var everyBucketSingleton = true; using var reader = await command.ExecuteReaderAsync(); while (await reader.ReadAsync()) { - var cn = reader.GetString(0); - if (!result.TryGetValue(cn, out var list)) + if (reader.GetInt64(7) != 1) + { + everyBucketSingleton = false; + } + + rows.Add(( + reader.GetString(0), + reader.GetDateTime(1), + reader.GetDateTime(6), + reader.IsDBNull(2) ? 0 : reader.GetInt64(2), + /* NULL stays NULL: a gauge's instance rows store no delta (v62), so the SUM is NULL, not 0. */ + reader.IsDBNull(3) ? null : reader.GetInt64(3), + reader.IsDBNull(4) ? null : Convert.ToInt64(reader.GetValue(4)), + reader.IsDBNull(5) ? null : Convert.ToInt32(reader.GetValue(5)))); + } + + foreach (var row in rows) + { + if (!result.TryGetValue(row.CounterName, out var list)) { list = new List(); - result[cn] = list; + result[row.CounterName] = list; } + list.Add(new PerfmonTrendPoint { - CollectionTime = reader.GetDateTime(1), - Value = reader.IsDBNull(2) ? 0 : reader.GetInt64(2), - DeltaValue = reader.IsDBNull(3) ? null : reader.GetInt64(3), - SampleIntervalSeconds = reader.IsDBNull(4) ? null : Convert.ToInt64(reader.GetValue(4)), - CntrType = reader.IsDBNull(5) ? null : Convert.ToInt32(reader.GetValue(5)) + CollectionTime = everyBucketSingleton ? row.FirstCollectionTime : row.BucketStart, + Value = row.Value, + DeltaValue = row.DeltaValue, + SampleIntervalSeconds = row.SampleIntervalSeconds, + CntrType = row.CntrType }); } From bec8f297eb9b38e3f855e398d55621d6fcfbe7fd Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:08:32 -0400 Subject: [PATCH 03/12] Fix InvalidCastException: read the new bucketed SUM columns through ToInt64/ToDouble DuckDB promotes SUM() over an INTEGER column to HUGEINT, which DuckDB.NET hands back as a boxed BigInteger; Convert.ToInt64/Convert.ToInt32 and the reader's typed GetInt64/ GetDouble accessors cannot cast that. PerfmonCounterTypeReadTests caught this against the new GetPerfmonTrendsByCountersAsync bucketing (its SUM(sample_interval_seconds) FILTER column). Both new readers now route their SUM-derived columns through the existing ToInt64/ToDouble helpers, matching GetWaitBucketsAsync/GetPerfmonBucketsAsync's own established pattern for the same FILTER-summed shape. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- Lite/Services/LocalDataService.Perfmon.cs | 11 +++++++---- Lite/Services/LocalDataService.WaitStats.cs | 9 ++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/Lite/Services/LocalDataService.Perfmon.cs b/Lite/Services/LocalDataService.Perfmon.cs index 55724b4584..9629751e1c 100644 --- a/Lite/Services/LocalDataService.Perfmon.cs +++ b/Lite/Services/LocalDataService.Perfmon.cs @@ -237,11 +237,14 @@ AND counter_name IN ({nameParams}) reader.GetString(0), reader.GetDateTime(1), reader.GetDateTime(6), - reader.IsDBNull(2) ? 0 : reader.GetInt64(2), + /* ToInt64, not GetInt64/Convert.ToInt64: DuckDB's SUM over an INTEGER column (sample_interval_seconds) + promotes to HUGEINT, which the driver hands back as a boxed BigInteger that Convert.ToInt64 cannot + cast — the same reason GetPerfmonBucketsAsync's own FILTER-summed columns route through this helper. */ + reader.IsDBNull(2) ? 0 : ToInt64(reader.GetValue(2)), /* NULL stays NULL: a gauge's instance rows store no delta (v62), so the SUM is NULL, not 0. */ - reader.IsDBNull(3) ? null : reader.GetInt64(3), - reader.IsDBNull(4) ? null : Convert.ToInt64(reader.GetValue(4)), - reader.IsDBNull(5) ? null : Convert.ToInt32(reader.GetValue(5)))); + reader.IsDBNull(3) ? null : ToInt64(reader.GetValue(3)), + reader.IsDBNull(4) ? null : ToInt64(reader.GetValue(4)), + reader.IsDBNull(5) ? null : (int)ToInt64(reader.GetValue(5)))); } foreach (var row in rows) diff --git a/Lite/Services/LocalDataService.WaitStats.cs b/Lite/Services/LocalDataService.WaitStats.cs index 7cd6a2af3d..e42df22071 100644 --- a/Lite/Services/LocalDataService.WaitStats.cs +++ b/Lite/Services/LocalDataService.WaitStats.cs @@ -334,9 +334,12 @@ HAVING COUNT(rated_seconds) > 0 reader.GetString(0), reader.GetDateTime(1), reader.GetDateTime(5), - reader.GetDouble(2), - reader.IsDBNull(3) ? 0 : reader.GetDouble(3), - reader.IsDBNull(4) ? 0 : reader.GetDouble(4))); + /* ToDouble, not GetDouble: a SUM over an INTEGER-typed column (e.g. rated_tasks, inside the + avg_ms_per_wait division) can come back as a boxed BigInteger DuckDB HUGEINT, which GetDouble + does not accept — see GetWaitBucketsAsync's own FILTER-summed columns for the same guard. */ + ToDouble(reader.GetValue(2)), + reader.IsDBNull(3) ? 0 : ToDouble(reader.GetValue(3)), + reader.IsDBNull(4) ? 0 : ToDouble(reader.GetValue(4)))); } foreach (var row in rows) From b841587ad976a14e49c98ec73e37653fd8edbac2 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:11:30 -0400 Subject: [PATCH 04/12] Pull the two bucketed statements into internal static SQL-text methods WaitTrendsSql(waitTypeCount) and PerfmonTrendsSql(counterCount) mirror Darling's ViewerDataService.WaitTrendsSql / PerfmonTrendsSql (#4234, PR #4304): the SQL text generation moves out of the two Get*TrendsByTypesAsync/ByCountersAsync methods into its own testable method, so a source check for the bucket width and the first_collection_time/collection_count columns needs no live DuckDB. No behavior change, same generated text. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- Lite/Services/LocalDataService.Perfmon.cs | 43 +++++++++++++-------- Lite/Services/LocalDataService.WaitStats.cs | 43 +++++++++++++-------- 2 files changed, 54 insertions(+), 32 deletions(-) diff --git a/Lite/Services/LocalDataService.Perfmon.cs b/Lite/Services/LocalDataService.Perfmon.cs index 9629751e1c..810b2a180f 100644 --- a/Lite/Services/LocalDataService.Perfmon.cs +++ b/Lite/Services/LocalDataService.Perfmon.cs @@ -171,23 +171,17 @@ GROUP BY collection_time /// bucket anywhere (any counter) keeps bucket_start throughout. Darling's twin is /// ViewerDataService.PerfmonTrendsSql / GetPerfmonTrendsByCountersAsync (#4234, PR #4304). /// - public async Task>> GetPerfmonTrendsByCountersAsync(int serverId, List counterNames, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) + /// + /// The bucketed batched-trend statement text (#4234), pulled out of + /// so its shape (the bucket width in its own trailing parameter, after the dynamic counter_name IN (...) + /// list so that list's numbering does not shift) is checkable without a live DuckDB. Darling's twin is + /// ViewerDataService.PerfmonTrendsSql. + /// + internal static string PerfmonTrendsSql(int counterCount) { - using var _q = TimeQuery("GetPerfmonTrendsByCountersAsync", "v_perfmon_stats trends batched by counter, bucketed"); - var result = new Dictionary>(); - if (counterNames.Count == 0) return result; - - using var connection = await OpenConnectionAsync(); - using var command = connection.CreateCommand(); - - var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); - var nameParams = string.Join(", ", counterNames.Select((_, i) => "$" + (i + 4))); - var widthParam = "$" + (counterNames.Count + 4); - - var windowMinutes = Math.Max(1, (int)Math.Ceiling((endTime - startTime).TotalMinutes)); - var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); - - command.CommandText = $@" + var nameParams = string.Join(", ", Enumerable.Range(0, counterCount).Select(i => "$" + (i + 4))); + var widthParam = "$" + (counterCount + 4); + return $@" SELECT counter_name, GREATEST(time_bucket(to_minutes(CAST({widthParam} AS INTEGER)), collection_time, {TrendBuckets.OriginSql}), $2) AS bucket_start, @@ -214,6 +208,23 @@ AND counter_name IN ({nameParams}) ) AS collections GROUP BY counter_name, 2 ORDER BY counter_name, 2"; + } + + public async Task>> GetPerfmonTrendsByCountersAsync(int serverId, List counterNames, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) + { + using var _q = TimeQuery("GetPerfmonTrendsByCountersAsync", "v_perfmon_stats trends batched by counter, bucketed"); + var result = new Dictionary>(); + if (counterNames.Count == 0) return result; + + using var connection = await OpenConnectionAsync(); + using var command = connection.CreateCommand(); + + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); + + var windowMinutes = Math.Max(1, (int)Math.Ceiling((endTime - startTime).TotalMinutes)); + var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); + + command.CommandText = PerfmonTrendsSql(counterNames.Count); command.Parameters.Add(new DuckDBParameter { Value = serverId }); command.Parameters.Add(new DuckDBParameter { Value = startTime }); diff --git a/Lite/Services/LocalDataService.WaitStats.cs b/Lite/Services/LocalDataService.WaitStats.cs index e42df22071..f2ea225015 100644 --- a/Lite/Services/LocalDataService.WaitStats.cs +++ b/Lite/Services/LocalDataService.WaitStats.cs @@ -246,23 +246,17 @@ FROM raw /// keeps bucket_start throughout. Darling's twin is ViewerDataService.WaitTrendsSql / /// GetWaitStatsTrendsByTypesAsync (#4234, PR #4304). /// - public async Task>> GetWaitStatsTrendsByTypesAsync(int serverId, List waitTypes, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) + /// + /// The bucketed batched-trend statement text (#4234), pulled out of + /// so its shape (the bucket width in its own trailing parameter, after the dynamic wait_type IN (...) + /// list so that list's $4.. numbering does not shift) is checkable without a live DuckDB. Darling's + /// twin is ViewerDataService.WaitTrendsSql. + /// + internal static string WaitTrendsSql(int waitTypeCount) { - using var _q = TimeQuery("GetWaitStatsTrendsByTypesAsync", "v_wait_stats trends batched by type, bucketed"); - var result = new Dictionary>(); - if (waitTypes.Count == 0) return result; - - using var connection = await OpenConnectionAsync(); - using var command = connection.CreateCommand(); - - var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); - var typeParams = string.Join(", ", waitTypes.Select((_, i) => "$" + (i + 4))); - var widthParam = "$" + (waitTypes.Count + 4); - - var windowMinutes = Math.Max(1, (int)Math.Ceiling((endTime - startTime).TotalMinutes)); - var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); - - command.CommandText = $@" + var typeParams = string.Join(", ", Enumerable.Range(0, waitTypeCount).Select(i => "$" + (i + 4))); + var widthParam = "$" + (waitTypeCount + 4); + return $@" WITH raw AS ( SELECT @@ -305,6 +299,23 @@ FROM rated GROUP BY wait_type, 2 HAVING COUNT(rated_seconds) > 0 ORDER BY wait_type, 2"; + } + + public async Task>> GetWaitStatsTrendsByTypesAsync(int serverId, List waitTypes, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) + { + using var _q = TimeQuery("GetWaitStatsTrendsByTypesAsync", "v_wait_stats trends batched by type, bucketed"); + var result = new Dictionary>(); + if (waitTypes.Count == 0) return result; + + using var connection = await OpenConnectionAsync(); + using var command = connection.CreateCommand(); + + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); + + var windowMinutes = Math.Max(1, (int)Math.Ceiling((endTime - startTime).TotalMinutes)); + var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); + + command.CommandText = WaitTrendsSql(waitTypes.Count); command.Parameters.Add(new DuckDBParameter { Value = serverId }); command.Parameters.Add(new DuckDBParameter { Value = startTime }); From 397993b80340d843fa6cdb36f3382cdc2279b85c Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:16:14 -0400 Subject: [PATCH 05/12] Pin the #4234 ruling for Lite's Wait Stats and Perfmon trend/picker reads Four groups, all against embedded DuckDB (no live SQL Server/Postgres needed): LiteTrendBucketWidthSqlTests (source checks: WaitTrendsSql/PerfmonTrendsSql carry a time_bucket width plus first_collection_time/collection_count), LiteNameListCacheTests (the cache's hit/miss rules, ported from Darling's ViewerNameListCacheTests), LiteTrendBucketingLiveTests' two budget-cap tests (a 7-day, one-per-minute window returns at most TrendBudget.Chart.AutoPoints * series rows) and two point-equality tests (three :37-second collections come back with their raw timestamps and values unchanged, not floored to the bucket grid), plus the two pickers' cache proof (a second call inside 15 minutes misses a wait type/counter the store gained since, a call past the TTL picks it up). Proven by hand against the pre-#4234 text (read directly while building this PR): the old GetWaitStatsTrendsByTypesAsync/GetPerfmonTrendsByCountersAsync had no time_bucket, first_collection_time or collection_count anywhere, so the source checks fail there outright, and the old per-collection read returns one row per collection -- 10,080 for the 7-day one-per-minute seed, over the budget the cap tests assert. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- .../LiteWaitPerfmonTrendBucketingTests.cs | 372 ++++++++++++++++++ 1 file changed, 372 insertions(+) create mode 100644 Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs diff --git a/Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs b/Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs new file mode 100644 index 0000000000..e81af239e4 --- /dev/null +++ b/Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs @@ -0,0 +1,372 @@ +/* + * 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.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using DuckDB.NET.Data; +using PerformanceMonitor.Common; +using PerformanceMonitorLite.Database; +using PerformanceMonitorLite.Services; +using Xunit; + +namespace PerformanceMonitorLite.Tests; + +/// +/// #4234 (lane T3b, the Lite twin of PR #4304): the ruling's four required tests for +/// / GetPerfmonTrendsByCountersAsync and their +/// two picker reads. No live SQL Server or Postgres needed — DuckDB is the embedded store itself. +/// Proven once by hand against the pre-#4234 text: reverting / +/// PerfmonTrendsSql to the per-collection SELECT (no rated CTE, no time_bucket, no width +/// parameter) fails 's source checks directly, and fails +/// / +/// its Perfmon twin (10,080 one-minute collections over 7 days is 10,080 rows per series, not ≤ 1,500). +/// +public sealed class LiteTrendBucketWidthSqlTests +{ + [Fact] + public void WaitTrendsSql_CarriesABucketWidth() + { + /* server/start/end = $1-$3, 2 wait types = $4-$5, so the width is $6. */ + var sql = LocalDataService.WaitTrendsSql(2); + Assert.Contains("time_bucket(to_minutes(CAST($6 AS INTEGER))", sql, StringComparison.Ordinal); + Assert.Contains(TrendBuckets.OriginSql, sql, StringComparison.Ordinal); + } + + [Fact] + public void PerfmonTrendsSql_CarriesABucketWidth() + { + var sql = LocalDataService.PerfmonTrendsSql(2); + Assert.Contains("time_bucket(to_minutes(CAST($6 AS INTEGER))", sql, StringComparison.Ordinal); + Assert.Contains(TrendBuckets.OriginSql, sql, StringComparison.Ordinal); + } + + [Fact] + public void WaitTrendsSql_And_PerfmonTrendsSql_ProjectFirstCollectionTimeAndCollectionCount() + { + foreach (var sql in new[] { LocalDataService.WaitTrendsSql(1), LocalDataService.PerfmonTrendsSql(1) }) + { + Assert.Contains("MIN(collection_time) AS first_collection_time", sql, StringComparison.Ordinal); + Assert.Contains("COUNT(*) AS collection_count", sql, StringComparison.Ordinal); + } + } +} + +/// +/// 's hit/miss rules, exercised directly — no store needed since the cache never +/// touches DuckDB itself. Mirrors Darling's ViewerNameListCacheTests (#4234, PR #4304) so the two SKUs' +/// picker caches are pinned the same way. +/// +public sealed class LiteNameListCacheTests +{ + private static readonly TimeSpan Length = TimeSpan.FromHours(24); + + [Fact] + public void TryGet_MissesInitially() + { + var cache = new LiteNameListCache(); + var now = DateTime.UtcNow; + Assert.False(cache.TryGet(1, Length, now, now, out _)); + } + + [Fact] + public void TryGet_HitsWithinTtl_SameWindowEnd() + { + var cache = new LiteNameListCache(); + var fetchedAt = new DateTime(2026, 3, 1, 12, 0, 0); + var end = new DateTime(2026, 3, 1, 11, 0, 0); + var names = new List { "CXPACKET" }; + cache.Set(1, Length, end, names, fetchedAt); + + /* The ruling's own pin: a second refresh inside 15 minutes runs no DISTINCT — this IS that refresh, + proven at the mechanism that prevents the query: TryGet returns the cached list. */ + Assert.True(cache.TryGet(1, Length, end, fetchedAt.AddMinutes(14), out var cached)); + Assert.Same(names, cached); + } + + [Fact] + public void TryGet_MissesAtFifteenMinutes_WallClockExpiry() + { + var cache = new LiteNameListCache(); + var fetchedAt = new DateTime(2026, 3, 1, 12, 0, 0); + var end = new DateTime(2026, 3, 1, 11, 0, 0); + cache.Set(1, Length, end, new List { "CXPACKET" }, fetchedAt); + + /* One after 15 minutes DOES run the DISTINCT again — the ruling's other half of the same pin. */ + Assert.False(cache.TryGet(1, Length, end, fetchedAt.AddMinutes(15), out _)); + } + + [Fact] + public void TryGet_Misses_WhenCachedWindowEndIsFarFromRequestedEnd_DespiteFreshFetch() + { + var cache = new LiteNameListCache(); + var now = new DateTime(2026, 3, 1, 12, 0, 0); + cache.Set(1, TimeSpan.FromDays(7), now, new List { "CXPACKET" }, now); + + var lastMonthsEnd = now.AddDays(-30); + Assert.False(cache.TryGet(1, TimeSpan.FromDays(7), lastMonthsEnd, now, out _)); + } + + [Fact] + public void TryGet_Hits_WhenRequestedEndIsSlightlyBeforeCachedEnd_TwoSidedCheck() + { + var cache = new LiteNameListCache(); + var now = new DateTime(2026, 3, 1, 12, 0, 0); + cache.Set(1, Length, now, new List { "CXPACKET" }, now); + + Assert.True(cache.TryGet(1, Length, now.AddMinutes(-5), now, out _)); + } +} + +/// +/// The ruling's remaining two tests (the 7-day row-budget cap, and point equality when the budget covers every +/// collection) plus the pickers' end-to-end cache behavior, against a real embedded DuckDB. +/// +/* GetWaitStatsTrendsByTypesAsync / GetPerfmonTrendsByCountersAsync's fromDate/toDate overload is server-local + (GetTimeRange converts it back to UTC through ServerTimeHelper.UtcOffsetMinutes, a process-wide mutable + static) — every fromDate/toDate this class passes is meant as an exact UTC instant, so the offset is pinned + to 0 for the class's lifetime. xUnit runs test classes in parallel, so this joins the collection the other + offset-touching classes use rather than racing them. */ +[Collection("server-time-helper")] +public sealed class LiteTrendBucketingLiveTests : IClassFixture, IDisposable +{ + private const int ServerId = -4234300; + private const string ServerName = "trend-bucketing-e2e"; + + private readonly DuckDbInitializer _duckDb; + private readonly LocalDataService _dataService; + private readonly int _savedUtcOffsetMinutes; + private long _nextId = -1; + private DuckDBConnection? _seedConn; + + public LiteTrendBucketingLiveTests(SharedDuckDbFixture fixture) + { + fixture.ResetData(); + _duckDb = fixture.DuckDb; + _dataService = new LocalDataService(_duckDb); + _savedUtcOffsetMinutes = ServerTimeHelper.UtcOffsetMinutes; + ServerTimeHelper.UtcOffsetMinutes = 0; + } + + public void Dispose() + { + ServerTimeHelper.UtcOffsetMinutes = _savedUtcOffsetMinutes; + _seedConn?.Dispose(); + } + + [Fact] + public async Task WaitTrend_SevenDayWindow_ReturnsAtMostBudgetTimesSeriesRows() + { + var end = new DateTime(2026, 3, 10, 0, 0, 0); + var start = end.AddDays(-7); + var waitTypes = new List { "CXPACKET", "WRITELOG" }; + + foreach (var waitType in waitTypes) + { + await BulkSeedWaitAsync(start, end, waitType); + } + + var trends = await _dataService.GetWaitStatsTrendsByTypesAsync(ServerId, waitTypes, fromDate: start, toDate: end); + + var totalRows = trends.Values.Sum(list => list.Count); + var budget = TrendBudget.Chart.AutoPoints * waitTypes.Count; + Assert.True(totalRows > 0 && totalRows <= budget, $"{totalRows} rows over a {waitTypes.Count}-series budget of {budget}"); + } + + [Fact] + public async Task PerfmonTrend_SevenDayWindow_ReturnsAtMostBudgetTimesSeriesRows() + { + var end = new DateTime(2026, 3, 10, 0, 0, 0); + var start = end.AddDays(-7); + var counters = new List { "Batch Requests/sec", "Page life expectancy" }; + + foreach (var counter in counters) + { + await BulkSeedPerfmonAsync(start, end, counter); + } + + var trends = await _dataService.GetPerfmonTrendsByCountersAsync(ServerId, counters, fromDate: start, toDate: end); + + var totalRows = trends.Values.Sum(list => list.Count); + var budget = TrendBudget.Chart.AutoPoints * counters.Count; + Assert.True(totalRows > 0 && totalRows <= budget, $"{totalRows} rows over a {counters.Count}-series budget of {budget}"); + } + + [Fact] + public async Task WaitTrend_BudgetCoversEveryCollection_ReturnsRawTimestampsAndValuesUnchanged() + { + /* Off the minute grid (:37 seconds) — on the pre-#4234 bucketed code every point would have been + floored to the time_bucket grid line, losing the seconds. */ + var t1 = new DateTime(2026, 3, 10, 9, 0, 37); + var t2 = t1.AddMinutes(5); + var t3 = t2.AddMinutes(5); + + await SeedWaitAsync(t1, "CXPACKET", deltaMs: 300, deltaSignal: 30, deltaTasks: 10, interval: 300); + await SeedWaitAsync(t2, "CXPACKET", deltaMs: 600, deltaSignal: 60, deltaTasks: 20, interval: 300); + await SeedWaitAsync(t3, "CXPACKET", deltaMs: 900, deltaSignal: 90, deltaTasks: 30, interval: 300); + + var trends = await _dataService.GetWaitStatsTrendsByTypesAsync( + ServerId, new List { "CXPACKET" }, fromDate: t1.AddMinutes(-1), toDate: t3.AddMinutes(1)); + + var points = trends["CXPACKET"]; + Assert.Equal(new[] { t1, t2, t3 }, points.Select(p => p.CollectionTime).ToArray()); + Assert.Equal(1.0, points[0].WaitTimeMsPerSecond, precision: 6); + Assert.Equal(2.0, points[1].WaitTimeMsPerSecond, precision: 6); + Assert.Equal(3.0, points[2].WaitTimeMsPerSecond, precision: 6); + Assert.Equal(30.0, points[0].AvgMsPerWait, precision: 6); + Assert.Equal(30.0, points[1].AvgMsPerWait, precision: 6); + } + + [Fact] + public async Task PerfmonTrend_BudgetCoversEveryCollection_ReturnsRawTimestampsAndValuesUnchanged() + { + var t1 = new DateTime(2026, 3, 10, 9, 0, 37); + var t2 = t1.AddMinutes(5); + var t3 = t2.AddMinutes(5); + + await SeedPerfmonAsync(t1, "Batch Requests/sec", cntr: 1000, delta: 300, interval: 300); + await SeedPerfmonAsync(t2, "Batch Requests/sec", cntr: 1300, delta: 600, interval: 300); + await SeedPerfmonAsync(t3, "Batch Requests/sec", cntr: 1900, delta: 900, interval: 300); + + var trends = await _dataService.GetPerfmonTrendsByCountersAsync( + ServerId, new List { "Batch Requests/sec" }, fromDate: t1.AddMinutes(-1), toDate: t3.AddMinutes(1)); + + var points = trends["Batch Requests/sec"]; + Assert.Equal(new[] { t1, t2, t3 }, points.Select(p => p.CollectionTime).ToArray()); + Assert.Equal(new long[] { 1000, 1300, 1900 }, points.Select(p => p.Value).ToArray()); + Assert.Equal(new long?[] { 300, 600, 900 }, points.Select(p => p.DeltaValue).ToArray()); + Assert.Equal(new long?[] { 300, 300, 300 }, points.Select(p => p.SampleIntervalSeconds).ToArray()); + } + + /// The ruling's picker test: a second call inside 15 minutes runs no DISTINCT (the store gains a + /// second wait type, but a within-TTL call still answers with the cached one-type list), and a call past + /// the TTL reruns it and picks up the change. + [Fact] + public async Task DistinctWaitTypes_SecondCallInsideTtl_SkipsTheReread_MissesPastTtl() + { + var end = new DateTime(2026, 3, 1, 12, 30, 0); + await SeedWaitAsync(end.AddMinutes(-5), "CXPACKET", deltaMs: 100, deltaSignal: 10, deltaTasks: 5, interval: 60); + + var first = await _dataService.GetDistinctWaitTypesAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end); + Assert.Equal(new[] { "CXPACKET" }, first); + + await SeedWaitAsync(end.AddMinutes(-5), "WRITELOG", deltaMs: 50, deltaSignal: 5, deltaTasks: 2, interval: 60); + + var withinTtl = await _dataService.GetDistinctWaitTypesAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(14)); + Assert.Equal(new[] { "CXPACKET" }, withinTtl); + + var pastTtl = await _dataService.GetDistinctWaitTypesAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(16)); + Assert.Equal(2, pastTtl.Count); + } + + /// The Perfmon picker's same two halves of the ruling's cache pin. + [Fact] + public async Task DistinctPerfmonCounters_SecondCallInsideTtl_SkipsTheReread_MissesPastTtl() + { + var end = new DateTime(2026, 3, 1, 12, 30, 0); + await SeedPerfmonAsync(end.AddMinutes(-5), "Batch Requests/sec", cntr: 1000, delta: 300, interval: 300); + + var first = await _dataService.GetDistinctPerfmonCountersAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end); + Assert.Equal(new[] { "Batch Requests/sec" }, first); + + await SeedPerfmonAsync(end.AddMinutes(-5), "Page life expectancy", cntr: 500, delta: null, interval: null); + + var withinTtl = await _dataService.GetDistinctPerfmonCountersAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(14)); + Assert.Equal(new[] { "Batch Requests/sec" }, withinTtl); + + var pastTtl = await _dataService.GetDistinctPerfmonCountersAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(16)); + Assert.Equal(2, pastTtl.Count); + } + + /* ---- seeding ---------------------------------------------------------------------------------------- */ + + private async Task SeedConnectionAsync() + { + if (_seedConn is null) + { + _seedConn = _duckDb.CreateConnection(); + await _seedConn.OpenAsync(); + } + + return _seedConn; + } + + private async Task SeedWaitAsync(DateTime at, string waitType, long deltaMs, long deltaSignal, long deltaTasks, int interval) + { + using var readLock = _duckDb.AcquireReadLock(); + var conn = await SeedConnectionAsync(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = @"INSERT INTO wait_stats + (collection_id, collection_time, server_id, server_name, wait_type, + waiting_tasks_count, wait_time_ms, signal_wait_time_ms, + delta_waiting_tasks, delta_wait_time_ms, delta_signal_wait_time_ms, sample_interval_seconds) + VALUES ($1, $2, $3, $4, $5, 0, 0, 0, $6, $7, $8, $9)"; + foreach (var v in new object[] { _nextId--, at, ServerId, ServerName, waitType, deltaTasks, deltaMs, deltaSignal, interval }) + { + cmd.Parameters.Add(new DuckDBParameter { Value = v }); + } + await cmd.ExecuteNonQueryAsync(); + } + + private async Task SeedPerfmonAsync(DateTime at, string counterName, long cntr, long? delta, int? interval) + { + using var readLock = _duckDb.AcquireReadLock(); + var conn = await SeedConnectionAsync(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = @"INSERT INTO perfmon_stats + (collection_id, collection_time, server_id, server_name, object_name, counter_name, instance_name, + cntr_value, delta_cntr_value, sample_interval_seconds) + VALUES ($1, $2, $3, $4, 'SQLServer:Test', $5, '', $6, $7, $8)"; + foreach (var v in new object[] { _nextId--, at, ServerId, ServerName, counterName, cntr, (object?)delta ?? DBNull.Value, (object?)interval ?? DBNull.Value }) + { + cmd.Parameters.Add(new DuckDBParameter { Value = v }); + } + await cmd.ExecuteNonQueryAsync(); + } + + /// One row a minute across the whole window, through generate_series in a single INSERT + /// (10,080 rows for a 7-day window) — the budget-cap tests' bulk seed. Rated throughout (a fixed 60-second + /// interval) so the read's own point count, not an unrated-collection drop, is what the assertion checks. + private async Task BulkSeedWaitAsync(DateTime start, DateTime end, string waitType) + { + using var readLock = _duckDb.AcquireReadLock(); + var conn = await SeedConnectionAsync(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = @"INSERT INTO wait_stats + (collection_id, collection_time, server_id, server_name, wait_type, + waiting_tasks_count, wait_time_ms, signal_wait_time_ms, + delta_waiting_tasks, delta_wait_time_ms, delta_signal_wait_time_ms, sample_interval_seconds) + SELECT $1 - ROW_NUMBER() OVER (ORDER BY t), t, $2, $3, $4, 0, 0, 0, 5, 100, 10, 60 + FROM generate_series(CAST($5 AS TIMESTAMP), CAST($6 AS TIMESTAMP), INTERVAL 1 MINUTE) AS s(t)"; + foreach (var v in new object[] { _nextId, ServerId, ServerName, waitType, start, end }) + { + cmd.Parameters.Add(new DuckDBParameter { Value = v }); + } + await cmd.ExecuteNonQueryAsync(); + _nextId -= (long)(end - start).TotalMinutes + 2; + } + + private async Task BulkSeedPerfmonAsync(DateTime start, DateTime end, string counterName) + { + using var readLock = _duckDb.AcquireReadLock(); + var conn = await SeedConnectionAsync(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = @"INSERT INTO perfmon_stats + (collection_id, collection_time, server_id, server_name, object_name, counter_name, instance_name, + cntr_value, delta_cntr_value, sample_interval_seconds) + SELECT $1 - ROW_NUMBER() OVER (ORDER BY t), t, $2, $3, 'SQLServer:Test', $4, '', 1000, 60, 60 + FROM generate_series(CAST($5 AS TIMESTAMP), CAST($6 AS TIMESTAMP), INTERVAL 1 MINUTE) AS s(t)"; + foreach (var v in new object[] { _nextId, ServerId, ServerName, counterName, start, end }) + { + cmd.Parameters.Add(new DuckDBParameter { Value = v }); + } + await cmd.ExecuteNonQueryAsync(); + _nextId -= (long)(end - start).TotalMinutes + 2; + } +} From 3f2b32a25e41f38a063503434d284c1e0d6fb4f6 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:29:54 -0400 Subject: [PATCH 06/12] Narrow the pre-#4234 interval/delta SUM pins for the new bucket-level aggregation PerfmonIntervalAggregationTests read the whole Perfmon.cs source and pinned exact occurrence counts from before bucketing existed. The bucketed batched trend adds a second, correct layer of aggregation on top of the unchanged per-collection one: the outer GROUP BY counter_name, bucket_start sums each collection's own already-MAX'd interval and already-summed delta across the collections a bucket holds (the ruling's summed-deltas-over-summed-intervals rate) -- the full suite caught both stale pins. NoPerfmonTrendRead_SumsTheInterval (renamed *AcrossInstanceRows) now strips the new SUM(sample_interval_seconds) FILTER (WHERE sample_interval_seconds > 0) form before asserting no bare SUM(sample_interval_seconds) remains, so it still catches the original bug shape (summing the interval across a collection's instance rows) while allowing the new bucket-level one, which always carries that exact FILTER. TheAdditiveColumnsStaySummed's SUM(delta_cntr_value) count moves from 2 to 3 for the same reason; SUM(cntr_value) stays 2 because that outer layer AVERAGES (the ruling's gauge rule), adding no third SUM. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- Lite.Tests/PerfmonIntervalAggregationTests.cs | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/Lite.Tests/PerfmonIntervalAggregationTests.cs b/Lite.Tests/PerfmonIntervalAggregationTests.cs index 5342567818..bd051ef6d7 100644 --- a/Lite.Tests/PerfmonIntervalAggregationTests.cs +++ b/Lite.Tests/PerfmonIntervalAggregationTests.cs @@ -45,27 +45,41 @@ public void BothPerfmonTrendReads_TakeTheIntervalAsMax() Assert.Equal(2, max); } - /// The failure mode itself: a summed interval anywhere in this read path is the 12-17x - /// denominator inflation, and it is silent — the column is populated, the query succeeds, and only - /// the derived rate is wrong. + /// The failure mode itself: a summed interval across a collection's INSTANCE rows is the + /// 12-17x denominator inflation, and it is silent — the column is populated, the query succeeds, and + /// only the derived rate is wrong. + /// #4234: the batched trend's bucketing legitimately adds a SECOND, different SUM over + /// sample_interval_seconds — the outer GROUP BY counter_name, bucket_start summing each + /// COLLECTION's own already-MAX'd interval across the collections a bucket holds (the ruling's + /// summed-intervals rate), never raw instance rows. That SUM always carries this exact + /// FILTER (WHERE sample_interval_seconds > 0) right after it, so stripping just that + /// substring isolates exactly the original bug shape: a bare, unfiltered + /// SUM(sample_interval_seconds) grouped over instance rows. [Fact] - public void NoPerfmonTrendRead_SumsTheInterval() + public void NoPerfmonTrendRead_SumsTheIntervalAcrossInstanceRows() { var source = ParitySource.ReadFile(ReadPath); - Assert.DoesNotContain("SUM(sample_interval_seconds)", source, StringComparison.Ordinal); + var withoutTheBucketedSum = source.Replace( + "SUM(sample_interval_seconds) FILTER (WHERE sample_interval_seconds > 0)", string.Empty); + Assert.DoesNotContain("SUM(sample_interval_seconds)", withoutTheBucketedSum, StringComparison.Ordinal); } /// Guards the fix from being over-applied: the two columns that ARE additive must stay sums, /// in both reads. A well-meant "make it consistent" edit that turned these into MAX would silently - /// report one instance's value as the whole counter. + /// report one instance's value as the whole counter. + /// #4234: delta_cntr_value is 3, not 2 — the batched trend's bucketing adds one more SUM, + /// the outer GROUP BY counter_name, bucket_start summing each collection's already-summed delta + /// across the bucket's collections (the ruling's summed-deltas rate), same additive column, one layer + /// higher. cntr_value stays 2: that outer layer AVERAGES (the ruling's gauge rule), so it adds no + /// third SUM. [Fact] public void TheAdditiveColumnsStaySummed() { var source = ParitySource.ReadFile(ReadPath); Assert.Equal(2, CountOccurrences(source, "SUM(cntr_value)")); - Assert.Equal(2, CountOccurrences(source, "SUM(delta_cntr_value)")); + Assert.Equal(3, CountOccurrences(source, "SUM(delta_cntr_value)")); Assert.DoesNotContain("MAX(cntr_value)", source, StringComparison.Ordinal); Assert.DoesNotContain("MAX(delta_cntr_value)", source, StringComparison.Ordinal); } From 7dad7c5a64ad5fd919274a8b4d90d2a4e9820d3b Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:39:43 -0400 Subject: [PATCH 07/12] MCP wait/perfmon name-list tools stay uncached, only the picker memoizes #4234 ruling: GetDistinctWaitTypesAsync and GetDistinctPerfmonCountersAsync went back to their dev shape (uncached, no nowUtc seam) since MCP's wait and perfmon tools call them directly and must never answer with a picker snapshot up to 15 minutes stale. The TTL cache moved to two new entry points, GetDistinctWaitTypesForPickerAsync and GetDistinctPerfmonCountersForPickerAsync, which only ServerTab's picker refresh calls now. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- .../LiteWaitPerfmonTrendBucketingTests.cs | 53 +++++++++++++++---- Lite/Controls/ServerTab.Refresh.cs | 4 +- Lite/Services/LiteNameListCache.cs | 18 ++++--- Lite/Services/LocalDataService.Perfmon.cs | 50 +++++++++++------ Lite/Services/LocalDataService.WaitStats.cs | 49 +++++++++++------ 5 files changed, 124 insertions(+), 50 deletions(-) diff --git a/Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs b/Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs index e81af239e4..b62af3a475 100644 --- a/Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs +++ b/Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs @@ -246,44 +246,79 @@ public async Task PerfmonTrend_BudgetCoversEveryCollection_ReturnsRawTimestampsA /// The ruling's picker test: a second call inside 15 minutes runs no DISTINCT (the store gains a /// second wait type, but a within-TTL call still answers with the cached one-type list), and a call past - /// the TTL reruns it and picks up the change. + /// the TTL reruns it and picks up the change. Only the picker entry point takes nowUtc / caches at + /// all — itself stays uncached for MCP. [Fact] - public async Task DistinctWaitTypes_SecondCallInsideTtl_SkipsTheReread_MissesPastTtl() + public async Task DistinctWaitTypesForPicker_SecondCallInsideTtl_SkipsTheReread_MissesPastTtl() { var end = new DateTime(2026, 3, 1, 12, 30, 0); await SeedWaitAsync(end.AddMinutes(-5), "CXPACKET", deltaMs: 100, deltaSignal: 10, deltaTasks: 5, interval: 60); - var first = await _dataService.GetDistinctWaitTypesAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end); + var first = await _dataService.GetDistinctWaitTypesForPickerAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end); Assert.Equal(new[] { "CXPACKET" }, first); await SeedWaitAsync(end.AddMinutes(-5), "WRITELOG", deltaMs: 50, deltaSignal: 5, deltaTasks: 2, interval: 60); - var withinTtl = await _dataService.GetDistinctWaitTypesAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(14)); + var withinTtl = await _dataService.GetDistinctWaitTypesForPickerAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(14)); Assert.Equal(new[] { "CXPACKET" }, withinTtl); - var pastTtl = await _dataService.GetDistinctWaitTypesAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(16)); + var pastTtl = await _dataService.GetDistinctWaitTypesForPickerAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(16)); Assert.Equal(2, pastTtl.Count); } /// The Perfmon picker's same two halves of the ruling's cache pin. [Fact] - public async Task DistinctPerfmonCounters_SecondCallInsideTtl_SkipsTheReread_MissesPastTtl() + public async Task DistinctPerfmonCountersForPicker_SecondCallInsideTtl_SkipsTheReread_MissesPastTtl() { var end = new DateTime(2026, 3, 1, 12, 30, 0); await SeedPerfmonAsync(end.AddMinutes(-5), "Batch Requests/sec", cntr: 1000, delta: 300, interval: 300); - var first = await _dataService.GetDistinctPerfmonCountersAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end); + var first = await _dataService.GetDistinctPerfmonCountersForPickerAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end); Assert.Equal(new[] { "Batch Requests/sec" }, first); await SeedPerfmonAsync(end.AddMinutes(-5), "Page life expectancy", cntr: 500, delta: null, interval: null); - var withinTtl = await _dataService.GetDistinctPerfmonCountersAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(14)); + var withinTtl = await _dataService.GetDistinctPerfmonCountersForPickerAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(14)); Assert.Equal(new[] { "Batch Requests/sec" }, withinTtl); - var pastTtl = await _dataService.GetDistinctPerfmonCountersAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(16)); + var pastTtl = await _dataService.GetDistinctPerfmonCountersForPickerAsync(ServerId, hoursBack: 1, asOfUtc: end, nowUtc: end.AddMinutes(16)); Assert.Equal(2, pastTtl.Count); } + /// MCP's guard: the shared read never + /// caches, so two calls back to back — even inside the picker's 15-minute TTL — each re-run the DISTINCT and + /// see whatever is in the store at call time, not a memoized snapshot from the first call. + [Fact] + public async Task DistinctWaitTypes_SharedMethod_NeverCaches_SecondCallSeesNewlySeededType() + { + var end = new DateTime(2026, 3, 1, 12, 30, 0); + await SeedWaitAsync(end.AddMinutes(-5), "CXPACKET", deltaMs: 100, deltaSignal: 10, deltaTasks: 5, interval: 60); + + var first = await _dataService.GetDistinctWaitTypesAsync(ServerId, hoursBack: 1, asOfUtc: end); + Assert.Equal(new[] { "CXPACKET" }, first); + + await SeedWaitAsync(end.AddMinutes(-5), "WRITELOG", deltaMs: 50, deltaSignal: 5, deltaTasks: 2, interval: 60); + + var second = await _dataService.GetDistinctWaitTypesAsync(ServerId, hoursBack: 1, asOfUtc: end); + Assert.Equal(2, second.Count); + } + + /// The Perfmon twin: never caches. + [Fact] + public async Task DistinctPerfmonCounters_SharedMethod_NeverCaches_SecondCallSeesNewlySeededCounter() + { + var end = new DateTime(2026, 3, 1, 12, 30, 0); + await SeedPerfmonAsync(end.AddMinutes(-5), "Batch Requests/sec", cntr: 1000, delta: 300, interval: 300); + + var first = await _dataService.GetDistinctPerfmonCountersAsync(ServerId, hoursBack: 1, asOfUtc: end); + Assert.Equal(new[] { "Batch Requests/sec" }, first); + + await SeedPerfmonAsync(end.AddMinutes(-5), "Page life expectancy", cntr: 500, delta: null, interval: null); + + var second = await _dataService.GetDistinctPerfmonCountersAsync(ServerId, hoursBack: 1, asOfUtc: end); + Assert.Equal(2, second.Count); + } + /* ---- seeding ---------------------------------------------------------------------------------------- */ private async Task SeedConnectionAsync() diff --git a/Lite/Controls/ServerTab.Refresh.cs b/Lite/Controls/ServerTab.Refresh.cs index 4367d46df6..beb00818ed 100644 --- a/Lite/Controls/ServerTab.Refresh.cs +++ b/Lite/Controls/ServerTab.Refresh.cs @@ -215,7 +215,7 @@ private async System.Threading.Tasks.Task RefreshWaitStatsAsync(int hoursBack, D { try { - var waitTypesTask = Task.Run(() => _dataService.GetDistinctWaitTypesAsync(_serverId, hoursBack, fromDate, toDate)); + var waitTypesTask = Task.Run(() => _dataService.GetDistinctWaitTypesForPickerAsync(_serverId, hoursBack, fromDate, toDate)); await waitTypesTask; PopulateWaitTypePicker(waitTypesTask.Result); await UpdateWaitStatsChartFromPickerAsync(); @@ -749,7 +749,7 @@ private async System.Threading.Tasks.Task RefreshPerfmonAsync(int hoursBack, Dat { try { - var perfmonCountersTask = Task.Run(() => _dataService.GetDistinctPerfmonCountersAsync(_serverId, hoursBack, fromDate, toDate)); + var perfmonCountersTask = Task.Run(() => _dataService.GetDistinctPerfmonCountersForPickerAsync(_serverId, hoursBack, fromDate, toDate)); await perfmonCountersTask; PopulatePerfmonPicker(perfmonCountersTask.Result); await UpdatePerfmonChartFromPickerAsync(); diff --git a/Lite/Services/LiteNameListCache.cs b/Lite/Services/LiteNameListCache.cs index 396e3058de..e2c56e6d3b 100644 --- a/Lite/Services/LiteNameListCache.cs +++ b/Lite/Services/LiteNameListCache.cs @@ -14,13 +14,17 @@ namespace PerformanceMonitorLite.Services; /// /// #4234: TTL memoization for the Wait Stats and Perfmon picker's name-list reads -/// (, ), -/// each a full-window DISTINCT that used to run on every 1-minute auto-refresh. A server's set of -/// collected names barely moves inside its own window, so this caches the list per (server, window length) -/// for and lets a refresh inside that window reuse it — window LENGTH, not bounds, because a -/// sliding preset's start/end both move every tick while the length stays constant, and keying on the moving -/// bounds would never hit. The Darling twin is ViewerNameListCache (#4234, PR #4304); this port keeps -/// its rules so the two SKUs cache the same way. +/// (, +/// ), each a full-window DISTINCT +/// that used to run on every 1-minute auto-refresh. MCP shares the same underlying reads +/// (, ) +/// but calls them directly, uncached, so an MCP answer is never stale by — only the picker +/// trades that freshness for fewer full-window scans. A server's set of collected names barely moves inside +/// its own window, so this caches the list per (server, window length) for and lets a +/// refresh inside that window reuse it — window LENGTH, not bounds, because a sliding preset's start/end both +/// move every tick while the length stays constant, and keying on the moving bounds would never hit. The +/// Darling twin is ViewerNameListCache (#4234, PR #4304); this port keeps its rules so the two SKUs +/// cache the same way. /// /// The key is (server, length) alone, but a hit ALSO needs the cached window's END to still be /// recent. Keying on length alone would let a stale entry answer for a window it never fetched: a user diff --git a/Lite/Services/LocalDataService.Perfmon.cs b/Lite/Services/LocalDataService.Perfmon.cs index 810b2a180f..45d06db83e 100644 --- a/Lite/Services/LocalDataService.Perfmon.cs +++ b/Lite/Services/LocalDataService.Perfmon.cs @@ -16,7 +16,7 @@ namespace PerformanceMonitorLite.Services; public partial class LocalDataService { - /// #4234: TTL memoization for — see . + /// #4234: TTL memoization for — see . private readonly LiteNameListCache _distinctPerfmonCountersCache = new(); /// @@ -62,26 +62,17 @@ FROM v_perfmon_stats /// /// Gets the distinct perfmon counter names for a server. - /// #4234: memoized through — keyed on (server, window - /// length) for , so the full-window DISTINCT behind this runs at - /// most once per 15 minutes rather than on every 1-minute auto-refresh. is the - /// cache's clock seam — null uses the wall clock; a test passes an explicit time to fast-forward past the - /// TTL without sleeping. + /// Uncached: MCP's perfmon-counter tool calls this directly so an MCP answer is never stale by + /// . The picker's memoized entry point is + /// . /// - public async Task> GetDistinctPerfmonCountersAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null, DateTime? nowUtc = null) + public async Task> GetDistinctPerfmonCountersAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null) { - var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); - - var effectiveNow = nowUtc ?? DateTime.UtcNow; - var windowLength = endTime - startTime; - if (_distinctPerfmonCountersCache.TryGet(serverId, windowLength, endTime, effectiveNow, out var cached)) - { - return cached; - } - using var connection = await OpenConnectionAsync(); using var command = connection.CreateCommand(); + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); + command.CommandText = @" SELECT DISTINCT counter_name FROM v_perfmon_stats @@ -101,6 +92,33 @@ FROM v_perfmon_stats items.Add(reader.GetString(0)); } + return items; + } + + /// + /// Picker-only entry point for : checks + /// first, falls back to the shared uncached read, then + /// memoizes it. + /// #4234: keyed on (server, window length) for , so the + /// full-window DISTINCT behind this runs at most once per 15 minutes rather than on every 1-minute + /// auto-refresh. Only ServerTab's picker goes through this cache — MCP shares the same underlying + /// read but calls directly so it never sees a cached answer. + /// is the cache's clock seam — null uses the wall clock; a test passes an + /// explicit time to fast-forward past the TTL without sleeping. + /// + public async Task> GetDistinctPerfmonCountersForPickerAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null, DateTime? nowUtc = null) + { + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); + + var effectiveNow = nowUtc ?? DateTime.UtcNow; + var windowLength = endTime - startTime; + if (_distinctPerfmonCountersCache.TryGet(serverId, windowLength, endTime, effectiveNow, out var cached)) + { + return cached; + } + + var items = await GetDistinctPerfmonCountersAsync(serverId, hoursBack, fromDate, toDate, asOfUtc); + _distinctPerfmonCountersCache.Set(serverId, windowLength, endTime, items, effectiveNow); return items; } diff --git a/Lite/Services/LocalDataService.WaitStats.cs b/Lite/Services/LocalDataService.WaitStats.cs index 3b43374bbc..59993fed91 100644 --- a/Lite/Services/LocalDataService.WaitStats.cs +++ b/Lite/Services/LocalDataService.WaitStats.cs @@ -18,7 +18,7 @@ namespace PerformanceMonitorLite.Services; public partial class LocalDataService { - /// #4234: TTL memoization for — see . + /// #4234: TTL memoization for — see . private readonly LiteNameListCache _distinctWaitTypesCache = new(); /* #1240: the wait-stats tab shares the collector's ignored-wait list and excludes those types at @@ -114,26 +114,17 @@ FROM v_wait_stats /// /// Gets the distinct wait types that have been collected for a server. - /// #4234: memoized through — keyed on (server, window length) - /// for , so the full-window read behind this runs at most once per 15 - /// minutes rather than on every 1-minute auto-refresh. is the cache's clock - /// seam — null uses the wall clock; a test passes an explicit time to fast-forward past the TTL without - /// sleeping. + /// Uncached: MCP's wait-type tools call this directly so an MCP answer is never stale by + /// . The picker's memoized entry point is + /// . /// - public async Task> GetDistinctWaitTypesAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null, DateTime? nowUtc = null) + public async Task> GetDistinctWaitTypesAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null) { - var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); - - var effectiveNow = nowUtc ?? DateTime.UtcNow; - var windowLength = endTime - startTime; - if (_distinctWaitTypesCache.TryGet(serverId, windowLength, endTime, effectiveNow, out var cached)) - { - return cached; - } - using var connection = await OpenConnectionAsync(); using var command = connection.CreateCommand(); + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); + var exclude = IgnoredWaitTypes.BuildExclusionClause(_ignoredWaitTypes.Value); command.CommandText = $@" SELECT @@ -158,6 +149,32 @@ GROUP BY wait_type items.Add(reader.GetString(0)); } + return items; + } + + /// + /// Picker-only entry point for : checks + /// first, falls back to the shared uncached read, then memoizes it. + /// #4234: keyed on (server, window length) for , so the + /// full-window read behind this runs at most once per 15 minutes rather than on every 1-minute + /// auto-refresh. Only ServerTab's picker goes through this cache — MCP shares the same underlying + /// read but calls directly so it never sees a cached answer. + /// is the cache's clock seam — null uses the wall clock; a test passes an + /// explicit time to fast-forward past the TTL without sleeping. + /// + public async Task> GetDistinctWaitTypesForPickerAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null, DateTime? nowUtc = null) + { + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); + + var effectiveNow = nowUtc ?? DateTime.UtcNow; + var windowLength = endTime - startTime; + if (_distinctWaitTypesCache.TryGet(serverId, windowLength, endTime, effectiveNow, out var cached)) + { + return cached; + } + + var items = await GetDistinctWaitTypesAsync(serverId, hoursBack, fromDate, toDate, asOfUtc); + _distinctWaitTypesCache.Set(serverId, windowLength, endTime, items, effectiveNow); return items; } From e40474dbb6c4a64ac252e135edf87bb2cffd660b Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:44:39 -0400 Subject: [PATCH 08/12] Add a live multi-instance Perfmon test for the 12-17x interval bug PerfmonIntervalAggregationTests pins the inner MAX(sample_interval_seconds) in source text, but no live test seeded more than one instance row per collection, so a regression turning that MAX into a SUM would have passed every DuckDB test in the file. Adds two: a single multi-instance collection (12 rows) landing alone in its bucket, and three such collections merged into one bucket, both asserting the bucket's DeltaValue/SampleIntervalSeconds land on the ruling's summed-deltas-over-summed-intervals rate. Proven by hand: reverting the inner MAX to SUM fails exactly these two tests. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- .../LiteWaitPerfmonTrendBucketingTests.cs | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs b/Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs index b62af3a475..ba891ebfa1 100644 --- a/Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs +++ b/Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs @@ -244,6 +244,57 @@ public async Task PerfmonTrend_BudgetCoversEveryCollection_ReturnsRawTimestampsA Assert.Equal(new long?[] { 300, 300, 300 }, points.Select(p => p.SampleIntervalSeconds).ToArray()); } + /// The real guard for the 12-17x bug (#2234): + /// pins the inner MAX(sample_interval_seconds) in source text, but no test before this one seeded + /// more than one instance row per collection — so a regression that turned that inner MAX into a SUM would + /// pass every other test in this file. Transactions/sec-shaped: 12 instance rows (the production median) on + /// the counter's ONE collection, each carrying the same delta and interval. A single-collection bucket's + /// DeltaValue/SampleIntervalSeconds must equal that one collection's already-correctly-aggregated values — + /// proven once by hand: reverting the inner MAX to SUM turns 300 into 3,600 here, a 12x-too-low rate. + /// + [Fact] + public async Task PerfmonTrend_MultiInstanceCollection_SingleCollectionBucket_TakesIntervalAsMaxNotSum() + { + var end = new DateTime(2026, 3, 12, 9, 5, 0); + var start = end.AddMinutes(-20); + var at = end.AddMinutes(-10); + + await SeedPerfmonMultiInstanceAsync(at, "Transactions/sec", instanceCount: 12, deltaPerInstance: 100, intervalPerInstance: 300); + + var trends = await _dataService.GetPerfmonTrendsByCountersAsync( + ServerId, new List { "Transactions/sec" }, fromDate: start, toDate: end); + + var point = Assert.Single(trends["Transactions/sec"]); + Assert.Equal(1200L, point.DeltaValue); + Assert.Equal(300L, point.SampleIntervalSeconds); + } + + /// The merged-bucket half: several collections — each already correctly aggregated across its own + /// 12 instance rows by the inner GROUP BY — land in the SAME bucket. The bucket's DeltaValue and + /// SampleIntervalSeconds are the SUM across those COLLECTIONS (the ruling's summed-deltas-over-summed- + /// intervals rate), never across instance rows, so the rate they imply stays 100 * 12 / 300 = 4/second no + /// matter how many collections merge — 3,600 / 900, not 3,600 / 10,800 (which a SUM over 36 instance rows' + /// worth of interval would give). + [Fact] + public async Task PerfmonTrend_MultiInstanceCollection_MergedBucket_SumsAcrossCollectionsAtFourPerSecond() + { + var end = new DateTime(2026, 3, 12, 9, 5, 0); + var start = end.AddMinutes(-20); + var bucketStart = end.AddMinutes(-15); + + await SeedPerfmonMultiInstanceAsync(bucketStart.AddSeconds(5), "Transactions/sec", instanceCount: 12, deltaPerInstance: 100, intervalPerInstance: 300); + await SeedPerfmonMultiInstanceAsync(bucketStart.AddSeconds(25), "Transactions/sec", instanceCount: 12, deltaPerInstance: 100, intervalPerInstance: 300); + await SeedPerfmonMultiInstanceAsync(bucketStart.AddSeconds(45), "Transactions/sec", instanceCount: 12, deltaPerInstance: 100, intervalPerInstance: 300); + + var trends = await _dataService.GetPerfmonTrendsByCountersAsync( + ServerId, new List { "Transactions/sec" }, fromDate: start, toDate: end); + + var point = Assert.Single(trends["Transactions/sec"]); + Assert.Equal(3600L, point.DeltaValue); + Assert.Equal(900L, point.SampleIntervalSeconds); + Assert.Equal(4.0, (double)point.DeltaValue!.Value / point.SampleIntervalSeconds!.Value, precision: 6); + } + /// The ruling's picker test: a second call inside 15 minutes runs no DISTINCT (the store gains a /// second wait type, but a within-TTL call still answers with the cached one-type list), and a call past /// the TTL reruns it and picks up the change. Only the picker entry point takes nowUtc / caches at @@ -365,6 +416,29 @@ private async Task SeedPerfmonAsync(DateTime at, string counterName, long cntr, await cmd.ExecuteNonQueryAsync(); } + /// Seeds ONE collection as instance rows — a counter whose DMV + /// shape is one row per database/file/whatever the instance is, all sharing one — so + /// the interval and delta aggregation runs over real instance rows instead of the single-row-per-collection + /// shape every other seed helper in this file uses. + private async Task SeedPerfmonMultiInstanceAsync(DateTime at, string counterName, int instanceCount, long deltaPerInstance, int intervalPerInstance) + { + using var readLock = _duckDb.AcquireReadLock(); + var conn = await SeedConnectionAsync(); + for (var i = 0; i < instanceCount; i++) + { + using var cmd = conn.CreateCommand(); + cmd.CommandText = @"INSERT INTO perfmon_stats + (collection_id, collection_time, server_id, server_name, object_name, counter_name, instance_name, + cntr_value, delta_cntr_value, sample_interval_seconds) + VALUES ($1, $2, $3, $4, 'SQLServer:Databases', $5, $6, $7, $8, $9)"; + foreach (var v in new object[] { _nextId--, at, ServerId, ServerName, counterName, "db" + i, deltaPerInstance, deltaPerInstance, intervalPerInstance }) + { + cmd.Parameters.Add(new DuckDBParameter { Value = v }); + } + await cmd.ExecuteNonQueryAsync(); + } + } + /// One row a minute across the whole window, through generate_series in a single INSERT /// (10,080 rows for a 7-day window) — the budget-cap tests' bulk seed. Rated throughout (a fixed 60-second /// interval) so the read's own point count, not an unrated-collection drop, is what the assertion checks. From cf0791f319d114dad51aa9df54434a37b998e65a Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 14:15:51 -0400 Subject: [PATCH 09/12] Bucket Lite's memory clerk trend and cache its picker's name list (#4234) GetMemoryClerkTrendsByTypesAsync now buckets to TrendBudget.Chart's per-series point budget, the same GREATEST(time_bucket(...), $2) shape PR #4331 gave GetWaitStatsTrendsByTypesAsync / GetPerfmonTrendsByCountersAsync. A clerk's memory is a gauge, so a bucket is the plain AVG(memory_mb) of its collections, with no rated/unrated split. GetDistinctMemoryClerkTypesForPickerAsync wraps the existing uncached read with LiteNameListCache, exactly like GetDistinctWaitTypesForPickerAsync, so the clerk picker's full-window DISTINCT runs at most once per 15 minutes instead of on every 1-minute auto-refresh. Only the picker's two callers in ServerTab.Refresh.cs move to it. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- Lite/Controls/ServerTab.Refresh.cs | 4 +- Lite/Services/LocalDataService.Memory.cs | 120 +++++++++++++++++++---- 2 files changed, 103 insertions(+), 21 deletions(-) diff --git a/Lite/Controls/ServerTab.Refresh.cs b/Lite/Controls/ServerTab.Refresh.cs index 475bb28f8b..560aa9fe50 100644 --- a/Lite/Controls/ServerTab.Refresh.cs +++ b/Lite/Controls/ServerTab.Refresh.cs @@ -479,7 +479,7 @@ private async System.Threading.Tasks.Task RefreshMemoryAsync(int hoursBack, Date UpdateMemoryChart(memTrend, memGrantTrend, hoursBack, fromDate, toDate); break; case 1: // Memory Clerks - var clerkTypes = await Task.Run(() => _dataService.GetDistinctMemoryClerkTypesAsync(_serverId, hoursBack, fromDate, toDate)); + var clerkTypes = await Task.Run(() => _dataService.GetDistinctMemoryClerkTypesForPickerAsync(_serverId, hoursBack, fromDate, toDate)); PopulateMemoryClerkPicker(clerkTypes); await UpdateMemoryClerksChartFromPickerAsync(); break; @@ -498,7 +498,7 @@ private async System.Threading.Tasks.Task RefreshMemoryAsync(int hoursBack, Date /* Full refresh: load all sub-tabs */ var memoryTask = Helpers.MethodProfiler.TimeAsync("Memory.MemoryStats", () => Task.Run(() => _dataService.GetLatestMemoryStatsAsync(_serverId))); var memoryTrendTask = Helpers.MethodProfiler.TimeAsync("Memory.MemoryTrend", () => Task.Run(() => _dataService.GetMemoryTrendAsync(_serverId, hoursBack, fromDate, toDate))); - var memoryClerkTypesTask = Helpers.MethodProfiler.TimeAsync("Memory.MemoryClerks", () => Task.Run(() => _dataService.GetDistinctMemoryClerkTypesAsync(_serverId, hoursBack, fromDate, toDate))); + var memoryClerkTypesTask = Helpers.MethodProfiler.TimeAsync("Memory.MemoryClerks", () => Task.Run(() => _dataService.GetDistinctMemoryClerkTypesForPickerAsync(_serverId, hoursBack, fromDate, toDate))); var memoryGrantTrendTask = Helpers.MethodProfiler.TimeAsync("Memory.MemoryGrantTrend", () => Task.Run(() => _dataService.GetMemoryGrantTrendAsync(_serverId, hoursBack, fromDate, toDate))); var memoryGrantChartTask = Helpers.MethodProfiler.TimeAsync("Memory.MemoryGrants", () => Task.Run(() => _dataService.GetMemoryGrantChartDataAsync(_serverId, hoursBack, fromDate, toDate))); var memoryPressureEventsTask = Helpers.MethodProfiler.TimeAsync("Memory.MemoryPressureEvents", () => Task.Run(() => _dataService.GetMemoryPressureEventsAsync(_serverId, hoursBack, fromDate, toDate))); diff --git a/Lite/Services/LocalDataService.Memory.cs b/Lite/Services/LocalDataService.Memory.cs index a0741c4eac..331fd5c122 100644 --- a/Lite/Services/LocalDataService.Memory.cs +++ b/Lite/Services/LocalDataService.Memory.cs @@ -10,11 +10,16 @@ using System.Collections.Generic; using System.Threading.Tasks; using DuckDB.NET.Data; +using PerformanceMonitor.Common; namespace PerformanceMonitorLite.Services; public partial class LocalDataService { + /// #4234: TTL memoization for — see + /// . + private readonly LiteNameListCache _distinctMemoryClerkTypesCache = new(); + /// /// Gets the most recent memory stats snapshot for a server. /// @@ -135,6 +140,9 @@ FROM v_memory_stats /// /// Gets the distinct memory clerk types collected for a server, ordered by total memory descending. + /// Uncached: MCP shares no read with the clerk picker today, but this stays uncached on the same + /// footing as so an MCP caller added later never sees a stale + /// answer. The picker's memoized entry point is . /// public async Task> GetDistinctMemoryClerkTypesAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) { @@ -167,52 +175,126 @@ GROUP BY clerk_type } /// - /// Fetches the memory clerk trend for ALL selected clerk types in ONE query - /// (replacing an N+1 query-per-clerk loop), grouped by clerk type. + /// Picker-only entry point for : checks + /// first, falls back to the shared uncached read, then + /// memoizes it. + /// #4234: keyed on (server, window length) for , so the + /// full-window read behind this runs at most once per 15 minutes rather than on every 1-minute + /// auto-refresh. Only ServerTab's clerk picker goes through this cache. + /// is the cache's clock seam — null uses the wall clock; a test passes an + /// explicit time to fast-forward past the TTL without sleeping. /// - public async Task>> GetMemoryClerkTrendsByTypesAsync(int serverId, List clerkTypes, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) + public async Task> GetDistinctMemoryClerkTypesForPickerAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? nowUtc = null) { - using var _q = TimeQuery("GetMemoryClerkTrendsByTypesAsync", "v_memory_clerks trends batched by type"); - var result = new Dictionary>(); - if (clerkTypes.Count == 0) return result; + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); - using var connection = await OpenConnectionAsync(); - using var command = connection.CreateCommand(); + var effectiveNow = nowUtc ?? DateTime.UtcNow; + var windowLength = endTime - startTime; + if (_distinctMemoryClerkTypesCache.TryGet(serverId, windowLength, endTime, effectiveNow, out var cached)) + { + return cached; + } - var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); - var typeParams = string.Join(", ", clerkTypes.Select((_, i) => "$" + (i + 4))); + var items = await GetDistinctMemoryClerkTypesAsync(serverId, hoursBack, fromDate, toDate); - command.CommandText = $@" + _distinctMemoryClerkTypesCache.Set(serverId, windowLength, endTime, items, effectiveNow); + return items; + } + + /// + /// Batched sibling of the removed per-clerk loop: fetches the trend for ALL selected clerk types in ONE + /// query, grouped by clerk type. + /// #4234: buckets to 's point budget PER SERIES (seriesCount + /// always 1 into , the ruling's own wording). A clerk's memory is a + /// GAUGE, not a counter, so a bucket's value is the plain average of the collections inside it — there is + /// no unrated-row concept here (#3540 only marks delta rows), so every row in the window counts and no + /// HAVING is needed to drop a bucket. When EVERY bucket the whole call returned holds exactly one + /// physical collection, each point is stamped at its bucket's raw first_collection_time instead of + /// the time_bucket grid line; a single merged bucket anywhere (any clerk type) keeps + /// bucket_start throughout — the same rule and PerfmonTrendsSql + /// use. + /// + /// + /// The bucketed batched-trend statement text (#4234), pulled out of + /// so its shape (the bucket width in its own trailing parameter, after the dynamic clerk_type IN (...) + /// list so that list's numbering does not shift) is checkable without a live DuckDB. + /// + internal static string MemoryClerkTrendsSql(int clerkTypeCount) + { + var typeParams = string.Join(", ", Enumerable.Range(0, clerkTypeCount).Select(i => "$" + (i + 4))); + var widthParam = "$" + (clerkTypeCount + 4); + return $@" SELECT clerk_type, - collection_time, - memory_mb + GREATEST(time_bucket(to_minutes(CAST({widthParam} AS INTEGER)), collection_time, {TrendBuckets.OriginSql}), $2) AS bucket_start, + AVG(memory_mb) AS memory_mb, + MIN(collection_time) AS first_collection_time, + COUNT(*) AS collection_count FROM v_memory_clerks WHERE server_id = $1 AND collection_time >= $2 AND collection_time <= $3 AND clerk_type IN ({typeParams}) -ORDER BY clerk_type, collection_time"; +GROUP BY clerk_type, 2 +ORDER BY clerk_type, 2"; + } + + public async Task>> GetMemoryClerkTrendsByTypesAsync(int serverId, List clerkTypes, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) + { + using var _q = TimeQuery("GetMemoryClerkTrendsByTypesAsync", "v_memory_clerks trends batched by type, bucketed"); + var result = new Dictionary>(); + if (clerkTypes.Count == 0) return result; + + using var connection = await OpenConnectionAsync(); + using var command = connection.CreateCommand(); + + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); + + var windowMinutes = Math.Max(1, (int)Math.Ceiling((endTime - startTime).TotalMinutes)); + var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); + + command.CommandText = MemoryClerkTrendsSql(clerkTypes.Count); command.Parameters.Add(new DuckDBParameter { Value = serverId }); command.Parameters.Add(new DuckDBParameter { Value = startTime }); command.Parameters.Add(new DuckDBParameter { Value = endTime }); foreach (var ct in clerkTypes) command.Parameters.Add(new DuckDBParameter { Value = ct }); + command.Parameters.Add(new DuckDBParameter { Value = bucketMinutes }); + + var rows = new List<(string ClerkType, DateTime BucketStart, DateTime FirstCollectionTime, double MemoryMb)>(); + var everyBucketSingleton = true; using var reader = await command.ExecuteReaderAsync(); while (await reader.ReadAsync()) { - var ct = reader.GetString(0); - if (!result.TryGetValue(ct, out var list)) + if (reader.GetInt64(4) != 1) + { + everyBucketSingleton = false; + } + + rows.Add(( + reader.GetString(0), + reader.GetDateTime(1), + reader.GetDateTime(3), + /* ToDouble, not GetDouble: DuckDB's AVG over a DECIMAL column can hand back a boxed value + GetDouble does not accept — see GetWaitStatsTrendsByTypesAsync's own sum columns for the + same guard. */ + reader.IsDBNull(2) ? 0 : ToDouble(reader.GetValue(2)))); + } + + foreach (var row in rows) + { + if (!result.TryGetValue(row.ClerkType, out var list)) { list = new List(); - result[ct] = list; + result[row.ClerkType] = list; } + list.Add(new MemoryClerkTrendPoint { - CollectionTime = reader.GetDateTime(1), - MemoryMb = reader.IsDBNull(2) ? 0 : ToDouble(reader.GetValue(2)) + CollectionTime = everyBucketSingleton ? row.FirstCollectionTime : row.BucketStart, + MemoryMb = row.MemoryMb }); } From 8bb322f9d7b2e09602b41caeb0cc339bd1d1147b Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 14:19:53 -0400 Subject: [PATCH 10/12] Bucket Lite's File I/O latency and throughput trend reads (#4234) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetFileIoLatencyTrendAsync, GetFileIoThroughputTrendAsync and GetTempDbFileIoTrendAsync now bucket to TrendBudget.Chart's per-series point budget, the GREATEST(time_bucket(...), $2) shape PR #4331 gave the wait/perfmon trend reads. Latency stays a ratio (summed stall over summed operations, over rated rows only); throughput stays a rate (summed bytes over summed rated seconds). An unrated collection alone in a bucket still leaves that point absent (HAVING drops the bucket), same as before. The top-10-files ranking in the latency and throughput queries stays an unbucketed whole-window scan, unchanged. These reads feed the File I/O tab charts and the Overview correlated timeline's I/O lane (both its live and reference-period fetches), which read GetFileIoLatencyTrendAsync directly and need no changes of their own. Also fixes a stale comment in McpIoTools.cs that called the desktop chart's read "untouched" — it is bucketed now too, on its own budget, separately from the MCP tool's own GetFileIoTrendAsync. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- Lite/Mcp/McpIoTools.cs | 5 +- Lite/Services/LocalDataService.FileIo.cs | 328 ++++++++++++++++------- 2 files changed, 234 insertions(+), 99 deletions(-) diff --git a/Lite/Mcp/McpIoTools.cs b/Lite/Mcp/McpIoTools.cs index ef44c9da85..748489649e 100644 --- a/Lite/Mcp/McpIoTools.cs +++ b/Lite/Mcp/McpIoTools.cs @@ -89,8 +89,9 @@ because it depends on how many lines there are to draw. */ /* #3897: two reads, as on Darling. The ranking's length decides the bucket width — five lines and an "(other)" line need coarser buckets than two lines do to stay inside the same budget — and the - bucketed read charts exactly the series the ranking counted. The desktop chart's per-collection - read (GetFileIoLatencyTrendAsync) is untouched: a chart wants every collection. */ + bucketed read charts exactly the series the ranking counted. The desktop chart's own read + (GetFileIoLatencyTrendAsync) is ALSO bucketed now (#4234), to its own per-call budget — a + separate read, on a separate clock, from this tool's ranking-driven GetFileIoTrendAsync. */ var scope = string.IsNullOrWhiteSpace(database_name) ? null : database_name.Trim(); var series = await dataService.GetFileIoSeriesAsync(resolved.ServerId, hours_back, asOfUtc: windowEnd, scope); diff --git a/Lite/Services/LocalDataService.FileIo.cs b/Lite/Services/LocalDataService.FileIo.cs index 67c58e66f6..1b019c0f84 100644 --- a/Lite/Services/LocalDataService.FileIo.cs +++ b/Lite/Services/LocalDataService.FileIo.cs @@ -10,6 +10,7 @@ using System.Collections.Generic; using System.Threading.Tasks; using DuckDB.NET.Data; +using PerformanceMonitor.Common; namespace PerformanceMonitorLite.Services; @@ -71,15 +72,23 @@ FROM v_file_io_stats /// /// Gets file I/O latency trend data broken down by file for charting (top 10 files by I/O activity). + /// #4234: buckets to 's point budget PER SERIES (seriesCount + /// always 1 into ). The top-10 ranking (top_files) stays an + /// unbucketed scan of the whole call window, exactly as before — only the per-collection SELECT beneath it + /// buckets. Latency is a ratio (summed stall over summed operations), not a rate, so unlike throughput it + /// needs no interval arithmetic; a rated row is simply one whose stored interval is not the #3540 + /// sentinel 0, nulled (not filtered) so an unrated row still counts toward collection_count — the + /// same reason keeps its unrated rows in its CTE instead of excluding them + /// outright. HAVING drops a bucket with no rated row, so it stays absent, not 0. When EVERY bucket + /// the whole call returned holds exactly one physical collection, each point is stamped at its bucket's raw + /// first_collection_time instead of the time_bucket grid line; a single merged bucket + /// anywhere (any file) keeps bucket_start throughout. /// - public async Task> GetFileIoLatencyTrendAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null) - { - using var connection = await OpenConnectionAsync(); - using var command = connection.CreateCommand(); - - var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); - - command.CommandText = @" + /// + /// The bucketed statement text (#4234), pulled out of so its shape + /// is checkable without a live DuckDB. + /// + internal static readonly string FileIoLatencyTrendSql = $@" WITH top_files AS ( SELECT database_name, file_name FROM v_file_io_stats @@ -90,53 +99,95 @@ FROM v_file_io_stats GROUP BY database_name, file_name ORDER BY SUM(delta_reads + delta_writes) DESC LIMIT 10 +), +rated AS ( + SELECT + f.database_name, + f.file_name, + f.collection_time, + /* #3540: a stored interval of 0 is the calculator's no-delta-knowable marker — nulled here (not + filtered in the WHERE) so an unrated row still counts toward collection_count below. IS DISTINCT + FROM 0 keeps pre-v60 rows (NULL: interval never recorded), which carry on reading exactly as they + always did. */ + CASE WHEN f.sample_interval_seconds IS DISTINCT FROM 0 THEN f.delta_reads END AS rated_reads, + CASE WHEN f.sample_interval_seconds IS DISTINCT FROM 0 THEN f.delta_writes END AS rated_writes, + CASE WHEN f.sample_interval_seconds IS DISTINCT FROM 0 THEN CAST(f.delta_stall_read_ms AS DOUBLE PRECISION) END AS rated_stall_read_ms, + CASE WHEN f.sample_interval_seconds IS DISTINCT FROM 0 THEN CAST(f.delta_stall_write_ms AS DOUBLE PRECISION) END AS rated_stall_write_ms, + CASE WHEN f.sample_interval_seconds IS DISTINCT FROM 0 THEN CAST(COALESCE(f.delta_stall_queued_read_ms, 0) AS DOUBLE PRECISION) END AS rated_queued_read_ms, + CASE WHEN f.sample_interval_seconds IS DISTINCT FROM 0 THEN CAST(COALESCE(f.delta_stall_queued_write_ms, 0) AS DOUBLE PRECISION) END AS rated_queued_write_ms + FROM v_file_io_stats f + JOIN top_files tf ON tf.database_name = f.database_name AND tf.file_name = f.file_name + WHERE f.server_id = $1 + AND f.collection_time >= $2 + AND f.collection_time <= $3 ) SELECT - f.collection_time, - f.database_name, - f.file_name, - CASE WHEN SUM(f.delta_reads) > 0 - THEN SUM(CAST(f.delta_stall_read_ms AS DOUBLE PRECISION)) / SUM(f.delta_reads) - ELSE 0 END AS avg_read_latency_ms, - CASE WHEN SUM(f.delta_writes) > 0 - THEN SUM(CAST(f.delta_stall_write_ms AS DOUBLE PRECISION)) / SUM(f.delta_writes) - ELSE 0 END AS avg_write_latency_ms, - CASE WHEN SUM(f.delta_reads) > 0 - THEN SUM(CAST(COALESCE(f.delta_stall_queued_read_ms, 0) AS DOUBLE PRECISION)) / SUM(f.delta_reads) - ELSE 0 END AS avg_queued_read_latency_ms, - CASE WHEN SUM(f.delta_writes) > 0 - THEN SUM(CAST(COALESCE(f.delta_stall_queued_write_ms, 0) AS DOUBLE PRECISION)) / SUM(f.delta_writes) - ELSE 0 END AS avg_queued_write_latency_ms -FROM v_file_io_stats f -JOIN top_files tf ON tf.database_name = f.database_name AND tf.file_name = f.file_name -WHERE f.server_id = $1 -AND f.collection_time >= $2 -AND f.collection_time <= $3 -/* #3540: a stored interval of 0 is the calculator's no-delta-knowable marker (first sighting, - counter reset, a gap past the policy) — the row is dropped so the point is ABSENT rather than the - confident 0.00 ms a restart used to render. IS DISTINCT FROM 0 keeps pre-v60 rows (NULL: interval - never recorded), which carry on reading exactly as they always did. */ -AND f.sample_interval_seconds IS DISTINCT FROM 0 -GROUP BY f.collection_time, f.database_name, f.file_name -ORDER BY f.collection_time, f.database_name, f.file_name"; + database_name, + file_name, + GREATEST(time_bucket(to_minutes(CAST($4 AS INTEGER)), collection_time, {TrendBuckets.OriginSql}), $2) AS bucket_start, + CASE WHEN SUM(rated_reads) > 0 THEN SUM(rated_stall_read_ms) / SUM(rated_reads) ELSE 0 END AS avg_read_latency_ms, + CASE WHEN SUM(rated_writes) > 0 THEN SUM(rated_stall_write_ms) / SUM(rated_writes) ELSE 0 END AS avg_write_latency_ms, + CASE WHEN SUM(rated_reads) > 0 THEN SUM(rated_queued_read_ms) / SUM(rated_reads) ELSE 0 END AS avg_queued_read_latency_ms, + CASE WHEN SUM(rated_writes) > 0 THEN SUM(rated_queued_write_ms) / SUM(rated_writes) ELSE 0 END AS avg_queued_write_latency_ms, + MIN(collection_time) AS first_collection_time, + COUNT(*) AS collection_count +FROM rated +GROUP BY database_name, file_name, 3 +HAVING COUNT(rated_reads) > 0 +ORDER BY database_name, file_name, 3"; + + public async Task> GetFileIoLatencyTrendAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null) + { + using var _q = TimeQuery("GetFileIoLatencyTrendAsync", "v_file_io_stats top-10 files, bucketed"); + using var connection = await OpenConnectionAsync(); + using var command = connection.CreateCommand(); + + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc, SelectedServerTabUtcOffsetMinutes); + + var windowMinutes = Math.Max(1, (int)Math.Ceiling((endTime - startTime).TotalMinutes)); + var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); + + command.CommandText = FileIoLatencyTrendSql; command.Parameters.Add(new DuckDBParameter { Value = serverId }); command.Parameters.Add(new DuckDBParameter { Value = startTime }); command.Parameters.Add(new DuckDBParameter { Value = endTime }); + command.Parameters.Add(new DuckDBParameter { Value = bucketMinutes }); + + var rows = new List<(string DatabaseName, string FileName, DateTime BucketStart, DateTime FirstCollectionTime, double ReadLatency, double WriteLatency, double QueuedReadLatency, double QueuedWriteLatency)>(); + var everyBucketSingleton = true; - var items = new List(); using var reader = await command.ExecuteReaderAsync(); while (await reader.ReadAsync()) + { + if (reader.GetInt64(8) != 1) + { + everyBucketSingleton = false; + } + + rows.Add(( + reader.IsDBNull(0) ? "" : reader.GetString(0), + reader.IsDBNull(1) ? "" : reader.GetString(1), + reader.GetDateTime(2), + reader.GetDateTime(7), + reader.IsDBNull(3) ? 0 : ToDouble(reader.GetValue(3)), + reader.IsDBNull(4) ? 0 : ToDouble(reader.GetValue(4)), + reader.IsDBNull(5) ? 0 : ToDouble(reader.GetValue(5)), + reader.IsDBNull(6) ? 0 : ToDouble(reader.GetValue(6)))); + } + + var items = new List(); + foreach (var row in rows) { items.Add(new FileIoTrendPoint { - CollectionTime = reader.GetDateTime(0), - DatabaseName = reader.IsDBNull(1) ? "" : reader.GetString(1), - FileName = reader.IsDBNull(2) ? "" : reader.GetString(2), - AvgReadLatencyMs = reader.IsDBNull(3) ? 0 : ToDouble(reader.GetValue(3)), - AvgWriteLatencyMs = reader.IsDBNull(4) ? 0 : ToDouble(reader.GetValue(4)), - AvgQueuedReadLatencyMs = reader.IsDBNull(5) ? 0 : ToDouble(reader.GetValue(5)), - AvgQueuedWriteLatencyMs = reader.IsDBNull(6) ? 0 : ToDouble(reader.GetValue(6)) + CollectionTime = everyBucketSingleton ? row.FirstCollectionTime : row.BucketStart, + DatabaseName = row.DatabaseName, + FileName = row.FileName, + AvgReadLatencyMs = row.ReadLatency, + AvgWriteLatencyMs = row.WriteLatency, + AvgQueuedReadLatencyMs = row.QueuedReadLatency, + AvgQueuedWriteLatencyMs = row.QueuedWriteLatency }); } @@ -170,21 +221,19 @@ FROM v_file_io_stats /// Divides by each row's stored sample_interval_seconds; a pre-v60 row that never recorded one falls /// back to the LAG() over collection_time this read always used (#3540). /// #3653 (#3540 rule 1, readers NULL-not-0 on unknowable): the two rate arms end at END, - /// not ELSE 0. The outer WHERE interval_seconds IS NOT NULL AND interval_seconds > 0 - /// already excludes every row the ELSE could have run on, so this is the honest spelling of the arm and - /// not a behaviour change — the day the WHERE moves, a row with no knowable interval yields a NULL rate - /// rather than a measured 0.00 MB/s. The reader's IsDBNull ? 0 on the two rate columns is a - /// DBNull-safe belt that the WHERE keeps unreachable. Twin of the Darling Viewer's - /// FileIoThroughputTrendSql, edited in the same PR. + /// not ELSE 0. + /// #4234: buckets to 's point budget PER SERIES. The LAG-derived + /// interval is computed once over the RAW per-collection rows (with_interval), before bucketing — + /// a bucket's rate is summed bytes over summed rated seconds (time-weighted, never an average of + /// per-collection rates), and a bucket with no rated collection is dropped (HAVING), same as the + /// per-collection read always dropped that collection. Singleton stamping follows + /// 's rule. /// - public async Task> GetFileIoThroughputTrendAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) - { - using var connection = await OpenConnectionAsync(); - using var command = connection.CreateCommand(); - - var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); - - command.CommandText = @" + /// + /// The bucketed statement text (#4234), pulled out of so its + /// shape is checkable without a live DuckDB. + /// + internal static readonly string FileIoThroughputTrendSql = $@" WITH top_files AS ( SELECT database_name, file_name FROM v_file_io_stats @@ -203,8 +252,7 @@ with_interval AS ( f.delta_read_bytes, f.delta_write_bytes, /* #3540: the STORED interval where the row has one — 0 (no delta knowable) becomes NULL through - NULLIF and the outer WHERE drops the row, exactly as it always dropped the first row per file. - NULL (a pre-v60 row that never recorded one) falls back to the LAG this read always used. */ + NULLIF. NULL (a pre-v60 row that never recorded one) falls back to the LAG this read always used. */ CASE WHEN f.sample_interval_seconds IS NULL THEN EXTRACT(EPOCH FROM (f.collection_time - LAG(f.collection_time) OVER ( PARTITION BY f.server_id, f.database_name, f.file_name @@ -217,34 +265,76 @@ FROM v_file_io_stats f WHERE f.server_id = $1 AND f.collection_time >= $2 AND f.collection_time <= $3 +), +rated AS ( + SELECT + collection_time, + file_label, + CASE WHEN interval_seconds > 0 THEN CAST(delta_read_bytes AS DOUBLE PRECISION) END AS rated_read_bytes, + CASE WHEN interval_seconds > 0 THEN CAST(delta_write_bytes AS DOUBLE PRECISION) END AS rated_write_bytes, + CASE WHEN interval_seconds > 0 THEN interval_seconds END AS rated_seconds + FROM with_interval ) SELECT - collection_time, file_label, - CASE WHEN interval_seconds > 0 - THEN CAST(delta_read_bytes AS DOUBLE PRECISION) / interval_seconds / 1048576.0 - END AS read_mb_per_sec, - CASE WHEN interval_seconds > 0 - THEN CAST(delta_write_bytes AS DOUBLE PRECISION) / interval_seconds / 1048576.0 - END AS write_mb_per_sec -FROM with_interval -WHERE interval_seconds IS NOT NULL AND interval_seconds > 0 -ORDER BY collection_time, file_label"; + GREATEST(time_bucket(to_minutes(CAST($4 AS INTEGER)), collection_time, {TrendBuckets.OriginSql}), $2) AS bucket_start, + SUM(rated_read_bytes) / SUM(rated_seconds) / 1048576.0 AS read_mb_per_sec, + SUM(rated_write_bytes) / SUM(rated_seconds) / 1048576.0 AS write_mb_per_sec, + MIN(collection_time) AS first_collection_time, + COUNT(*) AS collection_count +FROM rated +GROUP BY file_label, 2 +HAVING COUNT(rated_seconds) > 0 +ORDER BY file_label, 2"; + + public async Task> GetFileIoThroughputTrendAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) + { + using var _q = TimeQuery("GetFileIoThroughputTrendAsync", "v_file_io_stats top-10 files by bytes, bucketed"); + using var connection = await OpenConnectionAsync(); + using var command = connection.CreateCommand(); + + var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); + + var windowMinutes = Math.Max(1, (int)Math.Ceiling((endTime - startTime).TotalMinutes)); + var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); + + command.CommandText = FileIoThroughputTrendSql; command.Parameters.Add(new DuckDBParameter { Value = serverId }); command.Parameters.Add(new DuckDBParameter { Value = startTime }); command.Parameters.Add(new DuckDBParameter { Value = endTime }); + command.Parameters.Add(new DuckDBParameter { Value = bucketMinutes }); + + var rows = new List<(string FileLabel, DateTime BucketStart, DateTime FirstCollectionTime, double ReadMbPerSec, double WriteMbPerSec)>(); + var everyBucketSingleton = true; - var items = new List(); using var reader = await command.ExecuteReaderAsync(); while (await reader.ReadAsync()) + { + if (reader.GetInt64(5) != 1) + { + everyBucketSingleton = false; + } + + rows.Add(( + reader.IsDBNull(0) ? "" : reader.GetString(0), + reader.GetDateTime(1), + reader.GetDateTime(4), + /* ToDouble, not GetDouble: a SUM over bytes can come back as a boxed DuckDB HUGEINT — see + GetWaitStatsTrendsByTypesAsync's own sum columns for the same guard. */ + reader.IsDBNull(2) ? 0 : ToDouble(reader.GetValue(2)), + reader.IsDBNull(3) ? 0 : ToDouble(reader.GetValue(3)))); + } + + var items = new List(); + foreach (var row in rows) { items.Add(new FileIoThroughputPoint { - CollectionTime = reader.GetDateTime(0), - FileLabel = reader.IsDBNull(1) ? "" : reader.GetString(1), - ReadMbPerSec = reader.IsDBNull(2) ? 0 : ToDouble(reader.GetValue(2)), - WriteMbPerSec = reader.IsDBNull(3) ? 0 : ToDouble(reader.GetValue(3)) + CollectionTime = everyBucketSingleton ? row.FirstCollectionTime : row.BucketStart, + FileLabel = row.FileLabel, + ReadMbPerSec = row.ReadMbPerSec, + WriteMbPerSec = row.WriteMbPerSec }); } @@ -253,47 +343,91 @@ WHERE interval_seconds IS NOT NULL AND interval_seconds > 0 /// /// Gets file I/O latency trend data for tempdb files only, broken down by file name. + /// #4234: bucketed the same way as — see that method's doc + /// for the rated/HAVING/singleton-stamping rules; this read has no top-10 ranking pass (tempdb's own file + /// count is already small) and no queued-latency columns (this table's caller chart never drew them). + /// DatabaseName carries the FILE name here, not the database (always tempdb) — preserved + /// exactly as the pre-#4234 reader mapped it, since ServerTab.Charts.UpdateTempDbFileIoChart groups + /// its series on that field. + /// + /// + /// The bucketed statement text (#4234), pulled out of so its shape + /// is checkable without a live DuckDB. /// + internal static readonly string TempDbFileIoTrendSql = $@" +WITH rated AS ( + SELECT + file_name, + collection_time, + /* #3540: see GetFileIoLatencyTrendAsync's rated CTE for why this nulls rather than filters. */ + CASE WHEN sample_interval_seconds IS DISTINCT FROM 0 THEN delta_reads END AS rated_reads, + CASE WHEN sample_interval_seconds IS DISTINCT FROM 0 THEN delta_writes END AS rated_writes, + CASE WHEN sample_interval_seconds IS DISTINCT FROM 0 THEN CAST(delta_stall_read_ms AS DOUBLE PRECISION) END AS rated_stall_read_ms, + CASE WHEN sample_interval_seconds IS DISTINCT FROM 0 THEN CAST(delta_stall_write_ms AS DOUBLE PRECISION) END AS rated_stall_write_ms + FROM v_file_io_stats + WHERE server_id = $1 + AND collection_time >= $2 + AND collection_time <= $3 + AND database_name = 'tempdb' +) +SELECT + file_name, + GREATEST(time_bucket(to_minutes(CAST($4 AS INTEGER)), collection_time, {TrendBuckets.OriginSql}), $2) AS bucket_start, + CASE WHEN SUM(rated_reads) > 0 THEN SUM(rated_stall_read_ms) / SUM(rated_reads) ELSE 0 END AS avg_read_latency_ms, + CASE WHEN SUM(rated_writes) > 0 THEN SUM(rated_stall_write_ms) / SUM(rated_writes) ELSE 0 END AS avg_write_latency_ms, + MIN(collection_time) AS first_collection_time, + COUNT(*) AS collection_count +FROM rated +GROUP BY file_name, 2 +HAVING COUNT(rated_reads) > 0 +ORDER BY file_name, 2"; + public async Task> GetTempDbFileIoTrendAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) { + using var _q = TimeQuery("GetTempDbFileIoTrendAsync", "v_file_io_stats tempdb files, bucketed"); using var connection = await OpenConnectionAsync(); using var command = connection.CreateCommand(); var (startTime, endTime) = GetTimeRange(hoursBack, fromDate, toDate, asOfUtc: null, SelectedServerTabUtcOffsetMinutes); - command.CommandText = @" -SELECT - collection_time, - file_name, - CASE WHEN SUM(delta_reads) > 0 THEN SUM(CAST(delta_stall_read_ms AS DOUBLE PRECISION)) / SUM(delta_reads) ELSE 0 END AS avg_read_latency_ms, - CASE WHEN SUM(delta_writes) > 0 THEN SUM(CAST(delta_stall_write_ms AS DOUBLE PRECISION)) / SUM(delta_writes) ELSE 0 END AS avg_write_latency_ms -FROM v_file_io_stats -WHERE server_id = $1 -AND collection_time >= $2 -AND collection_time <= $3 -AND database_name = 'tempdb' -/* #3540: a stored interval of 0 is the calculator's no-delta-knowable marker (first sighting, - counter reset, a gap past the policy) — the row is dropped so the point is ABSENT rather than the - confident 0.00 ms a restart used to render. IS DISTINCT FROM 0 keeps pre-v60 rows (NULL: interval - never recorded), which carry on reading exactly as they always did. */ -AND sample_interval_seconds IS DISTINCT FROM 0 -GROUP BY collection_time, file_name -ORDER BY collection_time, file_name"; + var windowMinutes = Math.Max(1, (int)Math.Ceiling((endTime - startTime).TotalMinutes)); + var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); + + command.CommandText = TempDbFileIoTrendSql; command.Parameters.Add(new DuckDBParameter { Value = serverId }); command.Parameters.Add(new DuckDBParameter { Value = startTime }); command.Parameters.Add(new DuckDBParameter { Value = endTime }); + command.Parameters.Add(new DuckDBParameter { Value = bucketMinutes }); + + var rows = new List<(string FileName, DateTime BucketStart, DateTime FirstCollectionTime, double ReadLatency, double WriteLatency)>(); + var everyBucketSingleton = true; - var items = new List(); using var reader = await command.ExecuteReaderAsync(); while (await reader.ReadAsync()) + { + if (reader.GetInt64(5) != 1) + { + everyBucketSingleton = false; + } + + rows.Add(( + reader.IsDBNull(0) ? "" : reader.GetString(0), + reader.GetDateTime(1), + reader.GetDateTime(4), + reader.IsDBNull(2) ? 0 : ToDouble(reader.GetValue(2)), + reader.IsDBNull(3) ? 0 : ToDouble(reader.GetValue(3)))); + } + + var items = new List(); + foreach (var row in rows) { items.Add(new FileIoTrendPoint { - CollectionTime = reader.GetDateTime(0), - DatabaseName = reader.IsDBNull(1) ? "" : reader.GetString(1), - AvgReadLatencyMs = reader.IsDBNull(2) ? 0 : ToDouble(reader.GetValue(2)), - AvgWriteLatencyMs = reader.IsDBNull(3) ? 0 : ToDouble(reader.GetValue(3)) + CollectionTime = everyBucketSingleton ? row.FirstCollectionTime : row.BucketStart, + DatabaseName = row.FileName, + AvgReadLatencyMs = row.ReadLatency, + AvgWriteLatencyMs = row.WriteLatency }); } From 4241b668be1056c727071432a7ff238db4017858 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 14:27:51 -0400 Subject: [PATCH 11/12] Add regression tests for Lite's clerk and File I/O bucketing (#4234) Source-check tests pin each new SQL statement's bucket-width parameter and first_collection_time/collection_count projection. Live DuckDB tests cover the 7-day row-budget cap, point equality when every bucket holds one collection, an unrated collection alone leaving its bucket absent, a merged bucket using summed-over-summed (not a mean of per-collection ratios/rates), and the clerk picker's TTL cache (hit inside 15 minutes, miss past it, shared read never caches). Proven against the pre-#4234 shape: none of these tests can even compile against it, since GetDistinctMemoryClerkTypesForPickerAsync, MemoryClerkTrendsSql, FileIoLatencyTrendSql, FileIoThroughputTrendSql and TempDbFileIoTrendSql did not exist before this branch. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- .../LiteMemoryFileIoTrendBucketingTests.cs | 424 ++++++++++++++++++ 1 file changed, 424 insertions(+) create mode 100644 Lite.Tests/LiteMemoryFileIoTrendBucketingTests.cs diff --git a/Lite.Tests/LiteMemoryFileIoTrendBucketingTests.cs b/Lite.Tests/LiteMemoryFileIoTrendBucketingTests.cs new file mode 100644 index 0000000000..d9195ad754 --- /dev/null +++ b/Lite.Tests/LiteMemoryFileIoTrendBucketingTests.cs @@ -0,0 +1,424 @@ +/* + * 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.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using DuckDB.NET.Data; +using PerformanceMonitor.Common; +using PerformanceMonitorLite.Database; +using PerformanceMonitorLite.Services; +using Xunit; + +namespace PerformanceMonitorLite.Tests; + +/// +/// #4234 (lane T3c, the Lite twin of PR #4304's clerk/File I/O reads): the ruling's required tests for +/// , , +/// , +/// and the clerk picker's cache. No live SQL Server or Postgres needed — DuckDB is the embedded store itself. +/// Proven once by hand against the pre-#4234 text: reverting the four SQL statements below to their +/// per-collection SELECTs (no rated CTE, no time_bucket, no width parameter) fails this file's +/// source checks directly, and fails 's budget-cap tests +/// (10,080 one-minute collections over 7 days is 10,080 rows, not ≤ 1,500). +/// +public sealed class LiteMemoryFileIoTrendBucketWidthSqlTests +{ + [Fact] + public void MemoryClerkTrendsSql_CarriesABucketWidth() + { + /* server/start/end = $1-$3, 2 clerk types = $4-$5, so the width is $6. */ + var sql = LocalDataService.MemoryClerkTrendsSql(2); + Assert.Contains("time_bucket(to_minutes(CAST($6 AS INTEGER))", sql, StringComparison.Ordinal); + Assert.Contains(TrendBuckets.OriginSql, sql, StringComparison.Ordinal); + } + + [Fact] + public void FileIoLatencyTrendSql_CarriesABucketWidth() + { + /* No dynamic series list (the top-10 ranking lives inside the statement), so the width is $4. */ + Assert.Contains("time_bucket(to_minutes(CAST($4 AS INTEGER))", LocalDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); + Assert.Contains(TrendBuckets.OriginSql, LocalDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); + } + + [Fact] + public void FileIoThroughputTrendSql_CarriesABucketWidth() + { + Assert.Contains("time_bucket(to_minutes(CAST($4 AS INTEGER))", LocalDataService.FileIoThroughputTrendSql, StringComparison.Ordinal); + Assert.Contains(TrendBuckets.OriginSql, LocalDataService.FileIoThroughputTrendSql, StringComparison.Ordinal); + } + + [Fact] + public void TempDbFileIoTrendSql_CarriesABucketWidth() + { + Assert.Contains("time_bucket(to_minutes(CAST($4 AS INTEGER))", LocalDataService.TempDbFileIoTrendSql, StringComparison.Ordinal); + Assert.Contains(TrendBuckets.OriginSql, LocalDataService.TempDbFileIoTrendSql, StringComparison.Ordinal); + } + + [Fact] + public void AllFour_ProjectFirstCollectionTimeAndCollectionCount() + { + foreach (var sql in new[] + { + LocalDataService.MemoryClerkTrendsSql(1), + LocalDataService.FileIoLatencyTrendSql, + LocalDataService.FileIoThroughputTrendSql, + LocalDataService.TempDbFileIoTrendSql + }) + { + Assert.Contains("MIN(collection_time) AS first_collection_time", sql, StringComparison.Ordinal); + Assert.Contains("COUNT(*) AS collection_count", sql, StringComparison.Ordinal); + } + } +} + +/// +/// The ruling's remaining tests (the 7-day row-budget cap, point equality when the budget covers every +/// collection, the unrated-drops-the-bucket and summed-not-mean pins, and the clerk picker's end-to-end cache +/// behavior) against a real embedded DuckDB. Mirrors LiteTrendBucketingLiveTests (PR #4331) for the +/// clerk and File I/O reads that lane left untouched. +/// +/* fromDate/toDate is server-local (GetTimeRange converts it back to UTC through ServerTimeHelper.UtcOffsetMinutes, + a process-wide mutable static) — every fromDate/toDate this class passes is meant as an exact UTC instant, so + the offset is pinned to 0 for the class's lifetime. xUnit runs test classes in parallel, so this joins the + collection the other offset-touching classes use rather than racing them. */ +[Collection("server-time-helper")] +public sealed class LiteMemoryFileIoTrendBucketingLiveTests : IClassFixture, IDisposable +{ + private const int ServerId = -4234301; + private const string ServerName = "clerk-fileio-bucketing-e2e"; + + private readonly DuckDbInitializer _duckDb; + private readonly LocalDataService _dataService; + private readonly int _savedUtcOffsetMinutes; + private long _nextId = -1; + private DuckDBConnection? _seedConn; + + public LiteMemoryFileIoTrendBucketingLiveTests(SharedDuckDbFixture fixture) + { + fixture.ResetData(); + _duckDb = fixture.DuckDb; + _dataService = new LocalDataService(_duckDb); + _savedUtcOffsetMinutes = ServerTimeHelper.UtcOffsetMinutes; + ServerTimeHelper.UtcOffsetMinutes = 0; + } + + public void Dispose() + { + ServerTimeHelper.UtcOffsetMinutes = _savedUtcOffsetMinutes; + _seedConn?.Dispose(); + } + + [Fact] + public async Task MemoryClerkTrend_SevenDayWindow_ReturnsAtMostBudgetTimesSeriesRows() + { + var end = new DateTime(2026, 3, 10, 0, 0, 0); + var start = end.AddDays(-7); + var clerkTypes = new List { "MEMORYCLERK_SQLBUFFERPOOL", "CACHESTORE_SQLCP" }; + + foreach (var clerkType in clerkTypes) + { + await BulkSeedMemoryClerkAsync(start, end, clerkType); + } + + var trends = await _dataService.GetMemoryClerkTrendsByTypesAsync(ServerId, clerkTypes, fromDate: start, toDate: end); + + var totalRows = trends.Values.Sum(list => list.Count); + var budget = TrendBudget.Chart.AutoPoints * clerkTypes.Count; + Assert.True(totalRows > 0 && totalRows <= budget, $"{totalRows} rows over a {clerkTypes.Count}-series budget of {budget}"); + } + + [Fact] + public async Task FileIoLatencyTrend_SevenDayWindow_ReturnsAtMostBudgetTimesSeriesRows() + { + var end = new DateTime(2026, 3, 11, 0, 0, 0); + var start = end.AddDays(-7); + await BulkSeedFileIoAsync(start, end, "AdventureWorks", "data1.mdf"); + + var points = await _dataService.GetFileIoLatencyTrendAsync(ServerId, fromDate: start, toDate: end); + + Assert.True(points.Count > 0 && points.Count <= TrendBudget.Chart.AutoPoints, $"{points.Count} rows over a 1-series budget of {TrendBudget.Chart.AutoPoints}"); + } + + [Fact] + public async Task FileIoThroughputTrend_SevenDayWindow_ReturnsAtMostBudgetTimesSeriesRows() + { + var end = new DateTime(2026, 3, 12, 0, 0, 0); + var start = end.AddDays(-7); + await BulkSeedFileIoAsync(start, end, "AdventureWorks", "data1.mdf"); + + var points = await _dataService.GetFileIoThroughputTrendAsync(ServerId, fromDate: start, toDate: end); + + Assert.True(points.Count > 0 && points.Count <= TrendBudget.Chart.AutoPoints, $"{points.Count} rows over a 1-series budget of {TrendBudget.Chart.AutoPoints}"); + } + + [Fact] + public async Task TempDbFileIoTrend_SevenDayWindow_ReturnsAtMostBudgetTimesRows() + { + var end = new DateTime(2026, 3, 13, 0, 0, 0); + var start = end.AddDays(-7); + await BulkSeedFileIoAsync(start, end, "tempdb", "tempdb.mdf"); + + var points = await _dataService.GetTempDbFileIoTrendAsync(ServerId, fromDate: start, toDate: end); + + Assert.True(points.Count > 0 && points.Count <= TrendBudget.Chart.AutoPoints, $"{points.Count} rows over a 1-series budget of {TrendBudget.Chart.AutoPoints}"); + } + + [Fact] + public async Task MemoryClerkTrend_BudgetCoversEveryCollection_ReturnsRawTimestampsAndValuesUnchanged() + { + /* Off the minute grid (:37 seconds) — on the pre-#4234 bucketed code every point would have been + floored to the time_bucket grid line, losing the seconds. */ + var t1 = new DateTime(2026, 3, 10, 9, 0, 37); + var t2 = t1.AddMinutes(5); + var t3 = t2.AddMinutes(5); + + await SeedMemoryClerkAsync(t1, "MEMORYCLERK_SQLBUFFERPOOL", 100); + await SeedMemoryClerkAsync(t2, "MEMORYCLERK_SQLBUFFERPOOL", 200); + await SeedMemoryClerkAsync(t3, "MEMORYCLERK_SQLBUFFERPOOL", 300); + + var trends = await _dataService.GetMemoryClerkTrendsByTypesAsync( + ServerId, new List { "MEMORYCLERK_SQLBUFFERPOOL" }, fromDate: t1.AddMinutes(-1), toDate: t3.AddMinutes(1)); + + var points = trends["MEMORYCLERK_SQLBUFFERPOOL"]; + Assert.Equal(new[] { t1, t2, t3 }, points.Select(p => p.CollectionTime).ToArray()); + Assert.Equal(new[] { 100.0, 200.0, 300.0 }, points.Select(p => p.MemoryMb).ToArray()); + } + + [Fact] + public async Task FileIoLatencyTrend_BudgetCoversEveryCollection_ReturnsRawTimestampsAndValuesUnchanged() + { + var t1 = new DateTime(2026, 3, 11, 9, 0, 37); + var t2 = t1.AddMinutes(5); + var t3 = t2.AddMinutes(5); + + await SeedFileIoAsync(t1, "AdventureWorks", "data1.mdf", reads: 10, writes: 5, readBytes: 0, writeBytes: 0, stallReadMs: 100, stallWriteMs: 25, interval: 300); + await SeedFileIoAsync(t2, "AdventureWorks", "data1.mdf", reads: 20, writes: 10, readBytes: 0, writeBytes: 0, stallReadMs: 300, stallWriteMs: 100, interval: 300); + await SeedFileIoAsync(t3, "AdventureWorks", "data1.mdf", reads: 30, writes: 15, readBytes: 0, writeBytes: 0, stallReadMs: 600, stallWriteMs: 225, interval: 300); + + var points = await _dataService.GetFileIoLatencyTrendAsync(ServerId, fromDate: t1.AddMinutes(-1), toDate: t3.AddMinutes(1)); + + Assert.Equal(new[] { t1, t2, t3 }, points.Select(p => p.CollectionTime).ToArray()); + Assert.Equal(new[] { 10.0, 15.0, 20.0 }, points.Select(p => p.AvgReadLatencyMs).ToArray()); + Assert.Equal(new[] { 5.0, 10.0, 15.0 }, points.Select(p => p.AvgWriteLatencyMs).ToArray()); + } + + [Fact] + public async Task FileIoThroughputTrend_BudgetCoversEveryCollection_ReturnsRawTimestampsAndValuesUnchanged() + { + var t1 = new DateTime(2026, 3, 12, 9, 0, 37); + var t2 = t1.AddMinutes(5); + + /* interval 300s: 314,572,800 bytes / 300s / 1,048,576 = 1.0 MB/s exactly. */ + await SeedFileIoAsync(t1, "AdventureWorks", "data1.mdf", reads: 0, writes: 0, readBytes: 314572800, writeBytes: 157286400, stallReadMs: 0, stallWriteMs: 0, interval: 300); + await SeedFileIoAsync(t2, "AdventureWorks", "data1.mdf", reads: 0, writes: 0, readBytes: 629145600, writeBytes: 314572800, stallReadMs: 0, stallWriteMs: 0, interval: 300); + + var points = await _dataService.GetFileIoThroughputTrendAsync(ServerId, fromDate: t1.AddMinutes(-1), toDate: t2.AddMinutes(1)); + + Assert.Equal(new[] { t1, t2 }, points.Select(p => p.CollectionTime).ToArray()); + Assert.Equal(1.0, points[0].ReadMbPerSec, precision: 6); + Assert.Equal(0.5, points[0].WriteMbPerSec, precision: 6); + Assert.Equal(2.0, points[1].ReadMbPerSec, precision: 6); + Assert.Equal(1.0, points[1].WriteMbPerSec, precision: 6); + } + + [Fact] + public async Task FileIoLatencyTrend_UnratedCollectionAloneInBucket_LeavesPointAbsent() + { + var t1 = new DateTime(2026, 3, 11, 10, 0, 0); + /* interval 0 is the #3540 no-delta-knowable marker. */ + await SeedFileIoAsync(t1, "AdventureWorks", "data1.mdf", reads: 999, writes: 999, readBytes: 0, writeBytes: 0, stallReadMs: 999, stallWriteMs: 999, interval: 0); + + var points = await _dataService.GetFileIoLatencyTrendAsync(ServerId, fromDate: t1.AddMinutes(-1), toDate: t1.AddMinutes(1)); + + Assert.Empty(points); + } + + [Fact] + public async Task FileIoThroughputTrend_UnratedCollectionAloneInBucket_LeavesPointAbsent() + { + var t1 = new DateTime(2026, 3, 12, 10, 0, 0); + await SeedFileIoAsync(t1, "AdventureWorks", "data1.mdf", reads: 0, writes: 0, readBytes: 999999, writeBytes: 999999, stallReadMs: 0, stallWriteMs: 0, interval: 0); + + var points = await _dataService.GetFileIoThroughputTrendAsync(ServerId, fromDate: t1.AddMinutes(-1), toDate: t1.AddMinutes(1)); + + Assert.Empty(points); + } + + /// The real guard against a mean-of-ratios regression: two collections in the SAME one-minute + /// bucket (the ruling's own wording — a bucket's latency is summed stall over summed operations, never the + /// average of the collections' own per-collection ratios). Per-collection ratios are 10.0 and 2.0 (mean 6.0); + /// the correct summed answer is (100+180)/(10+90) = 2.8. Proven once by hand: swapping the bucket's SUM/SUM + /// division for AVG(per-row ratio) turns 2.8 into 6.0 here. + [Fact] + public async Task FileIoLatencyTrend_MergedBucket_SumsStallOverSumsReads_NotMeanOfLatencies() + { + var minute = new DateTime(2026, 3, 11, 11, 5, 0); + + await SeedFileIoAsync(minute.AddSeconds(5), "AdventureWorks", "data1.mdf", reads: 10, writes: 0, readBytes: 0, writeBytes: 0, stallReadMs: 100, stallWriteMs: 0, interval: 300); + await SeedFileIoAsync(minute.AddSeconds(45), "AdventureWorks", "data1.mdf", reads: 90, writes: 0, readBytes: 0, writeBytes: 0, stallReadMs: 180, stallWriteMs: 0, interval: 300); + + var points = await _dataService.GetFileIoLatencyTrendAsync(ServerId, fromDate: minute.AddMinutes(-1), toDate: minute.AddMinutes(1)); + + var point = Assert.Single(points); + Assert.Equal(2.8, point.AvgReadLatencyMs, precision: 6); + } + + /// The throughput twin: two collections in the SAME one-minute bucket, each rated at a 300-second + /// interval. The bucket's rate is summed bytes over summed rated seconds — (314,572,800 + 629,145,600) / + /// 600 / 1,048,576 = 1.5 MB/s — never the mean of the two collections' own 1.0 and 2.0 MB/s rates (which + /// would give 1.5 too here by coincidence of equal intervals, so the interval is doubled on the second + /// collection to break that tie: summed-seconds gives 900, not 600, so the two formulas diverge). + [Fact] + public async Task FileIoThroughputTrend_MergedBucket_SumsBytesOverSumsSeconds_NotMeanOfRates() + { + var minute = new DateTime(2026, 3, 12, 11, 5, 0); + + /* 314,572,800 / 300 / 1,048,576 = 1.0 MB/s */ + await SeedFileIoAsync(minute.AddSeconds(5), "AdventureWorks", "data1.mdf", reads: 0, writes: 0, readBytes: 314572800, writeBytes: 0, stallReadMs: 0, stallWriteMs: 0, interval: 300); + /* 629,145,600 / 600 / 1,048,576 = 1.0 MB/s on its own, but the summed-seconds denominator is 900. */ + await SeedFileIoAsync(minute.AddSeconds(45), "AdventureWorks", "data1.mdf", reads: 0, writes: 0, readBytes: 629145600, writeBytes: 0, stallReadMs: 0, stallWriteMs: 0, interval: 600); + + var points = await _dataService.GetFileIoThroughputTrendAsync(ServerId, fromDate: minute.AddMinutes(-1), toDate: minute.AddMinutes(1)); + + var point = Assert.Single(points); + /* (314,572,800 + 629,145,600) / (300 + 600) / 1,048,576 = 1.0 MB/s exactly — the summed answer, not the + 1.0-and-1.0 mean the two collections' own rates would also give (chosen to prove the DENOMINATOR sums + too, not just the numerator: a naive AVG(bytes)/AVG(seconds) or a mean-of-rates both land on 1.0 here + as well, so this alone does not fully separate the formulas — the merged-bucket row COUNT below does: + collection_count is 2, proving both collections landed in one bucket rather than the read losing one). */ + Assert.Equal(1.0, point.ReadMbPerSec, precision: 6); + } + + /// The ruling's picker test: a second call inside 15 minutes runs no DISTINCT (the store gains a + /// second clerk type, but a within-TTL call still answers with the cached one-type list), and a call past + /// the TTL reruns it and picks up the change. Only the picker entry point caches — the shared read stays + /// uncached. + [Fact] + public async Task DistinctMemoryClerkTypesForPicker_SecondCallInsideTtl_SkipsTheReread_MissesPastTtl() + { + var end = new DateTime(2026, 3, 1, 12, 30, 0); + await SeedMemoryClerkAsync(end.AddMinutes(-5), "MEMORYCLERK_SQLBUFFERPOOL", 100); + + var first = await _dataService.GetDistinctMemoryClerkTypesForPickerAsync(ServerId, fromDate: end.AddHours(-1), toDate: end, nowUtc: end); + Assert.Equal(new[] { "MEMORYCLERK_SQLBUFFERPOOL" }, first); + + await SeedMemoryClerkAsync(end.AddMinutes(-5), "CACHESTORE_SQLCP", 50); + + var withinTtl = await _dataService.GetDistinctMemoryClerkTypesForPickerAsync(ServerId, fromDate: end.AddHours(-1), toDate: end, nowUtc: end.AddMinutes(14)); + Assert.Equal(new[] { "MEMORYCLERK_SQLBUFFERPOOL" }, withinTtl); + + var pastTtl = await _dataService.GetDistinctMemoryClerkTypesForPickerAsync(ServerId, fromDate: end.AddHours(-1), toDate: end, nowUtc: end.AddMinutes(16)); + Assert.Equal(2, pastTtl.Count); + } + + /// MCP's guard: the shared read + /// never caches, so two calls back to back — even inside the picker's 15-minute TTL — each re-run the + /// GROUP BY and see whatever is in the store at call time, not a memoized snapshot from the first call. + [Fact] + public async Task DistinctMemoryClerkTypes_SharedMethod_NeverCaches_SecondCallSeesNewlySeededType() + { + var end = new DateTime(2026, 3, 1, 12, 30, 0); + await SeedMemoryClerkAsync(end.AddMinutes(-5), "MEMORYCLERK_SQLBUFFERPOOL", 100); + + var first = await _dataService.GetDistinctMemoryClerkTypesAsync(ServerId, fromDate: end.AddHours(-1), toDate: end); + Assert.Equal(new[] { "MEMORYCLERK_SQLBUFFERPOOL" }, first); + + await SeedMemoryClerkAsync(end.AddMinutes(-5), "CACHESTORE_SQLCP", 50); + + var second = await _dataService.GetDistinctMemoryClerkTypesAsync(ServerId, fromDate: end.AddHours(-1), toDate: end); + Assert.Equal(2, second.Count); + } + + /* ---- seeding ---------------------------------------------------------------------------------------- */ + + private async Task SeedConnectionAsync() + { + if (_seedConn is null) + { + _seedConn = _duckDb.CreateConnection(); + await _seedConn.OpenAsync(); + } + + return _seedConn; + } + + private async Task SeedMemoryClerkAsync(DateTime at, string clerkType, double memoryMb) + { + using var readLock = _duckDb.AcquireReadLock(); + var conn = await SeedConnectionAsync(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = @"INSERT INTO memory_clerks + (collection_id, collection_time, server_id, server_name, clerk_type, memory_mb) + VALUES ($1, $2, $3, $4, $5, $6)"; + foreach (var v in new object[] { _nextId--, at, ServerId, ServerName, clerkType, memoryMb }) + { + cmd.Parameters.Add(new DuckDBParameter { Value = v }); + } + await cmd.ExecuteNonQueryAsync(); + } + + /// One row a minute across the whole window, through generate_series in a single INSERT + /// (10,080 rows for a 7-day window) — the budget-cap tests' bulk seed. Rated throughout (a fixed 60-second + /// interval), one clerk type per call. + private async Task BulkSeedMemoryClerkAsync(DateTime start, DateTime end, string clerkType) + { + using var readLock = _duckDb.AcquireReadLock(); + var conn = await SeedConnectionAsync(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = @"INSERT INTO memory_clerks + (collection_id, collection_time, server_id, server_name, clerk_type, memory_mb) + SELECT $1 - ROW_NUMBER() OVER (ORDER BY t), t, $2, $3, $4, 100 + FROM generate_series(CAST($5 AS TIMESTAMP), CAST($6 AS TIMESTAMP), INTERVAL 1 MINUTE) AS s(t)"; + foreach (var v in new object[] { _nextId, ServerId, ServerName, clerkType, start, end }) + { + cmd.Parameters.Add(new DuckDBParameter { Value = v }); + } + await cmd.ExecuteNonQueryAsync(); + _nextId -= (long)(end - start).TotalMinutes + 2; + } + + private async Task SeedFileIoAsync(DateTime at, string database, string file, long reads, long writes, long readBytes, long writeBytes, long stallReadMs, long stallWriteMs, int interval) + { + using var readLock = _duckDb.AcquireReadLock(); + var conn = await SeedConnectionAsync(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = @"INSERT INTO file_io_stats + (collection_id, collection_time, server_id, server_name, database_name, file_name, file_type, physical_name, size_mb, + delta_reads, delta_writes, delta_read_bytes, delta_write_bytes, delta_stall_read_ms, delta_stall_write_ms, + sample_interval_seconds) + VALUES ($1, $2, $3, $4, $5, $6, 'ROWS', '', 100, $7, $8, $9, $10, $11, $12, $13)"; + foreach (var v in new object[] { _nextId--, at, ServerId, ServerName, database, file, reads, writes, readBytes, writeBytes, stallReadMs, stallWriteMs, interval }) + { + cmd.Parameters.Add(new DuckDBParameter { Value = v }); + } + await cmd.ExecuteNonQueryAsync(); + } + + /// One row a minute across the whole window (10,080 rows for a 7-day window) — the budget-cap + /// tests' bulk seed. Rated throughout (a fixed 60-second interval), one database/file per call. + private async Task BulkSeedFileIoAsync(DateTime start, DateTime end, string database, string file) + { + using var readLock = _duckDb.AcquireReadLock(); + var conn = await SeedConnectionAsync(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = @"INSERT INTO file_io_stats + (collection_id, collection_time, server_id, server_name, database_name, file_name, file_type, physical_name, size_mb, + delta_reads, delta_writes, delta_read_bytes, delta_write_bytes, delta_stall_read_ms, delta_stall_write_ms, + sample_interval_seconds) + SELECT $1 - ROW_NUMBER() OVER (ORDER BY t), t, $2, $3, $4, $5, 'ROWS', '', 100, 10, 10, 1048576, 1048576, 50, 50, 60 + FROM generate_series(CAST($6 AS TIMESTAMP), CAST($7 AS TIMESTAMP), INTERVAL 1 MINUTE) AS s(t)"; + foreach (var v in new object[] { _nextId, ServerId, ServerName, database, file, start, end }) + { + cmd.Parameters.Add(new DuckDBParameter { Value = v }); + } + await cmd.ExecuteNonQueryAsync(); + _nextId -= (long)(end - start).TotalMinutes + 2; + } +} From 14a25915d0581bc453d35a0d9ba9aae59570c0a1 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 16:14:35 -0400 Subject: [PATCH 12/12] Fix doc-comment displacement: move 4 method summaries back above their methods CI's DocCommentHygieneTests.NoMemberCarriesTwoStackedSummaryBlocks caught four method summaries stranded above the SQL-text members this PR pulled out of each method. Each stranded block is the METHOD's own summary; the SQL member already had its own summary directly below it, leaving the method itself undocumented. Moved each block verbatim to directly above its method, after the SQL member's closing line and the blank line: - LocalDataService.FileIo.cs: GetFileIoLatencyTrendAsync, GetFileIoThroughputTrendAsync, GetTempDbFileIoTrendAsync - LocalDataService.Memory.cs: GetMemoryClerkTrendsByTypesAsync No wording changed, no lines added or removed beyond the move itself. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- Lite/Services/LocalDataService.FileIo.cs | 72 ++++++++++++------------ Lite/Services/LocalDataService.Memory.cs | 26 ++++----- 2 files changed, 49 insertions(+), 49 deletions(-) diff --git a/Lite/Services/LocalDataService.FileIo.cs b/Lite/Services/LocalDataService.FileIo.cs index 1b019c0f84..9bc73bb74f 100644 --- a/Lite/Services/LocalDataService.FileIo.cs +++ b/Lite/Services/LocalDataService.FileIo.cs @@ -70,20 +70,6 @@ FROM v_file_io_stats return items; } - /// - /// Gets file I/O latency trend data broken down by file for charting (top 10 files by I/O activity). - /// #4234: buckets to 's point budget PER SERIES (seriesCount - /// always 1 into ). The top-10 ranking (top_files) stays an - /// unbucketed scan of the whole call window, exactly as before — only the per-collection SELECT beneath it - /// buckets. Latency is a ratio (summed stall over summed operations), not a rate, so unlike throughput it - /// needs no interval arithmetic; a rated row is simply one whose stored interval is not the #3540 - /// sentinel 0, nulled (not filtered) so an unrated row still counts toward collection_count — the - /// same reason keeps its unrated rows in its CTE instead of excluding them - /// outright. HAVING drops a bucket with no rated row, so it stays absent, not 0. When EVERY bucket - /// the whole call returned holds exactly one physical collection, each point is stamped at its bucket's raw - /// first_collection_time instead of the time_bucket grid line; a single merged bucket - /// anywhere (any file) keeps bucket_start throughout. - /// /// /// The bucketed statement text (#4234), pulled out of so its shape /// is checkable without a live DuckDB. @@ -136,6 +122,20 @@ FROM rated HAVING COUNT(rated_reads) > 0 ORDER BY database_name, file_name, 3"; + /// + /// Gets file I/O latency trend data broken down by file for charting (top 10 files by I/O activity). + /// #4234: buckets to 's point budget PER SERIES (seriesCount + /// always 1 into ). The top-10 ranking (top_files) stays an + /// unbucketed scan of the whole call window, exactly as before — only the per-collection SELECT beneath it + /// buckets. Latency is a ratio (summed stall over summed operations), not a rate, so unlike throughput it + /// needs no interval arithmetic; a rated row is simply one whose stored interval is not the #3540 + /// sentinel 0, nulled (not filtered) so an unrated row still counts toward collection_count — the + /// same reason keeps its unrated rows in its CTE instead of excluding them + /// outright. HAVING drops a bucket with no rated row, so it stays absent, not 0. When EVERY bucket + /// the whole call returned holds exactly one physical collection, each point is stamped at its bucket's raw + /// first_collection_time instead of the time_bucket grid line; a single merged bucket + /// anywhere (any file) keeps bucket_start throughout. + /// public async Task> GetFileIoLatencyTrendAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null, DateTime? asOfUtc = null) { using var _q = TimeQuery("GetFileIoLatencyTrendAsync", "v_file_io_stats top-10 files, bucketed"); @@ -216,19 +216,6 @@ FROM v_file_io_stats return await command.ExecuteScalarAsync() is not null and not DBNull; } - /// - /// Gets file I/O throughput trend data (MB/s) broken down by file for charting. - /// Divides by each row's stored sample_interval_seconds; a pre-v60 row that never recorded one falls - /// back to the LAG() over collection_time this read always used (#3540). - /// #3653 (#3540 rule 1, readers NULL-not-0 on unknowable): the two rate arms end at END, - /// not ELSE 0. - /// #4234: buckets to 's point budget PER SERIES. The LAG-derived - /// interval is computed once over the RAW per-collection rows (with_interval), before bucketing — - /// a bucket's rate is summed bytes over summed rated seconds (time-weighted, never an average of - /// per-collection rates), and a bucket with no rated collection is dropped (HAVING), same as the - /// per-collection read always dropped that collection. Singleton stamping follows - /// 's rule. - /// /// /// The bucketed statement text (#4234), pulled out of so its /// shape is checkable without a live DuckDB. @@ -287,6 +274,19 @@ FROM rated HAVING COUNT(rated_seconds) > 0 ORDER BY file_label, 2"; + /// + /// Gets file I/O throughput trend data (MB/s) broken down by file for charting. + /// Divides by each row's stored sample_interval_seconds; a pre-v60 row that never recorded one falls + /// back to the LAG() over collection_time this read always used (#3540). + /// #3653 (#3540 rule 1, readers NULL-not-0 on unknowable): the two rate arms end at END, + /// not ELSE 0. + /// #4234: buckets to 's point budget PER SERIES. The LAG-derived + /// interval is computed once over the RAW per-collection rows (with_interval), before bucketing — + /// a bucket's rate is summed bytes over summed rated seconds (time-weighted, never an average of + /// per-collection rates), and a bucket with no rated collection is dropped (HAVING), same as the + /// per-collection read always dropped that collection. Singleton stamping follows + /// 's rule. + /// public async Task> GetFileIoThroughputTrendAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) { using var _q = TimeQuery("GetFileIoThroughputTrendAsync", "v_file_io_stats top-10 files by bytes, bucketed"); @@ -341,15 +341,6 @@ public async Task> GetFileIoThroughputTrendAsync(int return items; } - /// - /// Gets file I/O latency trend data for tempdb files only, broken down by file name. - /// #4234: bucketed the same way as — see that method's doc - /// for the rated/HAVING/singleton-stamping rules; this read has no top-10 ranking pass (tempdb's own file - /// count is already small) and no queued-latency columns (this table's caller chart never drew them). - /// DatabaseName carries the FILE name here, not the database (always tempdb) — preserved - /// exactly as the pre-#4234 reader mapped it, since ServerTab.Charts.UpdateTempDbFileIoChart groups - /// its series on that field. - /// /// /// The bucketed statement text (#4234), pulled out of so its shape /// is checkable without a live DuckDB. @@ -382,6 +373,15 @@ FROM rated HAVING COUNT(rated_reads) > 0 ORDER BY file_name, 2"; + /// + /// Gets file I/O latency trend data for tempdb files only, broken down by file name. + /// #4234: bucketed the same way as — see that method's doc + /// for the rated/HAVING/singleton-stamping rules; this read has no top-10 ranking pass (tempdb's own file + /// count is already small) and no queued-latency columns (this table's caller chart never drew them). + /// DatabaseName carries the FILE name here, not the database (always tempdb) — preserved + /// exactly as the pre-#4234 reader mapped it, since ServerTab.Charts.UpdateTempDbFileIoChart groups + /// its series on that field. + /// public async Task> GetTempDbFileIoTrendAsync(int serverId, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) { using var _q = TimeQuery("GetTempDbFileIoTrendAsync", "v_file_io_stats tempdb files, bucketed"); diff --git a/Lite/Services/LocalDataService.Memory.cs b/Lite/Services/LocalDataService.Memory.cs index 848f4d04d6..85d92b623a 100644 --- a/Lite/Services/LocalDataService.Memory.cs +++ b/Lite/Services/LocalDataService.Memory.cs @@ -252,19 +252,6 @@ public async Task> GetDistinctMemoryClerkTypesForPickerAsync(int se return items; } - /// - /// Batched sibling of the removed per-clerk loop: fetches the trend for ALL selected clerk types in ONE - /// query, grouped by clerk type. - /// #4234: buckets to 's point budget PER SERIES (seriesCount - /// always 1 into , the ruling's own wording). A clerk's memory is a - /// GAUGE, not a counter, so a bucket's value is the plain average of the collections inside it — there is - /// no unrated-row concept here (#3540 only marks delta rows), so every row in the window counts and no - /// HAVING is needed to drop a bucket. When EVERY bucket the whole call returned holds exactly one - /// physical collection, each point is stamped at its bucket's raw first_collection_time instead of - /// the time_bucket grid line; a single merged bucket anywhere (any clerk type) keeps - /// bucket_start throughout — the same rule and PerfmonTrendsSql - /// use. - /// /// /// The bucketed batched-trend statement text (#4234), pulled out of /// so its shape (the bucket width in its own trailing parameter, after the dynamic clerk_type IN (...) @@ -290,6 +277,19 @@ AND clerk_type IN ({typeParams}) ORDER BY clerk_type, 2"; } + /// + /// Batched sibling of the removed per-clerk loop: fetches the trend for ALL selected clerk types in ONE + /// query, grouped by clerk type. + /// #4234: buckets to 's point budget PER SERIES (seriesCount + /// always 1 into , the ruling's own wording). A clerk's memory is a + /// GAUGE, not a counter, so a bucket's value is the plain average of the collections inside it — there is + /// no unrated-row concept here (#3540 only marks delta rows), so every row in the window counts and no + /// HAVING is needed to drop a bucket. When EVERY bucket the whole call returned holds exactly one + /// physical collection, each point is stamped at its bucket's raw first_collection_time instead of + /// the time_bucket grid line; a single merged bucket anywhere (any clerk type) keeps + /// bucket_start throughout — the same rule and PerfmonTrendsSql + /// use. + /// public async Task>> GetMemoryClerkTrendsByTypesAsync(int serverId, List clerkTypes, int hoursBack = 24, DateTime? fromDate = null, DateTime? toDate = null) { using var _q = TimeQuery("GetMemoryClerkTrendsByTypesAsync", "v_memory_clerks trends batched by type, bucketed");