From 89e4a6f953ba724a8b50f204f569106f91417de7 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:04:59 -0400 Subject: [PATCH 1/4] Bucket the File I/O latency and throughput viewer trend reads (#4234) Both reads used to return every collection (measured 85,010 rows at 7 days for one store on the File I/O tab). Groups now by TrendBudget.Chart-sized date_bin buckets, same sizing as the MCP trend reads: latency averages summed stall-ms over summed read/write count deltas per bucket, throughput sums byte deltas over summed interval seconds. A bucket holding exactly one physical collection (rated or not) keeps that collection's own raw timestamp instead of the bucket_start grid line, matching WaitTrendsSql's pattern from PR #4304. Also pulls in the InternalsVisibleTo grant and ViewerNameListCache dependency shared with that PR's pattern. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- .../ViewerDataService.FileIo.cs | 241 +++++++++++++----- .../ViewerNameListCache.cs | 77 ++++++ .../PerformanceMonitor.Common.csproj | 3 + 3 files changed, 251 insertions(+), 70 deletions(-) create mode 100644 Darling/PerformanceMonitor.Darling.Viewer/ViewerNameListCache.cs diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.FileIo.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.FileIo.cs index 27fd99b132..eca0ac9c50 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.FileIo.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.FileIo.cs @@ -12,14 +12,20 @@ using System.Threading.Tasks; using Npgsql; +using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Storage; + namespace PerformanceMonitor.Darling.Viewer; /// /// One file's I/O-latency point for the File I/O tab's Latency sub-tab — a mirror of Lite's /// FileIoTrendPoint (LocalDataService.FileIo.cs). The read averages stall-ms / op per -/// (collection_time, database_name, file_name); the chart groups on database_name.file_name, +/// (bucket, database_name, file_name); the chart groups on database_name.file_name, /// sums read+write latency to rank the top files, and overlays the queued-I/O latencies as dashed /// lines when any are present. +/// #4234: CollectionTime is a date_bin bucket start, EXCEPT when every bucket the call +/// returned holds exactly one physical collection, in which case every point is stamped at its own raw +/// collection time instead — see . /// public sealed record FileIoLatencyPoint( DateTime CollectionTime, @@ -35,6 +41,9 @@ public sealed record FileIoLatencyPoint( /// FileIoThroughputPoint. file_label is the pre-concatenated database_name.file_name /// the read builds, and the per-second rates come from the delta bytes divided by the row's stored /// sample_interval_seconds (LAG-derived only for pre-V127 rows that never recorded one, #3540). +/// #4234: CollectionTime is a date_bin bucket start, EXCEPT when every bucket the call +/// returned holds exactly one physical collection, in which case every point is stamped at its own raw +/// collection time instead — see . /// public sealed record FileIoThroughputPoint( DateTime CollectionTime, @@ -45,16 +54,29 @@ public sealed record FileIoThroughputPoint( public sealed partial class ViewerDataService { /// - /// The File I/O latency read — Lite's GetFileIoLatencyTrendAsync ported to Postgres. Reads - /// v_file_io_stats (a SELECT * passthrough over file_io_stats) exactly as Lite's - /// view-based query does: a top_files CTE picks the 10 busiest (database, file) pairs by total - /// delta ops over the window, then per-collection average read/write (and queued read/write) latency - /// is computed as stall-ms / op with the delta-stall sums CAST to double precision before division. - /// The queued-stall columns are COALESCE'd to 0 so a server whose build predates them still reads 0. - /// Rows whose stored sample_interval_seconds is 0 — no delta knowable, a restart — are dropped - /// rather than rendered as 0.00 ms (#3540). $1 server_id, $2 window start, $3 window end (naive UTC). + /// The File I/O latency read — Lite's GetFileIoLatencyTrendAsync ported to Postgres, then bucketed + /// (#4234). Reads v_file_io_stats (a SELECT * passthrough over file_io_stats): a + /// top_files CTE picks the 10 busiest (database, file) pairs by total delta ops over the window — + /// unchanged by bucketing — then average read/write (and queued read/write) latency per bucket is computed + /// as summed stall-ms / summed ops, with the delta-stall sums CAST to double precision before division. The + /// queued-stall columns are COALESCE'd to 0 so a server whose build predates them still reads 0. + /// #3540, carried into the bucket: a row whose stored sample_interval_seconds is 0 (no delta + /// knowable — first sighting, counter reset, a gap past the policy) has its deltas nulled out of the sums + /// via the rated CTE rather than contributing a confident 0, exactly as the pre-bucket read dropped + /// the row outright. IS DISTINCT FROM 0 still keeps pre-V127 rows (NULL: interval never recorded). + /// Unlike the pre-bucket read, the row is NOT removed from the FROM clause — it still counts toward + /// collection_count below, so a bucket is never mistaken for a true singleton just because its one + /// physical collection happened to be unrated. HAVING COUNT(rated_reads) > 0 drops a bucket whose + /// every row was unrated, matching the pre-bucket read's "absent, not 0.00 ms" answer for that case; a + /// bucket with at least one rated row keeps its per-metric CASE ... ELSE 0 zero-safety exactly as + /// before (a rated row with 0 ops is a real 0 ms, not an absent point). + /// #4234 review (item 3): first_collection_time (MIN(collection_time), every row in + /// rated — rated or not) and collection_count (COUNT(*) over that same population) ride + /// along so the caller can tell a true singleton bucket from one the bucketing actually merged, exactly like + /// WaitTrendsSql. $1 server_id, $2 window start, $3 window end (naive UTC), $4 bucket width minutes. + /// /// - public const string FileIoLatencyTrendSql = """ + public const string FileIoLatencyTrendSql = $$""" WITH top_files AS ( SELECT database_name, file_name FROM v_file_io_stats @@ -65,56 +87,68 @@ 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.collection_time, + f.database_name, + f.file_name, + /* #3540: a stored interval of 0 means this row's own deltas are not knowable, so they are + nulled out of the sums below — the row still counts as a physical collection (collection_count) + but contributes nothing to a bucket's rate. */ + 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 f.delta_stall_read_ms END AS rated_stall_read_ms, + CASE WHEN f.sample_interval_seconds IS DISTINCT FROM 0 THEN f.delta_stall_write_ms END AS rated_stall_write_ms, + CASE WHEN f.sample_interval_seconds IS DISTINCT FROM 0 THEN COALESCE(f.delta_stall_queued_read_ms, 0) END AS rated_stall_queued_read_ms, + CASE WHEN f.sample_interval_seconds IS DISTINCT FROM 0 THEN COALESCE(f.delta_stall_queued_write_ms, 0) END AS rated_stall_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) + database_name, + file_name, + GREATEST(date_bin(CAST($4 AS integer) * INTERVAL '1 minute', collection_time, {{TrendBucketSql.OriginSql}}), $2) AS bucket_start, + CASE WHEN SUM(rated_reads) > 0 + THEN SUM(CAST(rated_stall_read_ms AS double precision)) / SUM(rated_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) + CASE WHEN SUM(rated_writes) > 0 + THEN SUM(CAST(rated_stall_write_ms AS double precision)) / SUM(rated_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) + CASE WHEN SUM(rated_reads) > 0 + THEN SUM(CAST(rated_stall_queued_read_ms AS double precision)) / SUM(rated_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-V127 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 + CASE WHEN SUM(rated_writes) > 0 + THEN SUM(CAST(rated_stall_queued_write_ms AS double precision)) / 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 """; /// - /// The File I/O throughput read — Lite's GetFileIoThroughputTrendAsync ported to Postgres - /// verbatim (LAG + EXTRACT(EPOCH ...) run identically on PG). A top_files CTE picks the 10 - /// busiest (database, file) pairs by total delta bytes, a with_interval CTE takes each row's - /// stored sample_interval_seconds (falling back to the LAG of the prior collection_time, - /// partitioned by server/database/file, for pre-V127 rows that never recorded one — #3540), and the - /// per-second MB rate is delta-bytes / interval-seconds / 1 MiB. Rows with no usable interval (NULL: - /// the first sample per file, or a stored 0 meaning no delta was knowable) are dropped, never plotted - /// as 0. - /// #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; it never coerces a NULL rate today because no - /// NULL rate reaches it. - /// $1 server_id, $2 window start, $3 window end (naive UTC). + /// The File I/O throughput read — Lite's GetFileIoThroughputTrendAsync ported to Postgres, then + /// bucketed (#4234). A top_files CTE picks the 10 busiest (database, file) pairs by total delta + /// bytes — unchanged by bucketing — a with_interval CTE takes each row's stored + /// sample_interval_seconds (falling back to the LAG of the prior collection_time, partitioned by + /// server/database/file, for pre-V127 rows that never recorded one — #3540), and a rated CTE nulls + /// out a row's bytes/interval when the interval is unknowable. A bucket's per-second MB rate is time-weighted + /// — summed rated bytes over summed rated interval-seconds / 1 MiB — never an average of per-collection + /// rates, mirroring WaitTrendsSql. HAVING COUNT(rated_seconds) > 0 drops a bucket with no + /// rated collection, same as the pre-bucket read always dropped that collection (#3653: NULL, never a + /// measured 0.00 MB/s). + /// #4234 review (item 3): first_collection_time / collection_count ride along exactly + /// like WaitTrendsSql's, counted over every physical row (rated or not) so a bucket is never mistaken + /// for a true singleton just because its one physical collection happened to be unrated. + /// $1 server_id, $2 window start, $3 window end (naive UTC), $4 bucket width minutes. /// - public const string FileIoThroughputTrendSql = """ + public const string FileIoThroughputTrendSql = $$""" WITH top_files AS ( SELECT database_name, file_name FROM v_file_io_stats @@ -133,9 +167,9 @@ 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-V127 row that never recorded one) falls back to the LAG this - read always used. */ + through NULLIF and is nulled out below, exactly as it always dropped the first row per + file. NULL (a pre-V127 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 @@ -148,30 +182,47 @@ 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 delta_read_bytes END AS rated_read_bytes, + CASE WHEN interval_seconds > 0 THEN delta_write_bytes 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(date_bin(CAST($4 AS integer) * INTERVAL '1 minute', collection_time, {{TrendBucketSql.OriginSql}}), $2) AS bucket_start, + CAST(SUM(rated_read_bytes) AS double precision) / SUM(rated_seconds) / 1048576.0 AS read_mb_per_sec, + CAST(SUM(rated_write_bytes) AS double precision) / 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 """; /// /// Per-file I/O-latency points (top 10 files by delta ops) for one server over the window, for the /// File I/O tab's Latency sub-tab. + /// #4234: buckets to 's point budget PER SERIES (file), like + /// GetWaitStatsTrendsByTypesAsync; 's window alone decides the width + /// (seriesCount is always 1 into , the ruling's own "per + /// series" wording). When every bucket the call returns holds exactly one physical collection (any file, + /// any bucket), every point is stamped at its own raw first_collection_time instead of the + /// bucket_start grid line — see . /// public async Task> GetFileIoLatencyTrendAsync( int serverId, DateTime startUtc, DateTime endUtc, CancellationToken cancellationToken = default) { var items = new List(); + var windowMinutes = Math.Max(1, (int)Math.Ceiling((endUtc - startUtc).TotalMinutes)); + var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); + await using var command = _dataSource.CreateCommand(FileIoLatencyTrendSql); command.CommandTimeout = ViewerCommandDeadlines.CurrentInteractiveReadSeconds; command.Parameters.Add(new NpgsqlParameter { TypedValue = serverId }); @@ -183,31 +234,61 @@ public async Task> GetFileIoLatencyTrendAsync( { TypedValue = DateTime.SpecifyKind(endUtc, DateTimeKind.Unspecified), }); + command.Parameters.Add(new NpgsqlParameter { TypedValue = bucketMinutes }); + + var rows = new List<(string DatabaseName, string FileName, DateTime BucketStart, DateTime FirstCollectionTime, + double AvgRead, double AvgWrite, double AvgQueuedRead, double AvgQueuedWrite)>(); + var everyBucketSingleton = true; + await using var reader = await command.ExecuteReaderAsync(cancellationToken); while (await reader.ReadAsync(cancellationToken)) { - items.Add(new FileIoLatencyPoint( - reader.GetDateTime(0), + if (reader.GetInt64(8) != 1) + { + everyBucketSingleton = false; + } + + rows.Add(( + reader.IsDBNull(0) ? "" : reader.GetString(0), reader.IsDBNull(1) ? "" : reader.GetString(1), - reader.IsDBNull(2) ? "" : reader.GetString(2), + reader.GetDateTime(2), + reader.GetDateTime(7), reader.IsDBNull(3) ? 0 : reader.GetDouble(3), reader.IsDBNull(4) ? 0 : reader.GetDouble(4), reader.IsDBNull(5) ? 0 : reader.GetDouble(5), reader.IsDBNull(6) ? 0 : reader.GetDouble(6))); } + foreach (var row in rows) + { + items.Add(new FileIoLatencyPoint( + everyBucketSingleton ? row.FirstCollectionTime : row.BucketStart, + row.DatabaseName, + row.FileName, + row.AvgRead, + row.AvgWrite, + row.AvgQueuedRead, + row.AvgQueuedWrite)); + } + return items; } /// /// Per-file throughput points (MB/s, top 10 files by delta bytes) for one server over the window, /// for the File I/O tab's Throughput sub-tab. + /// #4234: buckets to 's point budget PER SERIES (file), exactly like + /// — see its remarks for the width and singleton-stamping rules, + /// which this read shares verbatim. /// public async Task> GetFileIoThroughputTrendAsync( int serverId, DateTime startUtc, DateTime endUtc, CancellationToken cancellationToken = default) { var items = new List(); + var windowMinutes = Math.Max(1, (int)Math.Ceiling((endUtc - startUtc).TotalMinutes)); + var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); + await using var command = _dataSource.CreateCommand(FileIoThroughputTrendSql); command.CommandTimeout = ViewerCommandDeadlines.CurrentInteractiveReadSeconds; command.Parameters.Add(new NpgsqlParameter { TypedValue = serverId }); @@ -219,16 +300,36 @@ public async Task> GetFileIoThroughputTrendAsync( { TypedValue = DateTime.SpecifyKind(endUtc, DateTimeKind.Unspecified), }); + command.Parameters.Add(new NpgsqlParameter { TypedValue = bucketMinutes }); + + var rows = new List<(string FileLabel, DateTime BucketStart, DateTime FirstCollectionTime, double Read, double Write)>(); + var everyBucketSingleton = true; + await using var reader = await command.ExecuteReaderAsync(cancellationToken); while (await reader.ReadAsync(cancellationToken)) { - items.Add(new FileIoThroughputPoint( - reader.GetDateTime(0), - reader.IsDBNull(1) ? "" : reader.GetString(1), + 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 : reader.GetDouble(2), reader.IsDBNull(3) ? 0 : reader.GetDouble(3))); } + foreach (var row in rows) + { + items.Add(new FileIoThroughputPoint( + everyBucketSingleton ? row.FirstCollectionTime : row.BucketStart, + row.FileLabel, + row.Read, + row.Write)); + } + return items; } } diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerNameListCache.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerNameListCache.cs new file mode 100644 index 0000000000..807e6df146 --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerNameListCache.cs @@ -0,0 +1,77 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; + +namespace PerformanceMonitor.Darling.Viewer; + +/// +/// #4234: TTL memoization for the Wait Stats and Perfmon picker's name-list reads (DistinctWaitTypesSql, +/// DistinctPerfmonCountersSql), each a full-window DISTINCT that used to run on every 1-minute +/// auto-refresh (measured 2,079 ms cold for the Perfmon picker over a 7-day window on a production store). 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 key is (server, length) alone, but a hit ALSO needs the cached window's END to still be +/// recent. Keying on length alone 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, got THIS WEEK's list +/// under last month's window, because both windows share one length-keyed slot. A hit now 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. +/// +/// -style clock parameters throughout are the seam +/// already uses: the caller's wall clock +/// by default, an injected time in a test, so a live test can fast-forward past the TTL without a real +/// 15-minute sleep. +/// +internal sealed class ViewerNameListCache +{ + /// 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/PerformanceMonitor.Common/PerformanceMonitor.Common.csproj b/PerformanceMonitor.Common/PerformanceMonitor.Common.csproj index 612b4bcd0b..816b2805e2 100644 --- a/PerformanceMonitor.Common/PerformanceMonitor.Common.csproj +++ b/PerformanceMonitor.Common/PerformanceMonitor.Common.csproj @@ -31,6 +31,9 @@ + + From e67b88cef15d5f42860ec9de1f06cc10d5375cc8 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:07:05 -0400 Subject: [PATCH 2/4] Bucket the memory clerk viewer trend read and cache its picker (#4234) The clerk trend read (up to 20 clerks) returned every collection; it now buckets as a gauge (average memory_mb per bucket), same sizing as the MCP trend reads and the File I/O reads just bucketed. A bucket holding exactly one physical collection keeps that collection's own raw timestamp, matching WaitTrendsSql. DistinctMemoryClerkTypesSql (the clerk picker's population read) now sits behind ViewerNameListCache, same as DistinctWaitTypesSql in PR #4304, so the full-window DISTINCT runs at most once per 15 minutes instead of on every auto-refresh. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- .../ViewerDataService.Memory.cs | 104 ++++++++++++++---- 1 file changed, 83 insertions(+), 21 deletions(-) diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Memory.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Memory.cs index 227da4f7f3..d6d8350649 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Memory.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Memory.cs @@ -12,6 +12,7 @@ using System.Threading; using System.Threading.Tasks; using Npgsql; +using PerformanceMonitor.Common; using PerformanceMonitor.Darling.Storage; namespace PerformanceMonitor.Darling.Viewer; @@ -44,7 +45,13 @@ public sealed record MemoryGrantTrendPoint( /// One point on a selected memory clerk's trend line: its memory footprint (MB) at a /// collection. Mirror of Lite's MemoryClerkTrendPoint — the clerk type is the dictionary key of -/// the batched read, so it is not repeated on each point. +/// the batched read, so it is not repeated on each point. +/// #4234: a gauge, bucketed to 's point budget — MemoryMb is the +/// AVERAGE footprint across the bucket's collections. CollectionTime is a date_bin bucket start, +/// EXCEPT when every bucket the call returned holds exactly one physical collection, in which case every point +/// is stamped at its own raw collection time instead — see +/// . +/// public sealed record MemoryClerkTrendPoint( DateTime CollectionTime, double MemoryMb); @@ -93,6 +100,10 @@ public sealed record MemoryPressureEventRow( /// public sealed partial class ViewerDataService { + /// #4234: TTL memoization for — see + /// 's remarks. + private readonly ViewerNameListCache _distinctMemoryClerkTypesCache = new(); + /// /// The Overview summary snapshot — Lite's GetLatestMemoryStatsAsync ported to Postgres: the /// most recent memory_stats row for the server (no window; ORDER BY collection_time DESC LIMIT 1). @@ -140,6 +151,10 @@ ORDER BY collection_time /// Postgres verbatim: every clerk_type collected for the server over the window, ranked by total /// memory descending so the picker + "Top Clerks" fill see the heaviest clerks first. $1 server_id, /// $2 window start, $3 window end (naive UTC). + /// #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, exactly like + /// GetDistinctWaitTypesAsync. /// public const string DistinctMemoryClerkTypesSql = """ SELECT @@ -153,28 +168,39 @@ ORDER BY SUM(memory_mb) DESC """; /// - /// Lite's batched GetMemoryClerkTrendsByTypesAsync body, ported to Postgres: the trend for ALL - /// selected clerk types in ONE query (replacing an N+1 loop), grouped by clerk type. The - /// clerk_type IN (...) list is built dynamically from $4 onward - /// (server_id/start/end take $1/$2/$3), exactly like Lite's numbering. memory_mb - /// is numeric(18,2) in the store, CAST to double precision for the typed reader. A caller - /// passing = 0 is a bug the - /// guard prevents. + /// Lite's batched GetMemoryClerkTrendsByTypesAsync body, ported to Postgres, then bucketed (#4234): + /// the trend for ALL selected clerk types in ONE query (replacing an N+1 loop), grouped by clerk type and + /// date_bin bucket. The clerk_type IN (...) list is built dynamically from $4 onward + /// (server_id/start/end take $1/$2/$3, the bucket width takes the trailing parameter + /// after the list, exactly like WaitTrendsSql), exactly like Lite's numbering. memory_mb is + /// numeric(18,2) in the store, CAST to double precision for the typed reader. A caller passing + /// = 0 is a bug the guard + /// prevents. + /// #4234: a gauge — memory_mb is the AVERAGE footprint across the bucket's collections, not a + /// summed-delta rate (a clerk's memory_mb is a point-in-time reading, not an accumulating counter). + /// first_collection_time (MIN(collection_time)) and collection_count (COUNT(*)) + /// ride along per bucket, exactly like WaitTrendsSql, so the caller can tell a true singleton bucket + /// from one the bucketing merged. + /// $4.. clerk types, last $ the bucket width in minutes. /// public static string MemoryClerkTrendsSql(int typeCount) { var typeParams = string.Join(", ", Enumerable.Range(0, typeCount).Select(i => "$" + (i + 4))); - return $""" + var widthParam = "$" + (typeCount + 4); + return $$""" SELECT clerk_type, - collection_time, - CAST(memory_mb AS double precision) AS memory_mb + GREATEST(date_bin(CAST({{widthParam}} AS integer) * INTERVAL '1 minute', collection_time, {{TrendBucketSql.OriginSql}}), $2) AS bucket_start, + CAST(AVG(memory_mb) AS double precision) 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 + AND clerk_type IN ({{typeParams}}) + GROUP BY clerk_type, 2 + ORDER BY clerk_type, 2 """; } @@ -290,10 +316,20 @@ public async Task> GetMemoryGrantTrendAsync( } /// The distinct clerk types collected for one server in the window, ranked by total memory - /// descending — feeds the clerk picker's population + "Top Clerks" fill. + /// descending — feeds the clerk picker's population + "Top Clerks" fill. + /// 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> GetDistinctMemoryClerkTypesAsync( - int serverId, DateTime startUtc, DateTime endUtc, CancellationToken cancellationToken = default) + int serverId, DateTime startUtc, DateTime endUtc, DateTime? nowUtc = null, CancellationToken cancellationToken = default) { + var effectiveNow = nowUtc ?? DateTime.UtcNow; + var windowLength = endUtc - startUtc; + if (_distinctMemoryClerkTypesCache.TryGet(serverId, windowLength, endUtc, effectiveNow, out var cached)) + { + return cached; + } + var items = new List(); await using var command = _dataSource.CreateCommand(DistinctMemoryClerkTypesSql); @@ -314,11 +350,17 @@ public async Task> GetDistinctMemoryClerkTypesAsync( items.Add(reader.GetString(0)); } + _distinctMemoryClerkTypesCache.Set(serverId, windowLength, endUtc, items, effectiveNow); return items; } /// The memory trend for every selected clerk type in one query, grouped by clerk type. - /// Empty selection returns an empty map without touching the store. + /// Empty selection returns an empty map without touching the store. + /// #4234: buckets to 's point budget PER SERIES (clerk type), exactly + /// like GetWaitStatsTrendsByTypesAsync — see its remarks for the width and singleton-stamping rules, + /// which this read shares verbatim (only the metric itself differs: an averaged gauge here, a summed-delta + /// rate there). + /// public async Task>> GetMemoryClerkTrendsByTypesAsync( int serverId, List clerkTypes, DateTime startUtc, DateTime endUtc, CancellationToken cancellationToken = default) { @@ -328,6 +370,9 @@ public async Task>> GetMemoryCler return result; } + var windowMinutes = Math.Max(1, (int)Math.Ceiling((endUtc - startUtc).TotalMinutes)); + var bucketMinutes = TrendBuckets.AutoMinutes(windowMinutes, 1, TrendBudget.Chart.AutoPoints); + await using var command = _dataSource.CreateCommand(MemoryClerkTrendsSql(clerkTypes.Count)); command.CommandTimeout = ViewerCommandDeadlines.CurrentInteractiveReadSeconds; command.Parameters.Add(new NpgsqlParameter { TypedValue = serverId }); @@ -343,22 +388,39 @@ public async Task>> GetMemoryCler { command.Parameters.Add(new NpgsqlParameter { TypedValue = clerkType }); } + command.Parameters.Add(new NpgsqlParameter { TypedValue = bucketMinutes }); + + var rows = new List<(string ClerkType, DateTime BucketStart, DateTime FirstCollectionTime, double MemoryMb)>(); + var everyBucketSingleton = true; await using var reader = await command.ExecuteReaderAsync(cancellationToken); while (await reader.ReadAsync(cancellationToken)) { - var clerkType = reader.GetString(0); - if (!result.TryGetValue(clerkType, out var list)) + if (reader.GetInt64(4) != 1) { - list = new List(); - result[clerkType] = list; + everyBucketSingleton = false; } - list.Add(new MemoryClerkTrendPoint( + rows.Add(( + reader.GetString(0), reader.GetDateTime(1), + reader.GetDateTime(3), reader.IsDBNull(2) ? 0 : reader.GetDouble(2))); } + foreach (var row in rows) + { + if (!result.TryGetValue(row.ClerkType, out var list)) + { + list = new List(); + result[row.ClerkType] = list; + } + + list.Add(new MemoryClerkTrendPoint( + everyBucketSingleton ? row.FirstCollectionTime : row.BucketStart, + row.MemoryMb)); + } + return result; } From 9dda16da9d9f29bdcc9f5c5e12d5d7d24e1b88a8 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:15:06 -0400 Subject: [PATCH 3/4] Pin the File I/O and memory clerk bucketing with source and live tests (#4234) Updates the existing SQL-text assertions that the bucketing rewrite made stale (GROUP BY/ORDER BY shape, the rated-CTE column names, the CAST(memory_mb...) literal), and adds the ruling's four required tests per read: a bucket-width source check, a live 7-day-window row-budget cap, live point equality with the old shape when every bucket holds one collection (seeded off the minute grid at :37 seconds), and for the memory clerk picker, a live proof that a second call inside 15 minutes answers from cache and that a window end past the TTL misses. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- .../ViewerFileIoBlockingTests.cs | 164 +++++++++++++- Darling/Darling.Tests/ViewerMemoryTests.cs | 207 +++++++++++++++++- 2 files changed, 360 insertions(+), 11 deletions(-) diff --git a/Darling/Darling.Tests/ViewerFileIoBlockingTests.cs b/Darling/Darling.Tests/ViewerFileIoBlockingTests.cs index 9c238e345c..39de323e59 100644 --- a/Darling/Darling.Tests/ViewerFileIoBlockingTests.cs +++ b/Darling/Darling.Tests/ViewerFileIoBlockingTests.cs @@ -12,6 +12,7 @@ using Npgsql; using NpgsqlTypes; using PerformanceMonitor.Collectors; +using PerformanceMonitor.Common; using PerformanceMonitor.Darling.Storage; using PerformanceMonitor.Darling.Viewer; using Xunit; @@ -39,22 +40,41 @@ public void FileIoLatencyTrendSql_TopFilesCte_PerFileAverageLatency_WithQueuedCo Assert.Contains("ORDER BY SUM(delta_reads + delta_writes) DESC", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); Assert.Contains("LIMIT 10", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); - /* Read/write average latency casts the stall SUM to double before dividing by ops. */ - Assert.Contains("SUM(CAST(f.delta_stall_read_ms AS double precision)) / SUM(f.delta_reads)", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); - Assert.Contains("SUM(CAST(f.delta_stall_write_ms AS double precision)) / SUM(f.delta_writes)", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); + /* #4234: read/write average latency casts the bucket's summed stall to double before dividing by + the bucket's summed ops — a rated row's deltas only (unrated ones nulled out by the CASE below). */ + Assert.Contains("SUM(CAST(rated_stall_read_ms AS double precision)) / SUM(rated_reads)", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); + Assert.Contains("SUM(CAST(rated_stall_write_ms AS double precision)) / SUM(rated_writes)", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); /* Queued latency COALESCEs the queued-stall column to 0 (a pre-queued-column build reads 0). */ Assert.Contains("COALESCE(f.delta_stall_queued_read_ms, 0)", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); Assert.Contains("COALESCE(f.delta_stall_queued_write_ms, 0)", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); - Assert.Contains("GROUP BY f.collection_time, f.database_name, f.file_name", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); - Assert.Contains("ORDER BY f.collection_time, f.database_name, f.file_name", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); + /* #4234: buckets by date_bin, grouped per series (database_name, file_name) then the bucket start. */ + Assert.Contains("GROUP BY database_name, file_name, 3", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); + Assert.Contains("ORDER BY database_name, file_name, 3", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); + Assert.Contains("HAVING COUNT(rated_reads) > 0", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); /* Both window bounds are applied (top_files CTE + main query). */ Assert.Contains("collection_time >= $2", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); Assert.Contains("collection_time <= $3", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); } + /// #4234 ruling item 6 (source check): both File I/O reads carry a bucket width and project the + /// singleton-detection columns, mirroring WaitTrendsSql. Proven once by hand against the pre-#4234 + /// text: neither statement had a date_bin anywhere (a per-collection read has no bucket width), so + /// this assert fails there. + [Fact] + public void FileIoTrendSqls_CarryABucketWidth_AndProjectSingletonDetectionColumns() + { + foreach (var sql in new[] { ViewerDataService.FileIoLatencyTrendSql, ViewerDataService.FileIoThroughputTrendSql }) + { + Assert.Contains("date_bin(CAST($4 AS integer) * INTERVAL '1 minute'", sql, StringComparison.Ordinal); + Assert.Contains(TrendBucketSql.OriginSql, sql, StringComparison.Ordinal); + Assert.Contains("MIN(collection_time) AS first_collection_time", sql, StringComparison.Ordinal); + Assert.Contains("COUNT(*) AS collection_count", sql, StringComparison.Ordinal); + } + } + [Fact] public void FileIoThroughputTrendSql_LagInterval_PerSecondMbRate() { @@ -70,9 +90,11 @@ public void FileIoThroughputTrendSql_LagInterval_PerSecondMbRate() Assert.Contains("EXTRACT(EPOCH FROM", ViewerDataService.FileIoThroughputTrendSql, StringComparison.Ordinal); Assert.Contains("ELSE NULLIF(f.sample_interval_seconds, 0)", ViewerDataService.FileIoThroughputTrendSql, StringComparison.Ordinal); - /* Bytes / interval-seconds / 1 MiB, and the NULL-interval rows (first per file, or unknowable) are dropped. */ - Assert.Contains("/ interval_seconds / 1048576.0", ViewerDataService.FileIoThroughputTrendSql, StringComparison.Ordinal); - Assert.Contains("WHERE interval_seconds IS NOT NULL AND interval_seconds > 0", ViewerDataService.FileIoThroughputTrendSql, StringComparison.Ordinal); + /* #4234: time-weighted per bucket — summed rated bytes over summed rated interval-seconds / 1 MiB — + and a bucket with no rated collection is dropped (HAVING), same as the pre-bucket read always + dropped that collection. */ + Assert.Contains("CAST(SUM(rated_read_bytes) AS double precision) / SUM(rated_seconds) / 1048576.0", ViewerDataService.FileIoThroughputTrendSql, StringComparison.Ordinal); + Assert.Contains("HAVING COUNT(rated_seconds) > 0", ViewerDataService.FileIoThroughputTrendSql, StringComparison.Ordinal); } [Fact] @@ -130,7 +152,7 @@ public void LockWaitTrendSql_FiltersLckPrefix_PerSecondRateViaLag() [Fact] public void FileIoLatencyReads_DropTheUnknowableMarker_KeepPreV127Rows() { - Assert.Contains("AND f.sample_interval_seconds IS DISTINCT FROM 0", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); + Assert.Contains("CASE WHEN f.sample_interval_seconds IS DISTINCT FROM 0 THEN f.delta_reads END AS rated_reads", ViewerDataService.FileIoLatencyTrendSql, StringComparison.Ordinal); Assert.Contains("AND sample_interval_seconds IS DISTINCT FROM 0", ViewerDataService.TempDbFileIoTrendSql, StringComparison.Ordinal); } @@ -232,9 +254,108 @@ public sealed class ViewerFileIoBlockingLivePostgresTests private const int LockWaitServerId = -971005; private const int WaitingTaskServerId = -971006; private const int BlockedSessionServerId = -971007; + private const int FileIoBudgetServerId = -971008; + private const int FileIoSingletonServerId = -971009; private const string ServerName = "viewer-w1c-e2e"; + /// #4234 ruling items 2/6: a 7-day window at the real 1-minute file_io_stats cadence used to + /// return every collection (85,010 rows for one production store, per #4234's body) — now capped to + /// 's point budget PER SERIES (file). Bulk-seeded server-side + /// (generate_series) rather than one round trip per minute. + [Fact] + public async Task FileIoLatencyAndThroughput_SevenDayWindow_ReturnAtMostBudgetTimesSeriesRows() + { + var connectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(connectionString), + "Set DARLING_TEST_PG to a Postgres connection string to run the live File I/O budget-cap test."); + + using var connection = new NpgsqlConnection(connectionString); + await connection.OpenAsync(TestContext.Current.CancellationToken); + await PgMigrations.MigrateAsync(connection, TestContext.Current.CancellationToken); + await DeleteRowsAsync(connection, "file_io_stats", FileIoBudgetServerId, TestContext.Current.CancellationToken); + + await using var viewer = new ViewerDataService(connectionString!); + + var bodySucceeded = false; + try + { + var end = new DateTime(2026, 3, 10, 0, 0, 0); + var start = end.AddDays(-7); + var files = new[] { "f1", "f2" }; + var baseId = CollectionIdGenerator.Next() * 1_000_000L; + foreach (var file in files) + { + await BulkSeedFileIoAsync(connection, TestContext.Current.CancellationToken, baseId, FileIoBudgetServerId, start, end, "db1", file); + baseId += 20_000; + } + + var latency = await viewer.GetFileIoLatencyTrendAsync(FileIoBudgetServerId, start, end); + var throughput = await viewer.GetFileIoThroughputTrendAsync(FileIoBudgetServerId, start, end); + var budget = TrendBudget.Chart.AutoPoints * files.Length; + + Assert.True(latency.Count > 0 && latency.Count <= budget, $"latency: {latency.Count} rows over a {files.Length}-series budget of {budget}"); + Assert.True(throughput.Count > 0 && throughput.Count <= budget, $"throughput: {throughput.Count} rows over a {files.Length}-series budget of {budget}"); + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(connectionString!, bodySucceeded, async (cleanup, cleanupCt) => + await DeleteRowsAsync(cleanup, "file_io_stats", FileIoBudgetServerId, cleanupCt)); + } + } + + /// #4234 ruling item 3/6: when the budget covers every collection in the window, both File I/O + /// reads return the raw points unchanged, stamped at their own raw collection time — proven off the + /// minute grid (:37 seconds), which the pre-#4234 date_bin-always-floors code would have lost. + [Fact] + public async Task FileIoLatencyAndThroughput_BudgetCoversEveryCollection_ReturnsRawTimestampsUnchanged() + { + var connectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(connectionString), + "Set DARLING_TEST_PG to a Postgres connection string to run the live File I/O singleton-bucket test."); + + using var connection = new NpgsqlConnection(connectionString); + await connection.OpenAsync(TestContext.Current.CancellationToken); + await PgMigrations.MigrateAsync(connection, TestContext.Current.CancellationToken); + await DeleteRowsAsync(connection, "file_io_stats", FileIoSingletonServerId, TestContext.Current.CancellationToken); + + await using var viewer = new ViewerDataService(connectionString!); + + var bodySucceeded = false; + try + { + var baseTime = TruncateToSeconds(DateTime.UtcNow.AddMinutes(-30)); + var t1 = baseTime.AddSeconds(37 - baseTime.Second); + var t2 = t1.AddMinutes(5); + var t3 = t2.AddMinutes(5); + + foreach (var t in new[] { t1, t2, t3 }) + { + await InsertFileIoAsync(connection, FileIoSingletonServerId, t, "db1", "f1", + deltaReads: 10, deltaWrites: 4, deltaReadBytes: 1048576L * 60L, deltaWriteBytes: 1048576L * 60L, + deltaStallReadMs: 100, deltaStallWriteMs: 20, deltaStallQueuedReadMs: 0, deltaStallQueuedWriteMs: 0); + } + + var latency = await viewer.GetFileIoLatencyTrendAsync(FileIoSingletonServerId, t1.AddMinutes(-1), t3.AddMinutes(1)); + Assert.Equal(new[] { t1, t2, t3 }, latency.Select(p => p.CollectionTime).ToArray()); + Assert.All(latency, p => Assert.Equal(10.0, p.AvgReadLatencyMs, precision: 3)); + + /* Throughput drops the first collection (no prior LAG interval), leaving t2/t3. */ + var throughput = await viewer.GetFileIoThroughputTrendAsync(FileIoSingletonServerId, t1.AddMinutes(-1), t3.AddMinutes(1)); + Assert.Equal(new[] { t2, t3 }, throughput.Select(p => p.CollectionTime).ToArray()); + Assert.All(throughput, p => Assert.Equal(1.0, p.ReadMbPerSec, precision: 3)); + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(connectionString!, bodySucceeded, async (cleanup, cleanupCt) => + await DeleteRowsAsync(cleanup, "file_io_stats", FileIoSingletonServerId, cleanupCt)); + } + } + [Fact] public async Task FileIoLatency_PerFileAverage_WithQueuedOverlay_AgainstDevPostgres() { @@ -611,6 +732,31 @@ INSERT INTO file_io_stats await command.ExecuteNonQueryAsync(TestContext.Current.CancellationToken); } + /// One row per minute from to inclusive, + /// generated server-side (the real file_io_stats cadence) — the #4234 budget-cap live test's seed, + /// avoiding a round trip per minute over a 7-day window. Mirrors + /// ViewerTrendBucketingReviewTests.BulkSeedWaitAsync. + private static async Task BulkSeedFileIoAsync( + NpgsqlConnection connection, System.Threading.CancellationToken ct, long baseId, int serverId, + DateTime start, DateTime end, string databaseName, string fileName) + { + using var command = new NpgsqlCommand(@" +INSERT INTO file_io_stats + (collection_id, collection_time, server_id, server_name, + database_name, file_name, delta_reads, delta_writes, delta_read_bytes, delta_write_bytes, + delta_stall_read_ms, delta_stall_write_ms, delta_stall_queued_read_ms, delta_stall_queued_write_ms) +SELECT $1 + row_number() OVER (), g, $2, $3, $4, $5, 10, 4, 655360, 262144, 100, 20, 0, 0 +FROM generate_series($6::timestamp, $7::timestamp, interval '1 minute') AS g", connection); + command.Parameters.AddWithValue(baseId); + command.Parameters.AddWithValue(serverId); + command.Parameters.AddWithValue(ServerName); + command.Parameters.AddWithValue(databaseName); + command.Parameters.AddWithValue(fileName); + command.Parameters.AddWithValue(DateTime.SpecifyKind(start, DateTimeKind.Unspecified)); + command.Parameters.AddWithValue(DateTime.SpecifyKind(end, DateTimeKind.Unspecified)); + await command.ExecuteNonQueryAsync(ct); + } + private static async Task InsertBlockedProcessReportAsync( NpgsqlConnection connection, int serverId, DateTime collectionTimeUtc, DateTime eventTimeUtc) { diff --git a/Darling/Darling.Tests/ViewerMemoryTests.cs b/Darling/Darling.Tests/ViewerMemoryTests.cs index cd6167a7a0..271d99c636 100644 --- a/Darling/Darling.Tests/ViewerMemoryTests.cs +++ b/Darling/Darling.Tests/ViewerMemoryTests.cs @@ -13,6 +13,7 @@ using Npgsql; using NpgsqlTypes; using PerformanceMonitor.Collectors; +using PerformanceMonitor.Common; using PerformanceMonitor.Darling.Storage; using PerformanceMonitor.Darling.Viewer; using Xunit; @@ -102,8 +103,24 @@ public void MemoryClerkTrendsSql_BuildsDynamicInListFromFour_CastsMemoryToDouble Assert.Contains(inClause, sql, StringComparison.Ordinal); Assert.Contains(highestParam, sql, StringComparison.Ordinal); Assert.Contains("FROM v_memory_clerks", sql, StringComparison.Ordinal); - Assert.Contains("CAST(memory_mb AS double precision)", sql, StringComparison.Ordinal); - Assert.Contains("ORDER BY clerk_type, collection_time", sql, StringComparison.Ordinal); + /* #4234: a gauge — AVG per bucket, not the raw per-collection value. */ + Assert.Contains("CAST(AVG(memory_mb) AS double precision)", sql, StringComparison.Ordinal); + Assert.Contains("ORDER BY clerk_type, 2", sql, StringComparison.Ordinal); + } + + /// #4234 ruling item 6 (source check): the clerk trend carries a bucket width and projects the + /// singleton-detection columns, mirroring WaitTrendsSql. Proven once by hand against the pre-#4234 + /// text: it had no date_bin anywhere (a per-collection read has no bucket width), so this assert + /// fails there. + [Fact] + public void MemoryClerkTrendsSql_CarriesABucketWidth_AndProjectsSingletonDetectionColumns() + { + /* 1 clerk type = $4, so the width is the trailing $5. */ + var sql = ViewerDataService.MemoryClerkTrendsSql(1); + Assert.Contains("date_bin(CAST($5 AS integer) * INTERVAL '1 minute'", sql, StringComparison.Ordinal); + Assert.Contains(TrendBucketSql.OriginSql, sql, StringComparison.Ordinal); + Assert.Contains("MIN(collection_time) AS first_collection_time", sql, StringComparison.Ordinal); + Assert.Contains("COUNT(*) AS collection_count", sql, StringComparison.Ordinal); } [Fact] @@ -425,6 +442,152 @@ public sealed class ViewerMemoryLivePostgresTests private const int PressureServerId = -930005; private const string PressureServerName = "viewer-memory-pressure-e2e"; + private const int ClerkBudgetServerId = -930006; + private const int ClerkSingletonServerId = -930007; + private const int ClerkPickerServerId = -930008; + + /// #4234 ruling items 2/6: a 7-day window at the real 1-minute memory_clerks cadence used to + /// return every collection — now capped to 's point budget PER SERIES + /// (clerk type). Bulk-seeded server-side (generate_series) rather than one round trip per minute. + [Fact] + public async Task MemoryClerkTrend_SevenDayWindow_ReturnsAtMostBudgetTimesSeriesRows() + { + var connectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(connectionString), + "Set DARLING_TEST_PG to a Postgres connection string to run the live memory-clerk budget-cap test."); + + using var connection = new NpgsqlConnection(connectionString); + await connection.OpenAsync(TestContext.Current.CancellationToken); + await PgMigrations.MigrateAsync(connection, TestContext.Current.CancellationToken); + await DeleteRowsAsync(connection, "memory_clerks", ClerkBudgetServerId, TestContext.Current.CancellationToken); + + await using var viewer = new ViewerDataService(connectionString!); + + var bodySucceeded = false; + try + { + var end = new DateTime(2026, 3, 10, 0, 0, 0); + var start = end.AddDays(-7); + var clerkTypes = new List { "MEMORYCLERK_SQLBUFFERPOOL", "MEMORYCLERK_SQLGENERAL" }; + var baseId = CollectionIdGenerator.Next() * 1_000_000L; + foreach (var clerkType in clerkTypes) + { + await BulkSeedMemoryClerkAsync(connection, TestContext.Current.CancellationToken, baseId, start, end, clerkType); + baseId += 20_000; + } + + var trends = await viewer.GetMemoryClerkTrendsByTypesAsync(ClerkBudgetServerId, clerkTypes, start, 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}"); + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(connectionString!, bodySucceeded, async (cleanup, cleanupCt) => + await DeleteRowsAsync(cleanup, "memory_clerks", ClerkBudgetServerId, cleanupCt)); + } + } + + /// #4234 ruling item 3/6: when the budget covers every collection in the window, the clerk trend + /// returns the raw points unchanged, stamped at their own raw collection time — proven off the minute + /// grid (:37 seconds), which the pre-#4234 date_bin-always-floors code would have lost. + [Fact] + public async Task MemoryClerkTrend_BudgetCoversEveryCollection_ReturnsRawTimestampsAndAverageUnchanged() + { + var connectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(connectionString), + "Set DARLING_TEST_PG to a Postgres connection string to run the live memory-clerk singleton-bucket test."); + + using var connection = new NpgsqlConnection(connectionString); + await connection.OpenAsync(TestContext.Current.CancellationToken); + await PgMigrations.MigrateAsync(connection, TestContext.Current.CancellationToken); + await DeleteRowsAsync(connection, "memory_clerks", ClerkSingletonServerId, TestContext.Current.CancellationToken); + + await using var viewer = new ViewerDataService(connectionString!); + + var bodySucceeded = false; + try + { + var baseTime = TruncateToSeconds(DateTime.UtcNow.AddMinutes(-30)); + var t1 = baseTime.AddSeconds(37 - baseTime.Second); + var t2 = t1.AddMinutes(5); + var t3 = t2.AddMinutes(5); + + await InsertMemoryClerkAtServerAsync(connection, ClerkSingletonServerId, t1, "MEMORYCLERK_SQLBUFFERPOOL", 500.00m); + await InsertMemoryClerkAtServerAsync(connection, ClerkSingletonServerId, t2, "MEMORYCLERK_SQLBUFFERPOOL", 520.00m); + await InsertMemoryClerkAtServerAsync(connection, ClerkSingletonServerId, t3, "MEMORYCLERK_SQLBUFFERPOOL", 540.00m); + + var trends = await viewer.GetMemoryClerkTrendsByTypesAsync( + ClerkSingletonServerId, new List { "MEMORYCLERK_SQLBUFFERPOOL" }, t1.AddMinutes(-1), t3.AddMinutes(1)); + + var points = trends["MEMORYCLERK_SQLBUFFERPOOL"]; + Assert.Equal(new[] { t1, t2, t3 }, points.Select(p => p.CollectionTime).ToArray()); + Assert.Equal(new[] { 500.00, 520.00, 540.00 }, points.Select(p => p.MemoryMb).ToArray()); + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(connectionString!, bodySucceeded, async (cleanup, cleanupCt) => + await DeleteRowsAsync(cleanup, "memory_clerks", ClerkSingletonServerId, cleanupCt)); + } + } + + /// #4234 ruling items 4/6: the clerk picker (GetDistinctMemoryClerkTypesAsync) is cached + /// per (server, window length) for — a second call inside 15 minutes + /// returns the stale list without re-running the DISTINCT (proven by mutating the store between calls and + /// observing the new type is invisible until the cache is bypassed), and a window whose END has moved past + /// the TTL misses even though the wall clock hasn't. + [Fact] + public async Task MemoryClerkPicker_SecondCallWithinTtl_ReturnsStaleList_WindowEndPastTtl_Misses() + { + var connectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(connectionString), + "Set DARLING_TEST_PG to a Postgres connection string to run the live memory-clerk picker cache test."); + + using var connection = new NpgsqlConnection(connectionString); + await connection.OpenAsync(TestContext.Current.CancellationToken); + await PgMigrations.MigrateAsync(connection, TestContext.Current.CancellationToken); + await DeleteRowsAsync(connection, "memory_clerks", ClerkPickerServerId, TestContext.Current.CancellationToken); + + await using var viewer = new ViewerDataService(connectionString!); + + var bodySucceeded = false; + try + { + var now = new DateTime(2026, 3, 10, 12, 0, 0); + var start = now.AddHours(-1); + + await InsertMemoryClerkAtServerAsync(connection, ClerkPickerServerId, now.AddMinutes(-30), "MEMORYCLERK_SQLBUFFERPOOL", 500.00m); + + var first = await viewer.GetDistinctMemoryClerkTypesAsync(ClerkPickerServerId, start, now, now); + Assert.Equal(new[] { "MEMORYCLERK_SQLBUFFERPOOL" }, first); + + /* A new clerk type lands in the store, but a call 14 minutes later, same window end, still + answers from the cache — proof no DISTINCT ran. */ + await InsertMemoryClerkAtServerAsync(connection, ClerkPickerServerId, now.AddMinutes(-20), "MEMORYCLERK_XTP", 999.00m); + var stillCached = await viewer.GetDistinctMemoryClerkTypesAsync(ClerkPickerServerId, start, now, now.AddMinutes(14)); + Assert.Equal(new[] { "MEMORYCLERK_SQLBUFFERPOOL" }, stillCached); + + /* The window END moves forward past the TTL (a live auto-refresh), even though the wall clock + above hadn't reached 15 minutes yet on its own — this is a MISS, and the fresh DISTINCT sees + the new type. */ + var movedEnd = now.AddMinutes(20); + var afterWindowMoved = await viewer.GetDistinctMemoryClerkTypesAsync(ClerkPickerServerId, start.AddMinutes(20), movedEnd, now.AddMinutes(14)); + Assert.Equal(new[] { "MEMORYCLERK_XTP", "MEMORYCLERK_SQLBUFFERPOOL" }, afterWindowMoved); + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(connectionString!, bodySucceeded, async (cleanup, cleanupCt) => + await DeleteRowsAsync(cleanup, "memory_clerks", ClerkPickerServerId, cleanupCt)); + } + } + [Fact] public async Task LatestMemoryStats_ReturnsNewestSnapshot_MbAsDouble_StateStrings_AgainstDevPostgres() { @@ -749,6 +912,46 @@ INSERT INTO memory_clerks await command.ExecuteNonQueryAsync(TestContext.Current.CancellationToken); } + /// #4234 test helper: hardcodes ; + /// the budget/singleton/picker live tests each need their own server id so they cannot race each other's + /// row churn under the shared "live-postgres" collection. + private static async Task InsertMemoryClerkAtServerAsync( + NpgsqlConnection connection, int serverId, DateTime collectionTimeUtc, string clerkType, decimal memoryMb) + { + using var command = new NpgsqlCommand(@" +INSERT INTO memory_clerks + (collection_id, collection_time, server_id, server_name, clerk_type, memory_mb) +VALUES ($1, $2, $3, $4, $5, $6)", connection); + command.Parameters.AddWithValue(1L); + command.Parameters.AddWithValue(DateTime.SpecifyKind(collectionTimeUtc, DateTimeKind.Unspecified)); + command.Parameters.AddWithValue(serverId); + command.Parameters.AddWithValue(ClerkServerName); + command.Parameters.AddWithValue(clerkType); + command.Parameters.AddWithValue(memoryMb); + await command.ExecuteNonQueryAsync(TestContext.Current.CancellationToken); + } + + /// One row per minute from to inclusive, + /// generated server-side (the real memory_clerks cadence) — the #4234 budget-cap live test's seed, + /// avoiding a round trip per minute over a 7-day window. Mirrors + /// ViewerTrendBucketingReviewTests.BulkSeedWaitAsync. + private static async Task BulkSeedMemoryClerkAsync( + NpgsqlConnection connection, System.Threading.CancellationToken ct, long baseId, DateTime start, DateTime end, string clerkType) + { + using var command = new NpgsqlCommand(@" +INSERT INTO memory_clerks + (collection_id, collection_time, server_id, server_name, clerk_type, memory_mb) +SELECT $1 + row_number() OVER (), g, $2, $3, $4, 500.00 +FROM generate_series($5::timestamp, $6::timestamp, interval '1 minute') AS g", connection); + command.Parameters.AddWithValue(baseId); + command.Parameters.AddWithValue(ClerkBudgetServerId); + command.Parameters.AddWithValue(ClerkServerName); + command.Parameters.AddWithValue(clerkType); + command.Parameters.AddWithValue(DateTime.SpecifyKind(start, DateTimeKind.Unspecified)); + command.Parameters.AddWithValue(DateTime.SpecifyKind(end, DateTimeKind.Unspecified)); + await command.ExecuteNonQueryAsync(ct); + } + private static async Task InsertMemoryGrantAsync( NpgsqlConnection connection, int serverId, string serverName, DateTime collectionTimeUtc, int poolId, decimal availMb, decimal grantedMb, decimal usedMb, int grantee, int waiter, long timeoutDelta, long forcedDelta) From 565b7c7d9a91e7a80d089e96332b193474ea5c76 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:23:56 -0400 Subject: [PATCH 4/4] Fix a miscalibrated byte constant in the new FileIo throughput singleton test The singleton test's t1..t3 are 5 minutes apart, but the byte delta was calibrated for a 60-second interval (copied from the other throughput test, which is 60-second spaced) -- it read 0.2 MB/s instead of the intended 1.0. Caught by actually running the new test. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- Darling/Darling.Tests/ViewerFileIoBlockingTests.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Darling/Darling.Tests/ViewerFileIoBlockingTests.cs b/Darling/Darling.Tests/ViewerFileIoBlockingTests.cs index 39de323e59..0bbc78b41d 100644 --- a/Darling/Darling.Tests/ViewerFileIoBlockingTests.cs +++ b/Darling/Darling.Tests/ViewerFileIoBlockingTests.cs @@ -331,10 +331,13 @@ public async Task FileIoLatencyAndThroughput_BudgetCoversEveryCollection_Returns var t2 = t1.AddMinutes(5); var t3 = t2.AddMinutes(5); + /* t1..t3 are 5 minutes apart, so 1.0 MB/s needs bytes calibrated to a 300-second interval + (unlike the 60-second-spaced throughput test above). */ + const long oneMbPerSecOverFiveMinutes = 1048576L * 300L; foreach (var t in new[] { t1, t2, t3 }) { await InsertFileIoAsync(connection, FileIoSingletonServerId, t, "db1", "f1", - deltaReads: 10, deltaWrites: 4, deltaReadBytes: 1048576L * 60L, deltaWriteBytes: 1048576L * 60L, + deltaReads: 10, deltaWrites: 4, deltaReadBytes: oneMbPerSecOverFiveMinutes, deltaWriteBytes: oneMbPerSecOverFiveMinutes, deltaStallReadMs: 100, deltaStallWriteMs: 20, deltaStallQueuedReadMs: 0, deltaStallQueuedWriteMs: 0); }