From 94acd426f5aa1cf6f1d267554d8ade3052b2a05e Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 22 Sep 2026 19:30:49 -0400 Subject: [PATCH 1/3] The daily summary probes each day once, counts distinct queries by hashing, and reads rollup coverage from the shared cache (#3905) The month read's cost was the routed queries CTE, not the raw CTEs. Its not-carried probes (#3653 A6) were planned as a semi-join and an anti-join over the server's whole history in the probed relation, rescanned once per day and spilled past work_mem. Each probe now carries an OFFSET 0 fence, so it runs as a per-day index probe, and the source is probed only on the days the rollup probe found empty. Every tier counts distinct (day, hash) pairs instead of COUNT(DISTINCT), which PostgreSQL can hash instead of sort. Results are identical: a live test compares every tier against the old statement row for row. DarlingHealthReader reads rollup coverage through ComposeStoreAvailability instead of probing on every call. That cache is now single-flight, so concurrent readers share one probe. The web server page's Overview tab draws today's tile from the range read's anchor-day row: one fetch for both panels. Rig (one busy server): month 3,278 -> 186 ms, 120-day 12,359 -> 161 ms, single day 146 -> 45 ms. DARLING01 busiest server: month 729 -> 253 ms. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018Z4T51J4E6L8fjriPHpsif --- .../DailySummaryNotCarriedTests.cs | 35 +- .../DailySummaryReadShapeTests.cs | 615 ++++++++++++++++++ .../DarlingMcpHealthToolsTests.cs | 6 +- .../IntervalHonestHourlyRollupTests.cs | 4 +- .../Darling.Tests/RetentionTierRouterTests.cs | 10 +- Darling/Darling.Tests/ServerPageTabsTests.cs | 5 +- .../Compose/ComposeStoreAvailability.cs | 118 +++- .../Mcp/DarlingHealthReader.cs | 18 +- .../Mcp/DarlingMcpHealthTools.cs | 23 +- .../wwwroot/js/pages/server-tabs.js | 58 +- .../DailySummarySql.cs | 118 +++- 11 files changed, 912 insertions(+), 98 deletions(-) create mode 100644 Darling/Darling.Tests/DailySummaryReadShapeTests.cs diff --git a/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs b/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs index 4b527a0740..6d097b1c2f 100644 --- a/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs +++ b/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs @@ -92,25 +92,37 @@ public void EveryRoutedForm_AsksTheHoleScansTwoProbes_PerServer_AtDayGrain() var filter = TimescaleSupport.MaterializationHoleSourceFilterFor(target.CreateSql); Assert.Contains("SELECT b.d, NULL::bigint AS c", sql, StringComparison.Ordinal); - Assert.Contains("FROM generate_series(date_trunc('day', $2::timestamp), date_trunc('day', $3::timestamp), INTERVAL '1 day') AS b(d)", sql, StringComparison.Ordinal); - Assert.Contains("WHERE b.d < $3", sql, StringComparison.Ordinal); - Assert.Contains("AND b.d < COALESCE((SELECT last_day + INTERVAL '1 day' FROM queries_ceiling), $2)", sql, StringComparison.Ordinal); + Assert.Contains("FROM generate_series(date_trunc('day', $2::timestamp), date_trunc('day', $3::timestamp), INTERVAL '1 day') AS g(d)", sql, StringComparison.Ordinal); + Assert.Contains("WHERE g.d < $3", sql, StringComparison.Ordinal); + Assert.Contains("AND g.d < COALESCE((SELECT last_day + INTERVAL '1 day' FROM queries_ceiling), $2)", sql, StringComparison.Ordinal); /* Probe one: no rollup row for THIS server that day. */ var lf = sql.Replace("\r\n", "\n", StringComparison.Ordinal); Assert.Contains( - $"NOT EXISTS (\n SELECT 1 FROM collect.{relation} AS r\n WHERE r.server_id = $1 AND r.bucket >= b.d AND r.bucket < b.d + INTERVAL '1 day')", + $"NOT EXISTS (\n SELECT 1 FROM collect.{relation} AS r\n WHERE r.server_id = $1 AND r.bucket >= g.d AND r.bucket < g.d + INTERVAL '1 day'\n OFFSET 0)", lf, StringComparison.Ordinal); /* Probe two: an admitted source row for THIS server that day — the source and its time column are the repair's, and the filter is the aggregate's own WHERE, verbatim. */ var sourceProbe = $"SELECT 1 FROM collect.{target.Source} AS s\n WHERE s.server_id = $1 AND s.{target.SourceTimeColumn} >= b.d AND s.{target.SourceTimeColumn} < b.d + INTERVAL '1 day'" - + (filter.Length == 0 ? ")" : "\n AND " + filter + ")"); + + (filter.Length == 0 ? "\n OFFSET 0)" : "\n AND " + filter + "\n OFFSET 0)"); Assert.Contains(sourceProbe, lf, StringComparison.Ordinal); + /* #3905: the fences. Each probe carries OFFSET 0, which PostgreSQL refuses to pull up into a join, so + each stays a per-day SubPlan; the candidate days are a fenced subquery holding probe one, and probe + two filters its output, so the source is probed only on the days the rollup probe found empty. + DailySummaryProbePlanLiveTests proves the resulting plan on a store. */ + var member = lf[lf.IndexOf("SELECT b.d, NULL::bigint AS c", StringComparison.Ordinal)..]; + Assert.Equal(3, member.Split("OFFSET 0").Length - 1); + var rollupProbeAt = member.IndexOf($"SELECT 1 FROM collect.{relation} AS r", StringComparison.Ordinal); + var fenceAt = member.IndexOf(" OFFSET 0\n ) AS b\n WHERE EXISTS (", StringComparison.Ordinal); + var sourceProbeAt = member.IndexOf($"SELECT 1 FROM collect.{target.Source} AS s", StringComparison.Ordinal); + Assert.True(rollupProbeAt > 0 && fenceAt > rollupProbeAt && sourceProbeAt > fenceAt, + "probe one sits inside the fenced candidate-day subquery and probe two filters its output"); + /* The three members, in order: rollup half, raw tail, not-carried. */ Assert.Equal(2, sql.Split("UNION ALL").Length - 1); - var rollupAt = sql.IndexOf("SELECT date_trunc('day', bucket) AS d, COUNT(DISTINCT query_hash) AS c", StringComparison.Ordinal); + var rollupAt = sql.IndexOf("SELECT DISTINCT date_trunc('day', bucket) AS d, query_hash", StringComparison.Ordinal); var rawAt = sql.IndexOf("FROM v_query_stats", StringComparison.Ordinal); var notCarriedAt = sql.IndexOf("SELECT b.d, NULL::bigint AS c", StringComparison.Ordinal); Assert.True(rollupAt > 0 && rawAt > rollupAt && notCarriedAt > rawAt); @@ -150,8 +162,11 @@ public void ABucketWithZeroDistinctHashes_CannotExist_SoNullNeedsNoFlag() Assert.Contains("GROUP BY server_id, server_name, database_name, query_hash", create, StringComparison.Ordinal); } - /* And the rollup half counts that hash, so a rollup row's count is >= 1 by construction. */ - Assert.Contains("SELECT date_trunc('day', bucket) AS d, COUNT(DISTINCT query_hash) AS c", DailySummarySql.RangeSqlFor(RetentionTier.Daily), StringComparison.Ordinal); + /* And the rollup half counts that hash, so a rollup row's count is >= 1 by construction: the distinct + (day, hash) pairs of the rollup's rows, counted per day (#3905's hashable spelling of the same count). */ + var daily = DailySummarySql.RangeSqlFor(RetentionTier.Daily); + Assert.Contains("SELECT DISTINCT date_trunc('day', bucket) AS d, query_hash", daily, StringComparison.Ordinal); + Assert.Contains("SELECT x.d, COUNT(x.query_hash) AS c", daily, StringComparison.Ordinal); } /// The routed form refuses a relation the registry does not know before it reaches the store. @@ -218,11 +233,11 @@ public void EveryConsumer_KeepsTheNull_AndSpellsTheDisclosureOneWay() Assert.Contains("days_missing = row.UniqueQueries is null ? new[] { row.SummaryDate.ToString(\"yyyy-MM-dd\") } : Array.Empty()", tools, StringComparison.Ordinal); var page = RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "pages", "server-tabs.js"); - Assert.Contains("export function dailyCalendarPanel(server)", page, StringComparison.Ordinal); + Assert.Contains("export function dailySummaryPanels(server)", page, StringComparison.Ordinal); Assert.Contains("res.data.days_missing", page, StringComparison.Ordinal); Assert.Contains("row.unique_queries == null", page, StringComparison.Ordinal); Assert.Contains("[\"not materialized\"]", page, StringComparison.Ordinal); - Assert.Contains("dailyCalendarPanel(server),", page, StringComparison.Ordinal); + Assert.Contains("...dailySummaryPanels(server),", page, StringComparison.Ordinal); /* The descriptor it replaced is gone: one fetch of the one read, as before. */ Assert.DoesNotContain("\"Daily Health Calendar\",\n \"get_daily_summary_range\"", page.Replace("\r\n", "\n", StringComparison.Ordinal), StringComparison.Ordinal); diff --git a/Darling/Darling.Tests/DailySummaryReadShapeTests.cs b/Darling/Darling.Tests/DailySummaryReadShapeTests.cs new file mode 100644 index 0000000000..cd5648478e --- /dev/null +++ b/Darling/Darling.Tests/DailySummaryReadShapeTests.cs @@ -0,0 +1,615 @@ +/* + * 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.Globalization; +using System.Linq; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Npgsql; +using PerformanceMonitor.Darling.Service; +using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; +using Xunit; + +namespace Darling.Tests; + +/// +/// #3905: the daily summary's read shape. On the largest production store the web server page's two daily +/// reads were 26 of the Overview tab's 28 seconds, and the issue blamed two things: raw CTEs over the whole +/// window, and a per-call rollup-coverage probe. Measured, the probe half held and the CTE half did not. The +/// month read's cost was the routed queries CTE: its not-carried probes (#3653 A6) planned as joins over +/// the server's entire history, rescanned once per day, and its COUNT(DISTINCT) sorted every rollup row +/// of the month. These pins hold the fixed shapes. Results are identical by construction, and the live half proves +/// that against the old text as an oracle, row for row, on every tier. +/// +public sealed class DailySummaryReadShapeSqlTests +{ + private static readonly (RetentionTier Tier, string Relation)[] EveryForm = + { + (RetentionTier.Raw, TimescaleSupport.QueryStatsHourlyView), + (RetentionTier.Hourly, TimescaleSupport.QueryStatsHourlyView), + (RetentionTier.Hourly, TimescaleSupport.QueryStatsIntervalHourlyView), + (RetentionTier.Daily, TimescaleSupport.QueryStatsHourlyView), + }; + + /// + /// No tier counts with a DISTINCT aggregate: PostgreSQL evaluates one by sorting every input row of its + /// group, where the distinct pairs below can be hashed (1.10 s against 0.24 s for a busy server's month of + /// hourly rows on the rig). Every member of the CTE counts the same way, so every tier counts the same thing: + /// one member on raw, the rollup half and the raw tail on a routed tier. + /// + [Fact] + public void EveryTier_CountsDistinctPairs_NeverADistinctAggregate() + { + foreach (var (tier, relation) in EveryForm) + { + var sql = DailySummarySql.RangeSqlFor(tier, relation); + Assert.DoesNotContain("COUNT(DISTINCT", sql, StringComparison.OrdinalIgnoreCase); + + var counted = tier == RetentionTier.Raw ? 1 : 2; + Assert.Equal(counted, sql.Split("SELECT x.d, COUNT(x.query_hash) AS c").Length - 1); + Assert.Equal(counted, sql.Split("GROUP BY x.d").Length - 1); + Assert.Contains("SELECT DISTINCT date_trunc('day', collection_time) AS d, query_hash", sql, StringComparison.Ordinal); + if (tier != RetentionTier.Raw) + { + Assert.Contains("SELECT DISTINCT date_trunc('day', bucket) AS d, query_hash", sql, StringComparison.Ordinal); + } + } + } + + /// + /// The seam, as source: the MCP reader routes on the cached gate the compose panels read, and no longer runs + /// either probe itself. A per-call probe is what put a >= 1.7 s coverage read in front of every daily read, + /// twice per server-page load. The live half counts the probes. + /// + [Fact] + public void TheDailyReader_RoutesOnTheCachedGate_NotAPerCallProbe() + { + var reader = RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Service", "Mcp", "DarlingHealthReader.cs"); + Assert.Contains("await ComposeStoreAvailability.GetRollupsAsync(postgres, cancellationToken);", reader, StringComparison.Ordinal); + Assert.DoesNotContain("TimescaleSupport.DetectRollupsAsync(", reader, StringComparison.Ordinal); + Assert.DoesNotContain("TimescaleSupport.DetectRollupCoverageAsync(", reader, StringComparison.Ordinal); + } + + /// + /// The web server page's Overview tab draws today's tile and the month grid from ONE fetch of + /// get_daily_summary_range: the tile is the row whose date is the read's own to_date (the anchor + /// day), so the page derives no date of its own. It used to fetch get_daily_summary beside the range, which + /// ran the day's aggregate twice per load and could print two different query counts for today on one + /// page. + /// + [Fact] + public void TheOverviewTab_DrawsTodaysTileFromTheRangeRead_OneFetchForTwoPanels() + { + var page = RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "pages", "server-tabs.js"); + + var overviewAt = page.IndexOf("id: \"overview\",", StringComparison.Ordinal); + var nextTabAt = page.IndexOf("id: \"", overviewAt + 1, StringComparison.Ordinal); + Assert.True(overviewAt > 0 && nextTabAt > overviewAt, "the Overview tab's block must be found before it can be read"); + var overview = page[overviewAt..nextTabAt]; + Assert.Contains("...dailySummaryPanels(server),", overview, StringComparison.Ordinal); + Assert.DoesNotContain("\"get_daily_summary\"", overview, StringComparison.Ordinal); + + var composite = page[page.IndexOf("export function dailySummaryPanels(server)", StringComparison.Ordinal)..]; + composite = composite[..composite.IndexOf("\n}", StringComparison.Ordinal)]; + Assert.Equal(1, composite.Split("readTool(").Length - 1); + Assert.Contains("readTool(\"get_daily_summary_range\", { server, days_back: 30 })", composite, StringComparison.Ordinal); + Assert.Contains("days.find((day) => day.summary_date === res.data.to_date)", composite, StringComparison.Ordinal); + Assert.Contains("VIZ.stat(today, { stats: DAILY_STATS })", composite, StringComparison.Ordinal); + Assert.Contains("return [tile.panel, calendar.panel];", composite, StringComparison.Ordinal); + } +} + +/// +/// #3905: runs one coverage probe per data source at a time. The web +/// server page fires its daily reads together, and on the largest production store the probe is >= 1.7 s, so a +/// stale cache refreshed by every concurrent caller was that cost times the callers. Driven through the probe +/// seam with a gated fake, the pattern set for the fleet memo, so +/// callers can be parked on the probe before it answers, which is the only way to prove they shared it. +/// +public sealed class ComposeStoreAvailabilitySingleFlightTests +{ + private static readonly (RollupAvailability, RollupCoverage) Answer = (RollupAvailability.All, RollupCoverage.Unknown); + + private sealed class GatedProbe + { + private int _calls; + public TaskCompletionSource<(RollupAvailability, RollupCoverage)> Gate { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + public List TokensSeen { get; } = new(); + public int Calls => Volatile.Read(ref _calls); + + public Task<(RollupAvailability Rollups, RollupCoverage Coverage)> Run(NpgsqlDataSource dataSource, CancellationToken token) + { + Interlocked.Increment(ref _calls); + lock (TokensSeen) + { + TokensSeen.Add(token); + } + + return Gate.Task; + } + } + + /// A data source that is never opened: the cache keys on the instance, and nothing here reaches a + /// store. + private static NpgsqlDataSource UnopenedDataSource() => NpgsqlDataSource.Create("Host=unused.invalid;Database=none"); + + [Fact] + public async Task ConcurrentCallers_ShareOneProbe_AndTheAnswerIsThenCached() + { + await using var dataSource = UnopenedDataSource(); + var probe = new GatedProbe(); + + var callers = Enumerable.Range(0, 5) + .Select(_ => ComposeStoreAvailability.GetRollupsAsync(dataSource, probe.Run, CancellationToken.None).AsTask()) + .ToArray(); + + Assert.All(callers, c => Assert.False(c.IsCompleted)); + Assert.Equal(1, probe.Calls); + Assert.Equal(1, ComposeStoreAvailability.ProbesStartedFor(dataSource)); + + probe.Gate.SetResult(Answer); + var results = await Task.WhenAll(callers); + Assert.All(results, r => Assert.Equal(RollupAvailability.All, r.Rollups)); + + /* Inside ReprobeInterval the answer is served from memory: no second probe. */ + var cached = await ComposeStoreAvailability.GetRollupsAsync(dataSource, probe.Run, CancellationToken.None); + Assert.Equal(RollupAvailability.All, cached.Rollups); + Assert.Equal(1, probe.Calls); + Assert.Equal(1, ComposeStoreAvailability.ProbesStartedFor(dataSource)); + + /* The shared probe ran on no caller's token. */ + Assert.Equal(new[] { CancellationToken.None }, probe.TokensSeen); + } + + /// A caller that gives up releases itself and nobody else: the probe keeps running for the + /// others, and its answer is cached for the next caller. + [Fact] + public async Task ACallersCancellation_ReleasesOnlyThatCaller() + { + await using var dataSource = UnopenedDataSource(); + var probe = new GatedProbe(); + using var leaving = new CancellationTokenSource(); + + var leaver = ComposeStoreAvailability.GetRollupsAsync(dataSource, probe.Run, leaving.Token).AsTask(); + var stayer = ComposeStoreAvailability.GetRollupsAsync(dataSource, probe.Run, CancellationToken.None).AsTask(); + Assert.Equal(1, probe.Calls); + + await leaving.CancelAsync(); + await Assert.ThrowsAnyAsync(() => leaver); + Assert.False(stayer.IsCompleted); + + probe.Gate.SetResult(Answer); + Assert.Equal(RollupAvailability.All, (await stayer).Rollups); + Assert.Equal(RollupAvailability.All, (await ComposeStoreAvailability.GetRollupsAsync(dataSource, probe.Run, CancellationToken.None)).Rollups); + Assert.Equal(1, probe.Calls); + } + + /// A failed probe answers every waiter with the never-wrong fallback (route to raw, no coverage + /// evidence) and is cached like any answer, as it was before the probe was shared: the re-probe interval + /// retries it, and a burst of callers does not become a burst of failing probes. + [Fact] + public async Task AFailedProbe_AnswersRawForEveryWaiter_AndIsCached() + { + await using var dataSource = UnopenedDataSource(); + var probe = new GatedProbe(); + + var a = ComposeStoreAvailability.GetRollupsAsync(dataSource, probe.Run, CancellationToken.None).AsTask(); + var b = ComposeStoreAvailability.GetRollupsAsync(dataSource, probe.Run, CancellationToken.None).AsTask(); + probe.Gate.SetException(new NpgsqlException("57014: canceling statement due to statement timeout")); + + foreach (var answer in await Task.WhenAll(a, b)) + { + Assert.Equal(RollupAvailability.None, answer.Rollups); + Assert.Same(RollupCoverage.Unknown, answer.Coverage); + } + + Assert.Equal(RollupAvailability.None, (await ComposeStoreAvailability.GetRollupsAsync(dataSource, probe.Run, CancellationToken.None)).Rollups); + Assert.Equal(1, probe.Calls); + } +} + +/// +/// #3905, against a store: the rewritten statement answers exactly what the old one did on every tier, the +/// not-carried probes run once per day instead of as joins over the server's history, and the daily reads probe +/// coverage once however many race. +/// +/// The oracle is the old text. is the queries CTE as it stood +/// before #3905, for each tier, spliced into the current statement in place of the new one, so the two +/// statements differ in exactly the lines this issue changed. The seed plants the cases those lines decide: a +/// day the rollup skipped below its ceiling (NULL, named), a day with no rows at all (absent), a NULL +/// query_hash (skipped by both counts), a restart-only day (a hole for the legacy hourly, nothing for the +/// interval-honest successor), and the days past each tier's ceiling (the raw tail). +/// +[Collection("live-postgres")] +public sealed class DailySummaryReadShapeLiveTests +{ + private const int ServerId = -390501; + private const string ServerName = "read-shape-3905"; + + /// The pre-#3905 queries CTE, raw and routed, verbatim but for its comments. Placeholders: + /// the routed relation (the daily tier ignores , as + /// does), its registered source, the + /// source's time column and the source filter. + private static string OldQueriesCte(RetentionTier tier, string hourlyRelation) + { + var relation = tier == RetentionTier.Daily ? TimescaleSupport.QueryStatsDailyView : hourlyRelation; + if (tier == RetentionTier.Raw) + { + return """ + queries AS ( + SELECT date_trunc('day', collection_time) AS d, COUNT(DISTINCT query_hash) AS c + FROM v_query_stats + WHERE server_id = $1 AND collection_time >= $2 AND collection_time < $3 + GROUP BY 1 + ), + """; + } + + var target = TimescaleSupport.MaterializationHoleTargets.Single(t => t.View == relation); + var filter = TimescaleSupport.MaterializationHoleSourceFilterFor(target.CreateSql); + var sourceFilter = filter.Length == 0 ? string.Empty : " AND " + filter; + return $""" + queries_ceiling AS ( + SELECT date_trunc('day', max(bucket)) AS last_day + FROM collect.{relation} + WHERE server_id = $1 + ), + queries AS ( + SELECT date_trunc('day', bucket) AS d, COUNT(DISTINCT query_hash) AS c + FROM collect.{relation} + WHERE server_id = $1 AND bucket >= $2 AND bucket < $3 + GROUP BY 1 + UNION ALL + SELECT date_trunc('day', collection_time) AS d, COUNT(DISTINCT query_hash) AS c + FROM v_query_stats + WHERE server_id = $1 AND collection_time >= $2 AND collection_time < $3 + AND collection_time >= COALESCE((SELECT last_day + INTERVAL '1 day' FROM queries_ceiling), $2) + GROUP BY 1 + UNION ALL + SELECT b.d, NULL::bigint AS c + FROM generate_series(date_trunc('day', $2::timestamp), date_trunc('day', $3::timestamp), INTERVAL '1 day') AS b(d) + WHERE b.d < $3 + AND b.d < COALESCE((SELECT last_day + INTERVAL '1 day' FROM queries_ceiling), $2) + AND NOT EXISTS ( + SELECT 1 FROM collect.{relation} AS r + WHERE r.server_id = $1 AND r.bucket >= b.d AND r.bucket < b.d + INTERVAL '1 day') + AND EXISTS ( + SELECT 1 FROM collect.{target.Source} AS s + WHERE s.server_id = $1 AND s.{target.SourceTimeColumn} >= b.d AND s.{target.SourceTimeColumn} < b.d + INTERVAL '1 day'{sourceFilter}) + ), + """; + } + + /// The oracle statement: the current one with its queries region (from the first CTE this + /// issue touched to the deadlocks CTE that follows it) replaced by . + private static string OracleSql(RetentionTier tier, string relation) + { + var current = DailySummarySql.RangeSqlFor(tier, relation); + var start = current.IndexOf(tier == RetentionTier.Raw ? "queries AS (" : "queries_ceiling AS (", StringComparison.Ordinal); + var end = current.IndexOf("deadlocks AS (", StringComparison.Ordinal); + Assert.True(start > 0 && end > start, "the queries region must be found before it can be swapped"); + return current[..start] + OldQueriesCte(tier, relation) + "\n" + current[end..]; + } + + [Fact] + public async Task TheRewrittenStatement_AnswersExactlyWhatTheOldOneDid_OnEveryTier_AgainstDevPostgres() + { + await using var seed = await SeedAsync("the read-shape oracle test"); + var (connection, days) = (seed.Connection, seed.Days); + var ct = TestContext.Current.CancellationToken; + + var from = days[0].AddDays(-3); + var to = days[^1].AddDays(1); + var forms = new (RetentionTier Tier, string Relation)[] + { + (RetentionTier.Raw, TimescaleSupport.QueryStatsHourlyView), + (RetentionTier.Hourly, TimescaleSupport.QueryStatsHourlyView), + (RetentionTier.Hourly, TimescaleSupport.QueryStatsIntervalHourlyView), + (RetentionTier.Daily, TimescaleSupport.QueryStatsHourlyView), + }; + + var answers = new Dictionary>(StringComparer.Ordinal); + foreach (var (tier, relation) in forms) + { + var current = await ReadAllAsync(connection, DailySummarySql.RangeSqlFor(tier, relation), from, to, ct); + var oracle = await ReadAllAsync(connection, OracleSql(tier, relation), from, to, ct); + Assert.NotEmpty(current); + Assert.Equal(oracle, current); + answers[tier + "/" + relation] = current; + } + + /* Not vacuous: the forms reach the cases the changed lines decide. The legacy hourly names its skipped + day (D2); the NULL hash is skipped (D3 counts the 4 hashes, not the NULL); the successor's ceiling is + D3, so D4 onward is its raw tail, restart row included. On the daily tier D2 is absent rather than + NULL: its source, the legacy hourly, skipped the day too, so nothing witnesses it (the hole scan's + rule), and D8 comes from the raw tail past the daily's ceiling. */ + string Day(int n) => days[n].ToString("yyyy-MM-dd", CultureInfo.InvariantCulture); + var legacy = answers[RetentionTier.Hourly + "/" + TimescaleSupport.QueryStatsHourlyView]; + Assert.Contains(legacy, row => row.StartsWith(Day(2) + "|", StringComparison.Ordinal) && row.Split('|')[3] == "NULL"); + Assert.Contains(legacy, row => row.StartsWith(Day(3) + "|", StringComparison.Ordinal) && row.Split('|')[3] == "4"); + Assert.DoesNotContain(legacy, row => row.StartsWith(Day(1) + "|", StringComparison.Ordinal)); + var successor = answers[RetentionTier.Hourly + "/" + TimescaleSupport.QueryStatsIntervalHourlyView]; + Assert.Contains(successor, row => row.StartsWith(Day(4) + "|", StringComparison.Ordinal) && row.Split('|')[3] == "1"); + var daily = answers[RetentionTier.Daily + "/" + TimescaleSupport.QueryStatsHourlyView]; + Assert.DoesNotContain(daily, row => row.StartsWith(Day(2) + "|", StringComparison.Ordinal)); + Assert.Contains(daily, row => row.StartsWith(Day(8) + "|", StringComparison.Ordinal) && row.Split('|')[3] == "1"); + } + + /// + /// The plan, not the text: in the routed queries CTE nothing is materialized and nothing is a + /// semi-join or anti-join, which is how the old probes ran (the server's whole history materialized and + /// rescanned per day). Each probe is a SubPlan executed per day: the rollup probe once per candidate day at + /// or below the ceiling, the source probe only on the days the rollup probe found empty. The window is a + /// calendar month ending on the seed's last day, so most of it predates the seed and exercises both + /// probes. + /// + [Fact] + public async Task TheNotCarriedProbes_RunOncePerDay_AndTheSourceOnlyWhereTheRollupIsEmpty_AgainstDevPostgres() + { + await using var seed = await SeedAsync("the read-shape plan test"); + var (connection, days) = (seed.Connection, seed.Days); + var ct = TestContext.Current.CancellationToken; + + var to = days[^1].AddDays(1); + var from = to.AddDays(-30); + var sql = DailySummarySql.RangeSqlFor(RetentionTier.Hourly, TimescaleSupport.QueryStatsHourlyView); + + await using var explain = new NpgsqlCommand("EXPLAIN (ANALYZE, FORMAT JSON) " + sql, connection); + AddWindow(explain, from, to); + var json = (string)(await explain.ExecuteScalarAsync(ct))!; + using var plan = JsonDocument.Parse(json); + + var queries = Walk(plan.RootElement[0].GetProperty("Plan")) + .Single(n => n.TryGetProperty("Subplan Name", out var name) && name.GetString() == "CTE queries"); + var inQueries = Walk(queries).ToArray(); + Assert.DoesNotContain(inQueries, n => NodeType(n) == "Materialize"); + Assert.DoesNotContain(inQueries, n => NodeType(n) == "Nested Loop" + && n.TryGetProperty("Join Type", out var join) && join.GetString() is "Semi" or "Anti"); + + /* The legacy hourly's ceiling is D5 (the seed materializes D0, D3, D4 and D5); every window day at or + below it is a candidate, and the ones the rollup holds no row for are probed at the source. */ + var candidates = Enumerable.Range(0, (days[5] - from).Days + 1).Count(); + var rollupEmpty = candidates - 4; + + var candidateDays = inQueries.Single(n => NodeType(n) == "Function Scan" + && n.TryGetProperty("Function Name", out var fn) && fn.GetString() == "generate_series"); + Assert.Equal(candidates, LoopsOfTheOnlySubPlanUnder(candidateDays)); + + var notCarried = inQueries.Single(n => NodeType(n) == "Subquery Scan" && n.GetProperty("Alias").GetString()!.StartsWith('b')); + Assert.Equal(rollupEmpty, LoopsOfTheOnlySubPlanUnder(notCarried)); + } + + /// + /// The seam, counted: the single-day and range tools raced together (the web server page's old pattern, and + /// any agent's) and then called again cost ONE coverage probe on this data source. Before #3905 the reader + /// probed on every call and never touched the gate, so the count below was zero. + /// + [Fact] + public async Task EveryDailyRead_ProbesCoverageOnce_HoweverManyRace_AgainstDevPostgres() + { + var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), + "Set DARLING_TEST_PG to a Postgres connection string to run the live coverage-probe seam test."); + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + await using (var connection = new NpgsqlConnection(scratch.ConnectionString)) + { + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + await RegisterServerAsync(connection, ct); + } + + await using var postgres = NpgsqlDataSource.Create(scratch.ConnectionString); + Assert.Equal(0, ComposeStoreAvailability.ProbesStartedFor(postgres)); + + var raced = Enumerable.Range(0, 3) + .SelectMany(_ => new[] + { + DarlingMcpHealthTools.GetDailySummary(postgres, ServerName), + DarlingMcpHealthTools.GetDailySummaryRange(postgres, ServerName, 30), + }) + .ToArray(); + var payloads = (await Task.WhenAll(raced)).ToList(); + payloads.Add(await DarlingMcpHealthTools.GetDailySummaryRange(postgres, ServerName, 7)); + payloads.Add(await DarlingMcpHealthTools.GetDailySummary(postgres, ServerName)); + + foreach (var payload in payloads) + { + using var doc = JsonDocument.Parse(payload); + Assert.False(doc.RootElement.TryGetProperty("status", out var status) && status.GetString() is "error" or "invalid", payload); + } + + Assert.Equal(1, ComposeStoreAvailability.ProbesStartedFor(postgres)); + } + + /* ───────────────────────────── seed ───────────────────────────── */ + + /// + /// Nine whole UTC days, D0 thirteen days back, all past RawMaxAge and below every refresh policy's window, + /// so a background policy run cannot materialize what the seed leaves unmaterialized. D0: 3 hashes. D1: + /// nothing. D2: 5 hashes, skipped by the hourly and the daily (the hole). D3: 4 hashes and a NULL-hash row. + /// D4: one restart row (interval 0). D5: 6. D6: 2. D7: 3. D8: 1. Refreshed: the legacy hourly over D0-D1 and + /// D3-D5 (ceiling D5); the successor over D0 and D3 (ceiling D3; it rejects D4's restart row); the daily over + /// D0 and D3 (ceiling D3). + /// + private sealed class Seeded(ScratchPostgres scratch, NpgsqlConnection connection, DateTime[] days) : IAsyncDisposable + { + public NpgsqlConnection Connection { get; } = connection; + + public DateTime[] Days { get; } = days; + + public async ValueTask DisposeAsync() + { + await Connection.DisposeAsync(); + await scratch.DisposeAsync(); + } + } + + private static async Task SeedAsync(string what) + { + var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), + $"Set DARLING_TEST_PG to a Postgres connection string (with TimescaleDB installed) to run {what}."); + var ct = TestContext.Current.CancellationToken; + + var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + if (!await TimescaleSupport.TryEnableAsync(connection, null, ct)) + { + await connection.DisposeAsync(); + await scratch.DisposeAsync(); + Assert.Skip($"{what} needs TimescaleDB: a rollup tier exists only as a continuous aggregate."); + } + + await TimescaleSupport.ConvertToHypertablesAsync(connection, null, ct); + await TimescaleSupport.EnsureContinuousAggregatesAsync(connection, null, ct); + await RegisterServerAsync(connection, ct); + + var d0 = DateTime.SpecifyKind(DateTime.UtcNow.Date.AddDays(-13), DateTimeKind.Unspecified); + var days = Enumerable.Range(0, 9).Select(n => d0.AddDays(n)).ToArray(); + Assert.True(DateTime.UtcNow - days[^1].AddDays(1) > TimescaleSupport.DailyRefreshStartSpan, "the seed must sit below every refresh policy's window"); + + foreach (var (day, hashes) in new[] { (0, 3), (2, 5), (3, 4), (5, 6), (6, 2), (7, 3), (8, 1) }) + { + await PlantDayAsync(connection, days[day], hashes, ct); + } + + await PlantRowAsync(connection, days[3].AddHours(12), queryHash: null, intervalSeconds: 1200, ct); + await PlantRowAsync(connection, days[4].AddHours(6), queryHash: "0xRESTART", intervalSeconds: 0, ct); + + await RefreshAsync(connection, TimescaleSupport.QueryStatsHourlyView, days[0], days[2], ct); + await RefreshAsync(connection, TimescaleSupport.QueryStatsHourlyView, days[3], days[6], ct); + await RefreshAsync(connection, TimescaleSupport.QueryStatsIntervalHourlyView, days[0], days[1], ct); + await RefreshAsync(connection, TimescaleSupport.QueryStatsIntervalHourlyView, days[3], days[5], ct); + await RefreshAsync(connection, TimescaleSupport.QueryStatsDailyView, days[0], days[1], ct); + await RefreshAsync(connection, TimescaleSupport.QueryStatsDailyView, days[3], days[4], ct); + + return new Seeded(scratch, connection, days); + } + + private static async Task PlantDayAsync(NpgsqlConnection connection, DateTime day, int hashes, CancellationToken ct) + { + foreach (var hour in new[] { 1, 9, 17 }) + { + for (var hash = 0; hash < hashes; hash++) + { + await PlantRowAsync(connection, day.AddHours(hour), $"0xHASH{hash:D4}", 1200, ct); + } + } + } + + private static int s_collectionId; + + private static async Task PlantRowAsync(NpgsqlConnection connection, DateTime at, string? queryHash, int intervalSeconds, CancellationToken ct) + { + await using var insert = new NpgsqlCommand(@" +INSERT INTO collect.query_stats + (collection_id, collection_time, server_id, server_name, database_name, query_hash, sql_handle, + delta_worker_time, delta_elapsed_time, delta_execution_count, sample_interval_seconds) +VALUES ($1, $2, $3, $4, 'ShapeDb', $5, $6, 1000, 1000, 10, $7)", connection); + insert.Parameters.AddWithValue((long)Interlocked.Increment(ref s_collectionId)); + insert.Parameters.AddWithValue(at); + insert.Parameters.AddWithValue(ServerId); + insert.Parameters.AddWithValue(ServerName); + insert.Parameters.Add(new NpgsqlParameter { TypedValue = queryHash, NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Text }); + insert.Parameters.AddWithValue("0xHANDLE" + (queryHash ?? "NULL")); + insert.Parameters.AddWithValue(intervalSeconds); + await insert.ExecuteNonQueryAsync(ct); + } + + private static async Task RefreshAsync(NpgsqlConnection connection, string view, DateTime from, DateTime to, CancellationToken ct) + { + await using var refresh = new NpgsqlCommand($"CALL refresh_continuous_aggregate('collect.{view}'::regclass, $1::timestamp, $2::timestamp)", connection); + refresh.Parameters.AddWithValue(from); + refresh.Parameters.AddWithValue(to); + await refresh.ExecuteNonQueryAsync(ct); + } + + private static async Task RegisterServerAsync(NpgsqlConnection connection, CancellationToken ct) + { + await using var command = new NpgsqlCommand(@" +INSERT INTO servers (server_id, server_name, display_name, is_enabled, sql_major_version, created_date, modified_date) +VALUES ($1, $2, $3, TRUE, 15, $4, $4) +ON CONFLICT (server_id) DO UPDATE SET is_enabled = TRUE, sql_major_version = 15;", connection); + command.Parameters.AddWithValue(ServerId); + command.Parameters.AddWithValue(ServerName); + command.Parameters.AddWithValue(ServerName); + command.Parameters.AddWithValue(DateTime.SpecifyKind(DateTime.UtcNow, DateTimeKind.Unspecified)); + await command.ExecuteNonQueryAsync(ct); + } + + /* ───────────────────────────── reads ───────────────────────────── */ + + private static void AddWindow(NpgsqlCommand command, DateTime from, DateTime to) + { + command.Parameters.Add(new NpgsqlParameter { TypedValue = ServerId }); + command.Parameters.Add(new NpgsqlParameter { TypedValue = from }); + command.Parameters.Add(new NpgsqlParameter { TypedValue = to }); + } + + /// Every column of every row, as text, NULL spelled out: the comparison is the whole answer, not + /// the columns this issue touched. + private static async Task> ReadAllAsync(NpgsqlConnection connection, string sql, DateTime from, DateTime to, CancellationToken ct) + { + var rows = new List(); + await using var read = new NpgsqlCommand(sql, connection); + AddWindow(read, from, to); + await using var reader = await read.ExecuteReaderAsync(ct); + while (await reader.ReadAsync(ct)) + { + var cells = new string[reader.FieldCount]; + for (var i = 0; i < reader.FieldCount; i++) + { + cells[i] = reader.IsDBNull(i) + ? "NULL" + : reader.GetValue(i) is DateTime at + ? at.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture) + : Convert.ToString(reader.GetValue(i), CultureInfo.InvariantCulture)!; + } + + rows.Add(string.Join("|", cells)); + } + + return rows; + } + + private static IEnumerable Walk(JsonElement node) + { + yield return node; + if (node.TryGetProperty("Plans", out var plans)) + { + foreach (var child in plans.EnumerateArray()) + { + foreach (var descendant in Walk(child)) + { + yield return descendant; + } + } + } + } + + private static string? NodeType(JsonElement node) => node.GetProperty("Node Type").GetString(); + + /// How many times the one SubPlan hanging off ran: the probe's + /// per-row execution count, which is what tells a per-day probe from a join. + private static int LoopsOfTheOnlySubPlanUnder(JsonElement node) + { + var subPlans = node.GetProperty("Plans").EnumerateArray() + .Where(child => child.GetProperty("Parent Relationship").GetString() == "SubPlan") + .ToArray(); + Assert.Single(subPlans); + return subPlans[0].GetProperty("Actual Loops").GetInt32(); + } +} diff --git a/Darling/Darling.Tests/DarlingMcpHealthToolsTests.cs b/Darling/Darling.Tests/DarlingMcpHealthToolsTests.cs index 4efee5992d..6891a0c94a 100644 --- a/Darling/Darling.Tests/DarlingMcpHealthToolsTests.cs +++ b/Darling/Darling.Tests/DarlingMcpHealthToolsTests.cs @@ -77,9 +77,9 @@ private static (string Name, bool Optional)[] McpParams(string toolName) [InlineData("get_server_summary", "server_name")] [InlineData("get_daily_summary", "server_name,summary_date")] /* The range read is a SIBLING rather than a wider get_daily_summary: the single-day tool returns a flat - object of scalars and this returns rows, and the Overview tab reads both, which it could not do if they - were one read (no tab may fetch a read twice). Its span is in DAYS, so it carries the day-grained - anchor description rather than the hours one. */ + object of scalars and this returns rows. (The Overview tab reads only this one since #3905, drawing + today's tile from its anchor-day row.) Its span is in DAYS, so it carries the day-grained anchor + description rather than the hours one. */ [InlineData("get_daily_summary_range", "server_name,days_back,as_of")] public void ParamContract_MatchesContract(string toolName, string expectedCsv) { diff --git a/Darling/Darling.Tests/IntervalHonestHourlyRollupTests.cs b/Darling/Darling.Tests/IntervalHonestHourlyRollupTests.cs index 5ec9aff521..19d6b11e1c 100644 --- a/Darling/Darling.Tests/IntervalHonestHourlyRollupTests.cs +++ b/Darling/Darling.Tests/IntervalHonestHourlyRollupTests.cs @@ -395,9 +395,11 @@ legacy text in that name alone. */ const string SourceProbeTail = "AND s.collection_time < b.d + INTERVAL '1 day'"; var legacyRouted = DailySummarySql.RangeSqlFor(RetentionTier.Hourly); Assert.Equal(1, legacyRouted.Split(SourceProbeTail).Length - 1); + /* #3905: the probe now ends in its OFFSET 0 fence on the next line, so the filter is inserted after + the tail itself — which the count above proves appears exactly once. */ var expectedSuccessorRouted = legacyRouted .Replace($"FROM collect.{legacy}", $"FROM collect.{successor}", StringComparison.Ordinal) - .Replace(SourceProbeTail + ")", SourceProbeTail + "\n AND " + successorFilter + ")", StringComparison.Ordinal); + .Replace(SourceProbeTail, SourceProbeTail + "\n AND " + successorFilter, StringComparison.Ordinal); Assert.Equal(expectedSuccessorRouted, DailySummarySql.RangeSqlFor(RetentionTier.Hourly, successor)); Assert.Contains(successor, DailySummarySql.RangeSqlFor(RetentionTier.Hourly, successor), StringComparison.Ordinal); Assert.DoesNotContain(legacy, DailySummarySql.RangeSqlFor(RetentionTier.Hourly, successor), StringComparison.Ordinal); diff --git a/Darling/Darling.Tests/RetentionTierRouterTests.cs b/Darling/Darling.Tests/RetentionTierRouterTests.cs index 2de173f398..d3d77027f8 100644 --- a/Darling/Darling.Tests/RetentionTierRouterTests.cs +++ b/Darling/Darling.Tests/RetentionTierRouterTests.cs @@ -200,7 +200,9 @@ public void DailySummary_RollupTier_RoutesOnlyTheQueryCte(RetentionTier tier, st var sql = ViewerDataService.DailySummaryRangeSqlFor(tier); Assert.Contains($"FROM collect.{expectedRelation}", sql, StringComparison.Ordinal); - Assert.Contains("SELECT date_trunc('day', bucket) AS d, COUNT(DISTINCT query_hash) AS c", sql, StringComparison.Ordinal); + /* #3905: the distinct (day, hash) pairs, counted per day -- COUNT(DISTINCT query_hash)'s answer in a + shape the planner can hash. */ + Assert.Contains("SELECT DISTINCT date_trunc('day', bucket) AS d, query_hash", sql, StringComparison.Ordinal); /* The routed CTE reads the raw passthrough view ONLY for the days past the rollup's last materialized DAY (#3653: the daily rollup lags a day or two behind the clock, and those days used to print @@ -209,7 +211,7 @@ public void DailySummary_RollupTier_RoutesOnlyTheQueryCte(RetentionTier tier, st var rawReads = sql.Split("FROM v_query_stats").Length - 1; Assert.Equal(1, rawReads); var ceilingAt = sql.IndexOf("SELECT date_trunc('day', max(bucket)) AS last_day", StringComparison.Ordinal); - var rollupReadAt = sql.IndexOf("SELECT date_trunc('day', bucket) AS d, COUNT(DISTINCT query_hash) AS c", StringComparison.Ordinal); + var rollupReadAt = sql.IndexOf("SELECT DISTINCT date_trunc('day', bucket) AS d, query_hash", StringComparison.Ordinal); var unionAt = sql.IndexOf("UNION ALL", StringComparison.Ordinal); var rawReadAt = sql.IndexOf("FROM v_query_stats", StringComparison.Ordinal); var tailGateAt = sql.IndexOf("AND collection_time >= COALESCE((SELECT last_day + INTERVAL '1 day' FROM queries_ceiling), $2)", StringComparison.Ordinal); @@ -221,14 +223,14 @@ public void DailySummary_RollupTier_RoutesOnlyTheQueryCte(RetentionTier tier, st /* #3653 A6: the THIRD member — the days at or below the ceiling the rollup holds no row for while its source still holds admitted rows — emits the day with a NULL count, after the raw tail, gated on the - same ceiling from the other side (b.d < last_day + 1 day) so no day can be both "past the ceiling, + same ceiling from the other side (g.d < last_day + 1 day) so no day can be both "past the ceiling, answered from raw" and "below it, not carried". Its two probes are the hole scan's, per server: NOT EXISTS on the routed relation, EXISTS on the relation's registered SOURCE (the daily's is the legacy hourly it is hierarchical from; the hourly's is raw query_stats, read as collect.query_stats and not through the passthrough view, so the raw-read count above stays one). DailySummaryNotCarriedTests carries the rest of this member's pins; this one holds the ORDER and the gate. */ var notCarriedAt = sql.IndexOf("SELECT b.d, NULL::bigint AS c", StringComparison.Ordinal); - var notCarriedGateAt = sql.IndexOf("AND b.d < COALESCE((SELECT last_day + INTERVAL '1 day' FROM queries_ceiling), $2)", StringComparison.Ordinal); + var notCarriedGateAt = sql.IndexOf("AND g.d < COALESCE((SELECT last_day + INTERVAL '1 day' FROM queries_ceiling), $2)", StringComparison.Ordinal); Assert.True(notCarriedAt > tailGateAt, "the not-carried member is the last UNION ALL member, after the raw tail"); Assert.True(notCarriedGateAt > notCarriedAt, "the not-carried member is gated to the days at or below the ceiling"); Assert.Contains($"SELECT 1 FROM collect.{expectedRelation} AS r", sql[notCarriedAt..], StringComparison.Ordinal); diff --git a/Darling/Darling.Tests/ServerPageTabsTests.cs b/Darling/Darling.Tests/ServerPageTabsTests.cs index d40a0f063e..5fc651980f 100644 --- a/Darling/Darling.Tests/ServerPageTabsTests.cs +++ b/Darling/Darling.Tests/ServerPageTabsTests.cs @@ -1032,8 +1032,9 @@ public void NoTab_FetchesTheSameReadTwice() ["waitsPanel("] = new[] { "get_wait_stats", "get_wait_trend" }, ["fileIoPanel("] = new[] { "get_file_io_trend" }, /* #3653 A6: the Daily Health Calendar became a composite so the read's days_missing[] renders as a - line above the grid; it still fetches exactly the one read the table() descriptor did. */ - ["dailyCalendarPanel("] = new[] { "get_daily_summary_range" }, + line above the grid. #3905: it also renders today's tile, from the same one fetch, where the tile + used to be a get_daily_summary descriptor of its own. */ + ["dailySummaryPanels("] = new[] { "get_daily_summary_range" }, ["perfmonPanel("] = new[] { "get_perfmon_stats", "get_perfmon_trend" }, ["topQueriesPanel("] = new[] { "get_top_queries_by_cpu", "get_query_trend" }, }; diff --git a/Darling/PerformanceMonitor.Darling.Service/Compose/ComposeStoreAvailability.cs b/Darling/PerformanceMonitor.Darling.Service/Compose/ComposeStoreAvailability.cs index 193c799155..8932924cc8 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Compose/ComposeStoreAvailability.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Compose/ComposeStoreAvailability.cs @@ -18,16 +18,16 @@ namespace PerformanceMonitor.Darling.Service; /// -/// The compose runner's cached answer to "which retention rollups does this store have?" (#1665 — the -/// composer's arm of the #1664 availability guard), plus the "partial window, and says so" notice for the -/// routes that availability (or the value-remap/dimension-coverage gates) forced below what the window's age -/// wanted. Caching mirrors the viewer's GetRollupAvailabilityAsync semantics exactly: once every -/// rollup exists the answer is cached for the store's lifetime (a created CAGG is never dropped outside the -/// service's own reshape sweep); while the store reports none/partial, re-probe at most every -/// so an ensure sweep finishing mid-session converges without a restart; a -/// failed probe answers — raw always exists, so "route everything to -/// raw" is the never-wrong fallback. Keyed per data source (not a bare static) so gated-live tests spinning -/// several stores in one process can never bleed one store's shape into another's. +/// The service's cached answer to "which retention rollups does this store have, and how far back has each +/// materialized?" (#1665 — the composer's arm of the #1664 availability guard; since #3905 also the gate the +/// daily-summary reader routes on), plus the "partial window, and says so" notice for the routes that +/// availability (or the value-remap/dimension-coverage gates) forced below what the window's age wanted. +/// Caching follows the viewer's GetRollupAvailabilityAsync: the answer is re-probed at most every +/// , unconditionally since #1759 (see ), +/// so an ensure sweep or a backfill finishing mid-session converges without a restart; a failed probe answers +/// — raw always exists, so "route everything to raw" is the never-wrong +/// fallback. Keyed per data source (not a bare static) so gated-live tests spinning several stores in one +/// process can never bleed one store's shape into another's. /// internal static class ComposeStoreAvailability { @@ -37,6 +37,14 @@ private sealed class Entry public RollupCoverage Coverage = RollupCoverage.Unknown; public bool Probed; public DateTime ProbedAtUtc; + + /// The probe the current callers share (#3905): non-null and incomplete while one is running. + /// A completed task here is history, and the next caller who finds the cache stale starts a new one. + public Task<(RollupAvailability Rollups, RollupCoverage Coverage)>? InFlight; + + /// Probes started for this data source over its whole life. Diagnostic; see + /// . + public int ProbesStarted; } private static readonly ConditionalWeakTable s_entries = new(); @@ -48,8 +56,7 @@ private sealed class Entry /// /// The store's rollup availability AND each rollup's materialized-coverage floor, probed lazily and - /// cached per data source. Benignly racy: concurrent panel runs may probe twice; the probes are two - /// small catalog/aggregate lookups and last-write-wins caches the same answer. + /// cached per data source. /// /// Coverage expires even when availability does not (#1759). Availability is permanent once /// complete — a created aggregate is never dropped outside the service's own reshape sweep — but a @@ -58,31 +65,106 @@ private sealed class Entry /// life of the process, so an operator who just backfilled would keep getting raw fallbacks until a /// restart. The TTL therefore applies unconditionally, and the /// AllPresent shortcut is deliberately gone. + /// + /// One probe in flight per data source (#3905). This used to be "benignly racy": concurrent + /// callers that found the cache stale each ran the probe, on the argument that it was two small lookups. + /// Neither half held. The daily-summary reader now reads coverage through here too, the web server page + /// fires its two daily reads together, and on the largest production store the coverage probe is + /// >= 1.7 s on a quiet minute, not a small lookup. So a stale cache is refreshed by exactly one probe and + /// every caller that arrives while it runs awaits that one: the single-flight shape of + /// DarlingFleetReader.CollectionHealthMemo (#3735), for its reasons. The shared probe runs on + /// , so a caller's token releases only that caller's wait, and the + /// probe cannot fault the shared task: a failure is the cached raw fallback below, as before. /// - internal static async ValueTask<(RollupAvailability Rollups, RollupCoverage Coverage)> GetRollupsAsync( + internal static ValueTask<(RollupAvailability Rollups, RollupCoverage Coverage)> GetRollupsAsync( NpgsqlDataSource postgres, CancellationToken cancellationToken) + => GetRollupsAsync(postgres, ProbeAsync, cancellationToken); + + /// + /// with the probe substitutable, so a + /// test can count and pace executions without a store. is called with + /// : the cache, not the caller, decides the probe's lifetime. + /// + internal static async ValueTask<(RollupAvailability Rollups, RollupCoverage Coverage)> GetRollupsAsync( + NpgsqlDataSource postgres, + Func> probe, + CancellationToken cancellationToken) { + ArgumentNullException.ThrowIfNull(postgres); + ArgumentNullException.ThrowIfNull(probe); + var entry = s_entries.GetOrCreateValue(postgres); + Task<(RollupAvailability Rollups, RollupCoverage Coverage)> shared; + TaskCompletionSource<(RollupAvailability Rollups, RollupCoverage Coverage)>? lead = null; lock (entry) { if (entry.Probed && DateTime.UtcNow - entry.ProbedAtUtc < ReprobeInterval) { return (entry.Rollups, entry.Coverage); } + + if (entry.InFlight is not { IsCompleted: false }) + { + /* RunContinuationsAsynchronously: the waiters' continuations (the rest of each panel or daily + read) must not run inline on whichever thread completes the probe. */ + lead = new TaskCompletionSource<(RollupAvailability Rollups, RollupCoverage Coverage)>(TaskCreationOptions.RunContinuationsAsynchronously); + entry.InFlight = lead.Task; + entry.ProbesStarted++; + } + + shared = entry.InFlight; + } + + if (lead is not null) + { + /* Started outside the lock, and not awaited here: this caller is one waiter among any number, and + its own cancellation below must not be the probe's. RunProbeAsync completes the source on every + path and never throws, so the discarded task cannot fault. */ + _ = RunProbeAsync(entry, lead, postgres, probe); } + return await shared.WaitAsync(cancellationToken); + } + + /// How many probes has started + /// for — the figure a live test compares against the number of reads it raced + /// (#3905). Diagnostic; nothing reads it in production. + internal static int ProbesStartedFor(NpgsqlDataSource postgres) + { + var entry = s_entries.GetOrCreateValue(postgres); + lock (entry) + { + return entry.ProbesStarted; + } + } + + private static async Task<(RollupAvailability Rollups, RollupCoverage Coverage)> ProbeAsync( + NpgsqlDataSource postgres, CancellationToken cancellationToken) + { + var rollups = await TimescaleSupport.DetectRollupsAsync(postgres, cancellationToken); + var coverage = await TimescaleSupport.DetectRollupCoverageAsync(postgres, rollups, cancellationToken); + return (rollups, coverage); + } + + private static async Task RunProbeAsync( + Entry entry, + TaskCompletionSource<(RollupAvailability Rollups, RollupCoverage Coverage)> lead, + NpgsqlDataSource postgres, + Func> probe) + { RollupAvailability rollups; RollupCoverage coverage; try { - rollups = await TimescaleSupport.DetectRollupsAsync(postgres, cancellationToken); - coverage = await TimescaleSupport.DetectRollupCoverageAsync(postgres, rollups, cancellationToken); + (rollups, coverage) = await probe(postgres, CancellationToken.None); } - catch (Exception ex) when (ex is not OperationCanceledException) + catch (Exception ex) { /* A store hiccup mid-probe must not fail the panel — raw is the safe answer for availability, - and "no coverage evidence" leaves the age ladder in charge. The re-probe interval retries soon. */ + and "no coverage evidence" leaves the age ladder in charge. The re-probe interval retries soon. + Every exception, cancellation included: the probe runs on no caller's token, so nothing here + is a caller asking to stop, and the shared task must complete for every waiter. */ rollups = RollupAvailability.None; coverage = RollupCoverage.Unknown; _ = ex; @@ -96,7 +178,7 @@ private sealed class Entry entry.ProbedAtUtc = DateTime.UtcNow; } - return (rollups, coverage); + lead.SetResult((rollups, coverage)); } /// diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs index ac6ff0823e..3e4f895210 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs @@ -381,10 +381,20 @@ public static async Task GetDailySummaryRangeAsync( a separate question — a rollup created over pre-existing history answers old windows with silence. BOTH gates or neither: this tool and the viewer's calendar answer the same question off the same SQL, so routing them differently would have them report different query counts for the same day on exactly - the affected stores, with no way to tell which was right. Probed per call, uncached: get_daily_health - runs at human/model cadence and these are two small lookups. */ - var rollups = await TimescaleSupport.DetectRollupsAsync(postgres, cancellationToken); - var coverage = await TimescaleSupport.DetectRollupCoverageAsync(postgres, rollups, cancellationToken); + the affected stores, with no way to tell which was right. + + #3905: through ComposeStoreAvailability, the gate the compose panels already read: cached per data + source for its ReprobeInterval, one probe in flight at a time. This used to probe on every call on + the reasoning that it "runs at human/model cadence and these are two small lookups". The web server + page calls it twice per load, and on the largest production store the coverage probe alone measured + >= 1.7 s, because each min() over a compressed oldest chunk seq-scans and sorts that chunk's batch + list. A cached floor cannot produce a wrong count, only a different tier for at most the interval: + a floor moves back only on a backfill (stale, it under-claims, so a window stays where it was or + degrades), and forward only on a retention drop, which RetentionTierRouter's one-day RouteMargin keeps + every age-routed window clear of. The query CTE's ceiling is read live in the statement, and a day a + tier never carried is named NULL either way. The viewer's calendar has cached the same gate for the + same interval all along (ViewerDataService.GetRollupAvailabilityAsync). */ + var (rollups, coverage) = await ComposeStoreAvailability.GetRollupsAsync(postgres, cancellationToken); var tier = RetentionTierRouter.Resolve( DateTime.UtcNow, fromDate, rollups.QueryGrainHourly, rollups.QueryGrainDaily, coverage.For(TimescaleSupport.QueryStatsHourlyView, TimescaleSupport.QueryStatsDailyView)); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs index c405d66368..43ae990934 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs @@ -29,18 +29,17 @@ namespace PerformanceMonitor.Darling.Service.Mcp; /// into the SHARED composite health band (DailyHealthBandCalculator), so an agent gets the same /// Healthy / Warning / Critical verdict the Performance Calendar shows. /// -/// Why the range is a SIBLING rather than a wider get_daily_summary. Four reasons, and the first -/// two are mechanical. (1) The response SHAPE is the contract: get_daily_summary returns a flat object of -/// scalars, which is what a stat tile consumes and what its own description promises; a range returns rows. A -/// single tool that returned either depending on whether a span argument arrived would make every consumer -/// branch on a parameter it may not have sent, and the web stat panel would simply stop rendering. (2) The web -/// server page must not fetch one read twice — there is a pin for it — and the Overview tab already reads -/// get_daily_summary for today's band, so the month grid beside it CANNOT be the same read. (3) They are -/// different questions with different defaults: "how was Tuesday" versus "which of the last thirty days were -/// bad", the second of which is a screening read whose answer is the shape of the month rather than one day's -/// numbers. (4) get_daily_summary is a shipped name on both SKUs; changing its payload shape would break -/// callers for no gain. The two share the ONE aggregate underneath (DailySummarySql.RangeSql), which is -/// what stops them ever disagreeing about a day. +/// Why the range is a SIBLING rather than a wider get_daily_summary. Three reasons, and the first +/// is mechanical. (1) The response SHAPE is the contract: get_daily_summary returns a flat object of scalars, +/// which is what a stat tile consumes and what its own description promises; a range returns rows. A single +/// tool that returned either depending on whether a span argument arrived would make every consumer branch on +/// a parameter it may not have sent. (2) They are different questions with different defaults: "how was +/// Tuesday" versus "which of the last thirty days were bad", the second of which is a screening read whose +/// answer is the shape of the month rather than one day's numbers. (3) get_daily_summary is a shipped name on +/// both SKUs; changing its payload shape would break callers for no gain. The two share the ONE aggregate +/// underneath (DailySummarySql.RangeSql), which is what stops them ever disagreeing about a day. The web +/// server page used to fetch both, the tile beside the month grid; since #3905 it draws the tile from the +/// range's anchor-day row, one fetch for two panels, which changes neither tool. /// [McpServerToolType] public sealed class DarlingMcpHealthTools diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js index 6466c6c9be..d253346bf9 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js @@ -483,8 +483,20 @@ export function fileIoPanel(server, ctx) { } /** - * The Daily Health Calendar (#2484): the desktop viewer's month grid as a table, plus the one disclosure its - * rows cannot carry on their own. + * The Overview tab's two daily-summary panels, today's tile and the Daily Health Calendar (#2484), over ONE + * fetch of get_daily_summary_range (#3905). + * + * They used to be two reads: get_daily_summary for the tile and the range for the grid. On the largest + * production store that pair was 26 of the tab's 28 seconds, mostly for store-side reasons fixed in the + * statement and the coverage gate, but the tile's read also recomputed a day the range had just computed. The + * range already holds today: its last day is the anchor day, `to_date`, banded against the same clock off the + * same aggregate. So the tile is that row, and the store runs one statement per load instead of two. One + * figure can change, and the change makes the two panels agree: the tile's unique_queries for today is now + * the range's own count, which comes from the rollup tier's materialized hours when the month routes to a + * rollup (#3653 A6), where the single-day read counted raw. Before, the tile and the grid's last row could + * print different numbers for the same day on the same page. The row is matched on the server's `to_date` + * rather than on a date the page computes (R1). A day with nothing collected yet has no row, and the tile + * says so rather than rendering dashes. * * #3653 A6: a day the store's rollup tier never materialized for this server answers `unique_queries: null` * (NOT 0 — the tier did not carry the day; nothing says the server ran no queries) and the read names every @@ -496,13 +508,30 @@ export function fileIoPanel(server, ctx) { * 30 days and says so, deliberately not ctx.hours — the page's range tops out well short of a month, and a * calendar drawn over six hours is not a calendar. */ -export function dailyCalendarPanel(server) { - const { panel, body } = panelShell("Daily Health Calendar", "last 30 days (UTC)"); +export function dailySummaryPanels(server) { + const tile = panelShell("Daily Summary", "today (UTC)"); + const calendar = panelShell("Daily Health Calendar", "last 30 days (UTC)"); (async () => { const res = await readTool("get_daily_summary_range", { server, days_back: 30 }); - if (res.kind === "error") return mount(body, readErrorStrip(res.message)); - if (res.kind === "empty") return mount(body, emptyStrip(res.message)); + if (res.kind === "error") { + mount(tile.body, readErrorStrip(res.message)); + return mount(calendar.body, readErrorStrip(res.message)); + } + if (res.kind === "empty") { + mount(tile.body, emptyStrip(res.message)); + return mount(calendar.body, emptyStrip(res.message)); + } + + const days = Array.isArray(res.data.days) ? res.data.days : []; + const today = days.find((day) => day.summary_date === res.data.to_date); + mount( + tile.body, + today + ? VIZ.stat(today, { stats: DAILY_STATS }) + : emptyStrip("Nothing has been collected for this server yet on " + res.data.to_date + " (UTC).") + ); + const body = calendar.body; const grid = VIZ.table(res.data, { rowsKey: "days", columns: DAILY_RANGE_COLUMNS, @@ -522,7 +551,7 @@ export function dailyCalendarPanel(server) { grid, ]); })(); - return panel; + return [tile.panel, calendar.panel]; } /** Reshape flat rows into per-series points, keeping the top `maxSeries` series by peak value. */ @@ -654,12 +683,11 @@ export const SERVER_TABS = [ ctx.label, "No findings in this window. Findings are written by the analysis pass, which needs at least 24 hours of collected history." ), - stat("Daily Summary", "get_daily_summary", { server }, DAILY_STATS, "today (UTC)", 2), - /* #2484: the month range behind the desktop viewer's Performance Calendar. A SECOND read rather than a - wider get_daily_summary, which is also why it can sit beside the tile above: a tab must not fetch one - read twice, so the today tile and the month grid cannot be the same read. A composite since #3653 A6 - (dailyCalendarPanel) so the read's days_missing[] can be rendered as one line above the grid. */ - dailyCalendarPanel(server), + /* #2484: today's tile and the month range behind the desktop viewer's Performance Calendar. #3905: both + from ONE fetch of get_daily_summary_range, today's tile being the range's anchor-day row, where they + used to be two reads (get_daily_summary beside the range) that cost the store the day's aggregate + twice per load. See dailySummaryPanels. */ + ...dailySummaryPanels(server), ], }, @@ -2379,7 +2407,7 @@ const PROPERTY_STATS = [ const DAILY_STATS = [ { key: "summary_date", label: "Date", format: "text", small: true }, - /* One health card, not two. get_daily_summary returns overall_health as the band's LABEL + /* One health card, not two. The daily-summary row carried overall_health as the band's LABEL (DarlingHealthReader: OverallHealth => DailyHealthBandCalculator.Label(HealthBand)) — the same value health_band carries — so an "overall_health" card duplicated this one AND, formatted as num1, rendered the label "Critical" as NaN. #2807. A numeric health SCORE would be a new backend field, not this label. */ @@ -2402,7 +2430,7 @@ const DAILY_RANGE_COLUMNS = [ { key: "total_wait_time_sec", label: "Total wait (s)", format: "int" }, /* #3653 A6: null is "not carried at this tier" (the rollup never materialized this server's day), which is the opposite claim from 0 and from the page's "—" ("there was none of this"), so it gets its own words; - dailyCalendarPanel lists the same days above the grid from the read's days_missing[]. */ + dailySummaryPanels lists the same days above the grid from the read's days_missing[]. */ { key: "unique_queries", label: "Unique queries", diff --git a/Darling/PerformanceMonitor.Darling.Storage/DailySummarySql.cs b/Darling/PerformanceMonitor.Darling.Storage/DailySummarySql.cs index 4dba39db73..791346d7ff 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/DailySummarySql.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DailySummarySql.cs @@ -54,10 +54,15 @@ FROM wait_totals t LEFT JOIN wait_top tp ON tp.d = t.d ), queries AS ( - SELECT date_trunc('day', collection_time) AS d, COUNT(DISTINCT query_hash) AS c - FROM v_query_stats - WHERE server_id = $1 AND collection_time >= $2 AND collection_time < $3 - GROUP BY 1 + /* #3905: distinct (day, hash) pairs, then counted -- the distinct-hash count per day, in a shape + the planner can hash rather than sort. See DailySummarySql.QueriesCteRaw. */ + SELECT x.d, COUNT(x.query_hash) AS c + FROM ( + SELECT DISTINCT date_trunc('day', collection_time) AS d, query_hash + FROM v_query_stats + WHERE server_id = $1 AND collection_time >= $2 AND collection_time < $3 + ) AS x + GROUP BY x.d ), deadlocks AS ( SELECT date_trunc('day', collection_time) AS d, COUNT(*) AS c @@ -202,13 +207,37 @@ ORDER BY s.d /// The queries CTE exactly as it appears in — the ONE part of the /// daily summary that reads a table subject to the 4-day raw drop. Every other source here (wait_stats, /// deadlocks, alerts, memory) keeps DarlingRetention's 30-day default, so they need no routing. + /// + /// No other CTE has a tier to route to (#3905, checked rather than assumed). The continuous + /// aggregates over this statement's other sources are the anomaly baseline's per-collection supplies and + /// the fleet card's collection-health rollup, and none of them answers its CTE exactly: the wait supply + /// sums every wait type into one figure (no top wait), the blocked-process supply counts reports but + /// carries no wait time (no peak block), the deadlock supply's 35-day horizon is shorter than a raw + /// retention an operator may lengthen, and collection_health_hourly keeps eight days of a month. + /// CPU, memory pressure and the DMV blocking snapshots have no aggregate at all. The eight raw CTEs are a + /// bounded per-server read of the window; what made the month read slow was this CTE's routed form (the + /// not-carried probes, ) and the sort its COUNT(DISTINCT) forced, + /// both fixed here. + /// + /// Distinct pairs, then counted (#3905). COUNT(x.query_hash) over the distinct + /// (day, query_hash) pairs is COUNT(DISTINCT query_hash) per day, NULL hash included (both + /// skip it), but PostgreSQL evaluates a DISTINCT aggregate by SORTING every input row of its group, while + /// a SELECT DISTINCT can be hashed. On a busy server's month of hourly rollup rows that sort was + /// the calendar's largest remaining cost once the probes were fixed: 1.10 s for 573,600 rows on the rig + /// against 0.24 s for the same rows hashed, and 140 ms against 48 ms for a single raw day. The same shape + /// is used on every tier, so every tier still counts the same thing. /// private const string QueriesCteRaw = """ queries AS ( - SELECT date_trunc('day', collection_time) AS d, COUNT(DISTINCT query_hash) AS c - FROM v_query_stats - WHERE server_id = $1 AND collection_time >= $2 AND collection_time < $3 - GROUP BY 1 + /* #3905: distinct (day, hash) pairs, then counted -- the distinct-hash count per day, in a shape + the planner can hash rather than sort. See DailySummarySql.QueriesCteRaw. */ + SELECT x.d, COUNT(x.query_hash) AS c + FROM ( + SELECT DISTINCT date_trunc('day', collection_time) AS d, query_hash + FROM v_query_stats + WHERE server_id = $1 AND collection_time >= $2 AND collection_time < $3 + ) AS x + GROUP BY x.d ), """; @@ -270,9 +299,25 @@ GROUP BY 1 /// by its retention state. A day BELOW the relation's floor that the source still holds /// (the straddle the router's essay accepts "with no signal to the caller") answers the same two probes /// the same way, so this is also that reader's partial-coverage notice, for as long as the source holds - /// the day. Each probe is one index range per day — the materialization's (server_id, bucket) - /// group index, the source's (server_id, time) index — over a series of at most a year of days, so - /// the cost follows the window's length and not the tables' weight, the scan's own argument. + /// the day. + /// + /// Each probe is one index range per day, and the SQL has to say so (#3905). This paragraph + /// used to end "each probe is one index range per day, so the cost follows the window's length and not the + /// tables' weight". That was the intent, not the plan. Written as a bare EXISTS and NOT EXISTS, + /// PostgreSQL pulls both up into a semi-join and an anti-join, and neither join can push its per-day range + /// into the scan, so the planner materialized the server's ENTIRE history in the probed relation (the + /// source's four days of raw rows, or the rollup's 90 days) and rescanned it once per day of the window. + /// That is O(days × retained rows), and past work_mem it spills and rereads a temp file per day. On + /// the rig (one busy server: 570,600 raw rows, 880,800 hourly) this member alone was 4.5 s of a cold 6.1 s + /// month read, 32,678 temp blocks read, and a 120-day read at the daily tier, whose source probe runs over + /// the hourly's 90 days, took 12.4 s. On DARLING01's busiest server it was 629 of 873 ms. So each probe now + /// carries OFFSET 0. PostgreSQL refuses to pull a sub-select with an OFFSET into a join + /// (simplify_EXISTS_query: "OFFSET 0 ... traditionally is used as an optimization fence"), which + /// leaves each probe a SubPlan run once per candidate day: an index range on the materialization's + /// (server_id, bucket) index and on the source's time index, pruned to one chunk at run time. The + /// candidate days are fenced the same way, so the source probe runs only for the days the rollup probe + /// found empty, which on a covered window is none. At that scale the member is now 10 ms, the month read + /// 186 ms and the 120-day read 161 ms, whatever the relations weigh. /// /// A bucket holding zero distinct hashes is not a case. The rollup groups by query_hash, /// so a bucket exists only where a source row did, and every bucket carries at least one hash: a row in the @@ -311,33 +356,48 @@ FROM collect.{relation} WHERE server_id = $1 ), queries AS ( - SELECT date_trunc('day', bucket) AS d, COUNT(DISTINCT query_hash) AS c - FROM collect.{relation} - WHERE server_id = $1 AND bucket >= $2 AND bucket < $3 - GROUP BY 1 + SELECT x.d, COUNT(x.query_hash) AS c + FROM ( + SELECT DISTINCT date_trunc('day', bucket) AS d, query_hash + FROM collect.{relation} + WHERE server_id = $1 AND bucket >= $2 AND bucket < $3 + ) AS x + GROUP BY x.d UNION ALL - SELECT date_trunc('day', collection_time) AS d, COUNT(DISTINCT query_hash) AS c - FROM v_query_stats - WHERE server_id = $1 AND collection_time >= $2 AND collection_time < $3 - AND collection_time >= COALESCE((SELECT last_day + INTERVAL '1 day' FROM queries_ceiling), $2) - GROUP BY 1 + SELECT x.d, COUNT(x.query_hash) AS c + FROM ( + SELECT DISTINCT date_trunc('day', collection_time) AS d, query_hash + FROM v_query_stats + WHERE server_id = $1 AND collection_time >= $2 AND collection_time < $3 + AND collection_time >= COALESCE((SELECT last_day + INTERVAL '1 day' FROM queries_ceiling), $2) + ) AS x + GROUP BY x.d UNION ALL /* #3653 A6: the days at or below this server's ceiling that the rollup holds NO row for while its source still holds rows the rollup's own WHERE admits -- "not carried at this tier". One row per such day with a NULL count; the outer select passes it through as unique_queries = NULL and the readers list the day in days_missing[]. The two probes are the hole scan's (TimescaleSupport.MaterializationHoleScanSql, the #3731 repair), per server, at day grain. A day - the source has no admitted row for is not named: the rollup is honestly empty there. */ + the source has no admitted row for is not named: the rollup is honestly empty there. + #3905: every OFFSET 0 below is load-bearing. It keeps each probe a per-day index probe instead of + a join over the server's whole history, and the source is probed only on the days the rollup + probe found empty. */ SELECT b.d, NULL::bigint AS c - FROM generate_series(date_trunc('day', $2::timestamp), date_trunc('day', $3::timestamp), INTERVAL '1 day') AS b(d) - WHERE b.d < $3 - AND b.d < COALESCE((SELECT last_day + INTERVAL '1 day' FROM queries_ceiling), $2) - AND NOT EXISTS ( - SELECT 1 FROM collect.{relation} AS r - WHERE r.server_id = $1 AND r.bucket >= b.d AND r.bucket < b.d + INTERVAL '1 day') - AND EXISTS ( + FROM ( + SELECT g.d + FROM generate_series(date_trunc('day', $2::timestamp), date_trunc('day', $3::timestamp), INTERVAL '1 day') AS g(d) + WHERE g.d < $3 + AND g.d < COALESCE((SELECT last_day + INTERVAL '1 day' FROM queries_ceiling), $2) + AND NOT EXISTS ( + SELECT 1 FROM collect.{relation} AS r + WHERE r.server_id = $1 AND r.bucket >= g.d AND r.bucket < g.d + INTERVAL '1 day' + OFFSET 0) + OFFSET 0 + ) AS b + WHERE EXISTS ( SELECT 1 FROM collect.{target.Source} AS s - WHERE s.server_id = $1 AND s.{target.SourceTimeColumn} >= b.d AND s.{target.SourceTimeColumn} < b.d + INTERVAL '1 day'{sourceFilter}) + WHERE s.server_id = $1 AND s.{target.SourceTimeColumn} >= b.d AND s.{target.SourceTimeColumn} < b.d + INTERVAL '1 day'{sourceFilter} + OFFSET 0) ), """; } From 0863ffbd2d1c0fc4268854ac4c56db8325c484ef Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 22 Sep 2026 19:41:14 -0400 Subject: [PATCH 2/3] Pin the viewer's daily SQL to the hashed count and probe TimescaleDB on its own connection (#3905) ViewerDailySummarySqlTests asserted COUNT(DISTINCT query_hash) in the shared statement; it now asserts the distinct-pair shape. The read-shape live seed enables TimescaleDB through LiveTimescaleProbe (#1922), and disposes its scratch database on a skip. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018Z4T51J4E6L8fjriPHpsif --- .../DailySummaryReadShapeTests.cs | 77 ++++++++++--------- .../Darling.Tests/ViewerDailyHealthTests.cs | 5 +- 2 files changed, 45 insertions(+), 37 deletions(-) diff --git a/Darling/Darling.Tests/DailySummaryReadShapeTests.cs b/Darling/Darling.Tests/DailySummaryReadShapeTests.cs index cd5648478e..a793b8565e 100644 --- a/Darling/Darling.Tests/DailySummaryReadShapeTests.cs +++ b/Darling/Darling.Tests/DailySummaryReadShapeTests.cs @@ -377,7 +377,7 @@ public async Task TheNotCarriedProbes_RunOncePerDay_AndTheSourceOnlyWhereTheRoll /* The legacy hourly's ceiling is D5 (the seed materializes D0, D3, D4 and D5); every window day at or below it is a candidate, and the ones the rollup holds no row for are probed at the source. */ - var candidates = Enumerable.Range(0, (days[5] - from).Days + 1).Count(); + var candidates = (days[5] - from).Days + 1; var rollupEmpty = candidates - 4; var candidateDays = inQueries.Single(n => NodeType(n) == "Function Scan" @@ -434,14 +434,7 @@ public async Task EveryDailyRead_ProbesCoverageOnce_HoweverManyRace_AgainstDevPo /* ───────────────────────────── seed ───────────────────────────── */ - /// - /// Nine whole UTC days, D0 thirteen days back, all past RawMaxAge and below every refresh policy's window, - /// so a background policy run cannot materialize what the seed leaves unmaterialized. D0: 3 hashes. D1: - /// nothing. D2: 5 hashes, skipped by the hourly and the daily (the hole). D3: 4 hashes and a NULL-hash row. - /// D4: one restart row (interval 0). D5: 6. D6: 2. D7: 3. D8: 1. Refreshed: the legacy hourly over D0-D1 and - /// D3-D5 (ceiling D5); the successor over D0 and D3 (ceiling D3; it rejects D4's restart row); the daily over - /// D0 and D3 (ceiling D3). - /// + /// A seeded scratch store: its open connection and the seed's days, disposed together. private sealed class Seeded(ScratchPostgres scratch, NpgsqlConnection connection, DateTime[] days) : IAsyncDisposable { public NpgsqlConnection Connection { get; } = connection; @@ -455,6 +448,14 @@ public async ValueTask DisposeAsync() } } + /// + /// Nine whole UTC days, D0 thirteen days back, all past RawMaxAge and below every refresh policy's window, + /// so a background policy run cannot materialize what the seed leaves unmaterialized. D0: 3 hashes. D1: + /// nothing. D2: 5 hashes, skipped by the hourly and the daily (the hole). D3: 4 hashes and a NULL-hash row. + /// D4: one restart row (interval 0). D5: 6. D6: 2. D7: 3. D8: 1. Refreshed: the legacy hourly over D0-D1 and + /// D3-D5 (ceiling D5); the successor over D0 and D3 (ceiling D3; it rejects D4's restart row); the daily over + /// D0 and D3 (ceiling D3). + /// private static async Task SeedAsync(string what) { var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); @@ -464,39 +465,45 @@ private static async Task SeedAsync(string what) var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); var connection = new NpgsqlConnection(scratch.ConnectionString); - await connection.OpenAsync(ct); - await PgMigrations.MigrateAsync(connection, ct); - if (!await TimescaleSupport.TryEnableAsync(connection, null, ct)) + try { - await connection.DisposeAsync(); - await scratch.DisposeAsync(); - Assert.Skip($"{what} needs TimescaleDB: a rollup tier exists only as a continuous aggregate."); - } + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + Assert.SkipWhen(!await LiveTimescaleProbe.TryEnableAsync(scratch.ConnectionString, ct), + $"{what} needs TimescaleDB: a rollup tier exists only as a continuous aggregate."); - await TimescaleSupport.ConvertToHypertablesAsync(connection, null, ct); - await TimescaleSupport.EnsureContinuousAggregatesAsync(connection, null, ct); - await RegisterServerAsync(connection, ct); + await TimescaleSupport.ConvertToHypertablesAsync(connection, null, ct); + await TimescaleSupport.EnsureContinuousAggregatesAsync(connection, null, ct); + await RegisterServerAsync(connection, ct); - var d0 = DateTime.SpecifyKind(DateTime.UtcNow.Date.AddDays(-13), DateTimeKind.Unspecified); - var days = Enumerable.Range(0, 9).Select(n => d0.AddDays(n)).ToArray(); - Assert.True(DateTime.UtcNow - days[^1].AddDays(1) > TimescaleSupport.DailyRefreshStartSpan, "the seed must sit below every refresh policy's window"); + var d0 = DateTime.SpecifyKind(DateTime.UtcNow.Date.AddDays(-13), DateTimeKind.Unspecified); + var days = Enumerable.Range(0, 9).Select(n => d0.AddDays(n)).ToArray(); + Assert.True(DateTime.UtcNow - days[^1].AddDays(1) > TimescaleSupport.DailyRefreshStartSpan, "the seed must sit below every refresh policy's window"); - foreach (var (day, hashes) in new[] { (0, 3), (2, 5), (3, 4), (5, 6), (6, 2), (7, 3), (8, 1) }) - { - await PlantDayAsync(connection, days[day], hashes, ct); - } + foreach (var (day, hashes) in new[] { (0, 3), (2, 5), (3, 4), (5, 6), (6, 2), (7, 3), (8, 1) }) + { + await PlantDayAsync(connection, days[day], hashes, ct); + } - await PlantRowAsync(connection, days[3].AddHours(12), queryHash: null, intervalSeconds: 1200, ct); - await PlantRowAsync(connection, days[4].AddHours(6), queryHash: "0xRESTART", intervalSeconds: 0, ct); + await PlantRowAsync(connection, days[3].AddHours(12), queryHash: null, intervalSeconds: 1200, ct); + await PlantRowAsync(connection, days[4].AddHours(6), queryHash: "0xRESTART", intervalSeconds: 0, ct); - await RefreshAsync(connection, TimescaleSupport.QueryStatsHourlyView, days[0], days[2], ct); - await RefreshAsync(connection, TimescaleSupport.QueryStatsHourlyView, days[3], days[6], ct); - await RefreshAsync(connection, TimescaleSupport.QueryStatsIntervalHourlyView, days[0], days[1], ct); - await RefreshAsync(connection, TimescaleSupport.QueryStatsIntervalHourlyView, days[3], days[5], ct); - await RefreshAsync(connection, TimescaleSupport.QueryStatsDailyView, days[0], days[1], ct); - await RefreshAsync(connection, TimescaleSupport.QueryStatsDailyView, days[3], days[4], ct); + await RefreshAsync(connection, TimescaleSupport.QueryStatsHourlyView, days[0], days[2], ct); + await RefreshAsync(connection, TimescaleSupport.QueryStatsHourlyView, days[3], days[6], ct); + await RefreshAsync(connection, TimescaleSupport.QueryStatsIntervalHourlyView, days[0], days[1], ct); + await RefreshAsync(connection, TimescaleSupport.QueryStatsIntervalHourlyView, days[3], days[5], ct); + await RefreshAsync(connection, TimescaleSupport.QueryStatsDailyView, days[0], days[1], ct); + await RefreshAsync(connection, TimescaleSupport.QueryStatsDailyView, days[3], days[4], ct); - return new Seeded(scratch, connection, days); + return new Seeded(scratch, connection, days); + } + catch + { + /* A skip is an exception too: the scratch database goes with it either way. */ + await connection.DisposeAsync(); + await scratch.DisposeAsync(); + throw; + } } private static async Task PlantDayAsync(NpgsqlConnection connection, DateTime day, int hashes, CancellationToken ct) diff --git a/Darling/Darling.Tests/ViewerDailyHealthTests.cs b/Darling/Darling.Tests/ViewerDailyHealthTests.cs index ead6358e24..e5a05fe577 100644 --- a/Darling/Darling.Tests/ViewerDailyHealthTests.cs +++ b/Darling/Darling.Tests/ViewerDailyHealthTests.cs @@ -43,8 +43,9 @@ public void DailySummaryRangeSql_RollsUpEverySource_GroupedPerDay() Assert.Contains("DISTINCT ON (d)", sql, StringComparison.Ordinal); Assert.Contains("ORDER BY d, ms DESC", sql, StringComparison.Ordinal); - /* Distinct query count, deadlock count. */ - Assert.Contains("COUNT(DISTINCT query_hash)", sql, StringComparison.Ordinal); + /* Distinct query count (the distinct day/hash pairs counted, #3905's hashable spelling), deadlock count. */ + Assert.Contains("SELECT DISTINCT date_trunc('day', collection_time) AS d, query_hash", sql, StringComparison.Ordinal); + Assert.Contains("SELECT x.d, COUNT(x.query_hash) AS c", sql, StringComparison.Ordinal); Assert.Contains("FROM v_query_stats", sql, StringComparison.Ordinal); Assert.Contains("FROM v_deadlocks", sql, StringComparison.Ordinal); From 0655b9e526caa219450effbba6209a11ba244547 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 22 Sep 2026 20:00:31 -0400 Subject: [PATCH 3/3] The daily summary never routes on a failed coverage probe's raw fallback, and each Overview panel renders on its own (#3905) A failed shared probe is cached as RollupAvailability.None, which routes every window to raw. That is the compose panels' accepted fallback, but on a store whose raw purges are armed it prints unique_queries = 0 for every purged day of a calendar. Before #3905 the daily reader's per-call probe failed the read instead. The cache now marks a failed answer, and the daily reader's accessor (GetMeasuredRollupsAsync) probes again on its own token rather than route on it, so a second failure surfaces as the tool's error. The Overview composite renders each panel through its own guard, so a render fault cannot leave both on their loading strips, and today's tile gets its own sentence for an empty range. Found in review. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018Z4T51J4E6L8fjriPHpsif --- .../DailySummaryNotCarriedTests.cs | 2 +- .../DailySummaryReadShapeTests.cs | 110 +++++++++++++++--- .../Compose/ComposeStoreAvailability.cs | 95 +++++++++++---- .../Mcp/DarlingHealthReader.cs | 12 +- .../wwwroot/js/pages/server-tabs.js | 67 ++++++----- 5 files changed, 214 insertions(+), 72 deletions(-) diff --git a/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs b/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs index 6d097b1c2f..6ee2f37f9d 100644 --- a/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs +++ b/Darling/Darling.Tests/DailySummaryNotCarriedTests.cs @@ -234,7 +234,7 @@ public void EveryConsumer_KeepsTheNull_AndSpellsTheDisclosureOneWay() var page = RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "pages", "server-tabs.js"); Assert.Contains("export function dailySummaryPanels(server)", page, StringComparison.Ordinal); - Assert.Contains("res.data.days_missing", page, StringComparison.Ordinal); + Assert.Contains("Array.isArray(data.days_missing) ? data.days_missing : []", page, StringComparison.Ordinal); Assert.Contains("row.unique_queries == null", page, StringComparison.Ordinal); Assert.Contains("[\"not materialized\"]", page, StringComparison.Ordinal); Assert.Contains("...dailySummaryPanels(server),", page, StringComparison.Ordinal); diff --git a/Darling/Darling.Tests/DailySummaryReadShapeTests.cs b/Darling/Darling.Tests/DailySummaryReadShapeTests.cs index a793b8565e..b09c75dd22 100644 --- a/Darling/Darling.Tests/DailySummaryReadShapeTests.cs +++ b/Darling/Darling.Tests/DailySummaryReadShapeTests.cs @@ -66,15 +66,17 @@ public void EveryTier_CountsDistinctPairs_NeverADistinctAggregate() } /// - /// The seam, as source: the MCP reader routes on the cached gate the compose panels read, and no longer runs - /// either probe itself. A per-call probe is what put a >= 1.7 s coverage read in front of every daily read, - /// twice per server-page load. The live half counts the probes. + /// The seam, as source: the MCP reader routes on the cached gate the compose panels read, through the + /// accessor that refuses a failed probe's raw fallback, and no longer runs either probe itself. A per-call + /// probe is what put a >= 1.7 s coverage read in front of every daily read, twice per server-page load. The + /// live half counts the probes. /// [Fact] public void TheDailyReader_RoutesOnTheCachedGate_NotAPerCallProbe() { var reader = RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Service", "Mcp", "DarlingHealthReader.cs"); - Assert.Contains("await ComposeStoreAvailability.GetRollupsAsync(postgres, cancellationToken);", reader, StringComparison.Ordinal); + Assert.Contains("await ComposeStoreAvailability.GetMeasuredRollupsAsync(postgres, cancellationToken);", reader, StringComparison.Ordinal); + Assert.DoesNotContain("ComposeStoreAvailability.GetRollupsAsync(", reader, StringComparison.Ordinal); Assert.DoesNotContain("TimescaleSupport.DetectRollupsAsync(", reader, StringComparison.Ordinal); Assert.DoesNotContain("TimescaleSupport.DetectRollupCoverageAsync(", reader, StringComparison.Ordinal); } @@ -102,18 +104,21 @@ public void TheOverviewTab_DrawsTodaysTileFromTheRangeRead_OneFetchForTwoPanels( composite = composite[..composite.IndexOf("\n}", StringComparison.Ordinal)]; Assert.Equal(1, composite.Split("readTool(").Length - 1); Assert.Contains("readTool(\"get_daily_summary_range\", { server, days_back: 30 })", composite, StringComparison.Ordinal); - Assert.Contains("days.find((day) => day.summary_date === res.data.to_date)", composite, StringComparison.Ordinal); + Assert.Contains("days.find((day) => day.summary_date === data.to_date)", composite, StringComparison.Ordinal); + /* Each panel renders through its own guard, so a fault in one cannot strand the other on its loading strip. */ + Assert.Equal(2, composite.Split("renderInto(").Length - 1); Assert.Contains("VIZ.stat(today, { stats: DAILY_STATS })", composite, StringComparison.Ordinal); Assert.Contains("return [tile.panel, calendar.panel];", composite, StringComparison.Ordinal); } } /// -/// #3905: runs one coverage probe per data source at a time. The web -/// server page fires its daily reads together, and on the largest production store the probe is >= 1.7 s, so a -/// stale cache refreshed by every concurrent caller was that cost times the callers. Driven through the probe -/// seam with a gated fake, the pattern set for the fleet memo, so -/// callers can be parked on the probe before it answers, which is the only way to prove they shared it. +/// #3905: runs one coverage probe per data source at a time. Agents race +/// their daily reads, the compose panels and trend tools read the same gate, and on the largest production store +/// the probe is >= 1.7 s, so a stale cache refreshed by every concurrent caller was that cost times the callers. +/// Driven through the probe seam with a gated fake, the pattern set +/// for the fleet memo, so callers can be parked on the probe before it answers, which is the only way to prove +/// they shared it. /// public sealed class ComposeStoreAvailabilitySingleFlightTests { @@ -215,6 +220,70 @@ public async Task AFailedProbe_AnswersRawForEveryWaiter_AndIsCached() Assert.Equal(RollupAvailability.None, (await ComposeStoreAvailability.GetRollupsAsync(dataSource, probe.Run, CancellationToken.None)).Rollups); Assert.Equal(1, probe.Calls); } + + /// + /// The daily summary never routes on that fallback. On a TimescaleDB store whose raw purges are armed, + /// raw-for-everything prints unique_queries = 0 for every day raw has purged, a zero that is not NULL and + /// not in days_missing. So while the cache holds a failure, the measured accessor probes again itself on + /// its caller's token. A success is used (and not written back: the compose panels keep their cached + /// fallback), and a second failure surfaces to the caller, the way the per-call probe failed the read + /// before #3905. A measured answer is the cached one, with no extra probe. + /// + [Fact] + public async Task TheMeasuredAccessor_NeverRoutesOnAFailedProbesFallback() + { + await using var dataSource = UnopenedDataSource(); + var calls = 0; + var outcomes = new Queue>(new Func<(RollupAvailability, RollupCoverage)>[] + { + () => throw new NpgsqlException("57014: canceling statement due to statement timeout"), + () => Answer, + () => throw new NpgsqlException("57014: again"), + }); + var tokens = new List(); + Task<(RollupAvailability Rollups, RollupCoverage Coverage)> Probe(NpgsqlDataSource _, CancellationToken token) + { + calls++; + tokens.Add(token); + return Task.FromResult<(RollupAvailability, RollupCoverage)>(outcomes.Dequeue()()); + } + + using var caller = new CancellationTokenSource(); + + /* The shared probe fails and is cached as the fallback; the measured accessor probes again, on its own + token, and routes on that answer. */ + var measured = await ComposeStoreAvailability.GetMeasuredRollupsAsync(dataSource, Probe, caller.Token); + Assert.Equal(RollupAvailability.All, measured.Rollups); + Assert.Equal(2, calls); + Assert.Equal(new[] { CancellationToken.None, caller.Token }, tokens); + Assert.Equal(1, ComposeStoreAvailability.ProbesStartedFor(dataSource)); + + /* Not written back: the compose accessor still reads the cached fallback. */ + Assert.Equal(RollupAvailability.None, (await ComposeStoreAvailability.GetRollupsAsync(dataSource, Probe, CancellationToken.None)).Rollups); + Assert.Equal(2, calls); + + /* A second failure is the caller's error, not a quiet raw route. */ + var ex = await Assert.ThrowsAsync(() => ComposeStoreAvailability.GetMeasuredRollupsAsync(dataSource, Probe, caller.Token).AsTask()); + Assert.Contains("again", ex.Message, StringComparison.Ordinal); + Assert.Equal(3, calls); + } + + /// And on a healthy store the measured accessor is the cache: one shared probe, then memory. + [Fact] + public async Task TheMeasuredAccessor_ReadsAMeasuredAnswerFromTheCache() + { + await using var dataSource = UnopenedDataSource(); + var probe = new GatedProbe(); + + var first = ComposeStoreAvailability.GetMeasuredRollupsAsync(dataSource, probe.Run, CancellationToken.None).AsTask(); + var alongside = ComposeStoreAvailability.GetRollupsAsync(dataSource, probe.Run, CancellationToken.None).AsTask(); + probe.Gate.SetResult(Answer); + + Assert.Equal(RollupAvailability.All, (await first).Rollups); + Assert.Equal(RollupAvailability.All, (await alongside).Rollups); + Assert.Equal(RollupAvailability.All, (await ComposeStoreAvailability.GetMeasuredRollupsAsync(dataSource, probe.Run, CancellationToken.None)).Rollups); + Assert.Equal(1, probe.Calls); + } } /// @@ -342,6 +411,13 @@ public async Task TheRewrittenStatement_AnswersExactlyWhatTheOldOneDid_OnEveryTi var daily = answers[RetentionTier.Daily + "/" + TimescaleSupport.QueryStatsHourlyView]; Assert.DoesNotContain(daily, row => row.StartsWith(Day(2) + "|", StringComparison.Ordinal)); Assert.Contains(daily, row => row.StartsWith(Day(8) + "|", StringComparison.Ordinal) && row.Split('|')[3] == "1"); + + /* A day whose only hash is NULL is still a day, counted 0, on every tier: the distinct pairs keep the + (day, NULL) pair and COUNT skips its NULL, where dropping the pair would have lost the day. */ + foreach (var answer in answers.Values) + { + Assert.Contains(answer, row => row.StartsWith(Day(9) + "|", StringComparison.Ordinal) && row.Split('|')[3] == "0"); + } } /// @@ -449,12 +525,13 @@ public async ValueTask DisposeAsync() } /// - /// Nine whole UTC days, D0 thirteen days back, all past RawMaxAge and below every refresh policy's window, + /// Ten whole UTC days, D0 fourteen days back, all past RawMaxAge and below every refresh policy's window, /// so a background policy run cannot materialize what the seed leaves unmaterialized. D0: 3 hashes. D1: /// nothing. D2: 5 hashes, skipped by the hourly and the daily (the hole). D3: 4 hashes and a NULL-hash row. - /// D4: one restart row (interval 0). D5: 6. D6: 2. D7: 3. D8: 1. Refreshed: the legacy hourly over D0-D1 and - /// D3-D5 (ceiling D5); the successor over D0 and D3 (ceiling D3; it rejects D4's restart row); the daily over - /// D0 and D3 (ceiling D3). + /// D4: one restart row (interval 0). D5: 6. D6: 2. D7: 3. D8: 1. D9: a NULL-hash row and nothing else, past + /// every ceiling, so every tier counts it from raw: the day still appears, with 0. Refreshed: the legacy + /// hourly over D0-D1 and D3-D5 (ceiling D5); the successor over D0 and D3 (ceiling D3; it rejects D4's + /// restart row); the daily over D0 and D3 (ceiling D3). /// private static async Task SeedAsync(string what) { @@ -476,8 +553,8 @@ private static async Task SeedAsync(string what) await TimescaleSupport.EnsureContinuousAggregatesAsync(connection, null, ct); await RegisterServerAsync(connection, ct); - var d0 = DateTime.SpecifyKind(DateTime.UtcNow.Date.AddDays(-13), DateTimeKind.Unspecified); - var days = Enumerable.Range(0, 9).Select(n => d0.AddDays(n)).ToArray(); + var d0 = DateTime.SpecifyKind(DateTime.UtcNow.Date.AddDays(-14), DateTimeKind.Unspecified); + var days = Enumerable.Range(0, 10).Select(n => d0.AddDays(n)).ToArray(); Assert.True(DateTime.UtcNow - days[^1].AddDays(1) > TimescaleSupport.DailyRefreshStartSpan, "the seed must sit below every refresh policy's window"); foreach (var (day, hashes) in new[] { (0, 3), (2, 5), (3, 4), (5, 6), (6, 2), (7, 3), (8, 1) }) @@ -487,6 +564,7 @@ private static async Task SeedAsync(string what) await PlantRowAsync(connection, days[3].AddHours(12), queryHash: null, intervalSeconds: 1200, ct); await PlantRowAsync(connection, days[4].AddHours(6), queryHash: "0xRESTART", intervalSeconds: 0, ct); + await PlantRowAsync(connection, days[9].AddHours(3), queryHash: null, intervalSeconds: 1200, ct); await RefreshAsync(connection, TimescaleSupport.QueryStatsHourlyView, days[0], days[2], ct); await RefreshAsync(connection, TimescaleSupport.QueryStatsHourlyView, days[3], days[6], ct); diff --git a/Darling/PerformanceMonitor.Darling.Service/Compose/ComposeStoreAvailability.cs b/Darling/PerformanceMonitor.Darling.Service/Compose/ComposeStoreAvailability.cs index 8932924cc8..7cb519d4d7 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Compose/ComposeStoreAvailability.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Compose/ComposeStoreAvailability.cs @@ -31,16 +31,19 @@ namespace PerformanceMonitor.Darling.Service; /// internal static class ComposeStoreAvailability { + /// One probe's answer, and whether it is a measurement or the fallback a failed probe left + /// (#3905): the compose panels read both alike, the daily summary may not. + private readonly record struct ProbeAnswer(RollupAvailability Rollups, RollupCoverage Coverage, bool Failed); + private sealed class Entry { - public RollupAvailability Rollups; - public RollupCoverage Coverage = RollupCoverage.Unknown; + public ProbeAnswer Answer = new(RollupAvailability.None, RollupCoverage.Unknown, Failed: false); public bool Probed; public DateTime ProbedAtUtc; /// The probe the current callers share (#3905): non-null and incomplete while one is running. /// A completed task here is history, and the next caller who finds the cache stale starts a new one. - public Task<(RollupAvailability Rollups, RollupCoverage Coverage)>? InFlight; + public Task? InFlight; /// Probes started for this data source over its whole life. Diagnostic; see /// . @@ -68,13 +71,15 @@ private sealed class Entry /// /// One probe in flight per data source (#3905). This used to be "benignly racy": concurrent /// callers that found the cache stale each ran the probe, on the argument that it was two small lookups. - /// Neither half held. The daily-summary reader now reads coverage through here too, the web server page - /// fires its two daily reads together, and on the largest production store the coverage probe is - /// >= 1.7 s on a quiet minute, not a small lookup. So a stale cache is refreshed by exactly one probe and - /// every caller that arrives while it runs awaits that one: the single-flight shape of - /// DarlingFleetReader.CollectionHealthMemo (#3735), for its reasons. The shared probe runs on - /// , so a caller's token releases only that caller's wait, and the - /// probe cannot fault the shared task: a failure is the cached raw fallback below, as before. + /// Neither half held. The daily-summary reader now reads coverage through here too, agents race their + /// daily reads, the compose panels, trend tools and custom alerts read the same gate, and on the largest + /// production store the coverage probe is >= 1.7 s on a quiet minute, not a small lookup. So a stale cache + /// is refreshed by exactly one probe and every caller that arrives while it runs awaits that one: the + /// single-flight shape of DarlingFleetReader.CollectionHealthMemo (#3735), for its reasons. The + /// shared probe runs on , so a caller's token releases only that + /// caller's wait, and the probe cannot fault the shared task: a failure is the cached raw fallback below, + /// as before. A reader that must not route on that fallback reads + /// instead. /// internal static ValueTask<(RollupAvailability Rollups, RollupCoverage Coverage)> GetRollupsAsync( NpgsqlDataSource postgres, CancellationToken cancellationToken) @@ -89,26 +94,69 @@ private sealed class Entry NpgsqlDataSource postgres, Func> probe, CancellationToken cancellationToken) + { + var answer = await GetAnswerAsync(postgres, probe, cancellationToken); + return (answer.Rollups, answer.Coverage); + } + + /// + /// The same cached gate, for a reader whose numbers the failure fallback would falsify (#3905: the daily + /// summary). + /// + /// The fallback is , and sends + /// None to raw for any window. The compose panels accept that: raw is complete on a plain-PostgreSQL store, + /// and on a TimescaleDB one a panel without its rollup reads as a partial window. The daily summary cannot. + /// On a TimescaleDB store whose raw purges are armed, raw query_stats holds four days, so a month + /// routed to raw prints unique_queries = 0 for every older day. That is not NULL and not in + /// days_missing: it is the #3653 A6 lie, told for as long as the failure stays cached. Before #3905 + /// a failed probe failed this reader's call instead, because it probed per call and let the exception + /// through. So a cached failure is not an answer here. This reader probes again itself, on its own token, + /// exactly as it did per call before, and a probe that fails again surfaces as the caller's error. A + /// success is used and not written back: the failure fallback and its interval are the compose panels' + /// contract. Every measured answer, which is every answer on a healthy store, is the cached one. + /// + internal static ValueTask<(RollupAvailability Rollups, RollupCoverage Coverage)> GetMeasuredRollupsAsync( + NpgsqlDataSource postgres, CancellationToken cancellationToken) + => GetMeasuredRollupsAsync(postgres, ProbeAsync, cancellationToken); + + /// with the probe + /// substitutable. The shared probe gets ; the direct re-probe after a + /// cached failure gets , since it is this caller's alone. + internal static async ValueTask<(RollupAvailability Rollups, RollupCoverage Coverage)> GetMeasuredRollupsAsync( + NpgsqlDataSource postgres, + Func> probe, + CancellationToken cancellationToken) + { + var answer = await GetAnswerAsync(postgres, probe, cancellationToken); + return answer.Failed + ? await probe(postgres, cancellationToken) + : (answer.Rollups, answer.Coverage); + } + + private static async ValueTask GetAnswerAsync( + NpgsqlDataSource postgres, + Func> probe, + CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(postgres); ArgumentNullException.ThrowIfNull(probe); var entry = s_entries.GetOrCreateValue(postgres); - Task<(RollupAvailability Rollups, RollupCoverage Coverage)> shared; - TaskCompletionSource<(RollupAvailability Rollups, RollupCoverage Coverage)>? lead = null; + Task shared; + TaskCompletionSource? lead = null; lock (entry) { if (entry.Probed && DateTime.UtcNow - entry.ProbedAtUtc < ReprobeInterval) { - return (entry.Rollups, entry.Coverage); + return entry.Answer; } if (entry.InFlight is not { IsCompleted: false }) { /* RunContinuationsAsynchronously: the waiters' continuations (the rest of each panel or daily read) must not run inline on whichever thread completes the probe. */ - lead = new TaskCompletionSource<(RollupAvailability Rollups, RollupCoverage Coverage)>(TaskCreationOptions.RunContinuationsAsynchronously); + lead = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); entry.InFlight = lead.Task; entry.ProbesStarted++; } @@ -149,36 +197,35 @@ internal static int ProbesStartedFor(NpgsqlDataSource postgres) private static async Task RunProbeAsync( Entry entry, - TaskCompletionSource<(RollupAvailability Rollups, RollupCoverage Coverage)> lead, + TaskCompletionSource lead, NpgsqlDataSource postgres, Func> probe) { - RollupAvailability rollups; - RollupCoverage coverage; + ProbeAnswer answer; try { - (rollups, coverage) = await probe(postgres, CancellationToken.None); + var (rollups, coverage) = await probe(postgres, CancellationToken.None); + answer = new ProbeAnswer(rollups, coverage, Failed: false); } catch (Exception ex) { /* A store hiccup mid-probe must not fail the panel — raw is the safe answer for availability, and "no coverage evidence" leaves the age ladder in charge. The re-probe interval retries soon. Every exception, cancellation included: the probe runs on no caller's token, so nothing here - is a caller asking to stop, and the shared task must complete for every waiter. */ - rollups = RollupAvailability.None; - coverage = RollupCoverage.Unknown; + is a caller asking to stop, and the shared task must complete for every waiter. Marked Failed + so GetMeasuredRollupsAsync can refuse to route on it. */ + answer = new ProbeAnswer(RollupAvailability.None, RollupCoverage.Unknown, Failed: true); _ = ex; } lock (entry) { - entry.Rollups = rollups; - entry.Coverage = coverage; + entry.Answer = answer; entry.Probed = true; entry.ProbedAtUtc = DateTime.UtcNow; } - lead.SetResult((rollups, coverage)); + lead.SetResult(answer); } /// diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs index 3e4f895210..0c7bec0061 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingHealthReader.cs @@ -386,15 +386,17 @@ a separate question — a rollup created over pre-existing history answers old w #3905: through ComposeStoreAvailability, the gate the compose panels already read: cached per data source for its ReprobeInterval, one probe in flight at a time. This used to probe on every call on the reasoning that it "runs at human/model cadence and these are two small lookups". The web server - page calls it twice per load, and on the largest production store the coverage probe alone measured + page called it twice per load, and on the largest production store the coverage probe alone measured >= 1.7 s, because each min() over a compressed oldest chunk seq-scans and sorts that chunk's batch list. A cached floor cannot produce a wrong count, only a different tier for at most the interval: a floor moves back only on a backfill (stale, it under-claims, so a window stays where it was or degrades), and forward only on a retention drop, which RetentionTierRouter's one-day RouteMargin keeps every age-routed window clear of. The query CTE's ceiling is read live in the statement, and a day a - tier never carried is named NULL either way. The viewer's calendar has cached the same gate for the - same interval all along (ViewerDataService.GetRollupAvailabilityAsync). */ - var (rollups, coverage) = await ComposeStoreAvailability.GetRollupsAsync(postgres, cancellationToken); + tier never carried is named NULL either way. A cached FAILURE is another matter: its raw fallback + would print 0 for every day raw has purged, so the measured accessor probes again rather than route + on it, and a probe that fails again fails this call as it always did. The viewer's calendar has + cached the same gate for the same interval all along (ViewerDataService.GetRollupAvailabilityAsync). */ + var (rollups, coverage) = await ComposeStoreAvailability.GetMeasuredRollupsAsync(postgres, cancellationToken); var tier = RetentionTierRouter.Resolve( DateTime.UtcNow, fromDate, rollups.QueryGrainHourly, rollups.QueryGrainDaily, coverage.For(TimescaleSupport.QueryStatsHourlyView, TimescaleSupport.QueryStatsDailyView)); @@ -463,7 +465,7 @@ private static async Task ReadDeadlockRateThresholdsAsyn /// per-server read (#3466 lane 2), which is minus two /// choices that are the calendar's contract rather than the SQL's: the .Date truncation /// (a sweep span is sub-day and starts at the previous sweep's instant, not midnight) and the - /// per-call rollup probe (a sweep span ends at "now" and is capped at one day by + /// rollup-tier gate (a sweep span ends at "now" and is capped at one day by /// FleetSweepCadence.IntervalMinutesCeiling, so it always sits inside the 4-day raw window /// and the raw tier is correct by construction rather than by routing). /// diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js index d253346bf9..a4639c4e2c 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js @@ -518,42 +518,57 @@ export function dailySummaryPanels(server) { return mount(calendar.body, readErrorStrip(res.message)); } if (res.kind === "empty") { - mount(tile.body, emptyStrip(res.message)); + /* The range's "empty" sentence is about the span and ends in MCP parameter advice (widen days_back, move + as_of); for today's tile the fact is simpler. "unavailable" (nothing ever collected) says the same thing + to both panels, so it is the server's sentence on both. */ + mount(tile.body, emptyStrip(res.status === "empty" ? "Nothing has been collected for this server today (UTC)." : res.message)); return mount(calendar.body, emptyStrip(res.message)); } - const days = Array.isArray(res.data.days) ? res.data.days : []; - const today = days.find((day) => day.summary_date === res.data.to_date); - mount( - tile.body, - today + /* Each panel renders on its own, the way fanout() guards its specs: a render fault in one says so in that + panel instead of leaving both on their loading strips. */ + const data = res.data || {}; + const days = Array.isArray(data.days) ? data.days : []; + renderInto(tile.body, () => { + const today = days.find((day) => day.summary_date === data.to_date); + return today ? VIZ.stat(today, { stats: DAILY_STATS }) - : emptyStrip("Nothing has been collected for this server yet on " + res.data.to_date + " (UTC).") - ); - - const body = calendar.body; - const grid = VIZ.table(res.data, { - rowsKey: "days", - columns: DAILY_RANGE_COLUMNS, - emptyText: - "No collected days in this range. A day with ANY collection appears here even when every signal was quiet, so a missing day is a gap in collection rather than a quiet one.", + : emptyStrip("Nothing has been collected for this server yet " + (data.to_date ? "on " + data.to_date : "today") + " (UTC)."); }); - const missing = Array.isArray(res.data.days_missing) ? res.data.days_missing : []; - if (!missing.length) return mount(body, grid); - const plural = missing.length === 1 ? "" : "s"; - mount(body, [ - noticeStrip( - "Unique queries not materialized for " + missing.length + " day" + plural + " (" + missing.join(", ") + "): " + - "the rollup tier that answers this range never carried " + (plural ? "those days" : "that day") + - " for this server, so the cell says so rather than 0. The service's start-up repair closes such days the next time it runs." - ), - grid, - ]); + renderInto(calendar.body, () => { + const grid = VIZ.table(data, { + rowsKey: "days", + columns: DAILY_RANGE_COLUMNS, + emptyText: + "No collected days in this range. A day with ANY collection appears here even when every signal was quiet, so a missing day is a gap in collection rather than a quiet one.", + }); + + const missing = Array.isArray(data.days_missing) ? data.days_missing : []; + if (!missing.length) return grid; + const plural = missing.length === 1 ? "" : "s"; + return [ + noticeStrip( + "Unique queries not materialized for " + missing.length + " day" + plural + " (" + missing.join(", ") + "): " + + "the rollup tier that answers this range never carried " + (plural ? "those days" : "that day") + + " for this server, so the cell says so rather than 0. The service's start-up repair closes such days the next time it runs." + ), + grid, + ]; + }); })(); return [tile.panel, calendar.panel]; } +/** Mount what `render` returns into `body`, or an error strip naming the fault if it throws. */ +function renderInto(body, render) { + try { + mount(body, render()); + } catch (e) { + mount(body, errorStrip("Could not render this panel: " + (e && e.message ? e.message : String(e)))); + } +} + /** Reshape flat rows into per-series points, keeping the top `maxSeries` series by peak value. */ function pivot(rows, { xKey, seriesKey, valueKey }, maxSeries = 8) { const byTime = new Map();