From e51870ceb129bde711af9188d25bcc9001b14edc Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sat, 22 Aug 2026 23:58:47 +0100 Subject: [PATCH 1/8] Move the nine PostgreSQL reads into the shared store layer (#2530) The WPF viewer cannot reference the service, so the PostgreSQL reads it needs for #2530 had to be reachable from somewhere both front ends see. They move to PerformanceMonitor.Darling.Storage, which the service, the viewer and the tests all already reference. Namespace change only: no query text, no ordinal mapping and no signature is touched, and every existing reader test still pins the same constants. Copying them into the viewer instead would have meant a second copy of, among others, a 200-line recursive blocking walk whose revisit guard, root attribution and truncation flag were each a separate review finding. Two copies of a query like that diverge, and the copy that diverges is never the one being read - the same reasoning CollectorEngineCapability's own comment gives for keeping one copy of a sentence both SKUs print. Storage gains InternalsVisibleTo("Darling.Tests") because the ordinal pins reach the internal Map*Row helpers, which followed the readers; the service keeps its own for everything that stayed behind. Co-Authored-By: Claude Opus 5 --- Darling/Darling.Tests/DarlingPgAutovacuumReaderTests.cs | 1 + .../Darling.Tests/DarlingPgBlockingReaderOrdinalTests.cs | 1 + Darling/Darling.Tests/DarlingPgBlockingReaderTests.cs | 1 + Darling/Darling.Tests/DarlingPgDatabaseReaderTests.cs | 1 + Darling/Darling.Tests/DarlingPgIoReaderTests.cs | 1 + Darling/Darling.Tests/DarlingPgSlotReaderTests.cs | 1 + Darling/Darling.Tests/DarlingPgStatementReaderTests.cs | 1 + Darling/Darling.Tests/DarlingPgWaitReaderTests.cs | 1 + Darling/Darling.Tests/DarlingPgWraparoundReaderTests.cs | 1 + Darling/Darling.Tests/DarlingPgXminReaderTests.cs | 1 + .../Mcp/DarlingMcpPgAutovacuumTools.cs | 1 + .../Mcp/DarlingMcpPgBlockingTools.cs | 1 + .../Mcp/DarlingMcpPgDatabaseTools.cs | 1 + .../Mcp/DarlingMcpPgIoTools.cs | 1 + .../Mcp/DarlingMcpPgSlotTools.cs | 1 + .../Mcp/DarlingMcpPgStatementTools.cs | 1 + .../Mcp/DarlingMcpPgWaitTools.cs | 1 + .../Mcp/DarlingMcpPgWraparoundTools.cs | 1 + .../Mcp/DarlingMcpPgXminTools.cs | 1 + .../DarlingPgAutovacuumReader.cs | 2 +- .../DarlingPgBlockingReader.cs | 2 +- .../DarlingPgDatabaseReader.cs | 2 +- .../DarlingPgIoReader.cs | 2 +- .../DarlingPgSlotReader.cs | 2 +- .../DarlingPgStatementReader.cs | 2 +- .../DarlingPgWaitReader.cs | 2 +- .../DarlingPgWraparoundReader.cs | 2 +- .../DarlingPgXminReader.cs | 2 +- .../PerformanceMonitor.Darling.Storage.csproj | 7 +++++++ 29 files changed, 35 insertions(+), 9 deletions(-) rename Darling/{PerformanceMonitor.Darling.Service/Mcp => PerformanceMonitor.Darling.Storage}/DarlingPgAutovacuumReader.cs (99%) rename Darling/{PerformanceMonitor.Darling.Service/Mcp => PerformanceMonitor.Darling.Storage}/DarlingPgBlockingReader.cs (99%) rename Darling/{PerformanceMonitor.Darling.Service/Mcp => PerformanceMonitor.Darling.Storage}/DarlingPgDatabaseReader.cs (99%) rename Darling/{PerformanceMonitor.Darling.Service/Mcp => PerformanceMonitor.Darling.Storage}/DarlingPgIoReader.cs (99%) rename Darling/{PerformanceMonitor.Darling.Service/Mcp => PerformanceMonitor.Darling.Storage}/DarlingPgSlotReader.cs (99%) rename Darling/{PerformanceMonitor.Darling.Service/Mcp => PerformanceMonitor.Darling.Storage}/DarlingPgStatementReader.cs (99%) rename Darling/{PerformanceMonitor.Darling.Service/Mcp => PerformanceMonitor.Darling.Storage}/DarlingPgWaitReader.cs (99%) rename Darling/{PerformanceMonitor.Darling.Service/Mcp => PerformanceMonitor.Darling.Storage}/DarlingPgWraparoundReader.cs (99%) rename Darling/{PerformanceMonitor.Darling.Service/Mcp => PerformanceMonitor.Darling.Storage}/DarlingPgXminReader.cs (98%) diff --git a/Darling/Darling.Tests/DarlingPgAutovacuumReaderTests.cs b/Darling/Darling.Tests/DarlingPgAutovacuumReaderTests.cs index bbbef5507a..1c1d230e2e 100644 --- a/Darling/Darling.Tests/DarlingPgAutovacuumReaderTests.cs +++ b/Darling/Darling.Tests/DarlingPgAutovacuumReaderTests.cs @@ -10,6 +10,7 @@ using System.Linq; using System.Text.RegularExpressions; using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; using Xunit; namespace Darling.Tests; diff --git a/Darling/Darling.Tests/DarlingPgBlockingReaderOrdinalTests.cs b/Darling/Darling.Tests/DarlingPgBlockingReaderOrdinalTests.cs index f5d803da84..0d80f3e672 100644 --- a/Darling/Darling.Tests/DarlingPgBlockingReaderOrdinalTests.cs +++ b/Darling/Darling.Tests/DarlingPgBlockingReaderOrdinalTests.cs @@ -13,6 +13,7 @@ using System.Linq; using System.Text.RegularExpressions; using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; using Xunit; namespace Darling.Tests; diff --git a/Darling/Darling.Tests/DarlingPgBlockingReaderTests.cs b/Darling/Darling.Tests/DarlingPgBlockingReaderTests.cs index 5e71e9485b..ea044074cd 100644 --- a/Darling/Darling.Tests/DarlingPgBlockingReaderTests.cs +++ b/Darling/Darling.Tests/DarlingPgBlockingReaderTests.cs @@ -10,6 +10,7 @@ using System.Linq; using System.Text.RegularExpressions; using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; using Xunit; namespace Darling.Tests; diff --git a/Darling/Darling.Tests/DarlingPgDatabaseReaderTests.cs b/Darling/Darling.Tests/DarlingPgDatabaseReaderTests.cs index 644644b315..0fa3f8a91f 100644 --- a/Darling/Darling.Tests/DarlingPgDatabaseReaderTests.cs +++ b/Darling/Darling.Tests/DarlingPgDatabaseReaderTests.cs @@ -9,6 +9,7 @@ using System; using System.Linq; using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; using Xunit; namespace Darling.Tests; diff --git a/Darling/Darling.Tests/DarlingPgIoReaderTests.cs b/Darling/Darling.Tests/DarlingPgIoReaderTests.cs index 9175791bfd..d8ea9f1d11 100644 --- a/Darling/Darling.Tests/DarlingPgIoReaderTests.cs +++ b/Darling/Darling.Tests/DarlingPgIoReaderTests.cs @@ -9,6 +9,7 @@ using System; using System.Linq; using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; using Xunit; namespace Darling.Tests; diff --git a/Darling/Darling.Tests/DarlingPgSlotReaderTests.cs b/Darling/Darling.Tests/DarlingPgSlotReaderTests.cs index 8a45d760b5..a62df35ce4 100644 --- a/Darling/Darling.Tests/DarlingPgSlotReaderTests.cs +++ b/Darling/Darling.Tests/DarlingPgSlotReaderTests.cs @@ -9,6 +9,7 @@ using System; using System.Linq; using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; using Xunit; namespace Darling.Tests; diff --git a/Darling/Darling.Tests/DarlingPgStatementReaderTests.cs b/Darling/Darling.Tests/DarlingPgStatementReaderTests.cs index 2ad6cdaf62..54ea137582 100644 --- a/Darling/Darling.Tests/DarlingPgStatementReaderTests.cs +++ b/Darling/Darling.Tests/DarlingPgStatementReaderTests.cs @@ -8,6 +8,7 @@ using System; using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; using Xunit; namespace Darling.Tests; diff --git a/Darling/Darling.Tests/DarlingPgWaitReaderTests.cs b/Darling/Darling.Tests/DarlingPgWaitReaderTests.cs index 7ba2cd4725..c2d0425958 100644 --- a/Darling/Darling.Tests/DarlingPgWaitReaderTests.cs +++ b/Darling/Darling.Tests/DarlingPgWaitReaderTests.cs @@ -8,6 +8,7 @@ using System; using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; using Xunit; namespace Darling.Tests; diff --git a/Darling/Darling.Tests/DarlingPgWraparoundReaderTests.cs b/Darling/Darling.Tests/DarlingPgWraparoundReaderTests.cs index c7dfe475ed..c1a9937b10 100644 --- a/Darling/Darling.Tests/DarlingPgWraparoundReaderTests.cs +++ b/Darling/Darling.Tests/DarlingPgWraparoundReaderTests.cs @@ -8,6 +8,7 @@ using System; using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; using Xunit; namespace Darling.Tests; diff --git a/Darling/Darling.Tests/DarlingPgXminReaderTests.cs b/Darling/Darling.Tests/DarlingPgXminReaderTests.cs index 05e7630a46..83a4c520e3 100644 --- a/Darling/Darling.Tests/DarlingPgXminReaderTests.cs +++ b/Darling/Darling.Tests/DarlingPgXminReaderTests.cs @@ -9,6 +9,7 @@ using System; using System.Linq; using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; using Xunit; namespace Darling.Tests; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs index 4f01af7427..5a092a3c60 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs @@ -14,6 +14,7 @@ using ModelContextProtocol.Server; using Npgsql; using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Storage; namespace PerformanceMonitor.Darling.Service.Mcp; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgBlockingTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgBlockingTools.cs index 6b3a71a4e4..eaf7138b2d 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgBlockingTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgBlockingTools.cs @@ -14,6 +14,7 @@ using ModelContextProtocol.Server; using Npgsql; using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Storage; namespace PerformanceMonitor.Darling.Service.Mcp; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgDatabaseTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgDatabaseTools.cs index a844997b6b..bae79d39ba 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgDatabaseTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgDatabaseTools.cs @@ -15,6 +15,7 @@ using ModelContextProtocol.Server; using Npgsql; using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Storage; namespace PerformanceMonitor.Darling.Service.Mcp; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs index 55367b9040..d4320a27be 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs @@ -14,6 +14,7 @@ using ModelContextProtocol.Server; using Npgsql; using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Storage; namespace PerformanceMonitor.Darling.Service.Mcp; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSlotTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSlotTools.cs index 683fba0886..af0eb207a9 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSlotTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSlotTools.cs @@ -14,6 +14,7 @@ using ModelContextProtocol.Server; using Npgsql; using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Storage; namespace PerformanceMonitor.Darling.Service.Mcp; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgStatementTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgStatementTools.cs index 08e821ae95..d9f83bba3d 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgStatementTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgStatementTools.cs @@ -14,6 +14,7 @@ using ModelContextProtocol.Server; using Npgsql; using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Storage; namespace PerformanceMonitor.Darling.Service.Mcp; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWaitTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWaitTools.cs index 257e6d2156..89e2e36eac 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWaitTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWaitTools.cs @@ -14,6 +14,7 @@ using ModelContextProtocol.Server; using Npgsql; using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Storage; namespace PerformanceMonitor.Darling.Service.Mcp; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWraparoundTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWraparoundTools.cs index 58ed6fbe01..5dfe9f793d 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWraparoundTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWraparoundTools.cs @@ -14,6 +14,7 @@ using ModelContextProtocol.Server; using Npgsql; using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Storage; namespace PerformanceMonitor.Darling.Service.Mcp; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgXminTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgXminTools.cs index 9164c60723..1448af572d 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgXminTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgXminTools.cs @@ -14,6 +14,7 @@ using ModelContextProtocol.Server; using Npgsql; using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Storage; namespace PerformanceMonitor.Darling.Service.Mcp; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgAutovacuumReader.cs b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgAutovacuumReader.cs similarity index 99% rename from Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgAutovacuumReader.cs rename to Darling/PerformanceMonitor.Darling.Storage/DarlingPgAutovacuumReader.cs index 5a49b0b8d2..c7eb3ef6df 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgAutovacuumReader.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgAutovacuumReader.cs @@ -12,7 +12,7 @@ using System.Threading.Tasks; using Npgsql; -namespace PerformanceMonitor.Darling.Service.Mcp; +namespace PerformanceMonitor.Darling.Storage; /// /// Reads per-table autovacuum state (pg_autovacuum_stats), ranked by how far past its own diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgBlockingReader.cs b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgBlockingReader.cs similarity index 99% rename from Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgBlockingReader.cs rename to Darling/PerformanceMonitor.Darling.Storage/DarlingPgBlockingReader.cs index 0a35574c6d..b485250e95 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgBlockingReader.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgBlockingReader.cs @@ -13,7 +13,7 @@ using System.Threading.Tasks; using Npgsql; -namespace PerformanceMonitor.Darling.Service.Mcp; +namespace PerformanceMonitor.Darling.Storage; /// /// Reads blocking chains from the stored edge list (pg_blocking_edges), assembled into one row per diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgDatabaseReader.cs b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgDatabaseReader.cs similarity index 99% rename from Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgDatabaseReader.cs rename to Darling/PerformanceMonitor.Darling.Storage/DarlingPgDatabaseReader.cs index febf307209..e0fb31889d 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgDatabaseReader.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgDatabaseReader.cs @@ -12,7 +12,7 @@ using System.Threading.Tasks; using Npgsql; -namespace PerformanceMonitor.Darling.Service.Mcp; +namespace PerformanceMonitor.Darling.Storage; /// /// Reads the per-database pg_stat_database counters from pg_database_stats, differenced across diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgIoReader.cs b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgIoReader.cs similarity index 99% rename from Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgIoReader.cs rename to Darling/PerformanceMonitor.Darling.Storage/DarlingPgIoReader.cs index 7f375b8f4d..18cca92c4b 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgIoReader.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgIoReader.cs @@ -12,7 +12,7 @@ using System.Threading.Tasks; using Npgsql; -namespace PerformanceMonitor.Darling.Service.Mcp; +namespace PerformanceMonitor.Darling.Storage; /// /// Reads I/O by (backend_type, object, context) from pg_io_stats, differenced across the window. diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgSlotReader.cs b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgSlotReader.cs similarity index 99% rename from Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgSlotReader.cs rename to Darling/PerformanceMonitor.Darling.Storage/DarlingPgSlotReader.cs index 9c93b8be10..11bcdd5f26 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgSlotReader.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgSlotReader.cs @@ -12,7 +12,7 @@ using System.Threading.Tasks; using Npgsql; -namespace PerformanceMonitor.Darling.Service.Mcp; +namespace PerformanceMonitor.Darling.Storage; /// /// Reads replication slot state (pg_replication_slots) — current state plus whether retained WAL diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgStatementReader.cs b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgStatementReader.cs similarity index 99% rename from Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgStatementReader.cs rename to Darling/PerformanceMonitor.Darling.Storage/DarlingPgStatementReader.cs index 1298a32db6..184918858e 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgStatementReader.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgStatementReader.cs @@ -12,7 +12,7 @@ using System.Threading.Tasks; using Npgsql; -namespace PerformanceMonitor.Darling.Service.Mcp; +namespace PerformanceMonitor.Darling.Storage; /// /// Reads the PostgreSQL statement store (pg_statement_stats) — the top-queries surface for an diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgWaitReader.cs b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgWaitReader.cs similarity index 99% rename from Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgWaitReader.cs rename to Darling/PerformanceMonitor.Darling.Storage/DarlingPgWaitReader.cs index ffc9c5eee5..2a5c10e73a 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgWaitReader.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgWaitReader.cs @@ -12,7 +12,7 @@ using System.Threading.Tasks; using Npgsql; -namespace PerformanceMonitor.Darling.Service.Mcp; +namespace PerformanceMonitor.Darling.Storage; /// /// Reads the PostgreSQL wait store (pg_wait_stats) — the counterpart of diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgWraparoundReader.cs b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgWraparoundReader.cs similarity index 99% rename from Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgWraparoundReader.cs rename to Darling/PerformanceMonitor.Darling.Storage/DarlingPgWraparoundReader.cs index 2523f344bb..13826d5a14 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgWraparoundReader.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgWraparoundReader.cs @@ -12,7 +12,7 @@ using System.Threading.Tasks; using Npgsql; -namespace PerformanceMonitor.Darling.Service.Mcp; +namespace PerformanceMonitor.Darling.Storage; /// /// Reads freeze headroom (pg_wraparound_stats) — the current distance to a write outage, per diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgXminReader.cs b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgXminReader.cs similarity index 98% rename from Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgXminReader.cs rename to Darling/PerformanceMonitor.Darling.Storage/DarlingPgXminReader.cs index 9106f1815e..009046cf1d 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingPgXminReader.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgXminReader.cs @@ -12,7 +12,7 @@ using System.Threading.Tasks; using Npgsql; -namespace PerformanceMonitor.Darling.Service.Mcp; +namespace PerformanceMonitor.Darling.Storage; /// /// Reads what is holding back the xmin horizon (pg_xmin_horizon), by cause. diff --git a/Darling/PerformanceMonitor.Darling.Storage/PerformanceMonitor.Darling.Storage.csproj b/Darling/PerformanceMonitor.Darling.Storage/PerformanceMonitor.Darling.Storage.csproj index f73174fcaf..832d107569 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/PerformanceMonitor.Darling.Storage.csproj +++ b/Darling/PerformanceMonitor.Darling.Storage/PerformanceMonitor.Darling.Storage.csproj @@ -20,4 +20,11 @@ + + + + + From e1a33bc7ba1653d8e7ce9b81357f3c9760c90fd3 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sat, 22 Aug 2026 23:59:33 +0100 Subject: [PATCH 2/8] Give a PostgreSQL target PostgreSQL tabs in the WPF viewer (#2530) `git grep IsPostgres` across the viewer returned nothing, so an Aurora target opened on the SQL Server correlated-lane Overview and got the whole nineteen-tab strip - tempdb, Trace Flags, Query Store, Plan Viewer, Always On - nearly all of it permanently empty and several tabs meaningless for the engine. It now gets six: Overview, Activity, Vacuum, Waits, I/O and Replication, the same six #2547 chose for the web, with the same ids and the same grouping so the two front ends do not teach one engine two shapes. Placement is DERIVED from CollectorCatalog in both directions rather than enumerated. All nine PostgreSQL collectors reach exactly one tab; a tenth turns the pin red naming itself, which is the check that did not exist while eight of them shipped MCP-only for three releases. The ViewerCollectorCoverageTests allow-list, which carried all nine as tracked debt, is now empty - and its reader-layer scan now follows the shared store readers the viewer NAMES, one hop outwards, so coverage still means "the viewer reads it" without demanding a duplicate copy of the SQL to satisfy a text match. Both Aurora-only panels are SHOWN on stock PostgreSQL, not hidden. Each prints CollectorEngineCapability.NotCollectedMessage - the same sentence the MCP surface and the web print, naming the server, the engine, the collector and the exact aurora_stat_* surface, ending "and never will". The defect here is unexplained emptiness, not emptiness; hiding would also make the tab strip a different shape on two PostgreSQL servers in one fleet. For the same reason the Overview grid is built from the CATALOG rather than from collection_log: a gated-off collector writes no log row at all, so a log-driven grid would drop precisely the row an operator most needs explained. Mechanics worth knowing: - The six TabItems CONTINUE the same TabControl at indices 19-24 rather than living in a second one. For a PostgreSQL server the nineteen SQL Server tabs are collapsed and these six shown, so both sets keep fixed indices and every drill-down that navigates by an index constant is untouched. - Each PostgreSQL index gets its OWN dispatch arm. Falling through to `default:` would run the SQL Server overview lanes - four collectors that cannot run on this engine - at a PostgreSQL server. - engine_kind and sql_engine_edition are selected by BOTH server reads. The sidebar uses ManagedServersSql on any seeded store, i.e. every real deployment, so a discriminator on only ServersSql would have left every PostgreSQL target on the SQL Server tabs while a unit test passed. - Only a POSITIVE claim switches. A null or unrecognised token keeps the SQL Server tabs and shows no engine badge at all, because the tabs such a server gets are a default rather than a finding. - The display projections keep the store's sentinels out of the cells: -1 is not a number, an untracked pg_stat_io write counter is not 0, a NULL recurrence is "cannot tell" and not "once", temp blocks are not bytes, and every timestamp goes through ViewerTimeHelper. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 4 + .../ViewerCollectorCoverageTests.cs | 88 ++-- .../Darling.Tests/ViewerPostgresTabsTests.cs | 487 +++++++++++++++++ Darling/Darling.Tests/packages.lock.json | 1 + .../PerformanceMonitor.Darling.Viewer.csproj | 5 + .../ViewerDataService.MonitoredServers.cs | 20 +- .../ViewerDataService.Postgres.cs | 292 +++++++++++ .../ViewerDataService.cs | 62 ++- .../ViewerPostgresDisplay.cs | 489 ++++++++++++++++++ .../ViewerPostgresTabs.cs | 160 ++++++ .../ViewerServerTab.Postgres.cs | 282 ++++++++++ .../ViewerServerTab.xaml | 377 ++++++++++++++ .../ViewerServerTab.xaml.cs | 59 ++- Darling/README.md | 4 +- docs/uat-onboarding.md | 35 +- 15 files changed, 2318 insertions(+), 47 deletions(-) create mode 100644 Darling/Darling.Tests/ViewerPostgresTabsTests.cs create mode 100644 Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Postgres.cs create mode 100644 Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs create mode 100644 Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs create mode 100644 Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3395fe5596..8fb1ce5a00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **The WPF viewer renders PostgreSQL tabs at a PostgreSQL target, and stops rendering nineteen empty SQL Server ones** ([#2530]) - `git grep IsPostgres` across `PerformanceMonitor.Darling.Viewer` returned **nothing**, so an Aurora target opened on the SQL Server correlated-lane Overview and got the whole nineteen-tab strip - tempdb, Trace Flags, Query Store, Plan Viewer, Always On - nearly all of it permanently empty and several tabs meaningless for the engine. It now gets **six**: Overview, Activity, Vacuum, Waits, I/O and Replication, the same six the web dashboard chose in [#2547], with the same ids and the same grouping so the two front ends do not teach one engine two shapes. All **nine** PostgreSQL collectors land on exactly one tab, and that placement is DERIVED from `CollectorCatalog` in both directions - a tenth collector turns the pin red naming itself, which is the check that did not exist while eight of them shipped MCP-only for three releases. The `ViewerCollectorCoverageTests` allow-list, which carried all nine as tracked debt, is now **empty**. +- **The two Aurora-only PostgreSQL panels are SHOWN on stock PostgreSQL, not hidden** ([#2530]) - `pg_wait_stats` and `pg_statement_stats` both gate on `IsAurora`, so on a stock target they can never have content. Each panel prints `CollectorEngineCapability.NotCollectedMessage` instead - the same sentence the MCP surface and the web dashboard print, naming the server, the engine, the collector and the exact `aurora_stat_*` surface, ending "and never will". The defect being closed is UNEXPLAINED emptiness, not emptiness; hiding them would also make the tab strip a different shape on two PostgreSQL servers in one fleet. The Overview tab is built from the CATALOG rather than from `collection_log` for the same reason: a gated-off collector writes no log row at all, so a log-driven grid would have dropped precisely the row an operator most needs explained. +- **One copy of the PostgreSQL read SQL, shared by the MCP surface and the desktop viewer** ([#2530]) - the nine `DarlingPg*Reader` classes moved from the service's MCP folder into `PerformanceMonitor.Darling.Storage`, which both front ends reference. Copying them into the viewer would have meant a second copy of, among others, a 200-line recursive blocking walk whose revisit guard, root attribution and truncation flag were each a separate review finding - and the copy that drifts is never the one being read. No query text changed; every existing reader test still pins the same constants. +- **The viewer's server rows carry the engine discriminator, from BOTH server reads** ([#2530]) - `DarlingServer` gained `EngineKind`, `EngineEdition`, `IsPostgres` and `EngineDescription`, and both `ServersSql` and `ManagedServersSql` select `engine_kind` and `sql_engine_edition`. Both, because the sidebar uses `ManagedServersSql` on any seeded store - i.e. every real deployment - so a column added to only the other one would have left every PostgreSQL target on the SQL Server tab set while a unit test passed. Only a POSITIVE claim switches: a null or unrecognised token keeps the SQL Server tabs and shows NO engine badge, because the tabs such a server gets are a default rather than a finding. - **PostgreSQL temp-file spills, cache hit ratio, deadlocks and the rollback ratio - one collector, one read** ([#2539]) - nothing touched `pg_stat_database`, so four separate questions had no answer on a PostgreSQL target. The one that mattered: **temp files are work `work_mem` could not hold**, which is the most common reason a PostgreSQL query is slow for a cause its plan shape does not show - and a spilling query looks identical to an expensive one in the statement statistics. `pg_statement_stats` does carry per-query `temp_blks_written`, but it reads `aurora_stat_statements()` and is **Aurora only**, so on stock PostgreSQL there was no temp-file evidence anywhere; on Aurora it is a top-N over statements `pg_stat_statements` still retains, and this is the ground truth those are a share of. Schema **v83** adds `collect.pg_database_stats`, the new `pg_database_stats` collector reads the view per minute, and `get_pg_database_stats` differences it across a window. Every major and every target shape: the newest column selected arrived in PostgreSQL 9.2 (verified live on 13, 16 and 17), it is core rather than Aurora, and a standby is included deliberately because a sort on a read replica spills exactly the way it does on a writer. - **A PostgreSQL statistics reset is reported AS a reset** ([#2539]) - `pg_stat_database`'s counters are cumulative since the last `pg_stat_reset()`, so a reset silently zeroes them, and the two obvious ways to difference them are both wrong: last-minus-first goes **negative**, max-minus-min **spikes** to the pre-reset lifetime total. Both look like measurements. The read clamps per interval so neither can happen, and then says so: `stats_reset_count` from the server's own per-database `stats_reset` moving, `counter_rewind_count` from a counter falling below its predecessor, and a per-database note that the window totals are LOWER BOUNDS. Both signals are carried because neither sees every reset - a reset followed by enough activity to climb back past the old value inside one collection interval leaves every difference positive, and only the timestamp knows. The sharpest case is a database's FIRST-EVER reset, where `stats_reset` moves NULL -> timestamp: the first sample of a series is excluded by its ROW_NUMBER rather than by its `LAG` being NULL, because `LAG(stats_reset)` is NULL both when there is no previous row AND when the previous row had never been reset - the ordinary state - so a guard on it would miss the one reset nothing else can see. Rows are stored **per database** for the same reason: `stats_reset` is per database, so an aggregate would have to pick one timestamp for rows that legitimately disagree, and a single database's reset would corrupt the cluster's delta with nothing left in the data to say so. - **The web dashboard renders PostgreSQL tabs at a PostgreSQL target, and stops rendering twelve empty SQL Server ones** ([#2530]) - the per-server page's `SERVER_TABS` was a FLAT list applied to every server, so an Aurora instance got Wait Stats, CPU, Memory, Blocking, File I/O, Queries, Configuration, Config Changes, tempdb and the rest - nearly all permanently empty, several meaningless for the engine - while the eight `get_pg_*` reads that DO have data were reachable only through MCP. The registry is now two registries, and `serverTabsFor(card)` chooses from the fleet card's server-derived `is_postgres`. A PostgreSQL target gets **six** tabs: **Overview** (freeze headroom, xmin horizon, autovacuum backlog and replication-slot vitals over findings and collection health), **Activity** (`get_pg_blocking` with its capture denominator, top query shapes, and [#2539]'s per-database temp-file spills directly beneath them, because a spill is the explanation a statement's own stats cannot give), **Vacuum**, **Waits**, **I/O** and **Replication**. Six against twelve is the design and not a shortfall: parity was explicitly not the constraint, because bloat, wraparound, the xmin horizon and autovacuum have no SQL Server analogue and are what actually pages a PostgreSQL DBA. **Vacuum is one tab on purpose** - an old xmin horizon starves vacuum, starved vacuum falls behind on freezing, freezing falling behind ends in wraparound; read separately each looks survivable and together they are one escalating story, so the three reads sit in that causal order under a note that says why. **Waits is SHOWN on stock PostgreSQL** even though `aurora_stat_system_waits()` means it can never fill there: [#2532] taught that read to answer `not_collected` naming the server, the engine, the collector and the exact Aurora surface and to say the gap is permanent, so the panel explains itself - and hiding it would have made the tab set change shape between two PostgreSQL servers in one fleet and made the one Aurora-only capability invisible from a stock instance. A **NULL** `engine_kind` keeps the SQL Server set, unchanged: only a positive PostgreSQL claim switches, because absence is not evidence for either engine. The bar and the grid wait for the ONE `/api/fleet` read the page already made rather than flashing the wrong twelve tabs and correcting them. The card also gained `engine_description` (the engine's name in words, from `MonitoredEngineKind`), so the header can badge the engine without any surface owning a second copy of the vocabulary - and `vizStat` gained the `emptyText` guard `vizLine` already had, because several PostgreSQL reads answer their HEALTHY case with a data body carrying prose and none of the summary keys, which used to render as a row of em-dashes. diff --git a/Darling/Darling.Tests/ViewerCollectorCoverageTests.cs b/Darling/Darling.Tests/ViewerCollectorCoverageTests.cs index 07e5dd9028..91a78879d5 100644 --- a/Darling/Darling.Tests/ViewerCollectorCoverageTests.cs +++ b/Darling/Darling.Tests/ViewerCollectorCoverageTests.cs @@ -11,6 +11,7 @@ using System.IO; using System.Linq; using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; using PerformanceMonitor.Collectors; using Xunit; @@ -20,7 +21,8 @@ namespace Darling.Tests; /// THE COVERAGE PIN (parity board Tier 0), Darling side — the mirror of Lite's /// CollectorViewerCoverageTests. Every collector table in must have /// a consumer in the Darling viewer's reader layer -/// (PerformanceMonitor.Darling.Viewer/ViewerDataService*.cs) — referenced directly by table name or +/// (PerformanceMonitor.Darling.Viewer/ViewerDataService*.cs, plus the shared store readers those +/// files CALL — see ) — referenced directly by table name or /// via its v_ view — OR be listed in . /// /// Darling currently reads EVERY catalog table, so the allow-list is empty and this pin is fully green. @@ -48,31 +50,9 @@ public sealed class ViewerCollectorCoverageTests // database_states is read by ViewerDataService.DatabaseStates.cs (the override editor's backing // store), so it is covered by the reader-layer scan and needs no allow-list entry. - // UNBUILT UI (parity board Tier 1) -- remove each when the PostgreSQL tab ships. - // The nine PostgreSQL collector tables are read through MCP today, not the WPF viewer: each has a - // reader class + MCP tool (get_pg_wait_stats, get_pg_top_queries, get_pg_wraparound_risk, - // get_pg_xmin_horizon, get_pg_replication_slots, get_pg_autovacuum_health, get_pg_io_stats, - // get_pg_blocking, get_pg_database_stats) and is exposed over /api/read, so the data is not - // invisible — it is just not on a - // WPF tab, because the viewer's surfaces are SQL-Server-shaped and a PostgreSQL target's signals do - // not slot into them. Deliberately listed one per line so removing one at a time is a one-line diff. - "pg_wait_stats", - "pg_statement_stats", - "pg_wraparound_stats", - "pg_xmin_horizon", - "pg_replication_slot_stats", - "pg_autovacuum_stats", - "pg_io_stats", - // pg_blocking_edges is the one with a genuine SQL Server counterpart on the Blocking tab, and it - // still cannot share it: that tab is built on blocked_process_report XML (an engine-recorded event - // with a deadlock graph and a threshold), while this table holds periodic samples of an edge list. - // Rendering samples through a surface labelled as an event log would misrepresent coverage, which is - // a worse outcome than the tab not existing yet. - "pg_blocking_edges", - // pg_database_stats (#2539) is read by get_pg_database_stats over /api/read, not by a WPF tab, for - // the same reason as the seven above: the viewer's surfaces are SQL-Server-shaped and there is no - // tab whose meaning survives being pointed at a PostgreSQL target. - "pg_database_stats", + // EMPTY, and it stays empty. The nine PostgreSQL collector tables were the last entries here, + // carrying "remove each when the PostgreSQL tab ships" — and #2530 shipped exactly those tabs, so + // all nine came off in one commit. Every table in the catalog now has a Darling viewer reader. }; [Fact] @@ -143,10 +123,27 @@ public void AllowList_HasNoEntryThatIsActuallyRead() private static bool ReferencedIn(string readerText, string table) => readerText.Contains(table, StringComparison.OrdinalIgnoreCase); - /// Concatenated text of the Darling viewer's reader layer + /// + /// Concatenated text of the Darling viewer's reader layer /// (PerformanceMonitor.Darling.Viewer/ViewerDataService*.cs), located from this test file's /// compile-time path (Darling.Tests sits at Darling/Darling.Tests/; the viewer is the sibling - /// Darling/PerformanceMonitor.Darling.Viewer/). + /// Darling/PerformanceMonitor.Darling.Viewer/) — PLUS the shared store readers those files name. + /// + /// Why the second half exists (#2530). The nine PostgreSQL reads run the SAME query text on + /// the MCP surface and on the WPF tabs, from DarlingPg-prefixed readers in + /// PerformanceMonitor.Darling.Storage. A scan that looked only at ViewerDataService*.cs + /// would have demanded a SECOND copy of that SQL — including a 200-line recursive blocking walk whose + /// revisit guard, root attribution and truncation flag were each a separate review finding — purely to + /// satisfy a text match. Duplicating load-bearing SQL so a coverage pin passes is the opposite of what + /// the pin is for. + /// + /// It is still DERIVED, and still viewer-shaped. A shared reader is followed only when the + /// viewer's own source NAMES its class — one hop, outwards from the viewer, never the reverse, so a + /// shared reader naming another cannot drag in coverage the viewer never asked for. Stop calling a + /// reader from the viewer and its table loses coverage immediately, which is exactly the failure this + /// pin exists to catch; proved by deleting one call and watching that table appear in the uncovered + /// list. + /// private static string ReaderLayerText([CallerFilePath] string thisFile = "") { var testDir = Path.GetDirectoryName(thisFile)!; @@ -167,7 +164,40 @@ private static string ReaderLayerText([CallerFilePath] string thisFile = "") $"No ViewerDataService*.cs files found under {readerDir} — the coverage pin cannot read the " + "Darling viewer's reader layer (did the reader layer move?)."); - return string.Join("\n", files.Select(f => StripSchemaProbeLines(File.ReadAllText(f)))); + var viewerText = string.Join("\n", files.Select(f => StripSchemaProbeLines(File.ReadAllText(f)))); + + return viewerText + "\n" + SharedReadersNamedBy(viewerText, testDir); + } + + /// + /// The source of every shared store reader the viewer's own reader layer names, concatenated. + /// A name that resolves to no file under PerformanceMonitor.Darling.Storage is skipped + /// rather than asserted on: the viewer legitimately mentions reader classes it cannot reference (the + /// service's own, in prose), and a missing file grants no coverage — which is the safe direction to + /// fail in. + /// + private static string SharedReadersNamedBy(string viewerText, string testDir) + { + var sharedDir = Path.GetFullPath(Path.Combine(testDir, "..", "PerformanceMonitor.Darling.Storage")); + + var named = Regex + .Matches(viewerText, @"\bDarling[A-Za-z0-9]*Reader\b") + .Select(m => m.Value) + .Distinct(StringComparer.Ordinal) + .OrderBy(n => n, StringComparer.Ordinal) + .ToList(); + + var followed = new List(); + foreach (var name in named) + { + var path = Path.Combine(sharedDir, name + ".cs"); + if (File.Exists(path) && !HasBuildOutputSegment(path)) + { + followed.Add(StripSchemaProbeLines(File.ReadAllText(path))); + } + } + + return string.Join("\n", followed); } /// diff --git a/Darling/Darling.Tests/ViewerPostgresTabsTests.cs b/Darling/Darling.Tests/ViewerPostgresTabsTests.cs new file mode 100644 index 0000000000..1dd89c56a3 --- /dev/null +++ b/Darling/Darling.Tests/ViewerPostgresTabsTests.cs @@ -0,0 +1,487 @@ +/* + * 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.IO; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; +using PerformanceMonitor.Collectors; +using PerformanceMonitor.Darling.Viewer; +using Xunit; + +namespace Darling.Tests; + +/// +/// The WPF viewer's PostgreSQL tab set (#2530) — the counterpart of ServerPageTabsTests, which +/// holds the web dashboard's two registries to the same standard. +/// +/// What these pins are for. The nine PostgreSQL collectors spent three releases MCP-only, and +/// nothing went red about it: no check could see that the graphical surface had fallen behind the data. +/// So the placement question is asked in BOTH directions and DERIVED from +/// — a tenth PostgreSQL collector turns +/// red naming itself, and a registry +/// entry for a collector that does not exist turns its twin red. An enumerated list of nine would have +/// passed through the whole failure it exists to prevent. +/// +public sealed class ViewerPostgresTabsTests +{ + // ── Placement, derived from the catalog in both directions ─────────────────────────────────── + + /// + /// Every PostgreSQL collector the catalog ships reaches exactly one PostgreSQL tab. The forward + /// direction: add a collector and forget the screen, and this names it. + /// + [Fact] + public void EveryPostgresCollector_IsShownOnExactlyOnePostgresTab() + { + var placements = ViewerPostgresTabs.All + .SelectMany(tab => tab.Collectors.Select(c => (Collector: c, tab.Id))) + .ToList(); + + var missing = new List(); + var duplicated = new List(); + + foreach (var definition in ViewerPostgresTabs.PostgresCollectors()) + { + var on = placements.Where(p => string.Equals(p.Collector, definition.Name, StringComparison.Ordinal)).ToList(); + if (on.Count == 0) + { + missing.Add($"{definition.Name} (stored in {definition.TargetTable})"); + } + else if (on.Count > 1) + { + duplicated.Add($"{definition.Name} on {string.Join(", ", on.Select(p => p.Id))}"); + } + } + + Assert.True(missing.Count == 0, + "The service collects these PostgreSQL signals and no PostgreSQL viewer tab shows them. Add a " + + "panel and list the collector on that tab in ViewerPostgresTabs.All — do NOT relax this pin, " + + "which exists because eight of them shipped invisible for three releases:\n " + + string.Join("\n ", missing)); + + Assert.True(duplicated.Count == 0, + "A collector is claimed by more than one PostgreSQL tab, so its panel has two homes and the " + + "'which screen shows this' question has two answers:\n " + string.Join("\n ", duplicated)); + } + + /// + /// The reverse: the registry may only name collectors that exist, are PostgreSQL, and whose store table + /// resolves. A panel wired to a name the catalog does not know can never fill. + /// + [Fact] + public void TheRegistry_NamesOnlyRealPostgresCollectors() + { + var bad = new List(); + + foreach (var tab in ViewerPostgresTabs.All) + { + foreach (var name in tab.Collectors) + { + var definition = CollectorCatalog.Find(name); + if (definition is null) + { + bad.Add($"{tab.Id}: '{name}' is not in CollectorCatalog"); + } + else if (definition.TargetEngine != CollectorTargetEngine.PostgreSql) + { + bad.Add($"{tab.Id}: '{name}' is a {definition.TargetEngine} collector"); + } + else if (string.IsNullOrWhiteSpace(ViewerPostgresTabs.TableOf(name))) + { + bad.Add($"{tab.Id}: '{name}' resolves to no store table"); + } + } + } + + Assert.True(bad.Count == 0, "PostgreSQL tab registry names collectors that cannot serve it:\n " + + string.Join("\n ", bad)); + } + + /// + /// The Aurora-only collectors are on the tab set, not hidden from it — the decision #2547 made for the + /// web, pinned here so the viewer cannot quietly diverge. pg_wait_stats and + /// pg_statement_stats gate on IsAurora, so on stock PostgreSQL they can never have + /// content; since #2532 they answer with a sentence naming the exact Aurora surface instead of a blank, + /// so the defect this issue is about — UNEXPLAINED emptiness — is closed by showing them, and hiding + /// them would make the tab strip a different shape on two PostgreSQL servers in one fleet. + /// Derived from the shipped gates rather than named: this reads which collectors are Aurora-only + /// out of and requires each to be placed. + /// + [Fact] + public void TheAuroraOnlyCollectors_AreShown_NotHidden() + { + var auroraOnly = ViewerPostgresTabs.PostgresCollectors() + .Where(d => !CollectorEngineCapability.IsCollectedOnEngineKind(d, MonitoredEngineKind.Postgres)) + .Where(d => CollectorEngineCapability.IsCollectedOnEngineKind(d, MonitoredEngineKind.AuroraPostgres)) + .Select(d => d.Name) + .OrderBy(n => n, StringComparer.Ordinal) + .ToList(); + + Assert.True(auroraOnly.Count >= 2, + "Expected at least two Aurora-only PostgreSQL collectors (pg_wait_stats and pg_statement_stats " + + "both read an aurora_stat_* surface). Found: " + string.Join(", ", auroraOnly)); + + var placed = ViewerPostgresTabs.All.SelectMany(t => t.Collectors).ToHashSet(StringComparer.Ordinal); + var hidden = auroraOnly.Where(n => !placed.Contains(n)).ToList(); + + Assert.True(hidden.Count == 0, + "Aurora-only collector(s) were dropped from the PostgreSQL tab set: " + string.Join(", ", hidden) + + ". They are SHOWN deliberately — the panel prints CollectorEngineCapability's sentence naming " + + "the Aurora surface, which is the opposite of the unexplained blank this issue is about."); + } + + // ── The registry, the XAML and the dispatch agree ──────────────────────────────────────────── + + /// + /// The registry's indices, headers and order are what the XAML actually declares. Three separate things + /// could drift — an index constant, a header string, the TabItem order — and each one silently sends a + /// tab click to the wrong loader, which is not a failure any of the other pins here can see. + /// + [Fact] + public void TheRegistry_MatchesTheTabItemsDeclaredInTheXaml() + { + var xaml = ViewerFile("ViewerServerTab.xaml"); + + /* Top-level TabItems only: the nested sub-tab groups (Queries, Memory, Activity...) are indented + further, and the inner TabControl's own items are not inner-tab indices. */ + var headers = Regex + .Matches(xaml, "^ ]*?Header=\"([^\"]+)\"", RegexOptions.Multiline) + .Select(m => m.Groups[1].Value) + .ToList(); + + Assert.True(headers.Count > ViewerPostgresTabs.All.Count, + $"Only {headers.Count} top-level TabItems found in ViewerServerTab.xaml — the scan is not " + + "matching the file's shape any more."); + + foreach (var tab in ViewerPostgresTabs.All) + { + Assert.True(tab.InnerTabIndex < headers.Count, + $"Registry tab '{tab.Id}' claims inner-tab index {tab.InnerTabIndex}, but ViewerServerTab.xaml " + + $"declares only {headers.Count} top-level TabItems."); + + Assert.Equal(tab.Header, Unescape(headers[tab.InnerTabIndex])); + } + + /* The PostgreSQL run is a CONTIGUOUS block at the END. That is what lets both engines keep fixed + indices: every SQL Server tab is collapsed for a PostgreSQL server and vice versa, and no + existing index constant moves. A tab inserted between them would renumber the SQL Server run and + break every drill-down that navigates by one of those constants. */ + var indices = ViewerPostgresTabs.All.Select(t => t.InnerTabIndex).ToList(); + Assert.Equal(indices.OrderBy(i => i).ToList(), indices); + Assert.Equal(headers.Count - ViewerPostgresTabs.All.Count, indices[0]); + Assert.Equal(headers.Count - 1, indices[^1]); + Assert.Equal(indices.Count, indices.Distinct().Count()); + } + + /// + /// Every PostgreSQL tab has its OWN arm in LoadInnerTabAsync. Without one it would fall through + /// to default: and run the SQL Server correlated-lane reads — four collectors that cannot run on + /// this engine — against a PostgreSQL server, painting the exact blank panels this issue is about. + /// + [Fact] + public void EveryPostgresTab_HasItsOwnArmInTheDispatch() + { + var code = ViewerFile("ViewerServerTab.xaml.cs"); + + var constants = new Dictionary(StringComparer.Ordinal) + { + ["overview"] = "PgOverviewInnerTabIndex", + ["activity"] = "PgActivityInnerTabIndex", + ["vacuum"] = "PgVacuumInnerTabIndex", + ["waits"] = "PgWaitsInnerTabIndex", + ["io"] = "PgIoInnerTabIndex", + ["replication"] = "PgReplicationInnerTabIndex", + }; + + var missing = new List(); + foreach (var tab in ViewerPostgresTabs.All) + { + Assert.True(constants.ContainsKey(tab.Id), $"No index constant is mapped for tab id '{tab.Id}'."); + + if (!code.Contains($"case {constants[tab.Id]}:", StringComparison.Ordinal)) + { + missing.Add($"{tab.Id} (case {constants[tab.Id]}:)"); + } + } + + Assert.True(missing.Count == 0, + "PostgreSQL tab(s) have no arm in LoadInnerTabAsync and would fall through to the SQL Server " + + "Overview lanes: " + string.Join(", ", missing)); + } + + /// + /// No SQL Server tab loader reaches a PostgreSQL read. The "and from no other tab" half of the web's + /// pin, in the shape the viewer has: the PostgreSQL reads live behind GetPg* on the data service, + /// and only the PostgreSQL loader file may call them. + /// + [Fact] + public void NoSqlServerTabLoader_CallsAPostgresRead() + { + var offenders = new List(); + + foreach (var file in ViewerFiles("ViewerServerTab")) + { + if (Path.GetFileName(file).Equals("ViewerServerTab.Postgres.cs", StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + var text = File.ReadAllText(file); + foreach (Match match in Regex.Matches(text, @"_dataService\.GetPg[A-Za-z]+Async")) + { + offenders.Add($"{Path.GetFileName(file)}: {match.Value}"); + } + } + + Assert.True(offenders.Count == 0, + "A SQL Server tab loader calls a PostgreSQL read. Those tabs are collapsed for a PostgreSQL " + + "server and shown for a SQL Server one, so this can only ever run against the wrong engine:\n " + + string.Join("\n ", offenders)); + } + + // ── The tab set switches only on a positive claim ──────────────────────────────────────────── + + /// + /// Only a POSITIVE PostgreSQL claim switches the tab set. Absence is not evidence for either engine, + /// and the unclaimed population is every server that has not reconnected since the engine-kind rung + /// landed — guessing PostgreSQL from a NULL would break every one of them. + /// + [Theory] + [InlineData(null, false)] + [InlineData("", false)] + [InlineData(" ", false)] + [InlineData("sqlserver", false)] + [InlineData("mysql", false)] + [InlineData("postgres", true)] + [InlineData("POSTGRES", true)] + [InlineData("aurora-postgres", true)] + public void TheTabSet_SwitchesOnlyOnAPositivePostgresClaim(string? engineKind, bool expected) + { + Assert.Equal(expected, Server(engineKind).IsPostgres); + } + + /// + /// The engine badge has three answers, and the asymmetry is the point: NO badge for a server the store + /// makes no claim about (the tabs it gets are a default, not a finding), the describer's words for a + /// recognised token, and the RAW TOKEN for one this build does not recognise — because the describer's + /// "an unrecognised engine" is worded to sit mid-sentence in the capability messages and reads wrong as + /// a label, while the literal string is what an operator would search their store for. + /// + [Fact] + public void TheEngineBadge_ShowsNothingRatherThanGuessing() + { + Assert.Null(Server(null).EngineDescription); + Assert.Null(Server("").EngineDescription); + Assert.Null(Server(" ").EngineDescription); + + Assert.Equal("SQL Server", Server("sqlserver").EngineDescription); + Assert.Equal("PostgreSQL", Server("postgres").EngineDescription); + Assert.Equal("Aurora PostgreSQL", Server("aurora-postgres").EngineDescription); + + Assert.Equal("cockroach", Server("cockroach").EngineDescription); + } + + /// + /// BOTH server reads carry the discriminator. The sidebar uses ManagedServersSql on any seeded + /// store — i.e. every real deployment — so a column added to only ServersSql would have left + /// every PostgreSQL target on the SQL Server tab set while a unit test over the other query passed. + /// + [Fact] + public void BothServerReads_CarryTheEngineDiscriminator() + { + foreach (var sql in new[] { ViewerDataService.ServersSql, ViewerDataService.ManagedServersSql }) + { + Assert.Contains("engine_kind", sql, StringComparison.Ordinal); + Assert.Contains("sql_engine_edition", sql, StringComparison.Ordinal); + } + } + + // ── The Overview grid is built from the catalog, not from the log ──────────────────────────── + + /// + /// The Overview grid shows every PostgreSQL collector, including the ones that wrote no + /// collection_log row because dispatch filtered them out — and it says why. A grid built from the + /// log alone would drop precisely the collector an operator most needs explained, which is the same + /// defect as a blank tab one layer down. + /// + [Fact] + public void TheOverviewGrid_ShowsAGatedOffCollector_WithTheReason() + { + var collectors = ViewerPostgresTabs.PostgresCollectors(); + var stock = Server(MonitoredEngineKind.Postgres); + + /* No log rows at all: the harshest case, and the one a stock PostgreSQL target actually produces + for the two Aurora-only collectors. */ + var rows = ViewerDataService.BuildPostgresCollectorHealth( + stock, collectors, Array.Empty()); + + Assert.Equal(collectors.Count, rows.Count); + + var waits = rows.Single(r => r.Collector == "pg_wait_stats"); + Assert.True(waits.IsPermanentGap); + Assert.False(waits.IsFault); + Assert.Contains("aurora_stat_system_waits()", waits.Explanation, StringComparison.Ordinal); + Assert.Contains("never will", waits.Explanation, StringComparison.Ordinal); + Assert.Contains(stock.ServerName, waits.Explanation, StringComparison.Ordinal); + + /* A collector that CAN run here and has no rows is a fault to chase, not a fact to read. The two + states are deliberately distinct because they need different responses. */ + var xmin = rows.Single(r => r.Collector == "pg_xmin_horizon"); + Assert.False(xmin.IsPermanentGap); + Assert.True(xmin.IsFault); + } + + /// On AURORA the same two collectors are not a gap at all — proving the row's state is derived + /// from the engine rather than from the collector's name. + [Fact] + public void TheOverviewGrid_DoesNotCallAnAuroraCollectorAGap_OnAurora() + { + var collectors = ViewerPostgresTabs.PostgresCollectors(); + var aurora = Server(MonitoredEngineKind.AuroraPostgres); + + var facts = new[] + { + new ViewerDataService.PostgresCollectorLogFacts( + "pg_wait_stats", new DateTime(2026, 8, 22, 12, 0, 0), new DateTime(2026, 8, 22, 12, 0, 0), + Runs: 60, FailedRuns: 0, RowsCollected: 1200, LastStatus: "SUCCESS", LastMessage: null), + }; + + var rows = ViewerDataService.BuildPostgresCollectorHealth(aurora, collectors, facts); + var waits = rows.Single(r => r.Collector == "pg_wait_stats"); + + Assert.False(waits.IsPermanentGap); + Assert.False(waits.IsFault); + Assert.Equal("Collecting", waits.Status); + Assert.Equal(60, waits.Runs); + Assert.Equal(1200, waits.RowsCollected); + Assert.Equal("", waits.Explanation); + } + + // ── Empty-state prose ──────────────────────────────────────────────────────────────────────── + + /// + /// Every note element the PostgreSQL XAML declares is actually filled by the loaders, and every panel + /// collector goes through PanelNote — the one place that asks + /// first. A panel whose note is never + /// assigned is a blank rectangle with no explanation, which is the defect, not a cosmetic gap. + /// + [Fact] + public void EveryPostgresPanel_ExplainsItsOwnEmptyState() + { + var xaml = ViewerFile("ViewerServerTab.xaml"); + var loaders = ViewerFile("ViewerServerTab.Postgres.cs"); + + var notes = Regex + .Matches(xaml, "x:Name=\"(Pg[A-Za-z]*Note)\"") + .Select(m => m.Groups[1].Value) + .Distinct(StringComparer.Ordinal) + .OrderBy(n => n, StringComparer.Ordinal) + .ToList(); + + Assert.True(notes.Count >= ViewerPostgresTabs.All.Count, + $"Only {notes.Count} PostgreSQL note elements found in the XAML; expected at least one per tab."); + + var unfilled = notes.Where(n => !loaders.Contains($"{n}.Text", StringComparison.Ordinal)).ToList(); + Assert.True(unfilled.Count == 0, + "PostgreSQL panel note(s) are declared in the XAML and never filled, so those panels can be " + + "blank with no explanation: " + string.Join(", ", unfilled)); + + var unasked = ViewerPostgresTabs.All + .SelectMany(t => t.Collectors) + .Distinct(StringComparer.Ordinal) + .Where(c => !loaders.Contains($"\"{c}\"", StringComparison.Ordinal)) + .OrderBy(c => c, StringComparer.Ordinal) + .ToList(); + + Assert.True(unasked.Count == 0, + "Panel collector(s) are never named in the loaders, so their panel never asks the capability " + + "question and cannot print the engine sentence: " + string.Join(", ", unasked)); + } + + // ── Display projections: the sentinels never reach a cell ──────────────────────────────────── + + /// + /// The store's -1 not-applicable sentinel never renders as a number. It is -1 and not + /// 0 precisely because 0 would read as a measurement — "started this instant", "retains + /// nothing" — and printing -1 in a grid makes exactly that mistake one step later. + /// + [Fact] + public void TheNotApplicableSentinel_NeverRendersAsANumber() + { + Assert.Equal(PgDisplay.NotApplicableText, PgDisplay.Bytes(-1)); + Assert.Equal(PgDisplay.NotApplicableText, PgDisplay.Count(-1)); + Assert.Equal(PgDisplay.NotApplicableText, PgDisplay.Milliseconds(-1)); + Assert.Equal(PgDisplay.NotApplicableText, PgDisplay.ByteDelta(-1, 4096)); + Assert.Equal(PgDisplay.NotApplicableText, PgDisplay.ByteDelta(4096, -1)); + + /* Zero is a real measurement and must NOT be swallowed by the same guard. */ + Assert.Equal("0 B", PgDisplay.Bytes(0)); + Assert.Equal("0", PgDisplay.Count(0)); + Assert.Equal("0 ms", PgDisplay.Milliseconds(0)); + } + + /// A percentage of nothing is not 0%. Dividing by a zero denominator has no answer, and "0%" + /// is a claim about a ratio that was never computable. + [Fact] + public void APercentageOfNothing_IsNotZeroPercent() + { + Assert.Equal(PgDisplay.NotApplicableText, PgDisplay.Percent(0, 0)); + Assert.StartsWith("0", PgDisplay.Percent(0, 100), StringComparison.Ordinal); + } + + /// + /// Aurora reports NULL across the whole pg_stat_io write side, because backends there do not + /// write data files. The read carries that as a flag so the cell can say "not tracked"; a 0 would claim + /// a measurement nobody took. + /// + [Fact] + public void AnUntrackedIoCounter_SaysSo_RatherThanReadingZero() + { + var aurora = PgDisplay.Io(new PerformanceMonitor.Darling.Storage.DarlingPgIoReader.PgIoRow( + "client backend", "relation", "normal", Reads: 10, ReadTimeMs: 1.0, Hits: 5, Extends: 0, + ExtendTimeMs: 0, Evictions: 0, Reuses: 0, Writes: 0, WriteTimeMs: 0, OpBytes: 8192, + WriteCountersTracked: false, StatsReset: null)); + + Assert.Equal("not tracked", aurora.Writes); + Assert.Equal("not tracked", aurora.WriteTimeMs); + + var selfManaged = PgDisplay.Io(new PerformanceMonitor.Darling.Storage.DarlingPgIoReader.PgIoRow( + "client backend", "relation", "normal", Reads: 10, ReadTimeMs: 1.0, Hits: 5, Extends: 0, + ExtendTimeMs: 0, Evictions: 0, Reuses: 0, Writes: 0, WriteTimeMs: 0, OpBytes: 8192, + WriteCountersTracked: true, StatsReset: null)); + + Assert.Equal("0", selfManaged.Writes); + } + + // ── Helpers ────────────────────────────────────────────────────────────────────────────────── + + private static DarlingServer Server(string? engineKind) => + new(1, "alpha-pg-01", "alpha", isEnabled: true, sqlMajorVersion: null, monthlyCostUsd: 0m, + engineKind: engineKind, engineEdition: CollectorEngineCapability.UnknownEngineEdition); + + private static string Unescape(string xmlAttributeValue) => + xmlAttributeValue.Replace("&", "&", StringComparison.Ordinal); + + private static string ViewerFile(string fileName, [CallerFilePath] string thisFile = "") + { + var path = Path.Combine(ViewerDir(thisFile), fileName); + Assert.True(File.Exists(path), $"Expected the viewer source {fileName} at {path}."); + return File.ReadAllText(path); + } + + private static IEnumerable ViewerFiles(string filePrefix, [CallerFilePath] string thisFile = "") => + Directory + .EnumerateFiles(ViewerDir(thisFile), "*.cs", SearchOption.TopDirectoryOnly) + .Where(f => Path.GetFileName(f).StartsWith(filePrefix, StringComparison.OrdinalIgnoreCase)); + + private static string ViewerDir(string thisFile) => + Path.GetFullPath(Path.Combine(Path.GetDirectoryName(thisFile)!, "..", "PerformanceMonitor.Darling.Viewer")); +} diff --git a/Darling/Darling.Tests/packages.lock.json b/Darling/Darling.Tests/packages.lock.json index ca736f2fbe..4d37abf68e 100644 --- a/Darling/Darling.Tests/packages.lock.json +++ b/Darling/Darling.Tests/packages.lock.json @@ -713,6 +713,7 @@ "Hardcodet.NotifyIcon.Wpf": "[2.0.1, )", "PerformanceMonitor.Alerting": "[1.0.0, )", "PerformanceMonitor.Analysis": "[1.0.0, )", + "PerformanceMonitor.Collectors": "[1.0.0, )", "PerformanceMonitor.Common": "[1.0.0, )", "PerformanceMonitor.Darling.Analysis": "[1.0.0, )", "PerformanceMonitor.Darling.Storage": "[1.0.0, )", diff --git a/Darling/PerformanceMonitor.Darling.Viewer/PerformanceMonitor.Darling.Viewer.csproj b/Darling/PerformanceMonitor.Darling.Viewer/PerformanceMonitor.Darling.Viewer.csproj index 149a8fb65d..9d6479020e 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/PerformanceMonitor.Darling.Viewer.csproj +++ b/Darling/PerformanceMonitor.Darling.Viewer/PerformanceMonitor.Darling.Viewer.csproj @@ -60,6 +60,11 @@ + + diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.MonitoredServers.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.MonitoredServers.cs index 51ebbd46ab..aef759ef5f 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.MonitoredServers.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.MonitoredServers.cs @@ -13,6 +13,7 @@ using System.Threading.Tasks; using Npgsql; using NpgsqlTypes; +using PerformanceMonitor.Collectors; using PerformanceMonitor.Common; using PerformanceMonitor.Notifications; @@ -176,6 +177,17 @@ AND database IS NOT DISTINCT FROM $2 /// viewer-added server appear immediately (before its first collection) and a removed one disappear at /// once, instead of waiting for the service's next reconcile. is_enabled and monthly_cost_usd /// come from config so a viewer toggle/edit is reflected without a round-trip through the service. + /// + /// The engine discriminator comes from the OBSERVED side (#2530). engine_kind and + /// sql_engine_edition are read from collect.servers, not from + /// config_monitored_servers.engine, which is the DESIRED configuration and cannot carry + /// Aurora-ness at all - that is probed from aurora_version at connect. A server the operator + /// added but the service has not connected to yet therefore has NO kind, which is the honest answer: + /// it gets the SQL Server tab set by default, exactly as it did before this column existed. + /// + /// This query, not ServersSql, is what the sidebar uses on any seeded store - i.e. every + /// real deployment - so the discriminator has to be on BOTH or the viewer would have kept rendering + /// SQL Server tabs at every PostgreSQL target while a unit test over the other query passed. /// public const string ManagedServersSql = @" SELECT @@ -184,7 +196,9 @@ AND database IS NOT DISTINCT FROM $2 COALESCE(s.display_name, c.name) AS display_name, c.is_enabled, s.sql_major_version, - c.monthly_cost_usd + c.monthly_cost_usd, + s.engine_kind, + COALESCE(s.sql_engine_edition, 0) AS sql_engine_edition FROM config_monitored_servers c LEFT JOIN servers s ON s.server_id = c.server_id ORDER BY COALESCE(s.display_name, c.name)"; @@ -215,7 +229,9 @@ public async Task> GetManagedServersAsync(CancellationToken reader.IsDBNull(2) ? serverName : reader.GetString(2), !reader.IsDBNull(3) && reader.GetBoolean(3), reader.IsDBNull(4) ? null : reader.GetInt32(4), - reader.IsDBNull(5) ? 0m : Convert.ToDecimal(reader.GetValue(5)))); + reader.IsDBNull(5) ? 0m : Convert.ToDecimal(reader.GetValue(5)), + reader.IsDBNull(6) ? null : reader.GetString(6), + reader.IsDBNull(7) ? CollectorEngineCapability.UnknownEngineEdition : reader.GetInt32(7))); } return servers; diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Postgres.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Postgres.cs new file mode 100644 index 0000000000..463b6a082a --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Postgres.cs @@ -0,0 +1,292 @@ +/* + * 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.Threading; +using System.Threading.Tasks; +using Npgsql; +using PerformanceMonitor.Collectors; +using PerformanceMonitor.Darling.Storage; + +namespace PerformanceMonitor.Darling.Viewer; + +/// +/// The viewer's PostgreSQL reader layer (#2530) — what the six PostgreSQL inner tabs +/// () read, and the projections that make stored rows renderable. +/// +/// The queries are not here, deliberately. All nine PostgreSQL reads run the SAME SQL the MCP +/// surface runs, from DarlingPg*Reader in PerformanceMonitor.Darling.Storage — moved there +/// from the service's MCP folder by this change so both front ends can reach them. Copying them into the +/// viewer would have meant a second copy of, among others, a 200-line recursive blocking walk whose revisit +/// guard, root attribution and truncation flag were each a separate review finding; two copies of that +/// diverge, and the copy that diverges is never the one being read. Every other seam in this repo that both +/// SKUs answer follows the same rule (CollectorEngineCapability's message, DescribeEngineKind, +/// the alert gates), and the reason is always this one. +/// +/// What IS here: the per-collector health read the Overview tab needs — which has no MCP twin +/// because the MCP answers it per-read rather than per-server — and the display projections. Those exist +/// because the stored rows carry sentinels and units a grid must not render raw: -1 means "not +/// measured" rather than "zero", pg_stat_io's write side is genuinely absent on Aurora rather than +/// zero, and every timestamp in the store is naive UTC that has to pass through +/// like every other timestamp the viewer shows. +/// +public sealed partial class ViewerDataService +{ + /// + /// Per-collector collection facts for one server over the window — the Overview tab's grid. + /// + /// Scoped by an explicit collector-name array rather than by a LIKE 'pg\_%' pattern: the + /// caller passes the names says are PostgreSQL collectors, so the set + /// tracks the catalog instead of a naming convention a future collector could break. Cast to + /// text[] explicitly — Npgsql's inference through = ANY($4) is a runtime failure, not a + /// compile one. + /// + /// A collector this engine can never run has NO row here at all, by design: dispatch filters it + /// out before it runs, so it writes no collection_log entry (a fake SUCCESS/0-rows would be + /// ~2,880 rows a day per server of noise). That absence is exactly why + /// composes this against the catalog rather than rendering it + /// directly — otherwise the one collector an operator most needs explained is the one row missing. + /// + /// $1 server_id, $2/$3 window (naive UTC), $4 collector names. + /// + public const string PostgresCollectorHealthSql = """ + SELECT + collector_name AS collector_name, + MAX(collection_time) AS last_run_at, + MAX(collection_time) FILTER (WHERE status = 'SUCCESS') AS last_success_at, + CAST(COUNT(*) AS bigint) AS runs, + CAST(COUNT(*) FILTER (WHERE status <> 'SUCCESS') AS bigint) AS failed_runs, + CAST(COALESCE(SUM(rows_collected), 0) AS bigint) AS rows_collected, + (ARRAY_AGG(status ORDER BY collection_time DESC))[1] AS last_status, + /* The newest NON-EMPTY message, not the newest message. A collector that failed an hour ago and + has succeeded quietly since would otherwise show a blank explanation for a non-zero failure + count, which reads as the grid having nothing to say about it. */ + (ARRAY_AGG(error_message ORDER BY (error_message IS NULL), collection_time DESC))[1] + AS last_message + FROM v_collection_log + WHERE server_id = $1 + AND collection_time >= $2 + AND collection_time <= $3 + AND collector_name = ANY($4::text[]) + GROUP BY collector_name + """; + + /// One collector's raw collection_log facts over the window, before the catalog join. + public sealed record PostgresCollectorLogFacts( + string CollectorName, + DateTime? LastRunAt, + DateTime? LastSuccessAt, + long Runs, + long FailedRuns, + long RowsCollected, + string? LastStatus, + string? LastMessage); + + /// Runs for the PostgreSQL collectors the catalog ships. + public async Task> GetPostgresCollectorLogFactsAsync( + int serverId, DateTime startUtc, DateTime endUtc, IReadOnlyList collectorNames, + CancellationToken cancellationToken = default) + { + var facts = new List(); + if (collectorNames.Count == 0) + { + return facts; + } + + await using var command = _dataSource.CreateCommand(PostgresCollectorHealthSql); + command.Parameters.Add(new NpgsqlParameter { TypedValue = serverId }); + /* Kind-Unspecified at the bind, per the store's naive-UTC discipline: a Kind=Utc DateTime makes + Npgsql infer timestamptz, and PostgreSQL then zone-shifts these naive columns to compare them, so + east of UTC the window slides off the data and the read silently returns nothing. */ + command.Parameters.Add(new NpgsqlParameter { TypedValue = DateTime.SpecifyKind(startUtc, DateTimeKind.Unspecified) }); + command.Parameters.Add(new NpgsqlParameter { TypedValue = DateTime.SpecifyKind(endUtc, DateTimeKind.Unspecified) }); + command.Parameters.Add(new NpgsqlParameter { TypedValue = collectorNames.ToArray() }); + + await using var reader = await command.ExecuteReaderAsync(cancellationToken); + while (await reader.ReadAsync(cancellationToken)) + { + facts.Add(new PostgresCollectorLogFacts( + reader.GetString(0), + reader.IsDBNull(1) ? null : reader.GetDateTime(1), + reader.IsDBNull(2) ? null : reader.GetDateTime(2), + reader.IsDBNull(3) ? 0 : reader.GetInt64(3), + reader.IsDBNull(4) ? 0 : reader.GetInt64(4), + reader.IsDBNull(5) ? 0 : reader.GetInt64(5), + reader.IsDBNull(6) ? null : reader.GetString(6), + reader.IsDBNull(7) ? null : reader.GetString(7))); + } + + return facts; + } + + /// One row of the Overview tab's grid: a PostgreSQL collector and what it did for this server. + public sealed class PostgresCollectorHealthRow + { + public string Collector { get; init; } = ""; + public string StoreTable { get; init; } = ""; + public string Status { get; init; } = ""; + public string LastRun { get; init; } = ""; + public long Runs { get; init; } + public long FailedRuns { get; init; } + public long RowsCollected { get; init; } + public string Explanation { get; init; } = ""; + + /// Drives the grid's row highlight: true for a collector this engine can never run, which + /// is a fact to read rather than a fault to chase. + public bool IsPermanentGap { get; init; } + + /// Drives the grid's row highlight for a collector that IS supposed to run and is not. + public bool IsFault { get; init; } + } + + /// + /// Composes the Overview grid from the catalog and the log — pure, so it is unit-tested without a store + /// or a window. + /// + /// Catalog-driven, not log-driven, and that is the whole design. Rows come from the nine + /// PostgreSQL collectors ships; the log only fills them in. A collector + /// with no log row is therefore VISIBLE, with the reason: on stock PostgreSQL the two Aurora-only + /// collectors read aurora_stat_system_waits() and + /// aurora_stat_statements(), which core PostgreSQL has in no version, and + /// says so in the one sentence the MCP + /// surface and the web dashboard also print. The defect #2530 is about is unexplained emptiness; a + /// grid that just dropped those two would have reproduced it one layer down. + /// + /// The three states are deliberately distinct, because they need three different responses: a + /// permanent engine gap is nothing to do, a collector that should be running and has no rows in the + /// window is something to chase, and a collector that ran and failed carries its own error text. + /// + public static List BuildPostgresCollectorHealth( + DarlingServer server, + IReadOnlyList postgresCollectors, + IReadOnlyList logFacts) + { + ArgumentNullException.ThrowIfNull(server); + ArgumentNullException.ThrowIfNull(postgresCollectors); + ArgumentNullException.ThrowIfNull(logFacts); + + var byName = logFacts.ToDictionary(f => f.CollectorName, StringComparer.Ordinal); + var rows = new List(); + + foreach (var definition in postgresCollectors) + { + var gap = CollectorEngineCapability.NotCollectedMessage( + server.ServerName, server.EngineEdition, server.EngineKind, definition.Name); + + byName.TryGetValue(definition.Name, out var facts); + + string status; + var isFault = false; + if (gap is not null) + { + status = "Not collected on this engine"; + } + else if (facts is null) + { + status = "No runs in this window"; + isFault = true; + } + else if (facts.FailedRuns > 0) + { + status = string.Equals(facts.LastStatus, "SUCCESS", StringComparison.Ordinal) + ? "Recovered" + : facts.LastStatus ?? "Failing"; + isFault = !string.Equals(facts.LastStatus, "SUCCESS", StringComparison.Ordinal); + } + else + { + status = "Collecting"; + } + + rows.Add(new PostgresCollectorHealthRow + { + Collector = definition.Name, + StoreTable = definition.TargetTable, + Status = status, + LastRun = facts?.LastRunAt is { } at + ? ViewerTimeHelper.ForDisplay(at).ToString("yyyy-MM-dd HH:mm", CultureInfo.CurrentCulture) + : "", + Runs = facts?.Runs ?? 0, + FailedRuns = facts?.FailedRuns ?? 0, + RowsCollected = facts?.RowsCollected ?? 0, + /* The engine sentence wins over the last error when both exist: a collector that cannot run + here has no error worth showing, and a stale one from before a migration would be the more + confusing of the two. */ + Explanation = gap ?? facts?.LastMessage ?? "", + IsPermanentGap = gap is not null, + IsFault = isFault, + }); + } + + return rows; + } + + // ───────────────────────────────────────────────────────────────────────────────────────────── + // The nine shared reads, as the viewer calls them. Thin on purpose: the SQL, the parameter binding + // and the ordinal mapping all live in PerformanceMonitor.Darling.Storage, shared byte-for-byte with + // the MCP surface, so these add a window and nothing else. + // ───────────────────────────────────────────────────────────────────────────────────────────── + + /// Vacuum tab, panel 1 — what is holding the xmin horizon back, by cause. + public Task> GetPgXminHorizonAsync( + int serverId, DateTime startUtc, DateTime endUtc, CancellationToken cancellationToken = default) => + DarlingPgXminReader.GetPgXminHorizonAsync(_dataSource, serverId, startUtc, endUtc, cancellationToken); + + /// Vacuum tab, panel 2 — per-table autovacuum backlog, ranked by ratio to the table's own threshold. + public Task> GetPgAutovacuumAsync( + int serverId, DateTime startUtc, DateTime endUtc, int limit = 100, CancellationToken cancellationToken = default) => + DarlingPgAutovacuumReader.GetPgAutovacuumAsync(_dataSource, serverId, startUtc, endUtc, limit, cancellationToken); + + /// Vacuum tab, panel 3 — per-database transaction-ID and multixact freeze headroom. + public Task> GetPgWraparoundAsync( + int serverId, DateTime startUtc, DateTime endUtc, CancellationToken cancellationToken = default) => + DarlingPgWraparoundReader.GetPgWraparoundAsync(_dataSource, serverId, startUtc, endUtc, cancellationToken); + + /// Waits tab — Aurora's cumulative wait counters, differenced over the window. + public Task> GetPgWaitStatsAsync( + int serverId, DateTime startUtc, DateTime endUtc, int limit = 50, CancellationToken cancellationToken = default) => + DarlingPgWaitReader.GetPgWaitStatsAsync(_dataSource, serverId, startUtc, endUtc, limit, cancellationToken); + + /// I/O tab — pg_stat_io per backend type / object / context, differenced over the window. + public Task> GetPgIoAsync( + int serverId, DateTime startUtc, DateTime endUtc, int limit = 100, CancellationToken cancellationToken = default) => + DarlingPgIoReader.GetPgIoAsync(_dataSource, serverId, startUtc, endUtc, limit, cancellationToken); + + /// Replication tab — slot WAL retention and the xmin each slot pins. + public Task> GetPgSlotsAsync( + int serverId, DateTime startUtc, DateTime endUtc, CancellationToken cancellationToken = default) => + DarlingPgSlotReader.GetPgSlotsAsync(_dataSource, serverId, startUtc, endUtc, cancellationToken); + + /// Activity tab, panel 1 — the sampling DENOMINATOR: how many captures ran, and how many saw blocking. + public Task GetPgBlockingCaptureCountsAsync( + int serverId, DateTime startUtc, DateTime endUtc, CancellationToken cancellationToken = default) => + DarlingPgBlockingReader.GetPgBlockingCaptureCountsAsync(_dataSource, serverId, startUtc, endUtc, cancellationToken); + + /// Activity tab, panel 2 — blocking chains by root. + public Task> GetPgBlockingChainsAsync( + int serverId, DateTime startUtc, DateTime endUtc, int limit = 50, CancellationToken cancellationToken = default) => + DarlingPgBlockingReader.GetPgBlockingChainsAsync(_dataSource, serverId, startUtc, endUtc, limit, cancellationToken); + + /// Activity tab, panel 3 — lock cycles, which have no root and so appear on no chain. + public Task> GetPgBlockingCyclesAsync( + int serverId, DateTime startUtc, DateTime endUtc, int limit = 50, CancellationToken cancellationToken = default) => + DarlingPgBlockingReader.GetPgBlockingCyclesAsync(_dataSource, serverId, startUtc, endUtc, limit, cancellationToken); + + /// Activity tab, panel 4 — top statement shapes by total execution time. + public Task> GetPgTopQueriesAsync( + int serverId, DateTime startUtc, DateTime endUtc, CancellationToken cancellationToken = default) => + DarlingPgStatementReader.GetPgTopQueriesAsync(_dataSource, serverId, startUtc, endUtc, cancellationToken); + + /// Activity tab, panel 5 — per-database counters: temp-file spills, cache hits, deadlocks, commit split. + public Task> GetPgDatabaseStatsAsync( + int serverId, DateTime startUtc, DateTime endUtc, int limit = 50, CancellationToken cancellationToken = default) => + DarlingPgDatabaseReader.GetPgDatabaseStatsAsync(_dataSource, serverId, startUtc, endUtc, limit, cancellationToken); +} diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs index 312ff9c024..37a9db15f7 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs @@ -14,6 +14,7 @@ using System.Threading; using System.Threading.Tasks; using Npgsql; +using PerformanceMonitor.Collectors; using PerformanceMonitor.Common; using PerformanceMonitor.Darling.Storage; @@ -32,13 +33,27 @@ namespace PerformanceMonitor.Darling.Viewer; /// public sealed class DarlingServer : INotifyPropertyChanged { + /// + /// servers.engine_kind as the store holds it (#2530) - one of + /// , or null for a row no connect has stamped since the V82 rung + /// landed. Defaulted so the two reader call sites are the only places that have to know the column + /// exists, and so the test fakes that construct a SQL Server row keep compiling unchanged. + /// + /// + /// servers.sql_engine_edition. Carried beside the kind because + /// takes BOTH axes and asks kind first: a + /// PostgreSQL target's edition is 0, which the edition axis correctly reads as "no claim". Passing 0 + /// here for a server whose edition has not been read is exactly that same silence. + /// public DarlingServer( int serverId, string serverName, string displayName, bool isEnabled, int? sqlMajorVersion, - decimal monthlyCostUsd = 0) + decimal monthlyCostUsd = 0, + string? engineKind = null, + int engineEdition = CollectorEngineCapability.UnknownEngineEdition) { ServerId = serverId; ServerName = serverName; @@ -46,6 +61,8 @@ public DarlingServer( IsEnabled = isEnabled; SqlMajorVersion = sqlMajorVersion; MonthlyCostUsd = monthlyCostUsd; + EngineKind = engineKind; + EngineEdition = engineEdition; } public int ServerId { get; } @@ -55,6 +72,35 @@ public DarlingServer( public int? SqlMajorVersion { get; } public decimal MonthlyCostUsd { get; } + /// The raw servers.engine_kind token, or null when the store makes no claim. Kept raw + /// rather than decoded to an enum so an unrecognised token written by a NEWER service survives the trip + /// to , which shows the operator the literal string to search for. + public string? EngineKind { get; } + + /// servers.sql_engine_edition; + /// when the probe has not run or the target has no such property (every PostgreSQL target). + public int EngineEdition { get; } + + /// + /// True only when the store SAYS this target is PostgreSQL. Absence is not evidence for either engine, + /// so a null or unrecognised token is false and the server keeps the SQL Server surface - the + /// pre-#2530 behaviour, which is the only safe default for the servers that have not reconnected since + /// the engine-kind rung landed. + /// + public bool IsPostgres => MonitoredEngineKind.IsPostgres(EngineKind); + + /// + /// How the engine reads in the per-server header, or null when the store makes no claim - in which case + /// the header shows NO engine label rather than "SQL Server", because the tabs such a server gets are a + /// default rather than a finding. An unrecognised token renders as the raw token: the describer's + /// "an unrecognised engine" is worded to sit mid-sentence in the capability messages and reads wrong as + /// a label, and the literal string is what an operator would grep their store for. + /// + public string? EngineDescription => + !MonitoredEngineKind.IsKnown(EngineKind) + ? (string.IsNullOrWhiteSpace(EngineKind) ? null : EngineKind) + : MonitoredEngineKind.DescribeEngineKind(EngineKind); + /// "SQL Server 2022"-style label for the server list; empty when the version is unknown. public string VersionLabel => ViewerDataService.SqlVersionLabel(SqlMajorVersion); @@ -303,8 +349,16 @@ public void SetSilenced(bool silenced) /// public sealed partial class ViewerDataService : IAsyncDisposable { + /// + /// The observed server registry. engine_kind (#2530) and sql_engine_edition ride along + /// because the per-server tab set and every PostgreSQL panel's own explanation are derived from them - + /// see . Both are read here AND in + /// ViewerDataService.MonitoredServers.cs's ManagedServersSql: that one is what the sidebar + /// actually uses on a seeded store, so a discriminator added to only this query would have left every + /// real deployment on the SQL Server tab set. + /// public const string ServersSql = - "SELECT server_id, server_name, display_name, is_enabled, sql_major_version, COALESCE(monthly_cost_usd, 0) FROM servers ORDER BY display_name"; + "SELECT server_id, server_name, display_name, is_enabled, sql_major_version, COALESCE(monthly_cost_usd, 0), engine_kind, COALESCE(sql_engine_edition, 0) FROM servers ORDER BY display_name"; /// /// The authoritative read-only probe (V8 security hardening): does the connected role hold INSERT @@ -1233,7 +1287,9 @@ public async Task> GetServersAsync(CancellationToken cancell reader.IsDBNull(2) ? serverName : reader.GetString(2), !reader.IsDBNull(3) && reader.GetBoolean(3), reader.IsDBNull(4) ? null : reader.GetInt32(4), - reader.IsDBNull(5) ? 0m : Convert.ToDecimal(reader.GetValue(5)))); + reader.IsDBNull(5) ? 0m : Convert.ToDecimal(reader.GetValue(5)), + reader.IsDBNull(6) ? null : reader.GetString(6), + reader.IsDBNull(7) ? CollectorEngineCapability.UnknownEngineEdition : reader.GetInt32(7))); } return servers; diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs new file mode 100644 index 0000000000..d878758117 --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs @@ -0,0 +1,489 @@ +/* + * 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 PerformanceMonitor.Darling.Storage; + +namespace PerformanceMonitor.Darling.Viewer; + +/// +/// Stored PostgreSQL rows, projected into what a grid may show (#2530). Pure and WPF-free, so every rule +/// below is unit-testable without a window — which matters, because the rules ARE the content: each one +/// exists to stop the grid printing a number that would be read as a measurement when it is not one. +/// +/// +/// -1 is not a value. It is the store's not-applicable sentinel for a duration, a WAL +/// size or a threshold. 0 was rejected for these deliberately — it reads as "started this instant", +/// "retains nothing", "already past its threshold" — so rendering -1 raw would be the same mistake +/// one step later. +/// Absent is not zero. pg_stat_io's write side is NULL across the board on Aurora, +/// because backends there do not write data files. A blank cell says that; a 0 claims a measurement +/// that was never taken. +/// NULL recurrence is "cannot tell", not "once". The blocking read returns NULL when the +/// root's own backend id did not resolve, and those are different claims. +/// Every timestamp is naive UTC in the store and goes through +/// , exactly like every other timestamp the viewer renders. +/// Blocks are not bytes. temp_blks_written counts blocks, and the block size is a +/// compile-time setting of the server, not a constant. It is labelled in blocks rather than multiplied by +/// an assumed 8kB. +/// +/// +internal static class PgDisplay +{ + /// The store's not-applicable sentinel. Named rather than repeated, because reading it as a + /// number anywhere is the defect this class exists to prevent. + internal const long NotApplicable = -1; + + /// What an unmeasured / not-applicable cell shows. An em dash, not "0" and not blank: blank + /// reads as "nothing here yet" and 0 reads as a measurement. + internal const string NotApplicableText = "—"; + + internal static string Bytes(long value) + { + if (value <= NotApplicable) + { + return NotApplicableText; + } + + string[] units = { "B", "KB", "MB", "GB", "TB", "PB" }; + double scaled = value; + var unit = 0; + while (scaled >= 1024 && unit < units.Length - 1) + { + scaled /= 1024; + unit++; + } + + return unit == 0 + ? $"{value:N0} B" + : string.Create(CultureInfo.CurrentCulture, $"{scaled:N1} {units[unit]}"); + } + + /// A signed byte delta, for the "is this growing" columns. Zero is stated rather than blanked: + /// "flat" is an answer, and an empty cell would read as "not measured". + internal static string ByteDelta(long from, long to) + { + if (from <= NotApplicable || to <= NotApplicable) + { + return NotApplicableText; + } + + var delta = to - from; + return delta == 0 ? "flat" : (delta > 0 ? "+" : "-") + Bytes(Math.Abs(delta)); + } + + internal static string Count(long value) => + value <= NotApplicable ? NotApplicableText : value.ToString("N0", CultureInfo.CurrentCulture); + + internal static string CountDelta(long from, long to) + { + var delta = to - from; + return delta == 0 ? "flat" : (delta > 0 ? "+" : "") + delta.ToString("N0", CultureInfo.CurrentCulture); + } + + /// A duration in milliseconds, or the not-applicable dash for the -1 sentinel. + internal static string Milliseconds(long ms) + { + if (ms <= NotApplicable) + { + return NotApplicableText; + } + + return ms < 1000 + ? $"{ms:N0} ms" + : TimeSpan.FromMilliseconds(ms).ToString(ms < 3_600_000 ? @"mm\:ss" : @"d\.hh\:mm\:ss", CultureInfo.CurrentCulture); + } + + internal static string Timestamp(DateTime? utc) => + utc is { } value + ? ViewerTimeHelper.ForDisplay(value).ToString("yyyy-MM-dd HH:mm", CultureInfo.CurrentCulture) + : string.Empty; + + /// A percentage of a total, or the dash when the total is zero — which is "nothing happened", + /// not "0%". + internal static string Percent(long part, long total) => + total <= 0 + ? NotApplicableText + : string.Create(CultureInfo.CurrentCulture, $"{part * 100.0 / total:N1}%"); + + // ── Vacuum ─────────────────────────────────────────────────────────────────────────────────── + + internal sealed class XminRow + { + public string Source { get; init; } = ""; + public string IsWinnerText { get; init; } = ""; + public string XminAge { get; init; } = ""; + public string PeakXminAge { get; init; } = ""; + public string WinnerShare { get; init; } = ""; + public string MeasuredAt { get; init; } = ""; + public string Holder { get; init; } = ""; + public string Detail { get; init; } = ""; + } + + internal static XminRow Xmin(DarlingPgXminReader.PgXminRow row) => new() + { + Source = row.Source, + IsWinnerText = row.IsWinner ? "yes" : "", + XminAge = Count(row.XminAge), + PeakXminAge = Count(row.PeakXminAge), + /* The share, not the raw count. A slot that won 58 of 60 samples is a standing problem someone + needs to own; a session that won twice is a query that ran long and finished. Reporting only the + current holder makes those two look the same. */ + WinnerShare = row.Samples <= 0 + ? NotApplicableText + : string.Create(CultureInfo.CurrentCulture, $"{row.SamplesAsWinner:N0} of {row.Samples:N0} samples"), + MeasuredAt = Timestamp(row.MeasuredAt), + Holder = row.Holder ?? "", + Detail = row.Detail ?? "", + }; + + internal sealed class AutovacuumRow + { + public string DatabaseName { get; init; } = ""; + public string SchemaName { get; init; } = ""; + public string TableName { get; init; } = ""; + public string AutovacuumState { get; init; } = ""; + public bool AutovacuumDisabled { get; init; } + public string DeadTuples { get; init; } = ""; + public string VacuumThreshold { get; init; } = ""; + public string ThresholdRatio { get; init; } = ""; + public string DeadTupleTrend { get; init; } = ""; + public string InsertsSinceVacuum { get; init; } = ""; + public string InsertVacuumThreshold { get; init; } = ""; + public string LastAutovacuum { get; init; } = ""; + public string LastAutoanalyze { get; init; } = ""; + public string TotalSize { get; init; } = ""; + } + + internal static AutovacuumRow Autovacuum(DarlingPgAutovacuumReader.PgAutovacuumRow row) => new() + { + DatabaseName = row.DatabaseName ?? "", + SchemaName = row.SchemaName ?? "", + TableName = row.TableName ?? "", + AutovacuumState = row.AutovacuumDisabled ? "DISABLED" : "enabled", + AutovacuumDisabled = row.AutovacuumDisabled, + DeadTuples = Count(row.DeadTuples), + VacuumThreshold = Count(row.VacuumThreshold), + /* Ratio to the table's OWN threshold, which is what makes a small hot table and a huge cold one + comparable at all. A never-analyzed table can have threshold 0, where a ratio is undefined + rather than infinite. */ + ThresholdRatio = row.VacuumThreshold <= 0 + ? NotApplicableText + : string.Create(CultureInfo.CurrentCulture, $"{row.DeadTuples / (double)row.VacuumThreshold:N1}x"), + /* Climbing dead tuples mean autovacuum is losing; a flat figure well past the threshold usually + means it is blocked or switched off. Those need different fixes, so the trend is a column rather + than something to work out from two visits. */ + DeadTupleTrend = CountDelta(row.FirstDeadTuples, row.DeadTuples), + InsertsSinceVacuum = Count(row.InsertsSinceVacuum), + /* -1 here is not "no threshold": it is a major with no autovacuum_vacuum_insert_threshold at all. */ + InsertVacuumThreshold = row.InsertVacuumThreshold <= NotApplicable + ? "n/a on this version" + : Count(row.InsertVacuumThreshold), + LastAutovacuum = Timestamp(row.LastAutovacuum), + LastAutoanalyze = Timestamp(row.LastAutoanalyze), + TotalSize = Bytes(row.TotalBytes), + }; + + internal sealed class WraparoundRow + { + public string DatabaseName { get; init; } = ""; + public string FrozenXidAge { get; init; } = ""; + public double PctTowardEmergencyVacuum { get; init; } + public double PctTowardWraparound { get; init; } + public string XidsRemaining { get; init; } = ""; + public string MinMultiXidAge { get; init; } = ""; + public double PctTowardMultixactEmergency { get; init; } + public string MultiXidsRemaining { get; init; } = ""; + public string WindowPeakFrozenXidAge { get; init; } = ""; + public string ConnectionsAllowed { get; init; } = ""; + public string MeasuredAt { get; init; } = ""; + } + + internal static WraparoundRow Wraparound(DarlingPgWraparoundReader.PgWraparoundRow row) => new() + { + DatabaseName = row.DatabaseName, + FrozenXidAge = Count(row.FrozenXidAge), + PctTowardEmergencyVacuum = Math.Round(row.PctTowardEmergencyVacuum, 1), + PctTowardWraparound = Math.Round(row.PctTowardWraparound, 1), + XidsRemaining = Count(row.XidsRemaining), + MinMultiXidAge = Count(row.MinMultiXidAge), + PctTowardMultixactEmergency = Math.Round(row.PctTowardMultixactEmergency, 1), + MultiXidsRemaining = Count(row.MultiXidsRemaining), + WindowPeakFrozenXidAge = Count(row.WindowPeakFrozenXidAge), + /* datallowconn=false is why a database can sit at 99% of wraparound and never be vacuumed by + anything that connects to it. It is the finding, not a footnote. */ + ConnectionsAllowed = row.AllowsConnections ? "allowed" : "NOT ALLOWED", + MeasuredAt = Timestamp(row.MeasuredAt), + }; + + // ── Waits, I/O, replication ────────────────────────────────────────────────────────────────── + + internal sealed class WaitRow + { + public string WaitType { get; init; } = ""; + public string WaitEvent { get; init; } = ""; + public string TotalWaits { get; init; } = ""; + public double TotalWaitTimeMs { get; init; } + public double AvgWaitTimeMs { get; init; } + } + + internal static WaitRow Wait(DarlingPgWaitReader.PgWaitRow row) => new() + { + WaitType = row.WaitType, + WaitEvent = row.WaitEvent, + TotalWaits = Count(row.TotalWaits), + TotalWaitTimeMs = Math.Round(row.TotalWaitTimeMs, 1), + AvgWaitTimeMs = Math.Round(row.AvgWaitTimeMs, 3), + }; + + internal sealed class IoRow + { + public string BackendType { get; init; } = ""; + public string ObjectType { get; init; } = ""; + public string Context { get; init; } = ""; + public string Reads { get; init; } = ""; + public double ReadTimeMs { get; init; } + public string Hits { get; init; } = ""; + public string Extends { get; init; } = ""; + public string Evictions { get; init; } = ""; + public string Reuses { get; init; } = ""; + public string Writes { get; init; } = ""; + public string WriteTimeMs { get; init; } = ""; + public string OpSize { get; init; } = ""; + public string StatsReset { get; init; } = ""; + } + + internal static IoRow Io(DarlingPgIoReader.PgIoRow row) => new() + { + BackendType = row.BackendType ?? "", + ObjectType = row.ObjectType ?? "", + Context = row.Context ?? "", + Reads = Count(row.Reads), + ReadTimeMs = Math.Round(row.ReadTimeMs, 1), + Hits = Count(row.Hits), + Extends = Count(row.Extends), + Evictions = Count(row.Evictions), + Reuses = Count(row.Reuses), + /* "not tracked", not 0. On Aurora the whole pg_stat_io write side is NULL because backends there + do not write data files, and the read carries WriteCountersTracked precisely so this cell can + tell that apart from a server that wrote nothing. */ + Writes = row.WriteCountersTracked ? Count(row.Writes) : "not tracked", + WriteTimeMs = row.WriteCountersTracked + ? Math.Round(row.WriteTimeMs, 1).ToString("N1", CultureInfo.CurrentCulture) + : "not tracked", + OpSize = Bytes(row.OpBytes), + StatsReset = Timestamp(row.StatsReset), + }; + + internal sealed class SlotRow + { + public string SlotName { get; init; } = ""; + public string SlotType { get; init; } = ""; + public string ActiveText { get; init; } = ""; + public string WalStatus { get; init; } = ""; + public string RetainedWal { get; init; } = ""; + public string RetainedWalTrend { get; init; } = ""; + public string SafeWalSize { get; init; } = ""; + public string XminAge { get; init; } = ""; + public string CatalogXminAge { get; init; } = ""; + public string InactiveSince { get; init; } = ""; + public string DatabaseName { get; init; } = ""; + public string Plugin { get; init; } = ""; + public string InvalidationReason { get; init; } = ""; + public bool IsInvalidated { get; init; } + public bool IsInactive { get; init; } + } + + internal static SlotRow Slot(DarlingPgSlotReader.PgSlotRow row) => new() + { + SlotName = row.SlotName, + SlotType = row.SlotType ?? "", + ActiveText = row.IsActive ? "active" : "INACTIVE", + WalStatus = row.WalStatus ?? "", + RetainedWal = Bytes(row.RetainedWalBytes), + RetainedWalTrend = ByteDelta(row.FirstRetainedWalBytes, row.RetainedWalBytes), + SafeWalSize = Bytes(row.SafeWalSizeBytes), + XminAge = Count(row.XminAge), + CatalogXminAge = Count(row.CatalogXminAge), + InactiveSince = Timestamp(row.InactiveSince), + DatabaseName = row.DatabaseName ?? "", + Plugin = row.Plugin ?? "", + InvalidationReason = row.InvalidationReason ?? "", + /* An invalidated slot has already lost its WAL: the replica behind it needs rebuilding, and that + is a different day from an inactive slot that is merely accumulating. */ + IsInvalidated = !string.IsNullOrEmpty(row.InvalidationReason) + || string.Equals(row.WalStatus, "lost", StringComparison.OrdinalIgnoreCase), + IsInactive = !row.IsActive, + }; + + // ── Activity ───────────────────────────────────────────────────────────────────────────────── + + internal sealed class ChainRow + { + public string CapturedAt { get; init; } = ""; + public int RootPid { get; init; } + public string Databases { get; init; } = ""; + public int TotalVictims { get; init; } + public int DirectVictims { get; init; } + public string Depth { get; init; } = ""; + public string WorstVictimWait { get; init; } = ""; + public string RootState { get; init; } = ""; + public string RootXactDuration { get; init; } = ""; + public string RootUsername { get; init; } = ""; + public string RootApplicationName { get; init; } = ""; + public string SamplesAsRoot { get; init; } = ""; + public string RootQuery { get; init; } = ""; + public string WorstVictimQuery { get; init; } = ""; + public string Caveats { get; init; } = ""; + } + + internal static ChainRow Chain(DarlingPgBlockingReader.PgBlockingChainRow row) + { + var caveats = new List(); + if (row.ChainMayBeTruncated) + { + caveats.Add("chain hit the 32-level walk cap"); + } + if (row.QueryTextMayBeTruncated) + { + caveats.Add("query text truncated at track_activity_query_size"); + } + + return new ChainRow + { + CapturedAt = Timestamp(row.CapturedAt), + RootPid = row.RootPid, + Databases = string.Join(", ", row.Databases.Where(d => !string.IsNullOrEmpty(d))), + TotalVictims = row.TotalVictims, + DirectVictims = row.DirectVictims, + Depth = row.MaxDepth.ToString(CultureInfo.CurrentCulture) + (row.ChainMayBeTruncated ? " (capped)" : ""), + WorstVictimWait = Milliseconds(row.WorstVictimWaitMs), + /* idle in transaction is the state worth naming on its own: the root is not running anything, + so nothing will finish and release the lock without intervention. */ + RootState = row.RootIsIdleInTransaction ? "idle in transaction" : row.RootState ?? "", + RootXactDuration = Milliseconds(row.RootXactDurationMs), + RootUsername = row.RootUsername ?? "", + RootApplicationName = row.RootApplicationName ?? "", + /* NULL is "cannot tell" — the blocker's own row had already left pg_stat_activity, so it landed + on the collector's synthetic backend id and counting it would conflate unrelated incidents. */ + SamplesAsRoot = row.SamplesAsRoot is { } samples + ? string.Create(CultureInfo.CurrentCulture, $"{samples:N0} captures") + : "unknown", + RootQuery = row.RootQuery ?? "", + WorstVictimQuery = row.WorstVictimQuery ?? "", + Caveats = string.Join("; ", caveats), + }; + } + + internal sealed class CycleRow + { + public string CapturedAt { get; init; } = ""; + public int ParticipantCount { get; init; } + public string Pids { get; init; } = ""; + public string DatabaseName { get; init; } = ""; + public string ApplicationName { get; init; } = ""; + public int BlockedBehindCount { get; init; } + public string BlockedBehindPids { get; init; } = ""; + } + + internal static CycleRow Cycle(DarlingPgBlockingReader.PgBlockingCycleRow row) => new() + { + CapturedAt = Timestamp(row.CapturedAt), + ParticipantCount = row.ParticipantCount, + Pids = string.Join(", ", row.Pids), + DatabaseName = row.DatabaseName ?? "", + ApplicationName = row.ApplicationName ?? "", + BlockedBehindCount = row.BlockedBehindCount, + BlockedBehindPids = string.Join(", ", row.BlockedBehindPids), + }; + + internal sealed class StatementRow + { + /// A string, not a number. PostgreSQL queryid is an int8 that routinely + /// exceeds 2^53, and rendering it through anything that rounds is the one field whose entire + /// purpose is joining back to pg_stat_statements (#2548). + public string QueryId { get; init; } = ""; + public string Calls { get; init; } = ""; + public string TotalExecTimeMs { get; init; } = ""; + public string AvgExecTimeMs { get; init; } = ""; + public double MaxExecTimeMs { get; init; } + public string RowsReturned { get; init; } = ""; + public string TempWritten { get; init; } = ""; + public string PeakMemory { get; init; } = ""; + public string WalWritten { get; init; } = ""; + public string QueryText { get; init; } = ""; + } + + internal static StatementRow Statement(DarlingPgStatementReader.PgStatementRow row) => new() + { + QueryId = row.QueryId.ToString(CultureInfo.InvariantCulture), + Calls = Count(row.Calls), + TotalExecTimeMs = Count(row.TotalExecTimeMs), + AvgExecTimeMs = row.Calls <= 0 + ? NotApplicableText + : string.Create(CultureInfo.CurrentCulture, $"{row.TotalExecTimeMs / (double)row.Calls:N1}"), + MaxExecTimeMs = Math.Round(row.MaxExecTimeMs, 1), + RowsReturned = Count(row.RowsReturned), + /* BLOCKS, and labelled as such. The block size is a compile-time setting of the server, so + multiplying by an assumed 8kB would put a fabricated byte count next to real ones. */ + TempWritten = row.TempBlocksWritten <= 0 + ? "0 blocks" + : string.Create(CultureInfo.CurrentCulture, $"{row.TempBlocksWritten:N0} blocks"), + PeakMemory = Bytes(row.MaxPeakMemBytes), + WalWritten = Bytes(row.WalBytes), + /* Null is "no text captured for this queryid yet" — text refreshes hourly, and a major-version + upgrade re-keys every queryid — which is a different statement from an empty query. */ + QueryText = row.QueryText ?? "(statement text not captured yet)", + }; + + internal sealed class DatabaseRow + { + public string DatabaseName { get; init; } = ""; + public string TempFiles { get; init; } = ""; + public string TempBytes { get; init; } = ""; + public string Deadlocks { get; init; } = ""; + public string CacheHitPct { get; init; } = ""; + public string XactCommit { get; init; } = ""; + public string XactRollback { get; init; } = ""; + public string RollbackPct { get; init; } = ""; + public int SampleCount { get; init; } + public string Caveats { get; init; } = ""; + } + + internal static DatabaseRow Database(DarlingPgDatabaseReader.PgDatabaseRow row) + { + var caveats = new List(); + /* Every total beside it is a LOWER BOUND when the counters were reset inside the window, so this + has to sit on the row rather than in a footnote nobody reads. */ + if (row.StatsResetCount > 0) + { + caveats.Add($"counters reset {row.StatsResetCount}x in window"); + } + if (row.CounterRewindCount > 0) + { + caveats.Add($"counters rewound {row.CounterRewindCount}x"); + } + + return new DatabaseRow + { + DatabaseName = row.DatabaseName ?? "", + TempFiles = Count(row.TempFiles), + TempBytes = Bytes(row.TempBytes), + Deadlocks = Count(row.Deadlocks), + CacheHitPct = Percent(row.BlksHit, row.BlksHit + row.BlksRead), + XactCommit = Count(row.XactCommit), + XactRollback = Count(row.XactRollback), + RollbackPct = Percent(row.XactRollback, row.XactCommit + row.XactRollback), + SampleCount = row.SampleCount, + Caveats = string.Join("; ", caveats), + }; + } +} diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs new file mode 100644 index 0000000000..261c0c6d1e --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs @@ -0,0 +1,160 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.Linq; +using PerformanceMonitor.Collectors; + +namespace PerformanceMonitor.Darling.Viewer; + +/// +/// One PostgreSQL inner tab: which inner-tab index it occupies, what its header reads, and which +/// COLLECTORS serve it. Panels are declared by collector NAME rather than by store table, so the table can +/// only ever be what says it is — the name is also the key +/// takes, so a panel's data and a panel's +/// explanation for having none are keyed on the same string by construction. +/// +/// Its position in ViewerServerTab.xaml's InnerTabs. +/// A stable identifier for pins and for the SQL Server registry's deep links to collide +/// with deliberately (overview means the same place on either engine). +/// The tab strip label, which must match the XAML <TabItem Header=>. +/// Every collector whose stored rows this tab renders, in panel order. +/// Why these panels sit together, when that is not obvious from the header. Rendered +/// above the tab's panels. Null for a tab whose single panel needs no framing. +internal sealed record ViewerPostgresTab( + int InnerTabIndex, + string Id, + string Header, + IReadOnlyList Collectors, + string? Note); + +/// +/// The PostgreSQL inner-tab registry — the viewer's half of #2530, and the direct counterpart of the web +/// dashboard's POSTGRES_TABS in server-tabs.js (#2547). Same six tabs, same ids, same +/// grouping, so the two front ends do not teach an operator two different shapes for one engine. +/// +/// Why a registry rather than "read the XAML". The SQL Server surface has nineteen inner tabs +/// declared in XAML and dispatched by integer index, and nothing there is machine-readable enough to ask +/// "does every PostgreSQL collector reach a screen?". That question is the whole point of this issue: the +/// nine PostgreSQL collectors spent three releases MCP-only precisely because no check could see that the +/// graphical surface had fallen behind the data. This list is what such a check reads, and +/// ViewerPostgresTabsTests derives the other side of it from — so a +/// TENTH PostgreSQL collector turns the pin red naming itself, rather than quietly shipping invisible. +/// +/// Six tabs against SQL Server's nineteen is the design. Parity was never the constraint; +/// the missing tabs (tempdb, Query Store, trace flags, plan cache, system_health, Always On) have no +/// PostgreSQL analogue to fill, and the signals that DO matter here — wraparound headroom, the xmin +/// horizon, vacuum backlog, WAL retention — have no SQL Server analogue either. A PostgreSQL surface built +/// by asking "what does the SQL Server viewer have" would have missed most of them. +/// +/// The Aurora-only panels are SHOWN, not hidden. pg_wait_stats and +/// pg_statement_stats both gate on CollectorTargetInfo.IsAurora, so on stock PostgreSQL they +/// can never have content — and since #2532 the reason is a sentence +/// () naming the server, the engine, the +/// collector and the exact Aurora surface, ending "and never will". The defect #2530 is about is +/// UNEXPLAINED emptiness, not emptiness: a panel that says that is the opposite of the twelve blank SQL +/// Server tabs it replaces. Hiding them would also make the tab strip a different shape on two PostgreSQL +/// servers in one fleet, and would make the one Aurora-specific capability the product has invisible from +/// a stock instance — while re-deriving in the viewer a gate the collectors already decide. +/// +internal static class ViewerPostgresTabs +{ + /// + /// The six tabs, in strip order. Indices continue the SQL Server run rather than displacing it: for a + /// PostgreSQL server every SQL Server TabItem is collapsed and these six are shown, so both sets + /// keep their fixed indices and ViewerServerTab's existing index constants — which drill-down + /// navigation keys on — are untouched by this feature. + /// + public static readonly IReadOnlyList All = new[] + { + new ViewerPostgresTab( + ViewerServerTab.PgOverviewInnerTabIndex, + "overview", + "Overview", + Array.Empty(), + /* No collector of its own: it reports on all nine, from collection_log joined to the catalog. + That join is the point — a collector gated off for this engine writes NO collection_log row + at all (pre-dispatch filtering, deliberately, so a permanent gap does not manufacture ~2,880 + fake SUCCESS rows a day), so a grid built from the log alone would show a PostgreSQL + operator eight rows and never mention the ninth. Derived from the catalog, the missing one + appears with the reason it is missing. */ + "Every PostgreSQL collector for this server: when it last ran, what it returned, and — for a " + + "collector this engine cannot run at all — why it never will."), + + new ViewerPostgresTab( + ViewerServerTab.PgActivityInnerTabIndex, + "activity", + "Activity", + new[] { "pg_blocking", "pg_statement_stats", "pg_database_stats" }, + /* pg_database_stats sits under the statement grid rather than on a tab of its own because it + answers the question the statement grid raises and cannot answer: a statement whose time + makes no sense from its row count usually spilled, and pg_stat_database's temp counters are + the only evidence of that we collect. */ + "What ran, what waited on what, and what it cost. Blocking is a periodic SAMPLE rather than an " + + "event log, so the capture counts above the grid are its denominator — three chains means " + + "something different in 60 captures than in 4."), + + new ViewerPostgresTab( + ViewerServerTab.PgVacuumInnerTabIndex, + "vacuum", + "Vacuum", + new[] { "pg_xmin_horizon", "pg_autovacuum_stats", "pg_wraparound_stats" }, + /* One tab, three panels, in causal order. Read separately each of the three looks survivable. */ + "One story in three panels: something holds the xmin horizon, the horizon starves vacuum, and " + + "vacuum falling behind ends in wraparound. Read on their own each of these looks survivable."), + + new ViewerPostgresTab( + ViewerServerTab.PgWaitsInnerTabIndex, + "waits", + "Waits", + new[] { "pg_wait_stats" }, + null), + + new ViewerPostgresTab( + ViewerServerTab.PgIoInnerTabIndex, + "io", + "I/O", + new[] { "pg_io_stats" }, + null), + + new ViewerPostgresTab( + ViewerServerTab.PgReplicationInnerTabIndex, + "replication", + "Replication", + new[] { "pg_replication_slots" }, + null), + }; + + /// The first PostgreSQL tab's index — what a PostgreSQL server's tab strip selects, since + /// every index below it belongs to a collapsed SQL Server tab. + public static int FirstInnerTabIndex => All[0].InnerTabIndex; + + /// True when belongs to this registry. + public static bool Owns(int innerTabIndex) => All.Any(t => t.InnerTabIndex == innerTabIndex); + + /// + /// The store table a declared collector writes, from the collector's own definition. Null for a name + /// the catalog does not know, which ViewerPostgresTabsTests refuses — the registry may not name + /// a collector that does not exist. + /// + public static string? TableOf(string collectorName) => + CollectorCatalog.Find(collectorName)?.TargetTable; + + /// + /// Every PostgreSQL collector the catalog ships, derived rather than listed. The pins compare this + /// against in BOTH directions: a collector missing from the registry is a screen that + /// was never built, and a registry entry that is not a PostgreSQL collector is a panel wired to + /// something that will never fill it. + /// + public static IReadOnlyList PostgresCollectors() => + CollectorCatalog.All + .Where(d => d.TargetEngine == CollectorTargetEngine.PostgreSql) + .OrderBy(d => d.Name, StringComparer.Ordinal) + .ToList(); +} diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs new file mode 100644 index 0000000000..f1dbfbce34 --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs @@ -0,0 +1,282 @@ +/* + * 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.Threading.Tasks; +using System.Windows; +using System.Windows.Controls; +using PerformanceMonitor.Collectors; +using PerformanceMonitor.Darling.Storage; + +namespace PerformanceMonitor.Darling.Viewer; + +/// +/// The six PostgreSQL inner tabs (#2530) — which tab set a server gets, what each tab loads, and the +/// projections that turn stored rows into something a grid may show. +/// +/// Why the projections exist rather than binding the reader records directly. Every one of +/// these reads carries at least one value a grid must not print raw. -1 is the store's +/// not-applicable sentinel for a duration or a size (0 was rejected because it reads as "started this +/// instant" / "retains nothing"); pg_stat_io's write counters are genuinely ABSENT on Aurora rather +/// than zero, because backends there do not write data files; recurrence is NULL when the blocker's own row +/// had already left pg_stat_activity, and "cannot tell" is a different claim from "seen once"; and +/// every timestamp is naive UTC that has to go through like every +/// other timestamp the viewer renders. A DataGrid bound straight at the record would show -1, a +/// misleading 0, and UTC. +/// +/// Every panel says why it is empty. asks +/// first — the same sentence the MCP surface and +/// the web dashboard print, naming the server, the engine, the collector and the exact surface it would have +/// read, ending "and never will". That is what makes the two Aurora-only panels worth SHOWING on stock +/// PostgreSQL rather than hiding: the defect #2530 is about is unexplained emptiness, not emptiness, and a +/// tab set that changed shape between two PostgreSQL servers in one fleet would be its own confusion. +/// +public partial class ViewerServerTab +{ + /// + /// A row limit for the per-tab PostgreSQL grids. Generous — these are already server-side ranked reads + /// — but bounded, because a fleet-sized autovacuum backlog is tens of thousands of tables and a WPF + /// DataGrid asked to realise all of them is a hung UI thread rather than a slow one. + /// + private const int PgGridRowLimit = 200; + + /// + /// Picks the engine's tab set, once, in the constructor. Only a POSITIVE PostgreSQL claim switches: + /// is false for an absent kind, an unrecognised token and a + /// server that has never connected, none of which is evidence for either engine. Falling back to the + /// SQL Server tabs there is the pre-#2530 behaviour, unchanged — and it has to be, because the + /// unclaimed population is every server that has not reconnected since the engine-kind rung landed. + /// + private void ApplyEngineTabSet() + { + if (_server.EngineDescription is { } engine) + { + ServerEngineText.Text = engine; + ServerEngineBadge.Visibility = Visibility.Visible; + } + + if (!_server.IsPostgres) + { + return; + } + + /* Collapse the SQL Server run and reveal the PostgreSQL one. Visibility rather than removal, so + both sets keep their fixed indices: LoadInnerTabAsync dispatches on SelectedIndex and every + drill-down in the viewer navigates by one of the index constants above. */ + for (var i = 0; i < InnerTabs.Items.Count; i++) + { + if (InnerTabs.Items[i] is TabItem item) + { + item.Visibility = ViewerPostgresTabs.Owns(i) ? Visibility.Visible : Visibility.Collapsed; + } + } + + /* A collapsed TabItem can still be the SELECTED one — WPF hides the header and shows the content + anyway — so the selection has to move explicitly. Index 0 (the SQL Server Overview lanes) would + otherwise be what a PostgreSQL server opens on. */ + InnerTabs.SelectedIndex = ViewerPostgresTabs.FirstInnerTabIndex; + + /* The per-server database filter drives the SQL Server database-scoped reads and nothing else. On a + PostgreSQL server it would sit there offering to filter views that never consult it, which is a + worse answer than not offering. */ + DatabaseFilterButton.Visibility = Visibility.Collapsed; + + PgEngineBanner.Text = $"{_server.DisplayName} runs {engineOrUnknown()}."; + PgOverviewNote.Text = ViewerPostgresTabs.All[0].Note ?? string.Empty; + PgActivityNote.Text = ViewerPostgresTabs.All[1].Note ?? string.Empty; + PgVacuumNote.Text = ViewerPostgresTabs.All[2].Note ?? string.Empty; + + string engineOrUnknown() => + _server.EngineDescription ?? "an engine the store has not recorded"; + } + + // ───────────────────────────────────────────────────────────────────────────────────────────── + // Empty-state prose + // ───────────────────────────────────────────────────────────────────────────────────────────── + + /// + /// What a panel says when it has nothing to show. Three different answers, because they need three + /// different responses from the person reading them: a collector this engine can never run (the shared + /// capability sentence — nothing to do), a zero-row window for a collector where zero IS the answer + /// (nothing wrong), and a zero-row window for a collector that should have data (something to chase). + /// Returns empty for a panel with rows: the rows speak for themselves and a standing sentence above a + /// full grid is noise. + /// + private string PanelNote(string collectorName, int rowCount, string healthyEmptyText) + { + var gap = CollectorEngineCapability.NotCollectedMessage( + _server.ServerName, _server.EngineEdition, _server.EngineKind, collectorName); + + if (gap is not null) + { + return gap; + } + + return rowCount > 0 ? string.Empty : healthyEmptyText; + } + + /// True when a panel's collector cannot run here — used to skip the read entirely rather than + /// spend a round trip proving a permanent gap is still permanent. + private bool PgCollectorIsGatedOff(string collectorName) => + CollectorEngineCapability.NotCollectedMessage( + _server.ServerName, _server.EngineEdition, _server.EngineKind, collectorName) is not null; + + // ───────────────────────────────────────────────────────────────────────────────────────────── + // Tab loaders + // ───────────────────────────────────────────────────────────────────────────────────────────── + + /// + /// Overview — every PostgreSQL collector for this server, from collection_log composed against + /// the CATALOG. Catalog-driven so a collector that is gated off for this engine (and therefore writes + /// no log row at all) is still a visible row carrying its own explanation, instead of the one line an + /// operator most needs being the one line missing. + /// + private async Task LoadPgOverviewAsync() + { + var (startUtc, endUtc) = GetWindowUtc(); + var collectors = ViewerPostgresTabs.PostgresCollectors(); + + var facts = await _dataService.GetPostgresCollectorLogFactsAsync( + _server.ServerId, startUtc, endUtc, collectors.Select(c => c.Name).ToList()); + + PgCollectorHealthGrid.ItemsSource = + ViewerDataService.BuildPostgresCollectorHealth(_server, collectors, facts); + } + + /// + /// Activity — blocking (denominator, chains, cycles) and query shapes (statements over per-database + /// counters). All five reads fire together: the sub-tabs are two views of one load, so switching + /// between them needs no second round trip. + /// + private async Task LoadPgActivityAsync() + { + var (startUtc, endUtc) = GetWindowUtc(); + + var countsTask = _dataService.GetPgBlockingCaptureCountsAsync(_server.ServerId, startUtc, endUtc); + var chainsTask = _dataService.GetPgBlockingChainsAsync(_server.ServerId, startUtc, endUtc, PgGridRowLimit); + var cyclesTask = _dataService.GetPgBlockingCyclesAsync(_server.ServerId, startUtc, endUtc, PgGridRowLimit); + + /* Aurora-only, so on stock PostgreSQL the read is skipped and the panel carries the capability + sentence instead. Skipped rather than run-and-discarded: the answer is decided by the collectors' + own gate and cannot change between now and the query returning. */ + var statementsGatedOff = PgCollectorIsGatedOff("pg_statement_stats"); + var statementsTask = statementsGatedOff + ? Task.FromResult(new List()) + : _dataService.GetPgTopQueriesAsync(_server.ServerId, startUtc, endUtc); + + var databasesTask = _dataService.GetPgDatabaseStatsAsync(_server.ServerId, startUtc, endUtc, PgGridRowLimit); + + await Task.WhenAll(countsTask, chainsTask, cyclesTask, statementsTask, databasesTask); + + var counts = countsTask.Result; + var chains = chainsTask.Result; + var cycles = cyclesTask.Result; + + PgBlockingChainsGrid.ItemsSource = chains.Select(PgDisplay.Chain).ToList(); + PgBlockingCyclesGrid.ItemsSource = cycles.Select(PgDisplay.Cycle).ToList(); + PgBlockingCyclesExpander.Visibility = cycles.Count > 0 ? Visibility.Visible : Visibility.Collapsed; + + /* The denominator, always — not only when the grid is empty. "Three chains" means something + different in a window of 60 captures than in a window of 4, and the edge table cannot tell an + absent capture from a capture that found nothing: both are an absence of rows. This is the whole + reason the read carries capture counts from collection_log. */ + PgBlockingNote.Text = PgCollectorIsGatedOff("pg_blocking") + ? PanelNote("pg_blocking", 0, string.Empty) + : counts.CapturesTotal == 0 + ? "No blocking capture ran for this server in this window, so this grid being empty says " + + "nothing about whether anything blocked. Check the Overview tab for the pg_blocking " + + "collector's status." + : $"{counts.CapturesWithBlocking:N0} of {counts.CapturesTotal:N0} captures in this window " + + $"saw blocking{(chains.Count == 0 && cycles.Count == 0 ? " — and none of them produced a chain or a cycle in view" : "")}."; + + PgTopQueriesGrid.ItemsSource = statementsTask.Result.Select(PgDisplay.Statement).ToList(); + PgStatementsNote.Text = PanelNote("pg_statement_stats", statementsTask.Result.Count, + "No statement accumulated execution time in this window."); + + PgDatabaseStatsGrid.ItemsSource = databasesTask.Result.Select(PgDisplay.Database).ToList(); + PgDatabasesNote.Text = PanelNote("pg_database_stats", databasesTask.Result.Count, + "No database counter moved in this window."); + } + + /// + /// Vacuum — the xmin horizon, the autovacuum backlog and the freeze headroom, in that causal order. + /// One load for all three: they are one story, and reading them a tab apart is how each of them ends up + /// looking survivable. + /// + private async Task LoadPgVacuumAsync() + { + var (startUtc, endUtc) = GetWindowUtc(); + + var xminTask = _dataService.GetPgXminHorizonAsync(_server.ServerId, startUtc, endUtc); + var autovacuumTask = _dataService.GetPgAutovacuumAsync(_server.ServerId, startUtc, endUtc, PgGridRowLimit); + var wraparoundTask = _dataService.GetPgWraparoundAsync(_server.ServerId, startUtc, endUtc); + + await Task.WhenAll(xminTask, autovacuumTask, wraparoundTask); + + PgXminHorizonGrid.ItemsSource = xminTask.Result.Select(PgDisplay.Xmin).ToList(); + PgXminNote.Text = PanelNote("pg_xmin_horizon", xminTask.Result.Count, + "Nothing held the xmin horizon back in this window — no long-running transaction, replication " + + "slot, standby feedback or prepared transaction pinned an old xmin. Zero rows is the healthy " + + "answer here, not a missing read."); + + PgAutovacuumGrid.ItemsSource = autovacuumTask.Result.Select(PgDisplay.Autovacuum).ToList(); + PgAutovacuumNote.Text = PanelNote("pg_autovacuum_stats", autovacuumTask.Result.Count, + "No table reported an autovacuum backlog in this window. This collector runs on the WRITER " + + "only — pg_stat_user_tables reports zeros on a replica — so an empty grid on a reader is " + + "expected rather than informative."); + + PgWraparoundGrid.ItemsSource = wraparoundTask.Result.Select(PgDisplay.Wraparound).ToList(); + PgWraparoundNote.Text = PanelNote("pg_wraparound_stats", wraparoundTask.Result.Count, + "No per-database freeze headroom has been collected in this window."); + } + + /// Waits — Aurora's cumulative wait counters. Shown on stock PostgreSQL too, where the panel + /// carries the capability sentence rather than a blank rectangle; see the type header. + private async Task LoadPgWaitsAsync() + { + var (startUtc, endUtc) = GetWindowUtc(); + + var rows = PgCollectorIsGatedOff("pg_wait_stats") + ? new List() + : await _dataService.GetPgWaitStatsAsync(_server.ServerId, startUtc, endUtc, PgGridRowLimit); + + PgWaitStatsGrid.ItemsSource = rows.Select(PgDisplay.Wait).ToList(); + PgWaitsNote.Text = PanelNote("pg_wait_stats", rows.Count, + "No wait time was recorded for this server in this window."); + } + + /// I/O — pg_stat_io, differenced over the window. + private async Task LoadPgIoAsync() + { + var (startUtc, endUtc) = GetWindowUtc(); + + var rows = await _dataService.GetPgIoAsync(_server.ServerId, startUtc, endUtc, PgGridRowLimit); + + PgIoStatsGrid.ItemsSource = rows.Select(PgDisplay.Io).ToList(); + PgIoNote.Text = PanelNote("pg_io_stats", rows.Count, + "No backend / object / context combination did any I/O in this window. This view needs " + + "PostgreSQL 16 or newer, where pg_stat_io exists; below that the collector does not run and " + + "the Overview tab says so."); + } + + /// Replication — slot WAL retention and the xmin each slot pins. + private async Task LoadPgReplicationAsync() + { + var (startUtc, endUtc) = GetWindowUtc(); + + var rows = await _dataService.GetPgSlotsAsync(_server.ServerId, startUtc, endUtc); + + PgReplicationSlotsGrid.ItemsSource = rows.Select(PgDisplay.Slot).ToList(); + PgReplicationNote.Text = PanelNote("pg_replication_slots", rows.Count, + "This server has no replication slots, so nothing is retaining WAL or pinning an xmin on their " + + "account. Zero rows is the healthy answer here, not a missing read."); + } +} diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml index c69b90b8e7..96f1a2756f 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml @@ -409,6 +409,15 @@ + + + + @@ -3592,6 +3601,374 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml.cs index dd31554532..0c6b0f393b 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml.cs @@ -67,9 +67,26 @@ resource run (Latches & Spinlocks was formerly at position 3, System Events dead /* #1496 Long Queries — the opt-in completion trace surface, appended after the diagnostics tail so no existing inner-tab index shifts (drill-down navigation keys on these constants). MUST remain the last - in ViewerServerTab.xaml's InnerTabs. */ + SQL SERVER in ViewerServerTab.xaml's InnerTabs. */ internal const int LongQueriesInnerTabIndex = 18; + /* ── The PostgreSQL run (#2530) ─────────────────────────────────────────────────────────────── + Indices 19-24 CONTINUE the same TabControl rather than living in a second one, and every index + above stays exactly where it was. That is the whole reason for the layout: for a PostgreSQL server + the nineteen SQL Server TabItems are collapsed and these six are shown, so the two sets never + renumber each other and no drill-down that keys on a constant above can be broken by this feature. + + ViewerPostgresTabs is the registry these belong to — headers, panel-to-collector mapping and the + framing notes live there, and ViewerPostgresTabsTests holds it to CollectorCatalog in both + directions so a tenth PostgreSQL collector cannot ship without a screen. Keep these constants, the + registry's InnerTabIndex values and the order in the XAML in step; two pins check it. */ + internal const int PgOverviewInnerTabIndex = 19; + internal const int PgActivityInnerTabIndex = 20; + internal const int PgVacuumInnerTabIndex = 21; + internal const int PgWaitsInnerTabIndex = 22; + internal const int PgIoInnerTabIndex = 23; + internal const int PgReplicationInnerTabIndex = 24; + private readonly ViewerDataService _dataService; private readonly DarlingServer _server; private readonly ViewerServerStore? _serverStore; @@ -137,6 +154,13 @@ chart chrome applies without any per-control theme plumbing. */ /* CPU Scheduler sub-tab chart (cpu_scheduler_stats parity): pressure trend + latest-snapshot grid. */ InitializeCpuSchedulerChart(); + /* #2530: a PostgreSQL target gets the PostgreSQL tab set. Done here, once, from the server record + the sidebar already loaded — not from an async probe — because the tab strip must be right on the + FIRST paint: the web half's review found that moving this decision into an async callback broke + the page's render model twice over, and the viewer has an even shorter path to get it right + since the engine kind arrives with the server row itself. */ + ApplyEngineTabSet(); + /* Memory inner-tab charts (copied from Lite): same up-front theme + hover for the five Memory charts (Overview trend, Clerks, Grant sizing/activity, Pressure events). */ InitializeMemoryCharts(); @@ -295,6 +319,14 @@ visible tab renders (the viewer's visible-only rule), so its offset wins. Loaded /// private async Task UpdatePermissionDeniedBadgeAsync() { + /* #2530: Collection Health is a SQL Server tab and is collapsed for a PostgreSQL server, which + gets the same answer on its own Overview tab instead. Writing a header nobody can see would be + harmless; issuing the read every refresh to do it would not. */ + if (_server.IsPostgres) + { + return; + } + try { var denied = await _dataService.GetPermissionDeniedCollectorCountAsync(_server.ServerId); @@ -392,6 +424,31 @@ not loaded on tab-switch. Explicit no-op so selecting it doesn't fall through to case LongQueriesInnerTabIndex: await LoadLongQueriesAsync(); break; + + /* #2530: the PostgreSQL run. Explicit arms, never the default: falling through to + LoadOverviewChartsAsync would point the SQL Server correlated-lane reads (CPU %, wait + ms/sec, buffer pool, file I/O latency — four collectors that cannot run on this engine) + at a PostgreSQL server and paint four permanently empty lanes, which is the exact defect + this issue is about. */ + case PgOverviewInnerTabIndex: + await LoadPgOverviewAsync(); + break; + case PgActivityInnerTabIndex: + await LoadPgActivityAsync(); + break; + case PgVacuumInnerTabIndex: + await LoadPgVacuumAsync(); + break; + case PgWaitsInnerTabIndex: + await LoadPgWaitsAsync(); + break; + case PgIoInnerTabIndex: + await LoadPgIoAsync(); + break; + case PgReplicationInnerTabIndex: + await LoadPgReplicationAsync(); + break; + case OverviewInnerTabIndex: default: await LoadOverviewChartsAsync(); diff --git a/Darling/README.md b/Darling/README.md index 4b571fbcfa..68bf6b042c 100644 --- a/Darling/README.md +++ b/Darling/README.md @@ -641,11 +641,11 @@ The two Aurora-only collectors are gated on Aurora detection, not on configurati Cadences and retention are the shared defaults, with no knobs, exactly as for SQL Server. -Three of the seven are outage predictors rather than performance metrics, which is deliberate: PostgreSQL's most damaging failures are quiet, slow, and fully predictable days ahead, and nothing in the engine raises its hand about them. The read surface for each is an MCP tool — see [the tool list](#mcp). +Three of the nine are outage predictors rather than performance metrics, which is deliberate: PostgreSQL's most damaging failures are quiet, slow, and fully predictable days ahead, and nothing in the engine raises its hand about them. Every one of the nine has an MCP tool (see [the tool list](#mcp)), a web tab and — since #2530 — a WPF viewer tab: a PostgreSQL target gets **six** inner tabs (Overview, Activity, Vacuum, Waits, I/O, Replication) in place of the nineteen SQL Server ones, chosen from `collect.servers.engine_kind`. The placement is derived from the collector catalog in both directions, so a tenth PostgreSQL collector cannot ship without a screen. ### What it does not do yet -Plan capture and the blocking-chain reads have no PostgreSQL equivalent in the store yet. Alerting and scheduled analysis are still SQL-Server-shaped, so a PostgreSQL target collects and is readable through MCP and the viewer, but does not yet raise alerts or produce analysis findings. **The three Tier 0 outage predictors DO alert.** Wraparound risk, a blocked vacuum horizon and replication-slot retention are evaluated on the alert cadence and delivered through the same deliverer, history and mute rules as every SQL Server alert, so they land in the same places and obey the same suppression. They ride alongside the shared engine rather than inside it, via a separate `IPostgresAlertReadAdapter` consulted only for PostgreSQL targets — Lite has no PostgreSQL target, and extending the shared adapter would have left it implementing three methods that can only return empty. Thresholds are derived from the server's own settings (wraparound grades against that cluster's `autovacuum_freeze_max_age`, not a constant) and are not yet configurable; see [`docs/postgres-alerting-design-note.md`](../docs/postgres-alerting-design-note.md). +Plan capture has no PostgreSQL equivalent in the store yet, and that is the largest remaining gap — plans are why people open a database monitoring tool. Alerting and scheduled analysis are still SQL-Server-shaped, so a PostgreSQL target collects and is readable through MCP, the web dashboard and the viewer, but does not yet raise alerts or produce analysis findings. The PostgreSQL tabs on both UIs are tables over the window: no charts, no correlated timeline, and no drill-down from a blocking root to the sessions behind it. **The three Tier 0 outage predictors DO alert.** Wraparound risk, a blocked vacuum horizon and replication-slot retention are evaluated on the alert cadence and delivered through the same deliverer, history and mute rules as every SQL Server alert, so they land in the same places and obey the same suppression. They ride alongside the shared engine rather than inside it, via a separate `IPostgresAlertReadAdapter` consulted only for PostgreSQL targets — Lite has no PostgreSQL target, and extending the shared adapter would have left it implementing three methods that can only return empty. Thresholds are derived from the server's own settings (wraparound grades against that cluster's `autovacuum_freeze_max_age`, not a constant) and are not yet configurable; see [`docs/postgres-alerting-design-note.md`](../docs/postgres-alerting-design-note.md). Scheduled ANALYSIS is still SQL-Server-shaped, so a PostgreSQL target does not yet produce analysis findings. diff --git a/docs/uat-onboarding.md b/docs/uat-onboarding.md index 90f6e35b7d..d7efd88a87 100644 --- a/docs/uat-onboarding.md +++ b/docs/uat-onboarding.md @@ -527,11 +527,20 @@ its avg CPU and avg duration over the window plus a grid of its per-collection s are exactly the queries the table shows** — the picker indexes into the rows rendered directly above it, rather than reading a second, wider list that could offer a query the table does not. -**The nine `get_pg_*` PostgreSQL reads are on the web now, and are still absent from the WPF viewer.** That -is the current state of [#2530](https://github.com/erikdarlingdata/PerformanceMonitor/issues/2530): the web -dashboard renders the six PostgreSQL tabs above at any target whose `engine_kind` says PostgreSQL, and the -desktop viewer has no PostgreSQL screens at all — so on Windows a PostgreSQL target is still readable only -through MCP. If you are evaluating this for PostgreSQL monitoring, know which surface you are evaluating. +**The nine PostgreSQL capture paths are now on BOTH surfaces.** +[#2530](https://github.com/erikdarlingdata/PerformanceMonitor/issues/2530) landed the web tabs first and the +WPF viewer second, and the desktop set is deliberately the same six with the same names and the same +grouping — Overview, Activity, Vacuum, Waits, I/O, Replication — so the two front ends do not teach one engine +two shapes. The desktop panels are grids rather than charts, and the desktop Overview is the per-collector +collection report (which of the nine ran, when, with what result, and for one that cannot run here, why), +which is the answer a PostgreSQL operator wants first and the one the web splits across tiles. + +Where the desktop set genuinely differs: the per-server **database filter is hidden** at a PostgreSQL target, +because it drives the SQL Server database-scoped reads and nothing else — offering to filter views that never +consult it is worse than not offering. And the Blocking panel prints its **sampling denominator** above the +grid whether or not the grid has rows: `pg_blocking` is a periodic sample, not an event log, so "two chains" +means something different in a window of 60 captures than in a window of 4, and an absent capture and a +capture that found nothing are the same absence of rows in the stored edge list. **Two of the nine PostgreSQL capture paths are Amazon Aurora-only**, so on a stock PostgreSQL target two panels are permanently empty — and neither is a fault. `get_pg_wait_stats` reads `aurora_stat_system_waits()` @@ -551,8 +560,10 @@ PostgreSQL target from a SQL Server one — a PostgreSQL target lands at `engine a SQL Server that has never connected lands at. The store now records the target engine explicitly (`collect.servers.engine_kind`, schema v82) and the card carries `engine_kind`, the derived `is_postgres` / `is_aurora` booleans, and `engine_description` — the engine's name in words, composed on the -server so no viewer owns a second copy of the vocabulary. **The web tab registry now branches on it**; the -WPF viewer does not, so a PostgreSQL target on the desktop still renders SQL Server tabs. +server so no viewer owns a second copy of the vocabulary. **Both tab registries now branch on it** — the web's +`serverTabsFor(card)` and the viewer's `ViewerPostgresTabs` — and the viewer's per-server header carries the +same engine label. A server the store makes no claim about gets **no badge at all** rather than one reading +"SQL Server": the tabs it gets are a default, not a finding. One consequence you may notice before the screens arrive: a SQL Server read aimed at a PostgreSQL target now answers `not_collected` naming the engine ("...runs Aurora PostgreSQL. The system_health_events collector is @@ -873,9 +884,13 @@ These are known, current, and not bugs: its page set is a deliberate subset of the viewer's. - **`network` blocks need a service restart.** There is no hot reload for exposure. The service now says so at startup, naming the block it loaded, so a config edit that appears to do nothing is at least explained. -- **PostgreSQL monitored targets** collect and are readable, but apart from the three Tier 0 outage predictors - they do not raise alerts or produce analysis findings yet, and there is no plan capture or blocking-chain - read for them. See [PostgreSQL Targets](../Darling/README.md#postgresql-targets). +- **PostgreSQL monitored targets** collect and are readable on both UIs, but apart from the three Tier 0 + outage predictors they do not raise alerts or produce analysis findings yet, and there is **no execution + plan capture at all** for them — which is the largest remaining gap, since plans are why people open a + database monitoring tool. See [PostgreSQL Targets](../Darling/README.md#postgresql-targets). +- **The PostgreSQL tabs have no charts and no drill-downs.** Every panel on both surfaces is a table over the + window; there is no PostgreSQL equivalent of the SQL Server Overview's correlated timeline lanes, no + click-through from a blocking root to the sessions behind it, and no per-query history window. If you hit something that is *not* on this list and *not* in [Troubleshooting](../Darling/README.md#troubleshooting), that is the interesting kind — please report it with From 8a98618e0217745c4894b5e693788748ef62555d Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:20:58 +0100 Subject: [PATCH 3/8] Address review: a dropped stored field is a category, not two columns All three claude[bot] findings were real. Two of them are the same defect - a field the shared reader goes to the trouble of returning that no viewer column shows - so the fix is an invariant rather than two columns, per the rule that the second instance of something is a sign you are fixing the wrong level. EveryFieldTheSharedReadersReturn_ReachesAColumn_OrIsFoldedWithAReason reflects over the ten reader row records against their display classes: every field must reach a column, or appear in FoldedFields naming the column that carries the fact and why that shape reads better. Derived both ways - a new reader field turns it red until something happens, and an exemption for a field that no longer exists is deleted the moment the reader stops returning it. Proved red by deleting exactly the two columns review found, each naming itself. It found three more on its first run, which is the point: PctTowardMultixactWraparound had no column at all (the multixact side has its own shutdown ceiling and a subtransaction-heavy workload reaches it first, with nothing on the XID columns to say so), and the two freeze_max_age settings were renamed rather than shown. The findings themselves: - Replication gains a Conflicting column, folded into the invalidated row highlight. A logical slot whose needed rows were vacuumed away by a recovery conflict is a different failure from invalidation-by-WAL-size, with a different cause (hot_standby_feedback, not max_slot_wal_keep_size), so it is its own column rather than folded into the invalidation reason. It was visible on the web and invisible on the desktop. - I/O gains ExtendTimeMs plus the three derivations the MCP tool already computed - avg read ms, a per-combination hit ratio, and each row's share of the window's read time (which is how the grid's order was decided, made legible). ContextMeaning moved to DarlingPgIoReader, beside the query that produces the value it explains, so the MCP surface and the viewer print one copy; it rides as the Context cell's tooltip because it is a paragraph. - The csproj comment's triple apostrophe, from my own shell escaping. Also filled while the pin was pointing at them: the autovacuum grid's ANALYZE half (a table can be vacuumed on schedule and still give the planner stale row counts), live tuples and the dead-tuple SHARE, the manual-vs-automatic run split, and autovacuum_count - zero being the classic wraparound route, since relfrozenxid never advances on a table autovacuum has never processed. Statements gain the database OID (half the read's grain), one cache-hit ratio over both Aurora cache tiers, and temp blocks READ beside written. Co-Authored-By: Claude Opus 5 --- .../Darling.Tests/ViewerPostgresTabsTests.cs | 157 +++++++++++++++++- .../Mcp/DarlingMcpPgIoTools.cs | 21 +-- .../DarlingPgIoReader.cs | 27 +++ .../PerformanceMonitor.Darling.Storage.csproj | 2 +- .../ViewerPostgresDisplay.cs | 151 ++++++++++++++++- .../ViewerServerTab.Postgres.cs | 2 +- .../ViewerServerTab.xaml | 40 ++++- 7 files changed, 368 insertions(+), 32 deletions(-) diff --git a/Darling/Darling.Tests/ViewerPostgresTabsTests.cs b/Darling/Darling.Tests/ViewerPostgresTabsTests.cs index 1dd89c56a3..e239fdc15e 100644 --- a/Darling/Darling.Tests/ViewerPostgresTabsTests.cs +++ b/Darling/Darling.Tests/ViewerPostgresTabsTests.cs @@ -10,9 +10,11 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Reflection; using System.Runtime.CompilerServices; using System.Text.RegularExpressions; using PerformanceMonitor.Collectors; +using PerformanceMonitor.Darling.Storage; using PerformanceMonitor.Darling.Viewer; using Xunit; @@ -448,7 +450,7 @@ public void AnUntrackedIoCounter_SaysSo_RatherThanReadingZero() var aurora = PgDisplay.Io(new PerformanceMonitor.Darling.Storage.DarlingPgIoReader.PgIoRow( "client backend", "relation", "normal", Reads: 10, ReadTimeMs: 1.0, Hits: 5, Extends: 0, ExtendTimeMs: 0, Evictions: 0, Reuses: 0, Writes: 0, WriteTimeMs: 0, OpBytes: 8192, - WriteCountersTracked: false, StatsReset: null)); + WriteCountersTracked: false, StatsReset: null), totalReadTimeMs: 1.0); Assert.Equal("not tracked", aurora.Writes); Assert.Equal("not tracked", aurora.WriteTimeMs); @@ -456,11 +458,162 @@ public void AnUntrackedIoCounter_SaysSo_RatherThanReadingZero() var selfManaged = PgDisplay.Io(new PerformanceMonitor.Darling.Storage.DarlingPgIoReader.PgIoRow( "client backend", "relation", "normal", Reads: 10, ReadTimeMs: 1.0, Hits: 5, Extends: 0, ExtendTimeMs: 0, Evictions: 0, Reuses: 0, Writes: 0, WriteTimeMs: 0, OpBytes: 8192, - WriteCountersTracked: true, StatsReset: null)); + WriteCountersTracked: true, StatsReset: null), totalReadTimeMs: 1.0); Assert.Equal("0", selfManaged.Writes); } + // ── Nothing the reader returns is dropped on the floor ─────────────────────────────────────── + + /// + /// Every field the shared PostgreSQL readers return reaches a viewer grid column, is FOLDED into one + /// with a stated reason, or the pin fails naming it. + /// + /// Why this exists as a category rather than as two fixes. Review found two instances of + /// the same defect in one PR — the replication grid silently dropped Conflicting (a broken + /// logical slot, invisible on the desktop while the web showed it) and the I/O grid dropped + /// ExtendTimeMs plus three derived columns. Two instances, one category: a stored field the + /// reader goes to the trouble of returning and no screen shows. Fixing the instances would have left + /// the third one to be found by someone else, so the invariant is asserted instead — a projection is + /// only allowed to omit a field by SAYING SO here, and the saying-so has to name a real field. + /// + /// Derived from the shipped record types by reflection, both directions: a field added to a + /// reader turns this red until a column or an exemption appears, and an exemption for a field that no + /// longer exists is deleted the moment the reader stops returning it. + /// + [Fact] + public void EveryFieldTheSharedReadersReturn_ReachesAColumn_OrIsFoldedWithAReason() + { + var missing = new List(); + + foreach (var (readerRow, displayRow) in ProjectionPairs()) + { + var shown = displayRow + .GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Select(p => p.Name) + .ToHashSet(StringComparer.Ordinal); + + foreach (var field in readerRow.GetProperties(BindingFlags.Public | BindingFlags.Instance)) + { + var key = $"{readerRow.Name}.{field.Name}"; + if (shown.Contains(field.Name) || FoldedFields.ContainsKey(key)) + { + continue; + } + + missing.Add(key); + } + } + + Assert.True(missing.Count == 0, + "Stored PostgreSQL field(s) the shared reader returns reach no viewer column and are not " + + "recorded as folded. Add a column, or add an entry to FoldedFields saying which column carries " + + "the fact and why that is the better shape:\n " + + string.Join("\n ", missing.OrderBy(m => m, StringComparer.Ordinal))); + } + + /// The ratchet's other half: an exemption may only name a field that still exists, so a + /// renamed or removed reader field cannot leave a stale excuse behind that quietly covers its + /// replacement. + [Fact] + public void EveryFoldedField_StillExists_AndSaysWhy() + { + var real = ProjectionPairs() + .SelectMany(pair => pair.Reader + .GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Select(p => $"{pair.Reader.Name}.{p.Name}")) + .ToHashSet(StringComparer.Ordinal); + + var stale = FoldedFields.Keys.Where(k => !real.Contains(k)).OrderBy(k => k, StringComparer.Ordinal).ToList(); + Assert.True(stale.Count == 0, + "FoldedFields names field(s) no shared reader returns any more — delete them: " + + string.Join(", ", stale)); + + var unexplained = FoldedFields + .Where(kv => string.IsNullOrWhiteSpace(kv.Value)) + .Select(kv => kv.Key) + .ToList(); + Assert.True(unexplained.Count == 0, + "FoldedFields entr(y/ies) with no reason: " + string.Join(", ", unexplained)); + } + + /// + /// Reader row type -> the display class that renders it. Named explicitly rather than matched by + /// convention, because a naming rule would silently pass a projection that was never wired up. + /// + private static IReadOnlyList<(Type Reader, Type Display)> ProjectionPairs() => new[] + { + (typeof(DarlingPgXminReader.PgXminRow), typeof(PgDisplay.XminRow)), + (typeof(DarlingPgAutovacuumReader.PgAutovacuumRow), typeof(PgDisplay.AutovacuumRow)), + (typeof(DarlingPgWraparoundReader.PgWraparoundRow), typeof(PgDisplay.WraparoundRow)), + (typeof(DarlingPgWaitReader.PgWaitRow), typeof(PgDisplay.WaitRow)), + (typeof(DarlingPgIoReader.PgIoRow), typeof(PgDisplay.IoRow)), + (typeof(DarlingPgSlotReader.PgSlotRow), typeof(PgDisplay.SlotRow)), + (typeof(DarlingPgBlockingReader.PgBlockingChainRow), typeof(PgDisplay.ChainRow)), + (typeof(DarlingPgBlockingReader.PgBlockingCycleRow), typeof(PgDisplay.CycleRow)), + (typeof(DarlingPgStatementReader.PgStatementRow), typeof(PgDisplay.StatementRow)), + (typeof(DarlingPgDatabaseReader.PgDatabaseRow), typeof(PgDisplay.DatabaseRow)), + }; + + /// + /// Fields a projection deliberately does not give a column of their own, and the column that carries + /// the fact instead. Every entry is a fold, never a drop: the fact still reaches the operator, in a + /// shape that reads better than the raw field would. + /// + private static readonly IReadOnlyDictionary FoldedFields = + new Dictionary(StringComparer.Ordinal) + { + ["PgXminRow.IsWinner"] = "IsWinnerText — rendered as a word rather than True/False", + ["PgXminRow.SamplesAsWinner"] = "WinnerShare — '58 of 60 samples' says more than either half alone", + ["PgXminRow.Samples"] = "WinnerShare — it is that fraction's denominator", + + ["PgAutovacuumRow.FirstDeadTuples"] = "DeadTupleTrend — the window's start, as a signed delta", + ["PgAutovacuumRow.FirstSeenAt"] = "DeadTupleTrend — the delta's other end; the window is the toolbar's", + ["PgAutovacuumRow.TotalBytes"] = "TotalSize — the same number, formatted", + + ["PgWraparoundRow.AllowsConnections"] = "ConnectionsAllowed — as words, because NOT ALLOWED is the finding", + ["PgWraparoundRow.AutovacuumFreezeMaxAge"] = "FreezeMaxAge — the same setting, without the column's prefix", + ["PgWraparoundRow.AutovacuumMultixactFreezeMaxAge"] = "MultixactFreezeMaxAge — same", + + ["PgIoRow.WriteCountersTracked"] = "Writes / WriteTimeMs — they read 'not tracked' rather than 0", + ["PgIoRow.OpBytes"] = "OpSize — the same number, formatted", + + ["PgSlotRow.FirstRetainedWalBytes"] = "RetainedWalTrend — the window's start, as a signed delta", + ["PgSlotRow.FirstSeenAt"] = "RetainedWalTrend — the delta's other end; the window is the toolbar's", + ["PgSlotRow.IsActive"] = "ActiveText, and the inactive row highlight", + ["PgSlotRow.RetainedWalBytes"] = "RetainedWal — the same number, formatted", + ["PgSlotRow.SafeWalSizeBytes"] = "SafeWalSize — the same number, formatted", + + ["PgBlockingChainRow.MaxDepth"] = "Depth — carries the '(capped)' marker with the number", + ["PgBlockingChainRow.ChainMayBeTruncated"] = "Depth's '(capped)' marker, and Caveats", + ["PgBlockingChainRow.QueryTextMayBeTruncated"] = "Caveats — one column for every qualifier on the row", + ["PgBlockingChainRow.RootIsIdleInTransaction"] = "RootState — 'idle in transaction' replaces the raw state", + ["PgBlockingChainRow.WorstVictimWaitMs"] = "WorstVictimWait — the -1 sentinel must not render as a number", + ["PgBlockingChainRow.RootXactDurationMs"] = "RootXactDuration — same sentinel handling", + ["PgBlockingChainRow.RootQueryDurationMs"] = + "RootXactDuration — the TRANSACTION's age is what holds the lock; a root idle in transaction " + + "has no query duration at all, so showing both invites reading the wrong one", + + ["PgStatementRow.SharedBlocksHit"] = "CacheHitPct — one ratio over both cache tiers and both miss sources", + ["PgStatementRow.OrcacheBlocksHit"] = "CacheHitPct — Aurora's Optimized Read cache, same ratio", + ["PgStatementRow.SharedBlocksRead"] = "CacheHitPct — the miss side of that ratio", + ["PgStatementRow.StorageBlocksRead"] = "CacheHitPct — Aurora's storage-layer miss, same ratio", + ["PgStatementRow.TempBlocksRead"] = "TempRead — labelled in blocks, not converted to bytes", + ["PgStatementRow.TempBlocksWritten"] = "TempWritten — labelled in blocks, not converted to bytes", + ["PgStatementRow.WalBytes"] = "WalWritten — the same number, formatted", + ["PgStatementRow.MaxPeakMemBytes"] = "PeakMemory — the same number, formatted", + ["PgStatementRow.TotalExecTimeMs"] = "TotalExecTimeMs is shown; AvgExecTimeMs derives from it and Calls", + + ["PgDatabaseRow.BlksHit"] = "CacheHitPct — the hit side of that ratio", + ["PgDatabaseRow.BlksRead"] = "CacheHitPct — the miss side of that ratio", + ["PgDatabaseRow.StatsResetCount"] = "Caveats — 'counters reset Nx in window', which is what it qualifies", + ["PgDatabaseRow.CounterRewindCount"] = "Caveats — the implicit-reset half of the same qualifier", + ["PgDatabaseRow.FirstSampleAt"] = + "SampleCount — the window's bounds are the toolbar's own and are shown there, so repeating " + + "them per row would be the same two timestamps on every line", + ["PgDatabaseRow.LastSampleAt"] = "SampleCount — the other end of that same window", + }; + // ── Helpers ────────────────────────────────────────────────────────────────────────────────── private static DarlingServer Server(string? engineKind) => diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs index d4320a27be..b7de2be754 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs @@ -27,24 +27,11 @@ public sealed class DarlingMcpPgIoTools /// /// Explains what a context value means, because it is the dimension with no SQL Server /// counterpart and the one that changes what you do about a number. + /// #2530 moved the prose to , beside the query that + /// produces the value it explains, so the MCP surface and the WPF viewer's I/O tab print one copy of it + /// rather than two that drift. Kept as a delegating member because this file's tests name it. /// - internal static string ContextMeaning(string? context) => context switch - { - "normal" => "Ordinary buffer-pool traffic. Reads here are cache misses that shared_buffers could " - + "have absorbed, so a high read share with a low hit share is the classic case for more " - + "memory or a better index.", - "bulkread" => "A sequential scan deliberately using a small ring buffer so it cannot evict the " - + "buffer pool. High volume here is a scan-heavy workload, NOT memory pressure — adding " - + "shared_buffers will not reduce it, because these reads bypass the pool by design.", - "bulkwrite" => "A bulk write (COPY, CREATE TABLE AS, some ALTER TABLE) using its own ring buffer.", - "vacuum" => "Vacuum's ring buffer. Volume here is autovacuum doing its job; pair it with " - + "get_pg_autovacuum_health to see whether it is keeping up.", - "index" => "Index-specific I/O, reported separately from the relation's own.", - "walreplay" => "A standby applying WAL. This is replica catch-up work, not query I/O, and it is the " - + "first thing to check when a reader lags.", - _ => "Unrecognized context — treat the raw counters as authoritative and check the PostgreSQL " - + "documentation for this server's major version.", - }; + internal static string ContextMeaning(string? context) => DarlingPgIoReader.ContextMeaning(context); [McpServerTool(Name = "get_pg_io_stats"), Description("Gets PostgreSQL I/O attributed to WHO did it, to WHAT, and WHY - the (backend_type, object, context) breakdown from pg_stat_io, differenced across the requested window. Richer than SQL Server's file-level dm_io_virtual_file_stats: instead of 'this file is busy' you get 'autovacuum workers are reading relations in the vacuum context', which names the cause. The context dimension is the one with no SQL Server equivalent and the one that changes the remedy - it separates ordinary buffer-pool misses (where more shared_buffers or a better index helps) from sequential scans that deliberately bypass the pool via a ring buffer (where it will not help at all), from vacuum's ring buffer, from a standby applying WAL. Reports whether write counters are TRACKED at all, because on Amazon Aurora they are always null - backends there do not write data files, the storage layer does - and a zero would otherwise read as 'no writes happened'. Requires PostgreSQL 16 or later; valid on a standby.")] public static async Task GetPgIoStats( diff --git a/Darling/PerformanceMonitor.Darling.Storage/DarlingPgIoReader.cs b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgIoReader.cs index 18cca92c4b..a0a2998886 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/DarlingPgIoReader.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgIoReader.cs @@ -146,4 +146,31 @@ row falls out of the window and the read silently returns nothing. Hidden by UTC return rows; } + + /// + /// What a context value MEANS — the dimension with no SQL Server counterpart, and the one that + /// changes what you do about a number rather than just how large it is. + /// + /// Lives beside the query that produces the value rather than in either front end (#2530): the MCP + /// tool and the WPF viewer's I/O tab both print it, and the copy that drifts is never the one being + /// read. A context this build has never seen gets an explicit "unrecognized" answer rather than silence, + /// because a blank explanation beside a large number reads as "nothing to say about it". + /// + public static string ContextMeaning(string? context) => context switch + { + "normal" => "Ordinary buffer-pool traffic. Reads here are cache misses that shared_buffers could " + + "have absorbed, so a high read share with a low hit share is the classic case for more " + + "memory or a better index.", + "bulkread" => "A sequential scan deliberately using a small ring buffer so it cannot evict the " + + "buffer pool. High volume here is a scan-heavy workload, NOT memory pressure — adding " + + "shared_buffers will not reduce it, because these reads bypass the pool by design.", + "bulkwrite" => "A bulk write (COPY, CREATE TABLE AS, some ALTER TABLE) using its own ring buffer.", + "vacuum" => "Vacuum's ring buffer. Volume here is autovacuum doing its job; pair it with " + + "get_pg_autovacuum_health to see whether it is keeping up.", + "index" => "Index-specific I/O, reported separately from the relation's own.", + "walreplay" => "A standby applying WAL. This is replica catch-up work, not query I/O, and it is the " + + "first thing to check when a reader lags.", + _ => "Unrecognized context — treat the raw counters as authoritative and check the PostgreSQL " + + "documentation for this server's major version.", + }; } diff --git a/Darling/PerformanceMonitor.Darling.Storage/PerformanceMonitor.Darling.Storage.csproj b/Darling/PerformanceMonitor.Darling.Storage/PerformanceMonitor.Darling.Storage.csproj index 832d107569..83208fd15f 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/PerformanceMonitor.Darling.Storage.csproj +++ b/Darling/PerformanceMonitor.Darling.Storage/PerformanceMonitor.Darling.Storage.csproj @@ -22,7 +22,7 @@ - diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs index d878758117..1060ba0ca7 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs @@ -80,6 +80,13 @@ internal static string ByteDelta(long from, long to) return delta == 0 ? "flat" : (delta > 0 ? "+" : "-") + Bytes(Math.Abs(delta)); } + /// A block count, LABELLED as blocks. PostgreSQL's block size is a compile-time setting of + /// the server, so converting to bytes here would put a fabricated figure next to measured ones. + internal static string Blocks(long value) => + value <= NotApplicable + ? NotApplicableText + : string.Create(CultureInfo.CurrentCulture, $"{value:N0} blocks"); + internal static string Count(long value) => value <= NotApplicable ? NotApplicableText : value.ToString("N0", CultureInfo.CurrentCulture); @@ -158,9 +165,17 @@ internal sealed class AutovacuumRow public string DeadTupleTrend { get; init; } = ""; public string InsertsSinceVacuum { get; init; } = ""; public string InsertVacuumThreshold { get; init; } = ""; + public string LastVacuum { get; init; } = ""; public string LastAutovacuum { get; init; } = ""; + public string LastAnalyze { get; init; } = ""; public string LastAutoanalyze { get; init; } = ""; + public string AutovacuumCount { get; init; } = ""; + public string LiveTuples { get; init; } = ""; + public string DeadTuplePct { get; init; } = ""; + public string ModsSinceAnalyze { get; init; } = ""; + public string AnalyzeThreshold { get; init; } = ""; public string TotalSize { get; init; } = ""; + public string MeasuredAt { get; init; } = ""; } internal static AutovacuumRow Autovacuum(DarlingPgAutovacuumReader.PgAutovacuumRow row) => new() @@ -187,9 +202,26 @@ than something to work out from two visits. */ InsertVacuumThreshold = row.InsertVacuumThreshold <= NotApplicable ? "n/a on this version" : Count(row.InsertVacuumThreshold), + /* Manual and automatic runs are separate columns because they answer different questions: a table + whose only vacuums are manual is one somebody is nursing, and that is a finding about the + configuration rather than about the workload. */ + LastVacuum = Timestamp(row.LastVacuum), LastAutovacuum = Timestamp(row.LastAutovacuum), + LastAnalyze = Timestamp(row.LastAnalyze), LastAutoanalyze = Timestamp(row.LastAutoanalyze), + /* Zero is the finding, not a missing value: a table autovacuum has NEVER processed is the classic + wraparound route, because relfrozenxid never advances. */ + AutovacuumCount = Count(row.AutovacuumCount), + LiveTuples = Count(row.LiveTuples), + /* The dead-tuple SHARE, which is what bloat actually is. The raw count beside a threshold says + whether vacuum is due; the share says how much of the table is already waste. */ + DeadTuplePct = Percent(row.DeadTuples, row.LiveTuples + row.DeadTuples), + /* The ANALYZE half of the same story, and it is genuinely separate: a table can be vacuumed on + schedule and still have statistics old enough to give the planner the wrong row counts. */ + ModsSinceAnalyze = Count(row.ModsSinceAnalyze), + AnalyzeThreshold = Count(row.AnalyzeThreshold), TotalSize = Bytes(row.TotalBytes), + MeasuredAt = Timestamp(row.MeasuredAt), }; internal sealed class WraparoundRow @@ -201,8 +233,12 @@ internal sealed class WraparoundRow public string XidsRemaining { get; init; } = ""; public string MinMultiXidAge { get; init; } = ""; public double PctTowardMultixactEmergency { get; init; } + public double PctTowardMultixactWraparound { get; init; } public string MultiXidsRemaining { get; init; } = ""; + public string FreezeMaxAge { get; init; } = ""; + public string MultixactFreezeMaxAge { get; init; } = ""; public string WindowPeakFrozenXidAge { get; init; } = ""; + public string WindowPeakMinMultiXidAge { get; init; } = ""; public string ConnectionsAllowed { get; init; } = ""; public string MeasuredAt { get; init; } = ""; } @@ -216,8 +252,18 @@ internal sealed class WraparoundRow XidsRemaining = Count(row.XidsRemaining), MinMultiXidAge = Count(row.MinMultiXidAge), PctTowardMultixactEmergency = Math.Round(row.PctTowardMultixactEmergency, 1), + /* The multixact side has its OWN shutdown ceiling, reached independently of the XID one - a + workload heavy on row-level share locks or subtransactions gets there first, and nothing on the + XID columns would say so. */ + PctTowardMultixactWraparound = Math.Round(row.PctTowardMultixactWraparound, 1), MultiXidsRemaining = Count(row.MultiXidsRemaining), + /* The percentages beside these are graded against THIS cluster's settings, not a constant, so the + settings themselves are a column: two databases at "80% to emergency" are different distances from + trouble when their freeze_max_age differs, and nothing else on the row would say so. */ + FreezeMaxAge = Count(row.AutovacuumFreezeMaxAge), + MultixactFreezeMaxAge = Count(row.AutovacuumMultixactFreezeMaxAge), WindowPeakFrozenXidAge = Count(row.WindowPeakFrozenXidAge), + WindowPeakMinMultiXidAge = Count(row.WindowPeakMinMultiXidAge), /* datallowconn=false is why a database can sit at 99% of wraparound and never be vacuumed by anything that connects to it. It is the finding, not a footnote. */ ConnectionsAllowed = row.AllowsConnections ? "allowed" : "NOT ALLOWED", @@ -249,10 +295,33 @@ internal sealed class IoRow public string BackendType { get; init; } = ""; public string ObjectType { get; init; } = ""; public string Context { get; init; } = ""; + + /// What this context MEANS, from the shared reader — the one copy the MCP surface also + /// prints. Shown as the Context cell's tooltip rather than a column: it is a paragraph, and it is + /// the dimension with no SQL Server counterpart, so an operator meeting `bulkread` for the first + /// time needs it and one who knows it does not want it eating the grid. + public string ContextMeaning { get; init; } = ""; + public string Reads { get; init; } = ""; public double ReadTimeMs { get; init; } + + /// Per-read latency — the figure that separates "a lot of I/O" from "slow I/O", which have + /// completely different remedies. + public string AvgReadMs { get; init; } = ""; + public string Hits { get; init; } = ""; + + /// A hit ratio scoped to THIS combination, the only scope where it means anything: a + /// server-wide ratio averages bulkread's deliberate misses together with normal-context ones and + /// understates both. + public string HitPct { get; init; } = ""; + + /// This combination's share of the window's total read TIME — how the grid's order was + /// decided, made legible. A row at 4% of the reads and 60% of the read time is the finding. + public string PctOfTotalReadTime { get; init; } = ""; + public string Extends { get; init; } = ""; + public string ExtendTimeMs { get; init; } = ""; public string Evictions { get; init; } = ""; public string Reuses { get; init; } = ""; public string Writes { get; init; } = ""; @@ -261,15 +330,37 @@ internal sealed class IoRow public string StatsReset { get; init; } = ""; } - internal static IoRow Io(DarlingPgIoReader.PgIoRow row) => new() + /// + /// Projects the whole I/O result at once, because two of its columns are SHARES of the window and a + /// per-row projection cannot compute a denominator it never sees. Ordering the grid by read time and + /// then not saying what share that is leaves the reader to divide by hand. + /// + internal static List IoRows(IReadOnlyList rows) + { + ArgumentNullException.ThrowIfNull(rows); + + var totalReadTime = rows.Sum(r => r.ReadTimeMs); + return rows.Select(r => Io(r, totalReadTime)).ToList(); + } + + internal static IoRow Io(DarlingPgIoReader.PgIoRow row, double totalReadTimeMs = 0) => new() { BackendType = row.BackendType ?? "", ObjectType = row.ObjectType ?? "", Context = row.Context ?? "", + ContextMeaning = DarlingPgIoReader.ContextMeaning(row.Context), Reads = Count(row.Reads), ReadTimeMs = Math.Round(row.ReadTimeMs, 1), + AvgReadMs = row.Reads > 0 + ? string.Create(CultureInfo.CurrentCulture, $"{row.ReadTimeMs / row.Reads:N3}") + : NotApplicableText, Hits = Count(row.Hits), + HitPct = Percent(row.Hits, row.Hits + row.Reads), + PctOfTotalReadTime = totalReadTimeMs > 0 + ? string.Create(CultureInfo.CurrentCulture, $"{row.ReadTimeMs / totalReadTimeMs * 100:N1}%") + : NotApplicableText, Extends = Count(row.Extends), + ExtendTimeMs = string.Create(CultureInfo.CurrentCulture, $"{Math.Round(row.ExtendTimeMs, 1):N1}"), Evictions = Count(row.Evictions), Reuses = Count(row.Reuses), /* "not tracked", not 0. On Aurora the whole pg_stat_io write side is NULL because backends there @@ -298,6 +389,15 @@ internal sealed class SlotRow public string DatabaseName { get; init; } = ""; public string Plugin { get; init; } = ""; public string InvalidationReason { get; init; } = ""; + + /// The stored conflicting flag: a logical slot whose needed rows were vacuumed away + /// by a recovery conflict. It is a DIFFERENT failure from invalidation-by-WAL-size and needs a + /// different response (the subscriber must be re-created either way, but the cause is + /// hot_standby_feedback rather than max_slot_wal_keep_size), so it is its own column + /// rather than folded into the invalidation reason. + public string Conflicting { get; init; } = ""; + + public string MeasuredAt { get; init; } = ""; public bool IsInvalidated { get; init; } public bool IsInactive { get; init; } } @@ -317,9 +417,14 @@ internal sealed class SlotRow DatabaseName = row.DatabaseName ?? "", Plugin = row.Plugin ?? "", InvalidationReason = row.InvalidationReason ?? "", + Conflicting = row.Conflicting ? "CONFLICTING" : "", + MeasuredAt = Timestamp(row.MeasuredAt), /* An invalidated slot has already lost its WAL: the replica behind it needs rebuilding, and that - is a different day from an inactive slot that is merely accumulating. */ + is a different day from an inactive slot that is merely accumulating. A CONFLICTING slot is in + the same category — its subscriber cannot continue — so it shares the highlight even though the + cause and the fix are different. */ IsInvalidated = !string.IsNullOrEmpty(row.InvalidationReason) + || row.Conflicting || string.Equals(row.WalStatus, "lost", StringComparison.OrdinalIgnoreCase), IsInactive = !row.IsActive, }; @@ -330,6 +435,12 @@ internal sealed class ChainRow { public string CapturedAt { get; init; } = ""; public int RootPid { get; init; } + + /// The collector's synthetic backend id, which is what "seen as root" is counted on — pids + /// are reused, so two captures agreeing on a pid are not necessarily the same backend. 0 is + /// the vanished-blocker sentinel and shows as unknown rather than as an id. + public string RootBackendId { get; init; } = ""; + public string Databases { get; init; } = ""; public int TotalVictims { get; init; } public int DirectVictims { get; init; } @@ -361,6 +472,9 @@ internal static ChainRow Chain(DarlingPgBlockingReader.PgBlockingChainRow row) { CapturedAt = Timestamp(row.CapturedAt), RootPid = row.RootPid, + RootBackendId = row.RootBackendId == 0 + ? "unknown" + : row.RootBackendId.ToString("N0", CultureInfo.CurrentCulture), Databases = string.Join(", ", row.Databases.Where(d => !string.IsNullOrEmpty(d))), TotalVictims = row.TotalVictims, DirectVictims = row.DirectVictims, @@ -411,11 +525,23 @@ internal sealed class StatementRow /// exceeds 2^53, and rendering it through anything that rounds is the one field whose entire /// purpose is joining back to pg_stat_statements (#2548). public string QueryId { get; init; } = ""; + + /// The database OID, which is half the grain of this read — one queryid appears once per + /// database it ran in, and two rows with the same statement text are not a duplicate. + public string DatabaseId { get; init; } = ""; + public string Calls { get; init; } = ""; public string TotalExecTimeMs { get; init; } = ""; public string AvgExecTimeMs { get; init; } = ""; public double MaxExecTimeMs { get; init; } public string RowsReturned { get; init; } = ""; + + /// Buffer-cache hit share for this statement, over BOTH cache tiers Aurora reports — the + /// shared buffers and the Optimized Read cache — against both miss sources. Reported as one ratio + /// rather than four block counters, which are the same fact in a form nobody reads correctly. + public string CacheHitPct { get; init; } = ""; + + public string TempRead { get; init; } = ""; public string TempWritten { get; init; } = ""; public string PeakMemory { get; init; } = ""; public string WalWritten { get; init; } = ""; @@ -425,6 +551,7 @@ internal sealed class StatementRow internal static StatementRow Statement(DarlingPgStatementReader.PgStatementRow row) => new() { QueryId = row.QueryId.ToString(CultureInfo.InvariantCulture), + DatabaseId = row.DatabaseId.ToString("N0", CultureInfo.CurrentCulture), Calls = Count(row.Calls), TotalExecTimeMs = Count(row.TotalExecTimeMs), AvgExecTimeMs = row.Calls <= 0 @@ -432,11 +559,15 @@ internal sealed class StatementRow : string.Create(CultureInfo.CurrentCulture, $"{row.TotalExecTimeMs / (double)row.Calls:N1}"), MaxExecTimeMs = Math.Round(row.MaxExecTimeMs, 1), RowsReturned = Count(row.RowsReturned), - /* BLOCKS, and labelled as such. The block size is a compile-time setting of the server, so - multiplying by an assumed 8kB would put a fabricated byte count next to real ones. */ - TempWritten = row.TempBlocksWritten <= 0 - ? "0 blocks" - : string.Create(CultureInfo.CurrentCulture, $"{row.TempBlocksWritten:N0} blocks"), + CacheHitPct = Percent( + row.SharedBlocksHit + row.OrcacheBlocksHit, + row.SharedBlocksHit + row.OrcacheBlocksHit + row.SharedBlocksRead + row.StorageBlocksRead), + /* BLOCKS, and labelled as such — both of them. The block size is a compile-time setting of the + server, so multiplying by an assumed 8kB would put a fabricated byte count next to real ones. + Temp READ sits beside temp written because a spill that is written and never read back is a + different (and cheaper) event than one the query then re-reads. */ + TempRead = Blocks(row.TempBlocksRead), + TempWritten = Blocks(row.TempBlocksWritten), PeakMemory = Bytes(row.MaxPeakMemBytes), WalWritten = Bytes(row.WalBytes), /* Null is "no text captured for this queryid yet" — text refreshes hourly, and a major-version @@ -455,6 +586,11 @@ internal sealed class DatabaseRow public string XactRollback { get; init; } = ""; public string RollbackPct { get; init; } = ""; public int SampleCount { get; init; } + + /// The stored stats_reset timestamp. Shown beside the caveat that counts resets in + /// the window, because "reset once" and "reset at 14:02" send you to different places. + public string StatsReset { get; init; } = ""; + public string Caveats { get; init; } = ""; } @@ -483,6 +619,7 @@ has to sit on the row rather than in a footnote nobody reads. */ XactRollback = Count(row.XactRollback), RollbackPct = Percent(row.XactRollback, row.XactCommit + row.XactRollback), SampleCount = row.SampleCount, + StatsReset = Timestamp(row.StatsReset), Caveats = string.Join("; ", caveats), }; } diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs index f1dbfbce34..5cb68faad5 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs @@ -260,7 +260,7 @@ private async Task LoadPgIoAsync() var rows = await _dataService.GetPgIoAsync(_server.ServerId, startUtc, endUtc, PgGridRowLimit); - PgIoStatsGrid.ItemsSource = rows.Select(PgDisplay.Io).ToList(); + PgIoStatsGrid.ItemsSource = PgDisplay.IoRows(rows); PgIoNote.Text = PanelNote("pg_io_stats", rows.Count, "No backend / object / context combination did any I/O in this window. This view needs " + "PostgreSQL 16 or newer, where pg_stat_io exists; below that the collector does not run and " diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml index 96f1a2756f..06a53b22a0 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml @@ -3686,6 +3686,7 @@ + @@ -3745,11 +3746,14 @@ HorizontalScrollBarVisibility="Auto" VerticalScrollBarVisibility="Auto" CanUserSortColumns="True"> + + + @@ -3777,6 +3781,7 @@ + @@ -3846,14 +3851,22 @@ + + + + + + - + + + @@ -3868,10 +3881,14 @@ + + - + + + @@ -3913,11 +3930,24 @@ - + + + + + + + + + + + + @@ -3956,6 +3986,7 @@ + @@ -3964,7 +3995,8 @@ - + + From 119386192e298a5535a4efb24cf20e9d08d53fee Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:22:13 +0100 Subject: [PATCH 4/8] Merge dev: point #2548's new pins at the readers' new home MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge itself was clean, but it carried a SEMANTIC conflict a text merge cannot see: PgInt64IdentityWireShapeTests arrived on dev naming DarlingPgStatementReader and DarlingPgBlockingReader, which this branch had already moved to PerformanceMonitor.Darling.Storage. One using. Nothing about #2548's fix changes here — queryid was already rendered as a string in the viewer's statement projection, for the same reason it is now a string on the wire: a PostgreSQL int8 queryid routinely exceeds 2^53, and the one field whose entire purpose is joining back to pg_stat_statements must not go through anything that rounds. Co-Authored-By: Claude Opus 5 --- Darling/Darling.Tests/PgInt64IdentityWireShapeTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Darling/Darling.Tests/PgInt64IdentityWireShapeTests.cs b/Darling/Darling.Tests/PgInt64IdentityWireShapeTests.cs index 9416a61ad5..c9bfa9b024 100644 --- a/Darling/Darling.Tests/PgInt64IdentityWireShapeTests.cs +++ b/Darling/Darling.Tests/PgInt64IdentityWireShapeTests.cs @@ -12,6 +12,7 @@ using System.Linq; using System.Text.Json; using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; using Xunit; namespace Darling.Tests; From 5aa0dcf0e2e5ddb2804a5e87dae252cdfd3a43d8 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:25:04 +0100 Subject: [PATCH 5/8] Look a tab's framing note up by id, not by position The three note assignments indexed All[0..2], which couples them to the registry's STRIP order - a list whose whole job is to be reorderable. The first reorder would have put the Vacuum note above the Activity grids, silently and correctly-compiling. NoteFor(id) instead, returning empty for an id the registry does not carry, because a missing note must never take a tab down with it. Co-Authored-By: Claude Opus 5 --- .../ViewerPostgresTabs.cs | 8 ++++++++ .../ViewerServerTab.Postgres.cs | 10 +++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs index 261c0c6d1e..68f9d13af0 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs @@ -138,6 +138,14 @@ the only evidence of that we collect. */ /// True when belongs to this registry. public static bool Owns(int innerTabIndex) => All.Any(t => t.InnerTabIndex == innerTabIndex); + /// + /// A tab's framing note by ID — empty for a tab that has none, and empty rather than throwing for an id + /// this registry does not carry, because a missing note must never take a tab down with it. Looked up by + /// id rather than by position so the strip order stays free to change. + /// + public static string NoteFor(string tabId) => + All.FirstOrDefault(t => string.Equals(t.Id, tabId, StringComparison.Ordinal))?.Note ?? string.Empty; + /// /// The store table a declared collector writes, from the collector's own definition. Null for a name /// the catalog does not know, which ViewerPostgresTabsTests refuses — the registry may not name diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs index 5cb68faad5..aed26c5ee6 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs @@ -90,9 +90,13 @@ worse answer than not offering. */ DatabaseFilterButton.Visibility = Visibility.Collapsed; PgEngineBanner.Text = $"{_server.DisplayName} runs {engineOrUnknown()}."; - PgOverviewNote.Text = ViewerPostgresTabs.All[0].Note ?? string.Empty; - PgActivityNote.Text = ViewerPostgresTabs.All[1].Note ?? string.Empty; - PgVacuumNote.Text = ViewerPostgresTabs.All[2].Note ?? string.Empty; + + /* Looked up by ID, never by position: the registry's order is the STRIP order and is free to change, + while these three assignments are about which tab gets which framing. Indexing All[0..2] would + have silently put the Vacuum note above the Activity grids the first time someone reordered. */ + PgOverviewNote.Text = ViewerPostgresTabs.NoteFor("overview"); + PgActivityNote.Text = ViewerPostgresTabs.NoteFor("activity"); + PgVacuumNote.Text = ViewerPostgresTabs.NoteFor("vacuum"); string engineOrUnknown() => _server.EngineDescription ?? "an engine the store has not recorded"; From c5909777a6dd85a1663c774388b61cca0b56857f Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:26:38 +0100 Subject: [PATCH 6/8] Drop an engine-name fallback that cannot be reached IsPostgres is only true for a token DescribeEngineKind recognises, so inside that branch EngineDescription always has words - the local function offering 'an engine the store has not recorded' was describing a state this code path excludes, which is the kind of defensive branch that later reads as a real case somebody should handle. Co-Authored-By: Claude Opus 5 --- .../ViewerServerTab.Postgres.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs index aed26c5ee6..cb8ae323f7 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs @@ -57,7 +57,8 @@ public partial class ViewerServerTab /// private void ApplyEngineTabSet() { - if (_server.EngineDescription is { } engine) + var engine = _server.EngineDescription; + if (engine is not null) { ServerEngineText.Text = engine; ServerEngineBadge.Visibility = Visibility.Visible; @@ -89,7 +90,9 @@ otherwise be what a PostgreSQL server opens on. */ worse answer than not offering. */ DatabaseFilterButton.Visibility = Visibility.Collapsed; - PgEngineBanner.Text = $"{_server.DisplayName} runs {engineOrUnknown()}."; + /* IsPostgres is only true for a token the describer recognises, so `engine` has words here by + construction; the coalesce is the compiler's, not a state this can reach. */ + PgEngineBanner.Text = $"{_server.DisplayName} runs {engine ?? "PostgreSQL"}."; /* Looked up by ID, never by position: the registry's order is the STRIP order and is free to change, while these three assignments are about which tab gets which framing. Indexing All[0..2] would @@ -97,9 +100,6 @@ have silently put the Vacuum note above the Activity grids the first time someon PgOverviewNote.Text = ViewerPostgresTabs.NoteFor("overview"); PgActivityNote.Text = ViewerPostgresTabs.NoteFor("activity"); PgVacuumNote.Text = ViewerPostgresTabs.NoteFor("vacuum"); - - string engineOrUnknown() => - _server.EngineDescription ?? "an engine the store has not recorded"; } // ───────────────────────────────────────────────────────────────────────────────────────────── From 1ff15955eb668ee0e258ef61b7f81e42c3d2a067 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:28:37 +0100 Subject: [PATCH 7/8] Give the statement grid the height its role implies, and trim the badge Two small ones the prose already promised and the markup did not deliver. The Query Shapes tab gave the per-database counters TWICE the height of the statement grid they exist to explain, which is backwards: they are the follow-up question (did this spill?), not a second list of equal standing. Swapped. And an unrecognised engine token reaches the badge raw so an operator can search their store for it; raw did not have to mean untrimmed, and a hand-edited row with surrounding whitespace would have rendered it. Co-Authored-By: Claude Opus 5 --- .../PerformanceMonitor.Darling.Viewer/ViewerDataService.cs | 2 +- .../PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs index 37a9db15f7..36770b07c2 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs @@ -98,7 +98,7 @@ public DarlingServer( /// public string? EngineDescription => !MonitoredEngineKind.IsKnown(EngineKind) - ? (string.IsNullOrWhiteSpace(EngineKind) ? null : EngineKind) + ? (string.IsNullOrWhiteSpace(EngineKind) ? null : EngineKind.Trim()) : MonitoredEngineKind.DescribeEngineKind(EngineKind); /// "SQL Server 2022"-style label for the server list; empty when the version is unknown. diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml index 06a53b22a0..c21d7babec 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml @@ -3735,10 +3735,13 @@ - + + - + From 1f41186afc781311bc780db1c3f3169e392e8876 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:44:09 +0100 Subject: [PATCH 8/8] Fix the CI red, and two more review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **The CI red is mine, and the guard that caught it did its job.** DarlingPgReadSqlParsesLiveTests discovers the shipped PostgreSQL reads by reflection, filtered on a HARDCODED namespace string — and this branch moved the readers, so the filter matched nothing and its own anti-vacuity floor failed the build saying exactly that: "the reflection filter has stopped matching, so this test is no longer checking anything". That assertion is the reason a silent pass was not what happened instead. The filter now takes the namespace from a reader TYPE it has to resolve anyway, so it follows the readers wherever they live. Verified the fix rather than assuming it: on macOS that test SKIPS before reaching its own count (no DARLING_TEST_PG), so "it passed" proves nothing — a throwaway host invoked the shipped private ShippedReadSql() directly and it discovers 12 constants across the nine readers, which is the number its own doc comment names. Proved red by restoring the literal: 0. **Two review findings, both real.** Bytes() chose the display unit against the UNROUNDED quotient and then formatted with N1, so one byte short of a megabyte rendered "1024.0 KB" — a number expressed in its own next unit, which reads as a typo rather than a size, on every column built over bytes. Re-checked after rounding. WPF applies the LAST matching DataTrigger, and IsInvalidated/IsInactive are not exclusive: an invalidated replication slot is usually also inactive, because its subscriber has already gone. Declaring inactive last painted the informational amber over the red on exactly the rows that needed the red. Swapped — and asserted as a RULE over every row style on these tabs rather than fixed on the one grid review found, since the same ordering pattern is one edit away from recurring: each flag carries a severity and a milder one may not follow a severer one. The Overview grid's pair is mutually exclusive today and is ordered that way anyway, so one rule covers the tab set instead of a rule and an exception. Both proved red by reverting only the thing under test. Co-Authored-By: Claude Opus 5 --- .../DarlingPgReadSqlParsesLiveTests.cs | 8 +- .../Darling.Tests/ViewerPostgresTabsTests.cs | 87 +++++++++++++++++++ .../ViewerPostgresDisplay.cs | 10 +++ .../ViewerServerTab.xaml | 22 +++-- 4 files changed, 120 insertions(+), 7 deletions(-) diff --git a/Darling/Darling.Tests/DarlingPgReadSqlParsesLiveTests.cs b/Darling/Darling.Tests/DarlingPgReadSqlParsesLiveTests.cs index 1296704f41..78d9d9bbe6 100644 --- a/Darling/Darling.Tests/DarlingPgReadSqlParsesLiveTests.cs +++ b/Darling/Darling.Tests/DarlingPgReadSqlParsesLiveTests.cs @@ -56,9 +56,15 @@ public sealed class DarlingPgReadSqlParsesLiveTests /// rather than the reader methods because a method would need a connection, a server and a window; the /// constant IS the shipped text, which is the thing under test. /// + /// The namespace comes from a reader TYPE rather than a string literal (#2530), for the reason + /// the discovery is reflective in the first place: the readers moved to + /// PerformanceMonitor.Darling.Storage so the WPF viewer could run the same query text, and a + /// hardcoded namespace matched nothing afterwards. The anti-vacuity floor below caught it — a guard that + /// stopped guarding, reported as one — but a filter anchored to a type it must find anyway cannot break + /// that way twice. private static IReadOnlyList<(string Name, string Sql)> ShippedReadSql() => typeof(DarlingPgStatementReader).Assembly.GetTypes() - .Where(t => t.Namespace == "PerformanceMonitor.Darling.Service.Mcp" + .Where(t => t.Namespace == typeof(DarlingPgStatementReader).Namespace && t.Name.StartsWith("DarlingPg", StringComparison.Ordinal) && t.Name.EndsWith("Reader", StringComparison.Ordinal)) .SelectMany(t => t diff --git a/Darling/Darling.Tests/ViewerPostgresTabsTests.cs b/Darling/Darling.Tests/ViewerPostgresTabsTests.cs index e239fdc15e..2d52ea7d68 100644 --- a/Darling/Darling.Tests/ViewerPostgresTabsTests.cs +++ b/Darling/Darling.Tests/ViewerPostgresTabsTests.cs @@ -430,6 +430,93 @@ public void TheNotApplicableSentinel_NeverRendersAsANumber() Assert.Equal("0 ms", PgDisplay.Milliseconds(0)); } + /// + /// A size never renders expressed in its own next unit. The unit is chosen against the quotient and the + /// quotient is then ROUNDED, so one byte short of a megabyte would otherwise print "1024.0 KB" — a + /// number that reads as a typo rather than as a size, on every column built over bytes. + /// + [Fact] + public void ASize_NeverRendersExpressedInItsOwnNextUnit() + { + var boundaries = new[] + { + 1024L * 1024 - 1, // one byte short of 1 MB + 1024L * 1024 * 1024 - 1, // one byte short of 1 GB + 1024L * 1024 * 1024 * 1024 - 1, // one byte short of 1 TB + }; + + foreach (var value in boundaries) + { + Assert.DoesNotContain("1024", PgDisplay.Bytes(value), StringComparison.Ordinal); + } + + /* The exact boundaries and the last value below them still read correctly — a guard that fixed the + rounding by breaking the ordinary case would be worse than the defect. */ + Assert.Equal("1,023 B", PgDisplay.Bytes(1023)); + Assert.StartsWith("1.0 KB", PgDisplay.Bytes(1024), StringComparison.Ordinal); + Assert.StartsWith("1.0 MB", PgDisplay.Bytes(1024 * 1024), StringComparison.Ordinal); + Assert.StartsWith("1.0 MB", PgDisplay.Bytes(1024L * 1024 - 1), StringComparison.Ordinal); + } + + /// + /// In every PostgreSQL row style, the LEAST severe highlight is declared first. + /// + /// WPF applies the LAST matching DataTrigger when several set the same property, and these + /// flags are not exclusive — an invalidated replication slot is usually also inactive, because its + /// subscriber has already gone. Declaring the informational amber last paints it over the red on exactly + /// the rows that need the red, which understates severity to someone scanning row colours. Asserted as a + /// rule over every row style on these tabs rather than fixed on the one grid where review found it. + /// + [Fact] + public void EveryPostgresRowStyle_DeclaresItsSeverestHighlightLast() + { + var xaml = ViewerFile("ViewerServerTab.xaml"); + var pgRegion = xaml[xaml.IndexOf("(StringComparer.Ordinal) + { + ["IsFault"] = 2, + ["IsInvalidated"] = 2, + ["AutovacuumDisabled"] = 2, + ["IsPermanentGap"] = 1, + ["IsInactive"] = 1, + }; + + var styles = Regex.Matches(pgRegion, "(.*?)", RegexOptions.Singleline); + Assert.True(styles.Count >= 3, + $"Only {styles.Count} PostgreSQL row styles found; the scan is not matching the XAML's shape."); + + var problems = new List(); + foreach (Match style in styles) + { + var flags = Regex + .Matches(style.Groups[1].Value, "DataTrigger Binding=\"\\{Binding (\\w+)\\}\"") + .Select(m => m.Groups[1].Value) + .ToList(); + + foreach (var flag in flags.Where(f => !severity.ContainsKey(f))) + { + problems.Add($"'{flag}' has no severity ranking — add one rather than leaving it unordered"); + } + + for (var i = 1; i < flags.Count; i++) + { + if (severity.TryGetValue(flags[i - 1], out var previous) && + severity.TryGetValue(flags[i], out var current) && + current < previous) + { + problems.Add($"'{flags[i]}' (severity {current}) is declared after '{flags[i - 1]}' " + + $"(severity {previous}); WPF applies the LAST match, so the milder state wins"); + } + } + } + + Assert.True(problems.Count == 0, + "PostgreSQL row highlight(s) declared in the wrong order:\n " + string.Join("\n ", problems)); + } + /// A percentage of nothing is not 0%. Dividing by a zero denominator has no answer, and "0%" /// is a claim about a ratio that was never computable. [Fact] diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs index 1060ba0ca7..1a5f8aa2ea 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresDisplay.cs @@ -62,6 +62,16 @@ internal static string Bytes(long value) unit++; } + /* The unit has to be chosen against the value AS RENDERED, not against the exact quotient. One byte + short of a megabyte the quotient is 1023.999…, which stays in KB and then rounds to "1024.0 KB" — + a number expressed in its own next unit, which reads as a typo rather than as a size. Re-check + once after rounding; one pass is enough, because the second division can only land on 1.0. */ + if (unit < units.Length - 1 && Math.Round(scaled, 1) >= 1024) + { + scaled /= 1024; + unit++; + } + return unit == 0 ? $"{value:N0} B" : string.Create(CultureInfo.CurrentCulture, $"{scaled:N1} {units[unit]}"); diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml index c21d7babec..953b72486a 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml @@ -3632,13 +3632,18 @@ HorizontalScrollBarVisibility="Auto" VerticalScrollBarVisibility="Auto" CanUserSortColumns="True"> @@ -3974,13 +3979,18 @@ HorizontalScrollBarVisibility="Auto" VerticalScrollBarVisibility="Auto" CanUserSortColumns="True">