From e8664d4c22d96df932b165ac6bf60b6aa67206eb Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:29:29 -0400 Subject: [PATCH 1/5] Add the closed-day range cache for the daily summary (#4232) DailySummaryRangeCache holds the closed-day portion of one daily-summary range read as one block for an hour; a refresh inside that hour re-runs the statement only over the still-open days (today, and yesterday during a two-hour post-midnight grace) and joins them to the block. Lives in Storage next to DailySummarySql so both the WPF viewer and the service can share it without seeing each other. Checked ruling item 7 first: no column DailySummarySql.RangeSqlFor returns depends on more than its own day (every CTE groups strictly within its own WHERE-bounded rows; no window function spans days), so there is no band or rank to protect from the cache. TRow is still the row from BEFORE any later banding/threshold/horizon stamp, so callers re-apply that judgment fresh after every read, cached or not. Unit pins (no database): cached-vs-fresh equality across a late row landing for yesterday inside the grace window, a second refresh reading only the open sub-range through the runRange seam, the one-hour block TTL and two-hour grace with a moved clock, an explicit end time skipping the cache every time, and a dedicated pin for the case where the grace boundary crosses mid-TTL (proved against the unguarded shape: without the closed-end boundary check, yesterday's row is silently dropped from the join). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- .../DailySummaryRangeCacheTests.cs | 190 ++++++++++++++++++ .../DailySummaryRangeCache.cs | 154 ++++++++++++++ 2 files changed, 344 insertions(+) create mode 100644 Darling/Darling.Tests/DailySummaryRangeCacheTests.cs create mode 100644 Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs diff --git a/Darling/Darling.Tests/DailySummaryRangeCacheTests.cs b/Darling/Darling.Tests/DailySummaryRangeCacheTests.cs new file mode 100644 index 0000000000..4032a98ef5 --- /dev/null +++ b/Darling/Darling.Tests/DailySummaryRangeCacheTests.cs @@ -0,0 +1,190 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using PerformanceMonitor.Darling.Storage; +using Xunit; + +namespace Darling.Tests; + +/// +/// #4232 ruling item 8: the closed-day range cache, pinned as pure logic against a stub runRange delegate +/// -- no database involved. stands in for a caller's per-day row (DailySummaryReadRow +/// / the viewer's raw parse); only Day matters to the cache itself. +/// +public sealed class DailySummaryRangeCacheTests +{ + private sealed record TestRow(DateTime Day, int Value); + + private const int ServerId = 1; + private const string RoutedSql = "SELECT 1 /* test */"; + + private static DateTime D(int day) => new(2026, 9, day, 0, 0, 0, DateTimeKind.Utc); + + [Fact] + public async Task GetRangeAsync_LateRowForYesterdayInsideGrace_CachedAnswerEqualsFreshFullStatement() + { + /* Clock at 01:00 UTC on the 25th: inside the two-hour grace, so the 24th (yesterday) is still open. */ + var now = new DateTime(2026, 9, 25, 1, 0, 0, DateTimeKind.Utc); + var cache = new DailySummaryRangeCache(() => now); + + /* The "true" store: mutable so a second read can see a late-arriving row for yesterday, exactly like a + collector landing a row a few minutes after midnight. */ + var store = new Dictionary + { + [D(1)] = 100, + [D(24)] = 1, // yesterday, before its late row lands + }; + + Task> Fresh(DateTime start, DateTime end, CancellationToken ct) => Task.FromResult( + store.Where(kv => kv.Key >= start && kv.Key < end) + .OrderBy(kv => kv.Key) + .Select(kv => new TestRow(kv.Key, kv.Value)) + .ToList()); + + var fromDate = D(1); + var toDate = D(26); // half-open range covering the 1st..25th + + var first = await cache.GetRangeAsync( + storeKey: "store", serverId: ServerId, fromDate: fromDate, toDate: toDate, routedSql: RoutedSql, + asOfNow: true, day: r => r.Day, runRange: Fresh, cancellationToken: CancellationToken.None); + Assert.Contains(first, r => r.Day == D(24) && r.Value == 1); + + /* The late row lands for yesterday. */ + store[D(24)] = 2; + + var second = await cache.GetRangeAsync( + storeKey: "store", serverId: ServerId, fromDate: fromDate, toDate: toDate, routedSql: RoutedSql, + asOfNow: true, day: r => r.Day, runRange: Fresh, cancellationToken: CancellationToken.None); + + var expected = await Fresh(fromDate, toDate, CancellationToken.None); + Assert.Equal(expected, second); + Assert.Contains(second, r => r.Day == D(24) && r.Value == 2); + } + + [Fact] + public async Task GetRangeAsync_SecondRefresh_RunsStatementOnlyOverOpenDays() + { + var now = new DateTime(2026, 9, 25, 12, 0, 0, DateTimeKind.Utc); // well past the grace: only the 25th is open + var cache = new DailySummaryRangeCache(() => now); + var calls = new List<(DateTime Start, DateTime End)>(); + + Task> Recording(DateTime start, DateTime end, CancellationToken ct) + { + calls.Add((start, end)); + return Task.FromResult(new List { new(start, 0) }); + } + + var fromDate = D(1); + var toDate = D(26); + + await cache.GetRangeAsync("store", ServerId, fromDate, toDate, RoutedSql, true, r => r.Day, Recording, CancellationToken.None); + await cache.GetRangeAsync("store", ServerId, fromDate, toDate, RoutedSql, true, r => r.Day, Recording, CancellationToken.None); + + Assert.Equal(2, calls.Count); + Assert.Equal((fromDate, toDate), calls[0]); // first refresh: the whole range + Assert.Equal((D(25), toDate), calls[1]); // second refresh: only the open day + } + + [Fact] + public async Task GetRangeAsync_WithinOneHour_ReusesBlock_AfterOneHour_RecomputesWhole() + { + var now = new DateTime(2026, 9, 25, 12, 0, 0, DateTimeKind.Utc); + var cache = new DailySummaryRangeCache(() => now); + var calls = new List<(DateTime Start, DateTime End)>(); + + Task> Recording(DateTime start, DateTime end, CancellationToken ct) + { + calls.Add((start, end)); + return Task.FromResult(new List { new(start, 0) }); + } + + var fromDate = D(1); + var toDate = D(26); + + await cache.GetRangeAsync("store", ServerId, fromDate, toDate, RoutedSql, true, r => r.Day, Recording, CancellationToken.None); + + now = now.AddMinutes(59); // still inside the one-hour block TTL + await cache.GetRangeAsync("store", ServerId, fromDate, toDate, RoutedSql, true, r => r.Day, Recording, CancellationToken.None); + Assert.Equal(2, calls.Count); + Assert.Equal((D(25), toDate), calls[1]); // open-only, block still trusted + + now = now.AddMinutes(2); // now 61 minutes after the block was built -- past BlockTtl + await cache.GetRangeAsync("store", ServerId, fromDate, toDate, RoutedSql, true, r => r.Day, Recording, CancellationToken.None); + Assert.Equal(3, calls.Count); + Assert.Equal((fromDate, toDate), calls[2]); // whole range recomputed + } + + [Fact] + public async Task GetRangeAsync_GraceBoundaryCrossesMidTtl_DoesNotDropYesterday() + { + /* Block built late in yesterday's grace window (01:45, grace ends 02:00), so the block's OWN + closed-end boundary excludes yesterday. Moved 25 minutes later (02:10): still inside the one-hour + BlockTtl from 01:45, but the grace has now expired, so a fresh call computes a DIFFERENT closed-end + boundary (today, not yesterday). A cache that trusted the stale block's ClosedRows here and only + re-read the (now-advanced) open portion would read neither the stale closed block's excluded + yesterday nor the open re-read's [today, end) -- yesterday would vanish from the join entirely. */ + var now = new DateTime(2026, 9, 25, 1, 45, 0, DateTimeKind.Utc); + var cache = new DailySummaryRangeCache(() => now); + + var store = new Dictionary { [D(23)] = 10, [D(24)] = 20, [D(25)] = 30 }; + Task> Fresh(DateTime start, DateTime end, CancellationToken ct) => Task.FromResult( + store.Where(kv => kv.Key >= start && kv.Key < end) + .OrderBy(kv => kv.Key) + .Select(kv => new TestRow(kv.Key, kv.Value)) + .ToList()); + + var fromDate = D(1); + var toDate = D(26); + + await cache.GetRangeAsync("store", ServerId, fromDate, toDate, RoutedSql, true, r => r.Day, Fresh, CancellationToken.None); + + now = now.AddMinutes(25); // 02:10 -- grace expired, still inside the one-hour block TTL + var second = await cache.GetRangeAsync("store", ServerId, fromDate, toDate, RoutedSql, true, r => r.Day, Fresh, CancellationToken.None); + + Assert.Contains(second, r => r.Day == D(24) && r.Value == 20); + } + + [Theory] + [InlineData(2026, 9, 25, 0, 0, 24)] // midnight exactly: still inside grace -> yesterday + [InlineData(2026, 9, 25, 1, 59, 24)] // just under two hours -> yesterday + [InlineData(2026, 9, 25, 2, 0, 25)] // exactly two hours -> today, grace just expired + [InlineData(2026, 9, 25, 12, 0, 25)] // mid-afternoon -> today + public void OpenStartUtc_TwoHourGraceAfterMidnight(int y, int m, int d, int h, int min, int expectedDay) + { + var now = new DateTime(y, m, d, h, min, 0, DateTimeKind.Utc); + Assert.Equal(D(expectedDay), DailySummaryRangeCache.OpenStartUtc(now)); + } + + [Fact] + public async Task GetRangeAsync_ExplicitEndTime_SkipsCacheEveryTime() + { + var now = new DateTime(2026, 9, 25, 12, 0, 0, DateTimeKind.Utc); + var cache = new DailySummaryRangeCache(() => now); + var calls = new List<(DateTime Start, DateTime End)>(); + + Task> Recording(DateTime start, DateTime end, CancellationToken ct) + { + calls.Add((start, end)); + return Task.FromResult(new List { new(start, 0) }); + } + + var fromDate = D(1); + var toDate = D(26); + + await cache.GetRangeAsync("store", ServerId, fromDate, toDate, RoutedSql, asOfNow: false, r => r.Day, Recording, CancellationToken.None); + await cache.GetRangeAsync("store", ServerId, fromDate, toDate, RoutedSql, asOfNow: false, r => r.Day, Recording, CancellationToken.None); + + Assert.Equal(2, calls.Count); + Assert.All(calls, call => Assert.Equal((fromDate, toDate), call)); // never narrowed -- no block ever built + } +} diff --git a/Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs b/Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs new file mode 100644 index 0000000000..b918a31ef6 --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs @@ -0,0 +1,154 @@ +/* + * 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; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; + +namespace PerformanceMonitor.Darling.Storage; + +/// +/// #4232: a reading-side cache for a range read. Both the WPF Daily Summary +/// calendar and the web/MCP get_daily_summary_range re-read a whole range of days on every poll even +/// though every day but today (and yesterday, briefly after midnight) is closed and its rows do not change. +/// This holds the CLOSED portion of one range as one block for one hour; every refresh inside that hour +/// re-runs the statement only over the days still open and joins them to the block. After an hour, one refresh +/// recomputes the whole range and repopulates it. No migration, no store job -- this lives entirely on the +/// reading side, next to so both its consumers (the viewer, which cannot see the +/// service, and the service) can share the shape. +/// +/// A day is closed two hours after it ends (ruling): a collector that missed its cadence by a +/// short outage still has time to land its row before the day is treated as settled. So the open portion is +/// always "today", plus "yesterday" during its first two hours. +/// +/// Callers own routing; this type never builds or chooses SQL. The caller resolves the tier-routed +/// statement text ONCE, for the range as a whole (exactly as it did before this cache existed), and its +/// runRange delegate (see ) must keep reading through that SAME text for +/// every sub-range this cache asks it to run, including the open-days-only re-read. A smaller [start, end) +/// resolved on its own could route to a different tier than the range it is part of -- a 30-day range old +/// enough to route its queries CTE to the hourly rollup, while a 1-day "yesterday" slice resolved fresh +/// would route to raw -- which would silently change what a day's unique_queries means between two rows +/// of the same read. The routed text also rides in the cache key: if the caller resolves a DIFFERENT statement +/// for the same server/range on a later call (a rollup materializes, coverage moves), the old block is not +/// reused. +/// +/// No band or rank lives here (#4232 ruling item 7, checked). Every column +/// DailySummarySql.RangeSqlFor returns is grouped strictly within its own day's rows -- nothing in the +/// statement compares one day to another, so there is no cross-range column to protect. should still be the RAW per-day row: callers must cache rows from BEFORE any later +/// banding/threshold/horizon judgment is stamped onto them, and re-apply that judgment to every row (cached or +/// fresh) after returns. A purge horizon or a deadlock-rate-threshold setting can +/// move inside the cache's one-hour lifetime, and a stale stamp baked into a cached closed day would be wrong +/// on its own terms, not just "a day compared to the range". should also be +/// immutable (a record or struct) -- the same row instances are handed to every caller within a block's +/// lifetime. +/// +public sealed class DailySummaryRangeCache +{ + /// How long one closed-days block is trusted before a refresh recomputes the whole range again. + public static readonly TimeSpan BlockTtl = TimeSpan.FromHours(1); + + /// How long after midnight UTC yesterday stays in the open (always-recomputed) set. + public static readonly TimeSpan ClosedGrace = TimeSpan.FromHours(2); + + private readonly Func _clock; + private readonly ConcurrentDictionary _blocks = new(); + + /// Defaults to the wall clock; tests pass one that can move (the same + /// Func<DateTime>? shape RdsLogSource already uses), so the one-hour and two-hour + /// boundaries can be pinned without a real sleep. + public DailySummaryRangeCache(Func? clock = null) + => _clock = clock ?? (() => DateTime.UtcNow); + + private readonly record struct BlockKey(object StoreKey, int ServerId, DateTime FromDate, DateTime ToDate, string RoutedSql); + + private sealed record CachedBlock(List ClosedRows, DateTime ClosedEndUtc, DateTime ComputedAtUtc); + + /// The UTC day at which the open (always-recomputed) portion of "now" begins: today, or yesterday + /// while inside of midnight. + internal static DateTime OpenStartUtc(DateTime nowUtc) + { + var today = nowUtc.Date; + return nowUtc < today.Add(ClosedGrace) ? today.AddDays(-1) : today; + } + + /// + /// Returns the rows for the half-open / UTC-day range, + /// using the closed-day cache when is true (#4232 ruling item 5 -- an explicit + /// end time is not "as of now" and always runs over the whole range, unmemoized). + /// + /// Distinguishes stores/connections sharing one cache instance (the service keeps ONE + /// cache for every store it serves). Reference-typed store handles (e.g. NpgsqlDataSource) compare by + /// identity here, which is exactly "same store"; a per-instance cache (the WPF viewer) can pass a constant. + /// The exact statement text the caller resolved for THIS range, reused for every + /// sub-range is asked to read (see the type doc) and folded into the cache key. + /// Reads a row's UTC day, to split a freshly-read whole range into its closed/open rows. + /// Runs the statement for an arbitrary half-open [start, end) sub-range and returns its + /// rows, oldest day first. Called with the WHOLE [, ) on a + /// cache miss, and with only the open sub-range on a hit. + public async Task> GetRangeAsync( + object storeKey, + int serverId, + DateTime fromDate, + DateTime toDate, + string routedSql, + bool asOfNow, + Func day, + Func>> runRange, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(day); + ArgumentNullException.ThrowIfNull(runRange); + + if (!asOfNow || fromDate >= toDate) + { + return await runRange(fromDate, toDate, cancellationToken).ConfigureAwait(false); + } + + var nowUtc = _clock(); + var openStartUtc = OpenStartUtc(nowUtc); + if (openStartUtc <= fromDate) + { + /* The whole requested window is still open (a short, recent range) -- nothing closed to cache. */ + return await runRange(fromDate, toDate, cancellationToken).ConfigureAwait(false); + } + + var closedEndUtc = openStartUtc < toDate ? openStartUtc : toDate; + var key = new BlockKey(storeKey, serverId, fromDate, toDate, routedSql); + + /* The block's OWN closed-end boundary must match what "now" computes THIS call, not just be within + BlockTtl -- otherwise a block built before yesterday's two-hour grace expired would still exclude + yesterday from ClosedRows, and a caller who only re-read [today, toDate) after the grace expired + would never read yesterday again: it is neither in the stale closed block nor in the open re-read. + Falling through to a full recompute below is what keeps that from being a silent gap. */ + if (_blocks.TryGetValue(key, out var cached) + && nowUtc - cached.ComputedAtUtc < BlockTtl + && cached.ClosedEndUtc == closedEndUtc) + { + if (closedEndUtc >= toDate) + { + return new List(cached.ClosedRows); + } + + var openFromUtc = fromDate > openStartUtc ? fromDate : openStartUtc; + var openRows = await runRange(openFromUtc, toDate, cancellationToken).ConfigureAwait(false); + var joined = new List(cached.ClosedRows.Count + openRows.Count); + joined.AddRange(cached.ClosedRows); + joined.AddRange(openRows); + return joined; + } + + var full = await runRange(fromDate, toDate, cancellationToken).ConfigureAwait(false); + var closedRows = full.Where(row => day(row) < closedEndUtc).ToList(); + _blocks[key] = new CachedBlock(closedRows, closedEndUtc, nowUtc); + return full; + } +} From a75cd1d18b2aa8a0077d46594f814f7c6735c54a Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:42:07 -0400 Subject: [PATCH 2/5] Wire the closed-day range cache into the service and WPF viewer (#4232) DarlingHealthReader.GetDailySummaryRangeAsync (get_daily_summary_range, shared by the web /api/read mirror and MCP clients) and ViewerDataService.ReadDailySummaryRangeAsync (the WPF Daily Summary calendar) now route their range reads through DailySummaryRangeCache instead of reading every day fresh on every refresh. Both resolve the tier-routed SQL text ONCE per call and reuse that same text for every sub-range the cache asks for (including the open-days-only re-read), so a 30-day range that routes to the hourly rollup cannot have its "today" slice quietly re-resolve to raw. Each reader splits its row read into a raw parse (what gets cached) and a banding/retention stamp (RateTiers/ReferenceUtc/DataState/ RetentionHorizon for the service, DataState/HealthBand for the viewer) applied fresh to every row, cached or not, on every call. The service keeps ONE static cache (DarlingHealthReader.RangeCache), keyed in part by the NpgsqlDataSource reference so distinct stores never collide. The viewer keeps a per-instance cache field, matching the existing GetRollupAvailabilityAsync pattern. get_daily_summary_range now passes asOfNow: as_of is null, so an explicit as_of always reads live per ruling item 5. Also: a pre-existing live test (DarlingDailySummaryRangeTests) shared one server ID between an empty-store check and a seeded-range check; the empty check now warms the cache's block for that server/range first, so the seeded read (same block, same hour) saw the stale empty block instead of the just-seeded rows. Split it onto its own server ID -- the "nothing ever collected" case does not need to share infrastructure with the gap-and-band case, and doing so was papering over two different stores as one. Added a new live pin that seeds a closed day and an open day, reads, seeds a second row on each, reads again, and asserts the closed day is unchanged (served from the cached block) while today picks up its new row (never cached). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- .../DarlingDailySummaryRangeTests.cs | 88 ++++++++++- .../Mcp/DarlingHealthReader.cs | 72 ++++++--- .../Mcp/DarlingMcpHealthTools.cs | 4 +- .../ViewerDataService.DailySummary.cs | 149 ++++++++++++++---- 4 files changed, 259 insertions(+), 54 deletions(-) diff --git a/Darling/Darling.Tests/DarlingDailySummaryRangeTests.cs b/Darling/Darling.Tests/DarlingDailySummaryRangeTests.cs index 56a8b19a43..68e1f5b4f7 100644 --- a/Darling/Darling.Tests/DarlingDailySummaryRangeTests.cs +++ b/Darling/Darling.Tests/DarlingDailySummaryRangeTests.cs @@ -46,6 +46,15 @@ public sealed class DarlingDailySummaryRangeTests private const int ServerId = -949578; private const string ServerName = "daily-summary-range"; + /// #4232: the "nothing ever collected" preamble below needs its OWN server — reading it through + /// would populate DarlingHealthReader.RangeCache's closed-day block for that + /// exact range from an EMPTY store, and the seeded read moments later (same range, same cache key, same + /// hour) would then see that stale empty block instead of the rows just seeded. That staleness is the + /// closed-day cache working as designed against a store that already had data — not a substitute for a + /// server nobody has ever collected from. + private const int NeverCollectedServerId = -949579; + private const string NeverCollectedServerName = "daily-summary-range-never-collected"; + private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); [Fact] @@ -67,13 +76,16 @@ public async Task TheCalendar_BandsEachDaySeparately_ShowsCollectionGaps_AndAnch try { await DarlingMcpTestData.RegisterServerAsync(connection, ServerId, ServerName, ct); + await DarlingMcpTestData.RegisterServerAsync(connection, NeverCollectedServerId, NeverCollectedServerName, ct); var today = DateTime.UtcNow.Date; var twoDaysAgo = today.AddDays(-2); var tenDaysAgo = today.AddDays(-10); - /* ── nothing collected at all: an empty calendar is a collection fault, not a quiet fortnight ── */ - var never = await DarlingMcpHealthTools.GetDailySummaryRange(dataSource, ServerName, 3); + /* ── nothing collected at all: an empty calendar is a collection fault, not a quiet fortnight ── + A dedicated server (see NeverCollectedServerId) so this empty read does not warm ServerId's + closed-day cache block from a store that has not been seeded yet. */ + var never = await DarlingMcpHealthTools.GetDailySummaryRange(dataSource, NeverCollectedServerName, 3); var neverDoc = JsonDocument.Parse(never); Assert.Equal("unavailable", neverDoc.RootElement.GetProperty("status").GetString()); var neverText = neverDoc.RootElement.GetProperty("message").GetString()!; @@ -192,7 +204,79 @@ INSERT INTO collection_log private static async Task DeleteRowsAsync(NpgsqlConnection connection, CancellationToken ct) { await DarlingMcpTestData.ExecAsync(connection, ct, "DELETE FROM collection_log WHERE server_id = $1", ServerId); + await DarlingMcpTestData.ExecAsync(connection, ct, "DELETE FROM wait_stats WHERE server_id = $1", ServerId); await DarlingMcpTestData.ExecAsync(connection, ct, "DELETE FROM servers WHERE server_id = $1", ServerId); await DarlingMcpTestData.ExecAsync(connection, ct, "DELETE FROM config_monitored_servers WHERE server_id = $1", ServerId); + await DarlingMcpTestData.ExecAsync(connection, ct, "DELETE FROM servers WHERE server_id = $1", NeverCollectedServerId); + await DarlingMcpTestData.ExecAsync(connection, ct, "DELETE FROM config_monitored_servers WHERE server_id = $1", NeverCollectedServerId); } + + /// + /// #4232: the closed-day cache, end to end against a real store. A closed day (two days ago) is seeded, + /// read once (populating the cache's block), then seeded with a SECOND wait row before a second read. If + /// the closed day were re-read from the store, the second read would see the new row; it must not — that + /// is exactly what "the closed portion is cached for an hour" means. Today (open) picks up its own second + /// row every time, because it is never in the cached block. + /// + [Fact] + public async Task GetDailySummaryRangeAsync_ClosedDayCache_SecondReadDoesNotSeeANewRowOnAClosedDay_ButDoesOnToday() + { + var cs = ConnectionString; + Assert.SkipWhen(string.IsNullOrEmpty(cs), + "Set DARLING_TEST_PG to a Postgres connection string to run the live daily-summary range cache test."); + + var ct = TestContext.Current.CancellationToken; + using var connection = new NpgsqlConnection(cs); + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + await DeleteRowsAsync(connection, ct); + + await using var dataSource = NpgsqlDataSource.Create(cs!); + var bodySucceeded = false; + + try + { + await DarlingMcpTestData.RegisterServerAsync(connection, ServerId, ServerName, ct); + + var today = DateTime.UtcNow.Date; + var twoDaysAgo = today.AddDays(-2); // well past the two-hour grace -- closed on every read below + var fromDate = today.AddDays(-3); + var toDate = today.AddDays(1); + + await PlantWaitAsync(connection, twoDaysAgo.AddHours(10), 1000, ct); + await PlantWaitAsync(connection, today.AddHours(1), 500, ct); + + var first = await DarlingHealthReader.GetDailySummaryRangeAsync(dataSource, ServerId, fromDate, toDate, cancellationToken: ct); + var firstClosedDay = first.Rows.Single(r => r.SummaryDate.Date == twoDaysAgo); + var firstToday = first.Rows.Single(r => r.SummaryDate.Date == today); + Assert.Equal(1000m, firstClosedDay.TotalWaitTimeSec * 1000m); // 1000 ms of wait, as seeded + Assert.Equal(500m, firstToday.TotalWaitTimeSec * 1000m); + + /* Land a second row on EACH day after the first read -- the closed day's cached block must not + see this; today, never cached, must. */ + await PlantWaitAsync(connection, twoDaysAgo.AddHours(11), 4000, ct); + await PlantWaitAsync(connection, today.AddHours(2), 2000, ct); + + var second = await DarlingHealthReader.GetDailySummaryRangeAsync(dataSource, ServerId, fromDate, toDate, cancellationToken: ct); + var secondClosedDay = second.Rows.Single(r => r.SummaryDate.Date == twoDaysAgo); + var secondToday = second.Rows.Single(r => r.SummaryDate.Date == today); + + Assert.Equal(1000m, secondClosedDay.TotalWaitTimeSec * 1000m); // unchanged: served from the cached block + Assert.Equal(2500m, secondToday.TotalWaitTimeSec * 1000m); // 500 + 2000: today is never cached + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(cs!, bodySucceeded, async (cleanup, cleanupCt) => + await DeleteRowsAsync(cleanup, cleanupCt)); + } + } + + private static async Task PlantWaitAsync(NpgsqlConnection connection, DateTime atUtc, long waitMs, CancellationToken ct) => + await DarlingMcpTestData.ExecAsync(connection, ct, @" +INSERT INTO wait_stats + (collection_id, collection_time, server_id, server_name, wait_type, delta_waiting_tasks, delta_wait_time_ms) +VALUES ($1, $2, $3, $4, $5, $6, $7)", + CollectionIdGenerator.Next(), DarlingMcpTestData.Naive(atUtc), ServerId, ServerName, "CXPACKET", 5L, waitMs); } diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs index 4565f177af..5efa2adb8f 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs @@ -390,9 +390,16 @@ private static async Task> ReadFleetRetentionOve /// #3541 A9: returns the rows AND the retention horizon they were judged against — see /// and the horizon note in the body. /// + /// #4232: ONE cache for every store this reader serves — get_daily_summary_range shares it + /// between the web viewer's /api/read mirror and MCP clients, because both paths call this same + /// method. Keyed inside by (store, server, range, routed + /// statement); has no value-equality override, so distinct stores — including + /// a live test's own ScratchPostgres database — never collide on one store's cached block. + internal static readonly DailySummaryRangeCache RangeCache = new(); + public static async Task GetDailySummaryRangeAsync( NpgsqlDataSource postgres, int serverId, DateTime fromDate, DateTime toDate, - DateTime? referenceUtc = null, CancellationToken cancellationToken = default) + DateTime? referenceUtc = null, bool asOfNow = true, CancellationToken cancellationToken = default) { /* #1664: gate the age decision on the rollups actually existing — a plain-PostgreSQL store has none (and never drops raw, so raw is complete there). #1759: and on what they have MATERIALIZED, which is @@ -434,29 +441,58 @@ horizon is a property of the store. DailySummaryRetention.HorizonFor documents t var shortestRetentionDays = ShortestSignalRetentionDays(fleetOverrides); var horizon = DailySummaryRetention.HorizonFor(DateTime.UtcNow, shortestRetentionDays); - var results = new List(); /* #3653 (Q12): tier over the legacy pair above; the hourly RELATION by the supply rule — the interval-honest successor where it reaches as far back as the legacy for this window. The viewer's - calendar makes the identical call, so the two still count the same queries for the same day. */ - await using var command = postgres.CreateCommand(DailySummarySql.RangeSqlFor(tier, coverage, fromDate)); - command.CommandTimeout = McpCommandDeadlines.ReadSeconds; - DarlingMcpReadParameters.AddInt(command, serverId); - DarlingMcpReadParameters.AddTimestamp(command, fromDate.Date); - DarlingMcpReadParameters.AddTimestamp(command, toDate.Date); - - await using var reader = await command.ExecuteReaderAsync(cancellationToken); - while (await reader.ReadAsync(cancellationToken)) + calendar makes the identical call, so the two still count the same queries for the same day. + #4232: resolved ONCE for the range as a whole and reused for every sub-range RunRangeAsync below is + asked to read (see DailySummaryRangeCache's own doc) — a sub-range resolved on its own could route to + a different tier than the range it is part of, which would silently change what a day's + unique_queries means between two rows of the same read. */ + var routedSql = DailySummarySql.RangeSqlFor(tier, coverage, fromDate); + + async Task> RunRangeAsync(DateTime start, DateTime end, CancellationToken ct) { - var row = ReadDailySummaryRow(reader); - results.Add(row with + var rows = new List(); + await using var command = postgres.CreateCommand(routedSql); + command.CommandTimeout = McpCommandDeadlines.ReadSeconds; + DarlingMcpReadParameters.AddInt(command, serverId); + DarlingMcpReadParameters.AddTimestamp(command, start.Date); + DarlingMcpReadParameters.AddTimestamp(command, end.Date); + + await using var reader = await command.ExecuteReaderAsync(ct); + while (await reader.ReadAsync(ct)) { - RateTiers = rateTiers, - ReferenceUtc = referenceUtc ?? DateTime.UtcNow, - DataState = DailySummaryRetention.StateFor(row.SummaryDate, row.CollectionRuns, row.SignalSourcesPresent, horizon), - RetentionHorizon = horizon, - }); + rows.Add(ReadDailySummaryRow(reader)); + } + + return rows; } + /* #4232: the closed-day cache. ReadDailySummaryRow above returns the row from BEFORE the RateTiers / + ReferenceUtc / DataState / RetentionHorizon stamp below — that raw row is what gets cached, and every + row (cached or freshly read) is re-stamped from THIS call's live thresholds and horizon, never from + whatever a cached block happened to compute them as up to an hour ago (ruling item 7: no column the + statement itself returns spans more than one day, but these post-read judgments do move between + refreshes, so they are re-applied after the join every time). */ + var rawResults = await RangeCache.GetRangeAsync( + storeKey: postgres, + serverId: serverId, + fromDate: fromDate, + toDate: toDate, + routedSql: routedSql, + asOfNow: asOfNow, + day: row => row.SummaryDate, + runRange: RunRangeAsync, + cancellationToken: cancellationToken); + + var results = rawResults.Select(row => row with + { + RateTiers = rateTiers, + ReferenceUtc = referenceUtc ?? DateTime.UtcNow, + DataState = DailySummaryRetention.StateFor(row.SummaryDate, row.CollectionRuns, row.SignalSourcesPresent, horizon), + RetentionHorizon = horizon, + }).ToList(); + return new DailySummaryRangeReadResult(results, horizon, shortestRetentionDays); } diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs index 130620e4d3..04ec1918fc 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs @@ -216,8 +216,10 @@ means the anchor day is the last one included rather than the first one excluded /* The anchor is also the clock the still-forming day's window clamps against (#3525 review): a backdated as_of must clamp its own "today" against ITSELF, not the process clock. */ + /* #4232 ruling item 5: only a read "as of now" (no as_of given) uses the closed-day cache — a + caller who pinned an explicit end time gets an unmemoized read of exactly that moment every time. */ var range = await DarlingHealthReader.GetDailySummaryRangeAsync( - postgres, resolved.ServerId, fromDate, toDate, referenceUtc: windowEnd); + postgres, resolved.ServerId, fromDate, toDate, referenceUtc: windowEnd, asOfNow: as_of is null); var rows = range.Rows; if (rows.Count == 0) diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.DailySummary.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.DailySummary.cs index 0e4b422c57..ef0b3e0637 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.DailySummary.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.DailySummary.cs @@ -9,6 +9,7 @@ using System; using System.Collections.Generic; using System.Data.Common; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Npgsql; @@ -48,6 +49,34 @@ public sealed partial class ViewerDataService /// . public static string DailySummaryRangeSqlFor(RetentionTier tier, RollupCoverage coverage, DateTime windowStartUtc) => DailySummarySql.RangeSqlFor(tier, coverage, windowStartUtc); + /// #4232: this viewer instance's own closed-day cache for the calendar's range read (ruling item 6 + /// — the WPF viewer keeps its own instance, distinct from the service's shared one). Benignly racy like + /// 's cache: concurrent tab loads may miss together and compute the + /// range twice, which is correct, just not maximally cache-efficient. + private readonly DailySummaryRangeCache _dailySummaryRangeCache = new(); + + /// #4232: one day's worth of 's raw output columns, from BEFORE any + /// banding or retention judgment — the unit actually caches. A record + /// with init-only properties (immutable), matching every row instance being shared across callers within + /// the cache's lifetime. + private sealed record RawDailySummaryDay + { + public DateTime Day { get; init; } + public decimal TotalWaitTimeSec { get; init; } + public string TopWaitType { get; init; } = ""; + public long? UniqueQueries { get; init; } + public long DeadlockCount { get; init; } + public long BlockingEvents { get; init; } + public long HighCpuEvents { get; init; } + public long CollectionErrors { get; init; } + public long MemoryPressureEvents { get; init; } + public long MemoryCriticalEvents { get; init; } + public long AlertCount { get; init; } + public long MaxBlockDurationMs { get; init; } + public long CollectionRuns { get; init; } + public int SignalSourcesPresent { get; init; } + } + /// /// Returns one per collected day in the half-open [fromDate, toDate) /// window. Powers the Performance Calendar month grid. @@ -99,20 +128,49 @@ Analysis assembly that owns it is one Storage does not reference. */ /* #3653 (Q12): tier over the legacy pair above; the hourly RELATION by the supply rule — the interval-honest successor where it reaches as far back as the legacy for this window, so the calendar - and get_daily_health (DarlingHealthReader, same call) count the same queries for the same day. */ - await using var command = _dataSource.CreateCommand(DailySummaryRangeSqlFor(tier, coverage, fromDate)); - command.CommandTimeout = ViewerCommandDeadlines.CurrentInteractiveReadSeconds; - command.Parameters.Add(new NpgsqlParameter { TypedValue = serverId }); - command.Parameters.Add(new NpgsqlParameter { TypedValue = DateTime.SpecifyKind(fromDate.Date, DateTimeKind.Unspecified) }); - command.Parameters.Add(new NpgsqlParameter { TypedValue = DateTime.SpecifyKind(toDate.Date, DateTimeKind.Unspecified) }); - - var results = new List(); - await using var reader = await command.ExecuteReaderAsync(cancellationToken); - while (await reader.ReadAsync(cancellationToken)) + and get_daily_health (DarlingHealthReader, same call) count the same queries for the same day. + #4232: resolved ONCE for the range as a whole and reused for every sub-range RunRangeAsync below is + asked to read (see DailySummaryRangeCache's own doc) — a sub-range resolved on its own could route to + a different tier than the range it is part of, which would silently change what a day's + unique_queries means between two rows of the same read. */ + var routedSql = DailySummaryRangeSqlFor(tier, coverage, fromDate); + + async Task> RunRangeAsync(DateTime start, DateTime end, CancellationToken ct) { - results.Add(ReadDailySummaryRow(reader, banding, horizon)); + var raws = new List(); + await using var command = _dataSource.CreateCommand(routedSql); + command.CommandTimeout = ViewerCommandDeadlines.CurrentInteractiveReadSeconds; + command.Parameters.Add(new NpgsqlParameter { TypedValue = serverId }); + command.Parameters.Add(new NpgsqlParameter { TypedValue = DateTime.SpecifyKind(start.Date, DateTimeKind.Unspecified) }); + command.Parameters.Add(new NpgsqlParameter { TypedValue = DateTime.SpecifyKind(end.Date, DateTimeKind.Unspecified) }); + + await using var reader = await command.ExecuteReaderAsync(ct); + while (await reader.ReadAsync(ct)) + { + raws.Add(ReadRawDailySummaryDay(reader)); + } + + return raws; } + /* #4232: the closed-day cache. The viewer's calendar reads are always live ("as of now"), so every + call is cache-eligible. ToDailySummaryRow re-reads the live banding/horizon on every row, cached or + fresh, every call — never from whatever a cached block happened to compute them as up to an hour ago + (ruling item 7: no column the statement itself returns spans more than one day, but this post-read + judgment does move between refreshes). */ + var rawResults = await _dailySummaryRangeCache.GetRangeAsync( + storeKey: string.Empty, + serverId: serverId, + fromDate: fromDate, + toDate: toDate, + routedSql: routedSql, + asOfNow: true, + day: raw => raw.Day, + runRange: RunRangeAsync, + cancellationToken: cancellationToken); + + var results = rawResults.Select(raw => ToDailySummaryRow(raw, banding, horizon)).ToList(); + return (results, horizon); } @@ -158,31 +216,56 @@ internal async Task ReadRetentionHorizonAsync(CancellationToken cancel return DailySummaryRetention.HorizonFor(DateTime.UtcNow, shortestRetentionDays); } - private static DailySummaryRow ReadDailySummaryRow(DbDataReader reader, DailyHealthThresholds banding, DateTime retentionHorizon) + /// #4232: the parse half of the old ReadDailySummaryRow, split out so the closed-day cache + /// () can hold rows from BEFORE any banding/retention judgment is + /// stamped onto them — see , which does the stamping, fresh, on every call. + private static RawDailySummaryDay ReadRawDailySummaryDay(DbDataReader reader) => new() + { + Day = reader.IsDBNull(0) ? DateTime.MinValue : Convert.ToDateTime(reader.GetValue(0)), + TotalWaitTimeSec = reader.IsDBNull(1) ? 0m : Convert.ToDecimal(reader.GetValue(1)), + TopWaitType = reader.IsDBNull(2) ? "" : reader.GetString(2), + /* #3653 A6: a NULL unique_queries is the routed statement's "not carried at this tier" (the rollup + never materialized this server's day while its source still holds the rows) and is KEPT as null + — the 0L this used to substitute drew a measured-zero on the day-detail panel for a day the tier + skipped. The MCP reader (DarlingHealthReader) makes the same read at the same ordinal. */ + UniqueQueries = reader.IsDBNull(3) ? null : Convert.ToInt64(reader.GetValue(3)), + DeadlockCount = reader.IsDBNull(4) ? 0L : Convert.ToInt64(reader.GetValue(4)), + BlockingEvents = reader.IsDBNull(5) ? 0L : Convert.ToInt64(reader.GetValue(5)), + HighCpuEvents = reader.IsDBNull(6) ? 0L : Convert.ToInt64(reader.GetValue(6)), + CollectionErrors = reader.IsDBNull(7) ? 0L : Convert.ToInt64(reader.GetValue(7)), + MemoryPressureEvents = reader.IsDBNull(8) ? 0L : Convert.ToInt64(reader.GetValue(8)), + MemoryCriticalEvents = reader.IsDBNull(9) ? 0L : Convert.ToInt64(reader.GetValue(9)), + AlertCount = reader.IsDBNull(10) ? 0L : Convert.ToInt64(reader.GetValue(10)), + MaxBlockDurationMs = reader.IsDBNull(11) ? 0L : Convert.ToInt64(reader.GetValue(11)), + /* #3539 A2: the trailing collection_runs column — the collection-error share's denominator. */ + CollectionRuns = reader.IsDBNull(12) ? 0L : Convert.ToInt64(reader.GetValue(12)), + /* #3541 A9 / #3653: the aggregate's LAST column, how many of the seven signal sources hold a row for + the day — the fact that tells a purged shell from a day the purge has not reached. */ + SignalSourcesPresent = reader.IsDBNull(13) ? 0 : Convert.ToInt32(reader.GetValue(13)), + }; + + /// #4232: the banding half of the old ReadDailySummaryRow — stamps a raw parsed day with the + /// per-range banding inputs. Called fresh for every row (cached or freshly read) on every range read, never + /// itself cached, so a settings save or a purge horizon crossing mid-cache-lifetime still repaints + /// correctly. + private static DailySummaryRow ToDailySummaryRow(RawDailySummaryDay raw, DailyHealthThresholds banding, DateTime retentionHorizon) { var row = new DailySummaryRow { - SummaryDate = reader.IsDBNull(0) ? DateTime.MinValue : Convert.ToDateTime(reader.GetValue(0)), - TotalWaitTimeSec = reader.IsDBNull(1) ? 0m : Convert.ToDecimal(reader.GetValue(1)), - TopWaitType = reader.IsDBNull(2) ? "" : reader.GetString(2), - /* #3653 A6: a NULL unique_queries is the routed statement's "not carried at this tier" (the rollup - never materialized this server's day while its source still holds the rows) and is KEPT as null - — the 0L this used to substitute drew a measured-zero on the day-detail panel for a day the tier - skipped. The MCP reader (DarlingHealthReader) makes the same read at the same ordinal. */ - UniqueQueries = reader.IsDBNull(3) ? null : Convert.ToInt64(reader.GetValue(3)), - DeadlockCount = reader.IsDBNull(4) ? 0L : Convert.ToInt64(reader.GetValue(4)), - BlockingEvents = reader.IsDBNull(5) ? 0L : Convert.ToInt64(reader.GetValue(5)), - HighCpuEvents = reader.IsDBNull(6) ? 0L : Convert.ToInt64(reader.GetValue(6)), - CollectionErrors = reader.IsDBNull(7) ? 0L : Convert.ToInt64(reader.GetValue(7)), - MemoryPressureEvents = reader.IsDBNull(8) ? 0L : Convert.ToInt64(reader.GetValue(8)), - MemoryCriticalEvents = reader.IsDBNull(9) ? 0L : Convert.ToInt64(reader.GetValue(9)), - AlertCount = reader.IsDBNull(10) ? 0L : Convert.ToInt64(reader.GetValue(10)), - MaxBlockDurationMs = reader.IsDBNull(11) ? 0L : Convert.ToInt64(reader.GetValue(11)), - /* #3539 A2: the trailing collection_runs column — the collection-error share's denominator. */ - CollectionRuns = reader.IsDBNull(12) ? 0L : Convert.ToInt64(reader.GetValue(12)), - /* #3541 A9 / #3653: the aggregate's LAST column, how many of the seven signal sources hold a row for - the day — the fact that tells a purged shell from a day the purge has not reached. */ - SignalSourcesPresent = reader.IsDBNull(13) ? 0 : Convert.ToInt32(reader.GetValue(13)), + SummaryDate = raw.Day, + TotalWaitTimeSec = raw.TotalWaitTimeSec, + TopWaitType = raw.TopWaitType, + UniqueQueries = raw.UniqueQueries, + DeadlockCount = raw.DeadlockCount, + BlockingEvents = raw.BlockingEvents, + HighCpuEvents = raw.HighCpuEvents, + CollectionErrors = raw.CollectionErrors, + MemoryPressureEvents = raw.MemoryPressureEvents, + MemoryCriticalEvents = raw.MemoryCriticalEvents, + AlertCount = raw.AlertCount, + MaxBlockDurationMs = raw.MaxBlockDurationMs, + CollectionRuns = raw.CollectionRuns, + SignalSourcesPresent = raw.SignalSourcesPresent, HasData = true, RetentionHorizon = retentionHorizon, }; From 8ce69505e60bc51bd4eb12b0fb5d034ac5aeb158 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 12:00:22 -0400 Subject: [PATCH 3/5] Add a test-only reset seam for the closed-day range cache (#4232) McpFilterSemanticsLivePostgresTests.DailySummary_StopsPaintingPurgedDaysGreen mutates a closed day (45 days old) between two range reads of the same server/range through the same store object, then asserts the second read reflects the mutation immediately. The #4232 cache serves that read from its one-hour block instead, by design (a real backfill shows up within the hour, not on the next statement) -- so the test was testing the cache's own contract, not a product bug. Add DailySummaryRangeCache.Clear() (public: Storage's InternalsVisibleTo reaches Darling.Tests only, not the service) and DarlingHealthReader.ResetRangeCacheForTests() (internal, visible to Darling.Tests through the service's own InternalsVisibleTo). Call it between each closed-day mutation and the re-read that expects to see it, in every test the census found doing this: - McpFilterSemanticsLivePostgresTests: the deadlock insert before the `survived` read, and the deadlock delete before the `shortened` read (same test, two separate mutate/reread pairs on the same ghost day). - DailySummaryNotCarriedTests.ASkippedDayBelowTheCeiling...: the repair pass doesn't touch the old server's rows, but the first read at the top of the test had already warmed that exact range's block, so the closing assertion was being served from cache and never reaching the store at all -- reset restores it as a real check. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- Darling/Darling.Tests/DailySummaryNotCarriedTests.cs | 6 ++++++ .../Darling.Tests/McpFilterSemanticsLivePostgresTests.cs | 8 ++++++++ .../Mcp/DarlingHealthReader.cs | 8 ++++++++ .../DailySummaryRangeCache.cs | 7 +++++++ 4 files changed, 29 insertions(+) diff --git a/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs b/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs index c1e3c517cf..79513fe3e8 100644 --- a/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs +++ b/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs @@ -429,6 +429,12 @@ than a band. What this leg pins is that the routed read behind it produced the r Assert.Equal(new long?[] { 3L, 5L, 7L, 2L }, repaired.Select(r => r.UniqueQueries).ToArray()); Assert.Equal(new[] { 1, 1, 1, 1 }, repaired.Select(r => r.SignalSourcesPresent).ToArray()); + /* #4232: the repair above never touches the old cluster (it is past the scan horizon), so this + assertion is not chasing a mutation -- but the FIRST read of this exact (postgres, OldServerId, + O(0), O(5)) range at the top of the test already warmed the closed-day block, and without a reset + this read would be served from that block and never reach the store at all. Reset so the read + actually re-queries and the assertion still means what it says: the old cluster still stands. */ + DarlingHealthReader.ResetRangeCacheForTests(); var oldAfter = await DarlingHealthReader.GetDailySummaryRangeAsync(postgres, OldServerId, O(0), O(5), cancellationToken: ct); Assert.Equal(new[] { O(2) }, oldAfter.DaysMissing); } diff --git a/Darling/Darling.Tests/McpFilterSemanticsLivePostgresTests.cs b/Darling/Darling.Tests/McpFilterSemanticsLivePostgresTests.cs index 9200e04cf9..1857661c7a 100644 --- a/Darling/Darling.Tests/McpFilterSemanticsLivePostgresTests.cs +++ b/Darling/Darling.Tests/McpFilterSemanticsLivePostgresTests.cs @@ -363,6 +363,10 @@ await DarlingMcpTestData.ExecAsync(connection, ct, VALUES ($1,$2,$3,$4,$5,$6,$7,$8)", CollectionIdGenerator.Next(), DarlingMcpTestData.Naive(ghostDay.AddHours(6)), ServerId, ServerName, DarlingMcpTestData.Naive(ghostDay.AddHours(6)), "process1", "DELETE FROM Posts", ""); + /* #4232: ghostDay is 45 days closed, well outside the cache's two-hour grace, so without this the + range read below would serve the block the FIRST read (above) warmed and never see this insert + -- a real backfill lands within the cache's one-hour contract, not this test's next statement. */ + DarlingHealthReader.ResetRangeCacheForTests(); var survived = JsonDocument.Parse(await DarlingMcpHealthTools.GetDailySummaryRange(postgres, ServerName, 60)).RootElement; Assert.Equal(1, survived.GetProperty("days_before_horizon").GetInt32()); Assert.Equal(0, survived.GetProperty("purged_day_count").GetInt32()); @@ -381,6 +385,10 @@ await DarlingMcpTestData.ExecAsync(connection, ct, collector to 10 days moves it, and the 20-day-old run becomes a ghost too. */ await DarlingMcpTestData.ExecAsync(connection, ct, "INSERT INTO config_collector_schedules (server_id, collector_name, retention_days, enabled) VALUES (NULL, 'deadlocks', 10, TRUE)"); + /* #4232: the DELETE above mutated ghostDay's rows again (the same closed day the reset before + `survived` already covered once) -- reset again so this read reflects it rather than the block + `survived` warmed. */ + DarlingHealthReader.ResetRangeCacheForTests(); var shortened = JsonDocument.Parse(await DarlingMcpHealthTools.GetDailySummaryRange(postgres, ServerName, 60)).RootElement; Assert.Equal(DailySummaryRetention.HorizonFor(DateTime.UtcNow, 10).ToString("yyyy-MM-dd"), shortened.GetProperty("retention_horizon").GetString()); Assert.Equal(2, shortened.GetProperty("days_before_horizon").GetInt32()); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs index 5efa2adb8f..f240e547d9 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs @@ -397,6 +397,14 @@ private static async Task> ReadFleetRetentionOve /// a live test's own ScratchPostgres database — never collide on one store's cached block. internal static readonly DailySummaryRangeCache RangeCache = new(); + /// #4232: test-only reset for . This cache is by design one hour stale for + /// a closed day -- a row that lands late for a closed day (an outage catch-up, a backfill) shows within the + /// hour, not immediately (the #4232 ruling). A live test that mutates a closed day's rows and reads the same + /// server/range again through the same store object is testing THAT contract, not a product bug, unless it + /// calls this between the mutation and the re-read to force the re-read to hit the store. Never call this + /// from product code -- only Darling.Tests can even see it (Service's own InternalsVisibleTo). + internal static void ResetRangeCacheForTests() => RangeCache.Clear(); + public static async Task GetDailySummaryRangeAsync( NpgsqlDataSource postgres, int serverId, DateTime fromDate, DateTime toDate, DateTime? referenceUtc = null, bool asOfNow = true, CancellationToken cancellationToken = default) diff --git a/Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs b/Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs index b918a31ef6..47705d5a26 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs @@ -151,4 +151,11 @@ Falling through to a full recompute below is what keeps that from being a silent _blocks[key] = new CachedBlock(closedRows, closedEndUtc, nowUtc); return full; } + + /// #4232: drops every cached block, for a test that needs its next read to hit the store rather than + /// serve a block a prior call in the same process warmed. Public rather than internal -- this type's + /// assembly (PerformanceMonitor.Darling.Storage) does not grant InternalsVisibleTo to the + /// service, only to Darling.Tests, so a caller reaching this through the service's own reset seam + /// (DarlingHealthReader.ResetRangeCacheForTests) needs a public member to call. + public void Clear() => _blocks.Clear(); } From 8ca50b9e4f0a0085659761de7a458b82049dc549 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 12:23:39 -0400 Subject: [PATCH 4/5] Fix a pre-existing stacked-doc-comment defect on DarlingHealthReader (#4232) The full Darling.Tests.exe run flagged DocCommentHygieneTests.NoMember CarriesTwoStackedSummaryBlocks: GetDailySummaryRangeAsync's own summary and RangeCache's summary sat back to back with no blank line between them, so both attached to RangeCache as trivia. Predates this branch's cache work (present in a75cd1d1, before this lane's changes); reordered so each member carries exactly one summary block, keeping the new ResetRangeCacheForTests doc in place. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- .../Mcp/DarlingHealthReader.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs index f240e547d9..7c1145d2d0 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs @@ -380,16 +380,6 @@ private static async Task> ReadFleetRetentionOve return overrides; } - /// One per collected day in the half-open [fromDate, toDate) - /// window (the viewer's GetDailySummaryRangeAsync). - /// - /// #1661: routes to the same retention tier the viewer's calendar does. This matters beyond - /// correctness — the calendar and this MCP tool answer the same question, so if only one routed they would - /// report different query counts for the same day and there would be no way to tell which was right. - /// - /// #3541 A9: returns the rows AND the retention horizon they were judged against — see - /// and the horizon note in the body. - /// /// #4232: ONE cache for every store this reader serves — get_daily_summary_range shares it /// between the web viewer's /api/read mirror and MCP clients, because both paths call this same /// method. Keyed inside by (store, server, range, routed @@ -405,6 +395,16 @@ private static async Task> ReadFleetRetentionOve /// from product code -- only Darling.Tests can even see it (Service's own InternalsVisibleTo). internal static void ResetRangeCacheForTests() => RangeCache.Clear(); + /// One per collected day in the half-open [fromDate, toDate) + /// window (the viewer's GetDailySummaryRangeAsync). + /// + /// #1661: routes to the same retention tier the viewer's calendar does. This matters beyond + /// correctness — the calendar and this MCP tool answer the same question, so if only one routed they would + /// report different query counts for the same day and there would be no way to tell which was right. + /// + /// #3541 A9: returns the rows AND the retention horizon they were judged against — see + /// and the horizon note in the body. + /// public static async Task GetDailySummaryRangeAsync( NpgsqlDataSource postgres, int serverId, DateTime fromDate, DateTime toDate, DateTime? referenceUtc = null, bool asOfNow = true, CancellationToken cancellationToken = default) From d012ac8c1cd62ce9569ac2c04c66bede122759b1 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 25 Sep 2026 12:49:06 -0400 Subject: [PATCH 5/5] Bound the closed-day range cache (#4232) DailySummaryRangeCache never removed a block: the key carries the range, so every days_back value, every server and every day added an entry nothing replaced. Trim expired blocks (past BlockTtl) and cap the total at MaxBlocks (1024), evicting the oldest first, on every cache miss. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3 --- .../DailySummaryRangeCacheTests.cs | 77 +++++++++++++++++++ .../DailySummaryRangeCache.cs | 35 ++++++++- 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/Darling/Darling.Tests/DailySummaryRangeCacheTests.cs b/Darling/Darling.Tests/DailySummaryRangeCacheTests.cs index 4032a98ef5..7fc50a1eb6 100644 --- a/Darling/Darling.Tests/DailySummaryRangeCacheTests.cs +++ b/Darling/Darling.Tests/DailySummaryRangeCacheTests.cs @@ -165,6 +165,83 @@ public void OpenStartUtc_TwoHourGraceAfterMidnight(int y, int m, int d, int h, i Assert.Equal(D(expectedDay), DailySummaryRangeCache.OpenStartUtc(now)); } + [Fact] + public async Task GetRangeAsync_ExpiredBlock_EvictedOnNextMissForAnotherKey_AndOldKeyMissesAgain() + { + /* #4232 follow-up: the cache never removed a block on its own -- every distinct server/range added one + that nothing replaced. Here two different servers share a range, so each gets its own block, and + moving the clock past BlockTtl before the second server's first (miss) call must sweep server 1's + now-stale block, not just leave it to rot. */ + var now = new DateTime(2026, 9, 25, 12, 0, 0, DateTimeKind.Utc); + var cache = new DailySummaryRangeCache(() => now); + var calls = new List<(int ServerId, DateTime Start, DateTime End)>(); + + Task> Recording(int serverId, DateTime start, DateTime end, CancellationToken ct) + { + calls.Add((serverId, start, end)); + return Task.FromResult(new List { new(start, 0) }); + } + + var fromDate = D(1); + var toDate = D(26); + + await cache.GetRangeAsync("store", 1, fromDate, toDate, RoutedSql, true, r => r.Day, + (s, e, ct) => Recording(1, s, e, ct), CancellationToken.None); + Assert.Equal(1, cache.Count); + + now = now.Add(DailySummaryRangeCache.BlockTtl).AddMinutes(1); // past BlockTtl for server 1's block + + await cache.GetRangeAsync("store", 2, fromDate, toDate, RoutedSql, true, r => r.Day, + (s, e, ct) => Recording(2, s, e, ct), CancellationToken.None); + Assert.Equal(1, cache.Count); // server 1's expired block swept, server 2's fresh block is the only one left + + var callsBeforeReread = calls.Count; + await cache.GetRangeAsync("store", 1, fromDate, toDate, RoutedSql, true, r => r.Day, + (s, e, ct) => Recording(1, s, e, ct), CancellationToken.None); + Assert.Equal(callsBeforeReread + 1, calls.Count); // server 1's block is gone -- this is a miss, not a hit + Assert.Equal((1, fromDate, toDate), calls[^1]); // whole range re-run, exactly like the very first read + } + + [Fact] + public async Task GetRangeAsync_MoreThanMaxBlocksDistinctKeys_CountStaysCapped_OldestKeysDropped() + { + /* #4232 follow-up: the key carries the range, so a fleet with many servers (or one MCP client stepping + through days_back values) can add a block per distinct key forever. Filling MaxBlocks + 10 of them, + each a fresh key, must cap Count at MaxBlocks by dropping the OLDEST blocks first, not the newest. */ + var now = new DateTime(2026, 9, 25, 12, 0, 0, DateTimeKind.Utc); + var cache = new DailySummaryRangeCache(() => now); + + var fromDate = D(1); + var toDate = D(26); + var total = DailySummaryRangeCache.MaxBlocks + 10; + + Task> Fresh(DateTime start, DateTime end, CancellationToken ct) + => Task.FromResult(new List { new(start, 0) }); + + for (var serverId = 1; serverId <= total; serverId++) + { + await cache.GetRangeAsync("store", serverId, fromDate, toDate, RoutedSql, true, r => r.Day, Fresh, CancellationToken.None); + now = now.AddSeconds(1); // distinct ComputedAtUtc per block, well inside BlockTtl for the whole loop + } + + Assert.Equal(DailySummaryRangeCache.MaxBlocks, cache.Count); + + /* The oldest 10 keys (server 1..10) were evicted to make room for the newest ones. A read for the + very first server is therefore a miss (runRange called again over the whole range). */ + var calls = new List<(DateTime Start, DateTime End)>(); + Task> Recording(DateTime start, DateTime end, CancellationToken ct) + { + calls.Add((start, end)); + return Task.FromResult(new List { new(start, 0) }); + } + + await cache.GetRangeAsync("store", 1, fromDate, toDate, RoutedSql, true, r => r.Day, Recording, CancellationToken.None); + Assert.Single(calls); + Assert.Equal((fromDate, toDate), calls[0]); // server 1's original block is gone -- whole range re-run + + Assert.True(cache.Count <= DailySummaryRangeCache.MaxBlocks); // the re-add still respects the cap + } + [Fact] public async Task GetRangeAsync_ExplicitEndTime_SkipsCacheEveryTime() { diff --git a/Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs b/Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs index 47705d5a26..1de78c469a 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DailySummaryRangeCache.cs @@ -21,7 +21,10 @@ namespace PerformanceMonitor.Darling.Storage; /// though every day but today (and yesterday, briefly after midnight) is closed and its rows do not change. /// This holds the CLOSED portion of one range as one block for one hour; every refresh inside that hour /// re-runs the statement only over the days still open and joins them to the block. After an hour, one refresh -/// recomputes the whole range and repopulates it. No migration, no store job -- this lives entirely on the +/// recomputes the whole range and repopulates it. Blocks older than are dropped on the +/// next miss, and the cache never holds more than blocks at once, evicting the oldest +/// first (#4232 follow-up: the key carries the range, so every distinct server/range/day combination otherwise +/// added a block that nothing ever removed). No migration, no store job -- this lives entirely on the /// reading side, next to so both its consumers (the viewer, which cannot see the /// service, and the service) can share the shape. /// @@ -59,9 +62,18 @@ public sealed class DailySummaryRangeCache /// How long after midnight UTC yesterday stays in the open (always-recomputed) set. public static readonly TimeSpan ClosedGrace = TimeSpan.FromHours(2); + /// The most blocks this cache holds at once. The key carries the store, server, range and routed + /// SQL, so an unbounded cache grows with every distinct combination a caller ever asks for; the fleet case + /// is one or two ranges per server, and a block holds at most 366 rows, so this cap is generous headroom, + /// not a tuned limit. + public const int MaxBlocks = 1024; + private readonly Func _clock; private readonly ConcurrentDictionary _blocks = new(); + /// The number of blocks currently cached, for tests. + internal int Count => _blocks.Count; + /// Defaults to the wall clock; tests pass one that can move (the same /// Func<DateTime>? shape RdsLogSource already uses), so the one-hour and two-hour /// boundaries can be pinned without a real sleep. @@ -148,6 +160,27 @@ Falling through to a full recompute below is what keeps that from being a silent var full = await runRange(fromDate, toDate, cancellationToken).ConfigureAwait(false); var closedRows = full.Where(row => day(row) < closedEndUtc).ToList(); + + /* #4232 follow-up: the key carries the range, so nothing ever replaces a block for a range nobody asks + for again (a one-off days_back value, a server that's gone). Trim on every miss rather than on a + timer -- this cache has no background thread, and a miss is exactly when growth happens. */ + foreach (var entry in _blocks) + { + if (nowUtc - entry.Value.ComputedAtUtc >= BlockTtl) + { + _blocks.TryRemove(entry); // KeyValuePair overload: never removes a block another caller just replaced + } + } + + while (_blocks.Count >= MaxBlocks && !_blocks.IsEmpty) + { + var oldest = _blocks.MinBy(entry => entry.Value.ComputedAtUtc); + if (!_blocks.TryRemove(oldest)) + { + break; // another caller changed it; the next miss trims again + } + } + _blocks[key] = new CachedBlock(closedRows, closedEndUtc, nowUtc); return full; }