diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 16b8853e..c25f2096 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -725,8 +725,8 @@ jobs: # # The values are the product's own derivation from the live hypertable count # (TimescaleSupport.HypertableCount = the 62-collector catalog + collection_log = 63): - # timescaledb.max_background_workers = HypertableCount + 2 = 69 - # max_worker_processes = 3 + (HypertableCount + 2) + 8 = 80 + # timescaledb.max_background_workers = HypertableCount + 2 = 70 + # max_worker_processes = 3 + (HypertableCount + 2) + 8 = 81 # Hard-coded here because a workflow cannot call into the product — so # CiClusterWorkerSizingTests parses THIS FILE and fails the build if either number stops # matching the formula as collectors are added, and CiClusterWorkerSizingLiveTests asserts @@ -744,8 +744,8 @@ jobs: Add-Content -Path "$dataDir\postgresql.conf" -Value "shared_preload_libraries = 'timescaledb'" Add-Content -Path "$dataDir\postgresql.conf" -Value "port = 5541" Add-Content -Path "$dataDir\postgresql.conf" -Value "listen_addresses = '127.0.0.1'" - Add-Content -Path "$dataDir\postgresql.conf" -Value "timescaledb.max_background_workers = 69" - Add-Content -Path "$dataDir\postgresql.conf" -Value "max_worker_processes = 80" + Add-Content -Path "$dataDir\postgresql.conf" -Value "timescaledb.max_background_workers = 70" + Add-Content -Path "$dataDir\postgresql.conf" -Value "max_worker_processes = 81" & "$bin\pg_ctl.exe" -D $dataDir -l $logFile -w start if ($LASTEXITCODE -ne 0) { if (Test-Path $logFile) { Get-Content $logFile -Tail 50 }; throw "pg_ctl start failed ($LASTEXITCODE)" } & "$bin\createdb.exe" -h 127.0.0.1 -p 5541 -U darling darling diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 94a64601..134e7b2d 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -369,8 +369,8 @@ jobs: # customer runs and made scheduler-racing failures luck-of-the-slot instead of reproducible. # Values are the product's own derivation from the live hypertable count # (TimescaleSupport.HypertableCount = the 62-collector catalog + collection_log = 63): - # timescaledb.max_background_workers = HypertableCount + 2 = 69 - # max_worker_processes = 3 + (HypertableCount + 2) + 8 = 80 + # timescaledb.max_background_workers = HypertableCount + 2 = 70 + # max_worker_processes = 3 + (HypertableCount + 2) + 8 = 81 # Kept honest by CiClusterWorkerSizingTests (parses this file against the formula) and # CiClusterWorkerSizingLiveTests (asserts the running cluster serves them). Must configure # the cluster identically to build.yml's darling-pg job: that guard parses the appended @@ -391,8 +391,8 @@ jobs: Add-Content -Path "$dataDir\postgresql.conf" -Value "shared_preload_libraries = 'timescaledb'" Add-Content -Path "$dataDir\postgresql.conf" -Value "port = 5541" Add-Content -Path "$dataDir\postgresql.conf" -Value "listen_addresses = '127.0.0.1'" - Add-Content -Path "$dataDir\postgresql.conf" -Value "timescaledb.max_background_workers = 69" - Add-Content -Path "$dataDir\postgresql.conf" -Value "max_worker_processes = 80" + Add-Content -Path "$dataDir\postgresql.conf" -Value "timescaledb.max_background_workers = 70" + Add-Content -Path "$dataDir\postgresql.conf" -Value "max_worker_processes = 81" & "$bin\pg_ctl.exe" -D $dataDir -l $logFile -w start if ($LASTEXITCODE -ne 0) { if (Test-Path $logFile) { Get-Content $logFile -Tail 50 }; throw "pg_ctl start failed ($LASTEXITCODE)" } & "$bin\createdb.exe" -h 127.0.0.1 -p 5541 -U darling darling diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f5fcc15..c6d4398f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **PostgreSQL configuration, and what changed in it** ([#2658]) - `pg_settings` was never collected, so neither "what is `work_mem` set to on this server" nor "what changed last Tuesday" had an answer, and the second is the kind that cannot be recovered later at any price: a configuration history nobody recorded is not sitting on the server waiting to be read. `get_pg_server_config` reports the settings somebody actually chose, non-default first, with where each value came from and whether changing it needs a restart or a reload. `get_pg_server_config_changes` reports value changes between snapshots, old beside new. Both name `pending_restart` loudly - the state where `postgresql.conf` has been edited and reloaded but the running server is still on the old value, so the file and the server disagree with no symptom until a restart months later changes behaviour during someone else's incident. - **Test an index from the predicate grid** ([#2612]) - right-click a row in PostgreSQL predicate statistics and ask whether the planner would actually use an index on that column. The command shipped with no caller: the only way to reach it was hand-writing a row into the command queue, which is how it was tested and is not a feature. It hangs off that grid and nowhere else, which is the shape it was scoped to - on demand only, never scheduled, driven from a row somebody is already looking at. The confirmation says what it costs the server before it runs (nothing executed, no index built, session reset), and a predicate whose estimate error is already large is flagged BEFORE the round trip, because an index does not fix a plan built on a wrong row count. - **Azure SQL DB now reports every database's size, not just the connected one** ([#2643], raised from the field) - `sys.database_files` is database-scoped, so a Viewer pointed at `master` showed `master`'s two files and nothing else, which is correct and reads exactly like a broken collector. `sys.resource_stats` is a master-only view carrying `storage_in_megabytes` per database, so from a `master` connection the siblings now appear too - as one row each, labelled `(whole database)` with a NULL `file_id`, because that view has no per-file breakdown and a fabricated file name would make the grid look complete and be wrong. The sibling read runs through `sp_executesql`: the view does not exist in a user database and name resolution happens at parse time, so a guarded UNION still fails with 208 everywhere else. Verified against a live Azure SQL Database from both a `master` and a user-database connection. - **Mark rows in grids** ([#2645], requested from the field) - right-click any FinOps grid and mark the selected rows **Done**, **To Do** or **Do Not Do**, so you can work through a result set and remember which rows you have dealt with and which you have decided against. Asked for on Index Analysis, where you decide index by index. Marks are held against the row objects, so they last exactly as long as the result set does: on a run-on-demand grid until you run it again, on a live grid until the next refresh. They are painted from `LoadingRow`, so no row model gained a property and no grid gained a column. @@ -77,6 +78,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Lite's portable ZIP is self-contained, which HALVED it** ([#2501]) - `Publish Lite` is now `-r win-x64 --self-contained` in both `build.yml` and `nightly.yml`, so neither Lite artifact has a .NET prerequisite any more and the failure [#2489] documented stops existing: a tester who unzips onto a stock Windows Server no longer meets the .NET host's bare `You must install .NET to run this application` before a line of our code runs. **The size went the opposite way from what bundling a runtime suggests.** The old publish was RID-agnostic, so it copied every platform its packages ship - **537 MB of `runtimes\` on a 565 MB tree** (osx 130, linux-x64 116, linux-arm64 70, win-arm64 56, then win-x86, musl, loongarch64 and riscv64), of which only the **52 MB `win-x64`** folder could ever load on Windows. `DuckDB.NET.Bindings.Full` is most of it, SkiaSharp and SqlClient behind it. Dropping ~485 MB of unloadable native payload beats the cost of bundling .NET, WPF and ASP.NET Core by roughly two to one: measured on one commit and one SDK, **565 MB tree / 212.7 MB zipped becomes 277 MB / 114.2 MB**. It matters most for the **nightly** ZIP, which is the UAT download and is not offered as a `Setup.exe` at all. **A RID-specific publish needed two more files than the flag.** `Lite/packages.lock.json` had only a `net10.0-windows7.0` target, and a RID restore adds `net10.0-windows7.0/win-x64` to it - after which the `dotnet restore --locked-mode` that BOTH workflows run before the publish fails `NU1004: the project's runtime identifiers have changed`, because locked mode compares the PROJECT's RID set (empty) against the lock file's (win-x64). Reproduced locally; that is a red CI run on every PR, not the future `--no-restore` trap it was filed as. The fix is `win-x64` in `PerformanceMonitorLite.csproj`, so the project itself asks for that graph and one committed lock file satisfies the RID-less locked-mode restore and the RID publish alike; `RuntimeIdentifiers` (plural) sets no RID on the build, so a plain `dotnet build` stays RID-agnostic and `Lite.Tests` is untouched. **SignPath needed nothing** - the `Lite` artifact-configuration slug already receives both shapes today, and the signed re-zip reads `signed/Lite/*`, inheriting whatever shape `publish/Lite` has. Auto-update is unaffected; the ZIP is not a Velopack channel. `LiteRuntimePrerequisiteDocsTests` went red on the flag alone (3 of its 7 facts) and was rewritten to state every claim BOTH ways round: [#2499]'s version asserted only that the docs DID name the runtimes, so two of its facts stayed green while the prose went stale. It now also derives the lock file's RID coverage from the `-r` flags in the workflows, and every new assertion was proven red with its fix reverted. ### Fixed +- **Ten PostgreSQL MCP tools were never registered with the host, so no agent could call them** ([#2659]) - `get_pg_write_stats`, `get_pg_buffer_usage`, `get_pg_extensions`, `get_pg_lock_stats`, `get_pg_index_bloat`, `get_pg_column_stats`, `get_pg_kernel_stats`, `get_pg_predicate_stats`, `get_pg_replication_stats` and `get_pg_wait_sampling` were implemented, documented, dispatched by the web dashboard and counted in the instructions census, and `tools/list` answered 116 tools where the census claimed 126. Registration is per class and explicit, and nothing failed when a class was left out: the inventory pin checks tool NAMES, which exist either way, and the tab pin exists to stop a read shipping reachable only through MCP - this was the exact inverse. A reflection-derived pin now asserts every `[McpServerToolType]` class is registered, so it fails when someone adds a class rather than when an agent next reaches for the tool. - **PostgreSQL 18 silently lost every I/O byte figure, and the estimate it replaced was off by an order of magnitude** ([#2655]) - 18 removed `op_bytes` from `pg_stat_io`, and both byte figures `get_pg_io_stats` serves were derived from it, so on 18 they came back null with no note, no status and nothing to distinguish a version change from a collector that had stopped. The columns 18 replaced it with are better than what was lost: `op_bytes` was the per-operation block size that the read multiplied by a count to ESTIMATE volume, while `read_bytes`/`write_bytes`/`extend_bytes` are measured totals. 18 also introduced vectored reads, so one entry in `reads` can cover several blocks and the old estimate undercounts - measured through the running service against a real 18.6 target, one combination reported 4,742 reads against 448,724,992 bytes where the estimate would have said 38,846,464, an 11.6x undercount, and three combinations ran 10x to 16x. They are now collected and served, with `bytes_source` on every row and on the envelope saying whether a figure was measured or estimated, because the two are not comparable and must never share a name silently. - **A PostgreSQL column that the server's VERSION removed read as a missing measurement** ([#2653]) - PostgreSQL 17 removed `buffers_backend` and `buffers_backend_fsync` from `pg_stat_bgwriter` outright, and `get_pg_write_stats` returned both as bare nulls under a note that went on explaining `buffers_backend` as a live backpressure signal. The collector was right - it emits NULL for them deliberately from 17 on - but nothing anywhere recorded the target's PostgreSQL major, so no read could tell a column the version does not have from one nothing collected. Seven PostgreSQL collectors gate on that version and the read layer had no access to it at all. The registry now carries it, stamped on every connect like the engine kind next door, and this read spends it: on 17 and later it names the removal, says it is not a measurement gap, and points at `get_pg_io_stats`, where the fact actually lives now. - **Self-hosted PostgreSQL had no query TEXT, so `test_hypothetical_index` could never work there** ([#2651]) - the statement-text store read `aurora_stat_statements()` with no vanilla path, so off Aurora `collect.pg_statement_text` was never populated. Two things failed silently: `get_pg_top_queries` returned `query_text: null` on every row forever, while that field's own documentation says null means "not captured YET" - true on Aurora, a lie here; and #2612's `test_hypothetical_index` resolves its statement from that table, so it always answered "no statement text is stored" and blamed a refresh cadence for a missing source. Fixing it exposed a second defect immediately: `pg_stat_statements` keys on `(queryid, userid, dbid, toplevel)`, so one queryid returns once per user and database, and the upsert - which keys on `(server_id, queryid)` - met those duplicates as `21000: ON CONFLICT DO UPDATE command cannot affect row a second time` and abandoned every batch. Both fixed and verified against a real self-hosted PostgreSQL: 47 statement texts stored where there were zero, and the hypothetical-index command then answered end to end for the first time - cost 1,059.34 to 438.71, a 58.6% reduction. @@ -3011,3 +3013,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#2564]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2564 [#2653]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2653 [#2655]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2655 +[#2658]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2658 +[#2659]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2659 diff --git a/Darling/Darling.Tests/McpToolTypeRegistrationTests.cs b/Darling/Darling.Tests/McpToolTypeRegistrationTests.cs new file mode 100644 index 00000000..57a00068 --- /dev/null +++ b/Darling/Darling.Tests/McpToolTypeRegistrationTests.cs @@ -0,0 +1,113 @@ +// Copyright (c) Erik Darling Data. All rights reserved. +// Licensed under the terms in the LICENSE file in the repository root. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Text.RegularExpressions; +using ModelContextProtocol.Server; +using PerformanceMonitor.Darling.Service.Mcp; +using Xunit; + +namespace Darling.Tests; + +/// +/// Every class must actually be registered with the MCP host +/// (#2659). +/// +/// Six were not. Ten shipped PostgreSQL reads — get_pg_write_stats, +/// get_pg_buffer_usage, get_pg_extensions, get_pg_lock_stats, +/// get_pg_index_bloat, get_pg_column_stats, get_pg_kernel_stats, +/// get_pg_predicate_stats, get_pg_replication_stats, get_pg_wait_sampling — were +/// implemented, documented, dispatched by the web API, counted in the instructions census and covered by +/// the name-based inventory pin, and an agent could not call any of them. Asked of the running service, +/// tools/list answered 116 tools where the census claimed 126. +/// +/// Why the existing guards could not see it. The inventory pin checks tool NAMES, and the +/// names exist — the attribute is on the method whether or not the class is registered. The +/// POSTGRES_TABS pin asserts every get_pg_* read reaches a web tab, and its own header says +/// it exists so a new read "cannot ship reachable only through MCP". This is the exact inverse, and there +/// was no pin for it: these shipped reachable only through the WEB. +/// +/// Derived, not enumerated. The check walks the assembly for the attribute and the host source +/// for its registrations, so it cannot go stale the way a hand-kept list does — and it fails the moment +/// someone adds a class, rather than whenever an agent next reaches for the tool. That is the same +/// reasoning as the tab pin being derived from the dispatch. +/// +public sealed class McpToolTypeRegistrationTests +{ + [Fact] + public void EveryMcpServerToolTypeClass_IsRegisteredWithTheHost() + { + var declared = typeof(DarlingMcpHostService).Assembly + .GetTypes() + .Where(t => t.GetCustomAttribute() is not null) + .Select(t => t.Name) + .OrderBy(n => n, StringComparer.Ordinal) + .ToList(); + + Assert.NotEmpty(declared); + + var registered = RegisteredToolTypeNames(); + + var missing = declared.Where(n => !registered.Contains(n)).ToList(); + + Assert.True( + missing.Count == 0, + "These [McpServerToolType] classes are never registered with the MCP host, so every tool they " + + "declare is unreachable over MCP even though its name exists and the web API dispatches it: " + + string.Join(", ", missing) + + ". Add a .WithGeminiCompatibleTools() line in DarlingMcpHostService."); + } + + /// + /// Reads the registrations out of the host SOURCE rather than by invoking the builder, because the + /// builder needs a host, a store and a live configuration, and this is a wiring question that should be + /// answerable without any of them. + /// + private static HashSet RegisteredToolTypeNames() + { + var path = HostSourcePath(); + var source = File.ReadAllText(path); + + var names = Regex + .Matches(source, @"WithGeminiCompatibleTools<(\w+)>") + .Select(m => m.Groups[1].Value) + .ToHashSet(StringComparer.Ordinal); + + Assert.True( + names.Count > 0, + $"Found no .WithGeminiCompatibleTools() registrations in {path}. If the registration style " + + "changed, this test needs to learn the new one rather than be deleted — it is the only thing " + + "standing between a new tools class and shipping unreachable."); + + return names; + } + + private static string HostSourcePath() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + + while (dir is not null) + { + var candidate = Path.Combine( + dir.FullName, + "Darling", + "PerformanceMonitor.Darling.Service", + "Mcp", + "DarlingMcpHostService.cs"); + + if (File.Exists(candidate)) + { + return candidate; + } + + dir = dir.Parent; + } + + throw new FileNotFoundException( + "Could not locate DarlingMcpHostService.cs by walking up from the test output directory."); + } +} diff --git a/Darling/Darling.Tests/PgSchemaGeneratorTests.cs b/Darling/Darling.Tests/PgSchemaGeneratorTests.cs index 0a0060b5..698aa0c9 100644 --- a/Darling/Darling.Tests/PgSchemaGeneratorTests.cs +++ b/Darling/Darling.Tests/PgSchemaGeneratorTests.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -51,7 +51,7 @@ Availability Group collectors (#991) = 38, plus plan_correction (#1952 automatic create tables and one store can hold both engines' data, so splitting it per engine would fragment DDL generation. Dispatch is gated separately, by engine, in CollectorCatalog.AppliesTo(definition, target). */ - Assert.Equal(66, CollectorCatalog.All.Count); + Assert.Equal(67, CollectorCatalog.All.Count); /* Uniqueness is asserted AGAINST THE COUNT rather than against a second literal. The literals here had drifted to 45 while the real figure tracked the count, so the test that exists to catch a @@ -636,6 +636,7 @@ public void EveryPostgresRung_IsIdenticalToTheGeneratedSchema() (97, PgKernelStatsCollector.Instance), (98, PgPredicateStatsCollector.Instance), (99, PgPlanCaptureCollector.Instance), + (102, PgServerConfigCollector.Instance), }; /* Every PostgreSQL collector must appear above. One added without a rung listed here would diff --git a/Darling/Darling.Tests/PgServerConfigTests.cs b/Darling/Darling.Tests/PgServerConfigTests.cs new file mode 100644 index 00000000..d5d1cc73 --- /dev/null +++ b/Darling/Darling.Tests/PgServerConfigTests.cs @@ -0,0 +1,177 @@ +// Copyright (c) Erik Darling Data. All rights reserved. +// Licensed under the terms in the LICENSE file in the repository root. + +using System; +using System.Linq; +using PerformanceMonitor.Collectors; +using PerformanceMonitor.Darling.Storage; +using Xunit; + +namespace Darling.Tests; + +/// +/// V102 (#2658) — the server's own configuration, from pg_settings. +/// +/// The two reads answer questions nothing else in the stack can. "What is this set to" was simply +/// missing on PostgreSQL; "what changed" is worse than missing, because a configuration history that was +/// never recorded cannot be reconstructed from the server afterwards at any price. +/// +public sealed class PgServerConfigTests +{ + /* ---------------- the rung ---------------- */ + + [Fact] + public void TheRungIsRegisteredAtTheTopOfADenseLadder() + { + var versions = PgMigrations.Scripts.Select(s => s.Version).ToList(); + + Assert.Equal("pg-server-config", PgMigrations.Scripts.Single(s => s.Version == 102).Name); + + Assert.Equal(StorageVersion.SchemaVersion, PgMigrations.Scripts[^1].Version); + Assert.Equal(StorageVersion.SchemaVersion, versions.Max()); + + Assert.Equal(versions.Distinct().OrderBy(v => v), versions); + var above = versions.Where(v => v > 45).OrderBy(v => v).ToList(); + Assert.Equal(Enumerable.Range(above[0], above.Count), above); + } + + /* ---------------- the collector ---------------- */ + + /// + /// Core catalog only, so it runs on every PostgreSQL target rather than being Aurora-gated — the same + /// tier as the wraparound collector. + /// + [Fact] + public void TheCollectorRunsOnEveryPostgresTarget_AndReadsOnlyPgSettings() + { + Assert.True(PgServerConfigCollector.Instance.AppliesTo( + new CollectorTargetInfo { Engine = CollectorTargetEngine.PostgreSql, PostgresMajorVersion = 14 })); + Assert.True(PgServerConfigCollector.Instance.AppliesTo( + new CollectorTargetInfo { Engine = CollectorTargetEngine.PostgreSql, PostgresMajorVersion = 18, IsAurora = true })); + + var sql = PgServerConfigCollector.Instance.BuildQuery(MakeContext()).Text; + + Assert.Contains("FROM pg_catalog.pg_settings", sql, StringComparison.Ordinal); + } + + /// + /// source is what separates the server's configuration from the collector's own session, so the + /// collector must STORE it — and must not filter on it. Dropping a session-scoped row at collection + /// time makes the evidence unrecoverable and leaves every later read guessing; the read does the + /// filtering, where it can be explained. + /// + [Fact] + public void TheCollectorStoresSourceAndDoesNotFilterOnIt() + { + var columns = PgServerConfigCollector.Instance.PayloadColumns.Select(c => c.Name).ToList(); + + Assert.Contains("source", columns); + Assert.Contains("boot_val", columns); + Assert.Contains("pending_restart", columns); + Assert.Contains("context", columns); + + var sql = PgServerConfigCollector.Instance.BuildQuery(MakeContext()).Text; + + Assert.DoesNotContain("WHERE", sql, StringComparison.Ordinal); + } + + /* ---------------- the reads ---------------- */ + + /// + /// pg_settings is a per-BACKEND view, so a client- or session-source row describes + /// the monitoring connection. Presenting one as the server's configuration would be wrong; reporting one + /// as a CHANGE would be worse, because the collector reconnects and application_name moves, and + /// the read would announce a configuration change nobody made. Both reads exclude them. + /// + [Fact] + public void BothReadsExcludeSessionScopedRows() + { + Assert.Contains("'client'", DarlingPgServerConfigReader.SessionScopedSources, StringComparison.Ordinal); + Assert.Contains("'session'", DarlingPgServerConfigReader.SessionScopedSources, StringComparison.Ordinal); + + /* Spelled out inline in both statements, so the constants are real SQL that parse analysis can + check — an earlier version substituted a SESSION_SCOPED token at call time and both reads failed + DarlingPgReadSqlParsesLiveTests with 42703. This assertion is what keeps the inline list and the + named constant from drifting now that they are written twice. */ + var expected = "NOT IN (" + DarlingPgServerConfigReader.SessionScopedSources + ")"; + + Assert.Contains(expected, DarlingPgServerConfigReader.CurrentConfigSql, StringComparison.Ordinal); + Assert.Contains(expected, DarlingPgServerConfigReader.ConfigChangesSql, StringComparison.Ordinal); + } + + /// + /// PostgreSQL's own source decides what counts as default, NOT a text comparison against + /// boot_val. Measured on a live 17.11 target, the string comparison invents non-defaults on a + /// server nobody configured: data_directory_mode reads 0700 against a boot_val of + /// 448 — one value in octal and decimal — archive_command reads (disabled) against + /// an empty default, and commit_timestamp_buffers reads 32 against 0 because 0 means auto-tune. + /// All three have source = 'default'. + /// + [Fact] + public void DefaultnessComesFromSource_NotFromComparingTextToBootVal() + { + var sql = DarlingPgServerConfigReader.CurrentConfigSql; + + Assert.Contains("(coalesce(c.source, 'default') = 'default') AS is_default", sql, StringComparison.Ordinal); + Assert.DoesNotContain("c.setting IS NOT DISTINCT FROM c.boot_val", sql, StringComparison.Ordinal); + } + + /// + /// The current-config read is anchored on the newest snapshot, never on an hours window. A + /// configuration has no window — it is the state now — and an hours filter would return NOTHING for a + /// server whose hourly collector last ran just outside it, which reads as "this server has no + /// configuration" rather than "ask again". + /// + [Fact] + public void TheCurrentReadAnchorsOnTheNewestSnapshot_NotAWindow() + { + var sql = DarlingPgServerConfigReader.CurrentConfigSql; + + Assert.Contains("MAX(collection_time)", sql, StringComparison.Ordinal); + Assert.DoesNotContain("collection_time >=", sql, StringComparison.Ordinal); + } + + /// + /// A setting APPEARING is not a change, and this guard is the whole difference between a useful read and + /// a useless one. LAG returns NULL for the first snapshot of every setting, so without it the + /// first collection would report several hundred fabricated changes — verified on the rig, where the + /// first snapshot carried 415 settings and the read correctly reported ONE change after a real edit. + /// It would fire again for every extension whose GUCs appear when the library is loaded. + /// + [Fact] + public void TheChangesReadIgnoresASettingAppearingForTheFirstTime() + { + var sql = DarlingPgServerConfigReader.ConfigChangesSql; + + Assert.Contains("LAG(c.setting) OVER (PARTITION BY c.name ORDER BY c.collection_time)", sql, StringComparison.Ordinal); + Assert.Contains("WHERE prev_time IS NOT NULL", sql, StringComparison.Ordinal); + Assert.Contains("setting IS DISTINCT FROM prev_setting", sql, StringComparison.Ordinal); + } + + private static CollectorContext MakeContext() => new() + { + ServerId = 42, + ServerName = "pg-target", + CollectionTime = new DateTime(2026, 8, 26, 12, 0, 0, DateTimeKind.Utc), + Deltas = NoDeltas.Instance, + Target = new CollectorTargetInfo + { + Engine = CollectorTargetEngine.PostgreSql, + PostgresMajorVersion = 17, + }, + ExcludedDatabases = Array.Empty(), + }; + + private sealed class NoDeltas : ICollectorDeltaCalculator + { + public static readonly NoDeltas Instance = new(); + + public long CalculateDelta(int serverId, string key, string metric, long current, DateTime? at = null, int i = 0) => 0; + + public long CalculateDeltaWithInterval(int serverId, string key, string metric, long current, out int seconds, DateTime? at = null, int i = 0) + { + seconds = 60; + return 0; + } + } +} diff --git a/Darling/Darling.Tests/ServerPageTabsTests.cs b/Darling/Darling.Tests/ServerPageTabsTests.cs index ded2cf4b..a002fa19 100644 --- a/Darling/Darling.Tests/ServerPageTabsTests.cs +++ b/Darling/Darling.Tests/ServerPageTabsTests.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -31,7 +31,7 @@ namespace Darling.Tests; /// stale list of its own. /// /// Two registries since #2530. SERVER_TABS is the SQL Server set and the default for a -/// server whose engine the store makes no claim about; POSTGRES_TABS is the seven-tab PostgreSQL set, and +/// server whose engine the store makes no claim about; POSTGRES_TABS is the eight-tab PostgreSQL set, and /// serverTabsFor(card) is the only thing that chooses. Several pins below scan ONE registry's region of /// the file rather than the whole file, because the two sets have different rules — a get_pg_* read is /// correct in one and a defect in the other — and a whole-file scan cannot tell them apart. @@ -82,6 +82,8 @@ public sealed class ServerPageTabsTests ["get_pg_extensions"] = "pg_extension_availability", ["get_pg_lock_stats"] = "pg_lock_stats", ["get_pg_write_stats"] = "pg_write_stats", + ["get_pg_server_config"] = "pg_server_config", + ["get_pg_server_config_changes"] = "pg_server_config", ["get_pg_replication_stats"] = "pg_replication_stats", ["get_pg_top_queries"] = "pg_statement_stats", ["get_pg_plans"] = "pg_plan_capture", @@ -649,7 +651,7 @@ on that registry's Overview rather than on a tab from the other set. */ /* Ids are unique WITHIN a registry — two tabs sharing one id makes the second unreachable and the bar's active state lie. ACROSS registries they may and do collide (overview, activity, waits, io), which is deliberate: those are the deep links that survive a server turning out to be the other engine. */ - foreach (var (registry, expected) in new[] { ("SERVER_TABS", 12), ("POSTGRES_TABS", 7) }) + foreach (var (registry, expected) in new[] { ("SERVER_TABS", 12), ("POSTGRES_TABS", 8) }) { var ids = TabIdsIn(RegistryRegion(ServerTabsJs, registry)); /* An exact count, not a floor. A floor would have let the prose in the CHANGELOG, the commit and diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs index 4525ab21..4bae52a6 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -1146,6 +1146,8 @@ private static CatalogRead R(string category, string description, params Catalog ["get_pg_extensions"] = R(CatData, "Which PostgreSQL extensions are installed, outdated, available or absent, per database. Usually the reason another read is empty.", PServer(), PHours(168), PLimit(50), PAsOf()), ["get_pg_lock_stats"] = R(CatData, "Sampled PostgreSQL lock activity by type, mode and relation. A sample of pg_locks, not an event log; for who blocks whom use get_pg_blocking.", PServer(), PHours(24), PLimit(25), PAsOf()), ["get_pg_write_stats"] = R(CatData, "Checkpoint and WAL write activity across the window: timed versus requested checkpoints, buffers written by whom, and WAL volume.", PServer(), PHours(24), PAsOf()), + ["get_pg_server_config"] = R(CatData, "The PostgreSQL server's configuration from pg_settings, non-default first, saying where each value came from and whether changing it needs a restart. Reports pending_restart, where the file and the running server disagree.", PServer(), PLimit(100), PBool("include_defaults", false)), + ["get_pg_server_config_changes"] = R(CatData, "PostgreSQL configuration parameters whose value CHANGED in the window, old beside new. Nothing else can reconstruct this after the fact.", PServer(), PHours(168), PLimit(100), PAsOf()), ["get_pg_replication_stats"] = R(CatData, "Health of CONNECTED replicas from pg_stat_replication, with the worst lag in the window beside the latest. Counterpart of get_pg_replication_slots.", PServer(), PHours(24), PLimit(25), PAsOf()), ["get_pg_blocking"] = R(CatData, "PostgreSQL blocking chains that were sampled, with the root blocker attributed. A sample, not an event log.", PServer(), PHours(24), PLimit(50), PAsOf()), ["get_pg_database_stats"] = R(CatData, "PostgreSQL per-database temp-file spills, cache hit ratio, deadlocks and commit/rollback split, differenced across the window.", PServer(), PHours(24), PLimit(20), PAsOf()), @@ -1622,6 +1624,8 @@ cannot parse exactly rather than silently matching nothing. */ ["get_pg_extensions"] = (c, pg, an) => DarlingMcpPgServerStateTools.GetPgExtensions(pg, Server(c), Hours(c, 168), Rows(c, "limit", 50), as_of: AsOf(c)), ["get_pg_lock_stats"] = (c, pg, an) => DarlingMcpPgServerStateTools.GetPgLockStats(pg, Server(c), Hours(c, 24), Rows(c, "limit", 25), as_of: AsOf(c)), ["get_pg_write_stats"] = (c, pg, an) => DarlingMcpPgServerStateTools.GetPgWriteStats(pg, Server(c), Hours(c, 24), as_of: AsOf(c)), + ["get_pg_server_config"] = (c, pg, an) => DarlingMcpPgServerStateTools.GetPgServerConfig(pg, Server(c), Rows(c, "limit", 100), QueryBool(c, "include_defaults", false)), + ["get_pg_server_config_changes"] = (c, pg, an) => DarlingMcpPgServerStateTools.GetPgServerConfigChanges(pg, Server(c), Hours(c, 168), Rows(c, "limit", 100), as_of: AsOf(c)), ["get_pg_replication_stats"] = (c, pg, an) => DarlingMcpPgReplicationStatsTools.GetPgReplicationStats(pg, Server(c), Hours(c, 24), Rows(c, "limit", 25), as_of: AsOf(c)), ["get_pg_blocking"] = (c, pg, an) => DarlingMcpPgBlockingTools.GetPgBlocking(pg, Server(c), Hours(c, 24), Rows(c, "limit", 50), as_of: AsOf(c)), ["get_pg_database_stats"] = (c, pg, an) => DarlingMcpPgDatabaseTools.GetPgDatabaseStats(pg, Server(c), Hours(c, 24), Rows(c, "limit", 20), as_of: AsOf(c)), diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs index 3020aa98..050ca6a8 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -4819,6 +4819,7 @@ await DarlingObservability.LogCollectionAsync( ["pg_wait_stats"] = (r, s, ct) => r.RunAsync(PgWaitStatsCollector.Instance, s, ct), ["pg_statement_stats"] = (r, s, ct) => r.RunAsync(PgStatementStatsCollector.Instance, s, ct), ["pg_wraparound_stats"] = (r, s, ct) => r.RunAsync(PgWraparoundStatsCollector.Instance, s, ct), + ["pg_server_config"] = (r, s, ct) => r.RunAsync(PgServerConfigCollector.Instance, s, ct), ["pg_xmin_horizon"] = (r, s, ct) => r.RunAsync(PgXminHorizonCollector.Instance, s, ct), ["pg_replication_slots"] = (r, s, ct) => r.RunAsync(PgReplicationSlotsCollector.Instance, s, ct), ["pg_autovacuum_stats"] = (r, s, ct) => r.RunAsync(PgAutovacuumStatsCollector.Instance, s, ct), diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs index e6841a0b..c441759c 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -641,6 +641,18 @@ job includes REFUSING a causal claim. get_pg_xmin_horizon says a session is hold idle-in-transaction session that looks identical is holding nothing at all, because a READ COMMITTED transaction that only read has already released its snapshot. */ .WithGeminiCompatibleTools() + /* #2659: these six shipped REGISTERED NOWHERE. They were implemented, documented, dispatched + by the web API and counted in the instructions census, and an agent could not call one of + them — the web dashboard could, which is why it went unnoticed. Registration here is + per-class and explicit, with no assembly scan, so a tools class is reachable only if + someone remembers this line and nothing failed when they did not. + McpToolTypeRegistrationTests now derives the check by reflection instead of trusting it. */ + .WithGeminiCompatibleTools() + .WithGeminiCompatibleTools() + .WithGeminiCompatibleTools() + .WithGeminiCompatibleTools() + .WithGeminiCompatibleTools() + .WithGeminiCompatibleTools() .WithGeminiCompatibleTools() .WithGeminiCompatibleTools() .WithGeminiCompatibleTools() diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs index c90e0197..63c7c74f 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -62,7 +62,7 @@ public static string Build(DarlingPeerDirectory.Snapshot peers) ## Tool Reference - This server exposes 126 tools. 86 are the same names Performance Monitor Lite exposes, spanning diagnostic analysis, plan analysis, data reads at core and diagnostic depth, resource contention + jobs, trends, system-health parse-on-read, alerts + health overview, and the Default Trace. The remaining 40 are unique to Darling: twenty-three are the PostgreSQL reads (Aurora/PostgreSQL targets only Darling's central store can hold), eight are the Custom Views tools (seven manage the saved views — the one view-authoring write surface — and `describe_custom_view_catalog` returns the read-only compose vocabulary those authoring tools draw from), three are alert-tuning write tools (`update_alert_settings` tunes the alert engine's thresholds; `create_mute_rule` / `delete_mute_rule` manage the mute rules) that write only the shared alert configuration in the monitoring store, two are server-onboarding write tools (`add_servers` bulk-adds monitored servers; `remove_server` removes one) that add or remove rows in the monitoring store's monitored-server registry, `get_fleet_overview` and `get_ag_health` are the two cross-server reads only a central store can answer, `get_store_metrics` reads the monitoring store's OWN hourly size/compression/growth series for capacity forecasting, and `get_blocking` is Darling's name for the blocked-process-report read that Lite exposes as `get_blocked_process_reports` — a naming difference, not a capability gap. Every data-read tool reads the data the collectors already captured into the store — a stored read, never a live query against the monitored server. + This server exposes 128 tools. 86 are the same names Performance Monitor Lite exposes, spanning diagnostic analysis, plan analysis, data reads at core and diagnostic depth, resource contention + jobs, trends, system-health parse-on-read, alerts + health overview, and the Default Trace. The remaining 42 are unique to Darling: twenty-five are the PostgreSQL reads (Aurora/PostgreSQL targets only Darling's central store can hold), eight are the Custom Views tools (seven manage the saved views — the one view-authoring write surface — and `describe_custom_view_catalog` returns the read-only compose vocabulary those authoring tools draw from), three are alert-tuning write tools (`update_alert_settings` tunes the alert engine's thresholds; `create_mute_rule` / `delete_mute_rule` manage the mute rules) that write only the shared alert configuration in the monitoring store, two are server-onboarding write tools (`add_servers` bulk-adds monitored servers; `remove_server` removes one) that add or remove rows in the monitoring store's monitored-server registry, `get_fleet_overview` and `get_ag_health` are the two cross-server reads only a central store can answer, `get_store_metrics` reads the monitoring store's OWN hourly size/compression/growth series for capacity forecasting, and `get_blocking` is Darling's name for the blocked-process-report read that Lite exposes as `get_blocked_process_reports` — a naming difference, not a capability gap. Every data-read tool reads the data the collectors already captured into the store — a stored read, never a live query against the monitored server. ### Reading an empty result diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgServerStateTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgServerStateTools.cs index 69971f6b..22e38c23 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgServerStateTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgServerStateTools.cs @@ -364,4 +364,144 @@ guessed one. */ return McpHelpers.Status("error", $"Reading PostgreSQL write stats failed: {ex.Message}"); } } + + [McpServerTool(Name = "get_pg_server_config"), Description("Gets the PostgreSQL server's configuration from pg_settings - what each parameter is set to, whether it differs from the compiled-in default, where the value came from (configuration file, command line, ALTER SYSTEM, per-database or per-role), and whether changing it needs a restart or only a reload. Non-default settings are listed FIRST, because a server has several hundred parameters and only the ones somebody chose are an answer. Reports pending_restart loudly: that means postgresql.conf was edited and reloaded but the running server is still using the old value, so the file and the server disagree with no symptom until the next restart. Session-scoped rows are excluded - pg_settings is a per-connection view and its client-source rows describe the monitoring connection, not the server. Snapshot from the most recent collection, not a window.")] + public static async Task GetPgServerConfig( + NpgsqlDataSource postgres, + [Description("Server name or display name.")] string? server_name = null, + [Description("Maximum settings to return. Default 100.")] int limit = 100, + [Description("When true, include settings still at their default. Default false - the non-default ones are the answer.")] bool include_defaults = false) + { + var (resolved, error) = await DarlingServerResolver.ResolveOrErrorAsync(postgres, server_name); + if (error != null) return error; + + var limitError = McpHelpers.ValidateTop(limit); + if (limitError != null) return McpHelpers.Status("error", limitError); + + try + { + var rows = await DarlingPgServerConfigReader.GetCurrentConfigAsync( + postgres, resolved.ServerId, limit); + + if (rows.Count == 0) + { + return await DarlingEngineCapability.NotCollectedStatusAsync( + postgres, resolved.ServerId, resolved.ServerName, "pg_server_config") + ?? McpHelpers.Status( + "empty", + $"No configuration snapshot has been collected for {resolved.ServerName} yet. " + + "This collector runs hourly, so a server registered in the last hour has not " + + "reached its first collection."); + } + + var shown = include_defaults ? rows : rows.Where(r => !r.IsDefault).ToList(); + var pendingRestart = rows.Where(r => r.PendingRestart).Select(r => r.Name).ToList(); + + return JsonSerializer.Serialize(new + { + server = resolved.ServerName, + status = "server_config", + /* Both counts, because they answer different questions and one without the other invites + the wrong conclusion: a small returned count is reassuring only if you know it was + filtered rather than truncated. */ + settings_returned = shown.Count, + non_default_count = rows.Count(r => !r.IsDefault), + truncated = rows.Count >= limit, + pending_restart_count = pendingRestart.Count, + pending_restart_settings = pendingRestart.Count > 0 ? pendingRestart : null, + note = pendingRestart.Count > 0 + ? "One or more settings are marked PENDING RESTART: the configuration file has been " + + "changed and reloaded, but the running server is still using the previous value. " + + "The file and the server disagree until the next restart, at which point the " + + "behaviour changes with no deployment to explain it." + : "Non-default settings first. 'source' says where the value came from; 'context' says " + + "what changing it would take - postmaster needs a restart, sighup a reload, user " + + "nothing. Session-scoped rows are excluded: pg_settings is a per-connection view " + + "and those describe the monitoring connection rather than the server.", + settings = shown.Select(r => new + { + name = r.Name, + setting = r.Setting, + unit = r.Unit, + /* The compiled-in default, so a reader can see what was moved away FROM without + needing a table of defaults that would rot at every major. */ + default_value = r.BootValue, + is_default = r.IsDefault, + source = r.Source, + context = r.Context, + requires_restart_to_change = string.Equals(r.Context, "postmaster", StringComparison.Ordinal), + pending_restart = r.PendingRestart, + category = r.Category, + description = r.ShortDescription, + }), + }, McpHelpers.JsonOptions); + } + catch (Exception ex) + { + return McpHelpers.Status("error", $"Reading PostgreSQL server config failed: {ex.Message}"); + } + } + + [McpServerTool(Name = "get_pg_server_config_changes"), Description("Gets PostgreSQL configuration parameters whose value CHANGED during the window, newest first, with the old and new value side by side. This is the read that answers 'this got slow sometime last month, what changed' - and nothing else in the stack can reconstruct it after the fact, because a configuration history that was not recorded cannot be recovered from the server. A setting appearing for the first time is deliberately NOT reported as a change: the first snapshot after an upgrade, or after an extension is loaded, would otherwise manufacture hundreds of changes nobody made. Session-scoped rows are excluded, so a monitoring reconnect does not read as a configuration change.")] + public static async Task GetPgServerConfigChanges( + NpgsqlDataSource postgres, + [Description("Server name or display name.")] string? server_name = null, + [Description("Hours of history to analyze. Default 168 (one week).")] int hours_back = 168, + [Description("Maximum changes to return. Default 100.")] int limit = 100, + [Description(McpHelpers.AsOfDescription)] string? as_of = null) + { + var (resolved, error) = await DarlingServerResolver.ResolveOrErrorAsync(postgres, server_name); + if (error != null) return error; + + var validation = McpHelpers.ValidateWindow(hours_back, as_of, out var windowEnd); + if (validation != null) return validation; + + var limitError = McpHelpers.ValidateTop(limit); + if (limitError != null) return McpHelpers.Status("error", limitError); + + try + { + var rows = await DarlingPgServerConfigReader.GetConfigChangesAsync( + postgres, resolved.ServerId, windowEnd.AddHours(-hours_back), windowEnd, limit); + + if (rows.Count == 0) + { + return await DarlingEngineCapability.NotCollectedStatusAsync( + postgres, resolved.ServerId, resolved.ServerName, "pg_server_config") + ?? McpHelpers.Status( + "no_changes", + $"No configuration parameter changed value on {resolved.ServerName} in the last " + + $"{hours_back} hour(s). That is a real finding rather than missing data - this " + + "read compares consecutive snapshots, so an unchanged server produces no rows."); + } + + return JsonSerializer.Serialize(new + { + server = resolved.ServerName, + hours_back, + status = "config_changes", + change_count = rows.Count, + truncated = rows.Count >= limit, + note = "changed_at is the time of the snapshot that FIRST reported the new value, so the " + + "change happened at some point in the hour before it - this collector runs hourly. " + + "A setting appearing for the first time is not reported here.", + changes = rows.Select(r => new + { + changed_at = r.ChangedAtUtc, + name = r.Name, + old_value = r.OldValue, + new_value = r.NewValue, + unit = r.Unit, + source = r.Source, + context = r.Context, + description = r.ShortDescription, + }), + }, McpHelpers.JsonOptions); + } + catch (Exception ex) + { + return McpHelpers.Status("error", $"Reading PostgreSQL config changes failed: {ex.Message}"); + } + } + } diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js index 9aec156f..a669ac46 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js @@ -13,7 +13,7 @@ * There are TWO, and serverTabsFor(card) picks between them from the fleet card's server-derived `is_postgres` * (#2530). SERVER_TABS is the SQL Server registry and also the default for a card that makes no engine claim — * the name is kept for that second job, because "no claim" has always rendered these tabs and still should. - * POSTGRES_TABS is the seven-tab PostgreSQL registry; its own header says why seven is the answer and not twelve. + * POSTGRES_TABS is the eight-tab PostgreSQL registry; its own header says why that is the answer and not twelve. * * Every entry is `{ id, label, note?, build(server, ctx) }` and `build` returns an array of nodes, almost all of * them PANEL DESCRIPTORS run through the unmodified renderPanel (the #1563 seam): a `read` naming an MCP tool @@ -1324,7 +1324,7 @@ export const SERVER_TABS = [ /* ─────────────────────────── the PostgreSQL tabs ─────────────────────────── */ /** - * The PostgreSQL registry (#2530). SEVEN tabs against the SQL Server registry's twelve, and the difference is the + * The PostgreSQL registry (#2530). EIGHT tabs against the SQL Server registry's twelve, and the difference is the * design rather than a shortfall: parity was explicitly not the constraint. Bloat, wraparound, the xmin horizon * and autovacuum have no SQL Server analogue and are what actually pages a PostgreSQL DBA; tempdb, Query Store, * trace flags, plan cache and the system_health ring buffer have no PostgreSQL analogue, and rendering them at a @@ -1907,6 +1907,39 @@ export const POSTGRES_TABS = [ ), ], }, + + /* #2658. The eighth tab, and the header's argument for seven admits it: what that argument rejects is + reproducing SQL-Server-only CONCEPTS at a PostgreSQL target, not adding a question PostgreSQL genuinely + has. "What is this server set to, and what changed" is one every engine has, and it was the only + remaining get_pg_* pair with nowhere to land. + + Changes ABOVE current settings, deliberately. Somebody opening this tab during an incident is asking + what moved, not what the server is; the full configuration is reference material and reads better as + the thing underneath it. */ + { + id: "config", + label: "Configuration", + build: (server, ctx) => [ + table( + "Configuration Changes", + "get_pg_server_config_changes", + { server, hours: ctx.hours }, + "changes", + PG_CONFIG_CHANGE_COLUMNS, + ctx.label + ", newest first; the collector runs hourly, so a change happened in the hour before it was seen", + "No configuration parameter changed value in this window. This compares consecutive snapshots, so an unchanged server is legitimately empty here - it is a finding, not missing data." + ), + table( + "Settings", + "get_pg_server_config", + { server }, + "settings", + PG_SERVER_CONFIG_COLUMNS, + "non-default first; pending restart means the file and the running server disagree", + "No configuration snapshot has been collected yet. This collector runs hourly, so a server registered within the last hour has not reached its first collection." + ), + ], + }, ]; /** @@ -2877,6 +2910,26 @@ const PG_IO_SUMMARY_STATS = [ buffers_backend is here rather than buried with the other buffer counters because it is the one that lands on a user query: a backend writing its own dirty buffer is a query paying for the write. */ +const PG_SERVER_CONFIG_COLUMNS = [ + { key: "name", label: "Setting" }, + { key: "setting", label: "Value" }, + { key: "unit", label: "Unit", small: true }, + { key: "default_value", label: "Default", small: true }, + { key: "source", label: "Source", small: true }, + { key: "context", label: "Change needs", small: true }, + { key: "pending_restart", label: "Pending restart", format: "bool", small: true }, + { key: "category", label: "Category", small: true }, +]; + +const PG_CONFIG_CHANGE_COLUMNS = [ + { key: "changed_at", label: "Seen at", format: "time" }, + { key: "name", label: "Setting" }, + { key: "old_value", label: "From" }, + { key: "new_value", label: "To" }, + { key: "unit", label: "Unit", small: true }, + { key: "source", label: "Source", small: true }, +]; + const PG_WRITE_STATS = [ { key: "checkpoints_timed", label: "Timed", format: "int" }, { key: "checkpoints_requested", label: "Requested", format: "int" }, diff --git a/Darling/PerformanceMonitor.Darling.Storage/DarlingPgServerConfigReader.cs b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgServerConfigReader.cs new file mode 100644 index 00000000..a51f02b0 --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Storage/DarlingPgServerConfigReader.cs @@ -0,0 +1,217 @@ +// Copyright (c) Erik Darling Data. All rights reserved. +// Licensed under the terms in the LICENSE file in the repository root. + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Npgsql; + +namespace PerformanceMonitor.Darling.Storage; + +/// +/// Reads the stored pg_settings snapshots (#2658): the server's configuration as of the newest +/// snapshot, and the changes between consecutive snapshots. +/// +/// Session-scoped rows are excluded here, not at collection. pg_settings is a +/// per-BACKEND view, so a row whose source is client or session describes the +/// collector's own connection rather than the server. The collector stores them deliberately — dropping +/// them there would make the evidence unrecoverable — and both reads filter them out, because presenting +/// one as the server's configuration would be wrong, and reporting one as a CHANGE would be worse: the +/// collector reconnects, application_name differs, and the read would announce a configuration +/// change nobody made. +/// +public static class DarlingPgServerConfigReader +{ + /// + /// The source values that describe THIS connection rather than the server. Deliberately a + /// whitelist of what to exclude rather than of what to keep: PostgreSQL adds source values between + /// majors, and an unknown one is far more likely to be a real server source than a new kind of + /// session state, so an unrecognised value should show up rather than vanish. + /// + /// Spelled out inline in both statements rather than substituted into them. The first + /// version of this reader used a SESSION_SCOPED token replaced at call time, which meant the + /// SQL constants were not SQL: DarlingPgReadSqlParsesLiveTests runs parse analysis on every + /// shipped read against a real server and both failed with 42703 column "session_scoped" does not + /// exist. A read whose text only becomes valid after a string substitution cannot be checked by + /// anything, which is worth more than the deduplication. The list stays honest because + /// PgServerConfigTests asserts both statements contain NOT IN ( plus this exact + /// string. + /// + public const string SessionScopedSources = "'client', 'session', 'override'"; + + public readonly record struct PgConfigRow( + string Name, + string? Setting, + string? Unit, + string? Category, + string? Context, + string? Source, + string? BootValue, + string? ResetValue, + string? SourceFile, + int SourceLine, + bool PendingRestart, + string? ShortDescription, + bool IsDefault); + + public readonly record struct PgConfigChangeRow( + DateTime ChangedAtUtc, + string Name, + string? OldValue, + string? NewValue, + string? Unit, + string? Context, + string? Source, + string? ShortDescription); + + /// + /// The newest snapshot, session-scoped rows removed. Anchored on MAX(collection_time) for the + /// server rather than on "within the last N hours": a configuration read has no window — it is the + /// state now — and an hours filter would return NOTHING on a server whose hourly collector last ran + /// just outside it, which reads as "this server has no configuration". + /// + public const string CurrentConfigSql = """ + SELECT + c.name, + c.setting, + c.unit, + c.category, + c.context, + c.source, + c.boot_val, + c.reset_val, + c.sourcefile, + coalesce(c.sourceline, 0), + coalesce(c.pending_restart, false), + c.short_desc, + /* PostgreSQL's OWN verdict, not a text comparison against boot_val. Comparing the strings + looks equivalent and is not, and the failures all point the same way — they invent + non-defaults on a server nobody has configured. Measured on the rig: data_directory_mode + reports setting '0700' against boot_val '448', the same value in octal and decimal; + archive_command reports '(disabled)' against an empty boot_val, which is a display + convention rather than a value; commit_timestamp_buffers reports 32 against a boot_val of 0, + because 0 means auto-tune and the server resolved it at startup. All three have + source = 'default', which is PostgreSQL saying plainly that nobody set them. + boot_val is still stored and returned — it is useful to SEE what the default is — it just + does not get to decide this. */ + (coalesce(c.source, 'default') = 'default') AS is_default + FROM pg_server_config AS c + WHERE c.server_id = $1 + AND c.collection_time = ( + SELECT MAX(collection_time) + FROM pg_server_config + WHERE server_id = $1) + AND coalesce(c.source, '') NOT IN ('client', 'session', 'override') + /* Non-default first: 415 settings sorted alphabetically is a dump, not an answer. pending_restart + outranks even that, because it is the one row that says the file and the running server + disagree. */ + ORDER BY coalesce(c.pending_restart, false) DESC, + (coalesce(c.source, 'default') = 'default'), + c.name + LIMIT $2 + """; + + /// + /// Value changes between consecutive snapshots, newest first. LAG over the per-setting series, + /// so a row appears only where the value actually moved. + /// + /// A setting that APPEARS is not a change. LAG returns NULL for the first snapshot + /// of every setting, and reporting that as "changed from nothing to 4MB" would turn the first + /// collection after an upgrade into hundreds of fabricated changes — and would do it again for every + /// extension whose GUCs appear when it is loaded. The prev IS NOT NULL guard is what makes this + /// read say only what it actually observed. + /// + public const string ConfigChangesSql = """ + WITH ordered AS ( + SELECT + c.collection_time, + c.name, + c.setting, + c.unit, + c.context, + c.source, + c.short_desc, + LAG(c.setting) OVER (PARTITION BY c.name ORDER BY c.collection_time) AS prev_setting, + LAG(c.collection_time) OVER (PARTITION BY c.name ORDER BY c.collection_time) AS prev_time + FROM pg_server_config AS c + WHERE c.server_id = $1 + AND c.collection_time >= $2 + AND c.collection_time <= $3 + AND coalesce(c.source, '') NOT IN ('client', 'session', 'override') + ) + SELECT + collection_time, + name, + prev_setting, + setting, + unit, + context, + source, + short_desc + FROM ordered + WHERE prev_time IS NOT NULL + AND setting IS DISTINCT FROM prev_setting + ORDER BY collection_time DESC, name + LIMIT $4 + """; + + public static async Task> GetCurrentConfigAsync( + NpgsqlDataSource postgres, int serverId, int limit, CancellationToken cancellationToken = default) + { + var rows = new List(); + await using var command = postgres.CreateCommand(CurrentConfigSql); + command.Parameters.AddWithValue(serverId); + command.Parameters.AddWithValue(limit); + await using var reader = await command.ExecuteReaderAsync(cancellationToken); + while (await reader.ReadAsync(cancellationToken)) + { + rows.Add(new PgConfigRow( + reader.GetString(0), + reader.IsDBNull(1) ? null : reader.GetString(1), + reader.IsDBNull(2) ? null : reader.GetString(2), + reader.IsDBNull(3) ? null : reader.GetString(3), + reader.IsDBNull(4) ? null : reader.GetString(4), + reader.IsDBNull(5) ? null : reader.GetString(5), + reader.IsDBNull(6) ? null : reader.GetString(6), + reader.IsDBNull(7) ? null : reader.GetString(7), + reader.IsDBNull(8) ? null : reader.GetString(8), + reader.GetInt32(9), + reader.GetBoolean(10), + reader.IsDBNull(11) ? null : reader.GetString(11), + !reader.IsDBNull(12) && reader.GetBoolean(12))); + } + + return rows; + } + + public static async Task> GetConfigChangesAsync( + NpgsqlDataSource postgres, int serverId, DateTime startUtc, DateTime endUtc, int limit, + CancellationToken cancellationToken = default) + { + var rows = new List(); + await using var command = postgres.CreateCommand(ConfigChangesSql); + command.Parameters.AddWithValue(serverId); + /* Kind-Unspecified at the BIND, per the store's naive-UTC discipline: a Kind=Utc DateTime makes + Npgsql infer timestamptz, and PostgreSQL then converts these naive columns at the store session's + TimeZone, which silently empties the window east of UTC. */ + command.Parameters.AddWithValue(DateTime.SpecifyKind(startUtc, DateTimeKind.Unspecified)); + command.Parameters.AddWithValue(DateTime.SpecifyKind(endUtc, DateTimeKind.Unspecified)); + command.Parameters.AddWithValue(limit); + await using var reader = await command.ExecuteReaderAsync(cancellationToken); + while (await reader.ReadAsync(cancellationToken)) + { + rows.Add(new PgConfigChangeRow( + reader.GetDateTime(0), + reader.GetString(1), + reader.IsDBNull(2) ? null : reader.GetString(2), + reader.IsDBNull(3) ? null : reader.GetString(3), + reader.IsDBNull(4) ? null : reader.GetString(4), + reader.IsDBNull(5) ? null : reader.GetString(5), + reader.IsDBNull(6) ? null : reader.GetString(6), + reader.IsDBNull(7) ? null : reader.GetString(7))); + } + + return rows; + } +} diff --git a/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs b/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs index 84b6f4f7..dc0a3e1c 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs @@ -158,6 +158,7 @@ here costs a fresh-through-this-rung store nothing and rung 54's own copy no-ops new Migration(99, "pg-plan-capture", V99Sql), new Migration(100, "pg-major-version", V100Sql), new Migration(101, "pg18-io-bytes", V101Sql), + new Migration(102, "pg-server-config", V102Sql), }; /// @@ -2278,6 +2279,61 @@ pool_buffers_used bigint CREATE INDEX IF NOT EXISTS idx_pg_buffer_usage_time ON collect.pg_buffer_usage(server_id, collection_time);"; + /// + /// V102 — collect.pg_server_config, the server's own configuration from pg_settings + /// (#2658). SQL Server answers this three ways and PostgreSQL had no answer at all: nothing stored a + /// setting, so both "what is work_mem here" and "what changed last Tuesday" were unanswerable + /// after the fact — the second permanently, because no other part of the stack can reconstruct a + /// configuration history that was never recorded. + /// + /// Every column is stored and nothing is filtered at collection. pg_settings is a + /// per-BACKEND view, so source ranges from default through configuration file to + /// client — and a client row is the collector's own session, not the server. Dropping + /// those here would make them unrecoverable and would still leave the read guessing; keeping + /// source lets the read state which rows are server configuration and which are not. The rule + /// lives at the read, where it can be enforced and explained. + /// + /// It IS a hypertable, like every collector table — TimescaleSupport.HypertableTables + /// is CollectorCatalog.All, so membership follows from being a collector and is not a per-table + /// choice. Worth stating because the shape argues the other way: this is a snapshot of something a + /// person changes, not a series of measurements, and the rows are wide-ish text that is nearly + /// identical from one hour to the next. Chunking and compression still earn their place on exactly that + /// data — a year of hourly near-duplicates is what compresses best — and the alternative would be a + /// special case in the one place that currently has none. It does mean CI's worker sizing moves: + /// CiClusterWorkerSizingTests derives the cluster's worker counts from the catalog count, so + /// adding a collector is also a workflow edit. + /// + /// All value columns nullable, including name, because the generated schema is what + /// a fresh store builds from and it declares them that way — see + /// PgSchemaGeneratorTests.EveryPostgresRung_IsIdenticalToTheGeneratedSchema, which requires this + /// text to be column-for-column identical to what the generator walks out of + /// PgServerConfigCollector.PayloadColumns. A NOT NULL added by hand here and not there is the + /// permanent, invisible divergence that test exists to catch. + /// + private const string V102Sql = @" +CREATE TABLE IF NOT EXISTS collect.pg_server_config ( + collection_id bigint NOT NULL, + collection_time timestamp NOT NULL, + server_id integer NOT NULL, + server_name text NOT NULL, + name text, + setting text, + unit text, + category text, + context text, + vartype text, + source text, + boot_val text, + reset_val text, + sourcefile text, + sourceline integer, + pending_restart boolean, + short_desc text +); + +CREATE INDEX IF NOT EXISTS idx_pg_server_config_time + ON collect.pg_server_config(server_id, collection_time);"; + /// /// V101 — the measured I/O byte totals PostgreSQL 18 gave pg_stat_io (#2655). /// diff --git a/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs b/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs index 6b329769..dea770e5 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs @@ -16,5 +16,5 @@ namespace PerformanceMonitor.Darling.Storage; /// public static class StorageVersion { - public const int SchemaVersion = 101; + public const int SchemaVersion = 102; } diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Postgres.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Postgres.cs index 055d8860..a99695be 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Postgres.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Postgres.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -324,6 +324,14 @@ confusing of the two. */ int serverId, DateTime startUtc, DateTime endUtc, CancellationToken cancellationToken = default) => DarlingPgWriteStatsReader.GetPgWriteStatsAsync(_dataSource, serverId, startUtc, endUtc, cancellationToken); + /// Overview tab - the server's own configuration from pg_settings (#2658), non-default first. + /// Anchored on the newest snapshot rather than the toolbar window, deliberately: a configuration is the + /// state NOW, and an hours filter would return nothing for a server whose hourly collector last ran just + /// outside it - which reads as "this server has no configuration" rather than "ask again". + public Task> GetPgServerConfigAsync( + int serverId, int limit = 500, CancellationToken cancellationToken = default) => + DarlingPgServerConfigReader.GetCurrentConfigAsync(_dataSource, serverId, limit, cancellationToken); + /// Overview tab - which extensions this target has, could have, or cannot have (#2545). Latest /// state per extension rather than the history: installing one is a rare deliberate act, so the window /// holds the same answer repeated daily. Monitoring-relevant extensions sort first, and within them the diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs index da3e3862..5b20b7bd 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs @@ -677,7 +677,8 @@ existence cannot separate the rungs and information_schema.columns is the only s EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = 'pg_predicate_stats'), EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = 'pg_plan_capture'), EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'servers' AND column_name = 'postgres_major_version'), - EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'pg_io_stats' AND column_name = 'read_bytes')"; + EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'pg_io_stats' AND column_name = 'read_bytes'), + EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = 'pg_server_config')"; /// The store schema version this viewer build requires — the highest migration it knows /// (). The connect-time gate blocks a store below this. @@ -698,7 +699,7 @@ existence cannot separate the rungs and information_schema.columns is the only s await using var reader = await command.ExecuteReaderAsync(cancellationToken); if (await reader.ReadAsync(cancellationToken)) { - return MapProbedSchemaVersion(reader.GetBoolean(0), reader.GetBoolean(1), reader.GetBoolean(2), reader.GetBoolean(3), reader.GetBoolean(4), reader.GetBoolean(5), reader.GetBoolean(6), reader.GetBoolean(7), reader.GetBoolean(8), reader.GetBoolean(9), reader.GetBoolean(10), reader.GetBoolean(11), reader.GetBoolean(12), reader.GetBoolean(13), reader.GetBoolean(14), reader.GetBoolean(15), reader.GetBoolean(16), reader.GetBoolean(17), reader.GetBoolean(18), reader.GetBoolean(19), reader.GetBoolean(20), reader.GetBoolean(21), reader.GetBoolean(22), reader.GetBoolean(23), reader.GetBoolean(24), reader.GetBoolean(25), reader.GetBoolean(26), reader.GetBoolean(27), reader.GetBoolean(28), reader.GetBoolean(29), reader.GetBoolean(30), reader.GetBoolean(31), reader.GetBoolean(32), reader.GetBoolean(33), reader.GetBoolean(34), reader.GetBoolean(35), reader.GetBoolean(36), reader.GetBoolean(37), reader.GetBoolean(38), reader.GetBoolean(39), reader.GetBoolean(40), reader.GetBoolean(41), reader.GetBoolean(42), reader.GetBoolean(43), reader.GetBoolean(44), reader.GetBoolean(45), reader.GetBoolean(46), reader.GetBoolean(47), reader.GetBoolean(48), reader.GetBoolean(49), reader.GetBoolean(50), reader.GetBoolean(51), reader.GetBoolean(52), reader.GetBoolean(53), reader.GetBoolean(54), reader.GetBoolean(55), reader.GetBoolean(56), reader.GetBoolean(57), reader.GetBoolean(58), reader.GetBoolean(59), reader.GetBoolean(60), reader.GetBoolean(61), reader.GetBoolean(62), reader.GetBoolean(63), reader.GetBoolean(64), reader.GetBoolean(65), reader.GetBoolean(66), reader.GetBoolean(67), reader.GetBoolean(68), reader.GetBoolean(69), reader.GetBoolean(70), reader.GetBoolean(71), reader.GetBoolean(72), reader.GetBoolean(73), reader.GetBoolean(74), reader.GetBoolean(75), reader.GetBoolean(76)); + return MapProbedSchemaVersion(reader.GetBoolean(0), reader.GetBoolean(1), reader.GetBoolean(2), reader.GetBoolean(3), reader.GetBoolean(4), reader.GetBoolean(5), reader.GetBoolean(6), reader.GetBoolean(7), reader.GetBoolean(8), reader.GetBoolean(9), reader.GetBoolean(10), reader.GetBoolean(11), reader.GetBoolean(12), reader.GetBoolean(13), reader.GetBoolean(14), reader.GetBoolean(15), reader.GetBoolean(16), reader.GetBoolean(17), reader.GetBoolean(18), reader.GetBoolean(19), reader.GetBoolean(20), reader.GetBoolean(21), reader.GetBoolean(22), reader.GetBoolean(23), reader.GetBoolean(24), reader.GetBoolean(25), reader.GetBoolean(26), reader.GetBoolean(27), reader.GetBoolean(28), reader.GetBoolean(29), reader.GetBoolean(30), reader.GetBoolean(31), reader.GetBoolean(32), reader.GetBoolean(33), reader.GetBoolean(34), reader.GetBoolean(35), reader.GetBoolean(36), reader.GetBoolean(37), reader.GetBoolean(38), reader.GetBoolean(39), reader.GetBoolean(40), reader.GetBoolean(41), reader.GetBoolean(42), reader.GetBoolean(43), reader.GetBoolean(44), reader.GetBoolean(45), reader.GetBoolean(46), reader.GetBoolean(47), reader.GetBoolean(48), reader.GetBoolean(49), reader.GetBoolean(50), reader.GetBoolean(51), reader.GetBoolean(52), reader.GetBoolean(53), reader.GetBoolean(54), reader.GetBoolean(55), reader.GetBoolean(56), reader.GetBoolean(57), reader.GetBoolean(58), reader.GetBoolean(59), reader.GetBoolean(60), reader.GetBoolean(61), reader.GetBoolean(62), reader.GetBoolean(63), reader.GetBoolean(64), reader.GetBoolean(65), reader.GetBoolean(66), reader.GetBoolean(67), reader.GetBoolean(68), reader.GetBoolean(69), reader.GetBoolean(70), reader.GetBoolean(71), reader.GetBoolean(72), reader.GetBoolean(73), reader.GetBoolean(74), reader.GetBoolean(75), reader.GetBoolean(76), reader.GetBoolean(77)); } return null; @@ -723,7 +724,7 @@ existence cannot separate the rungs and information_schema.columns is the only s /// is unit-tested without a live store; any schema bump past the newest arm trips the pinning test that keeps /// this in step with . /// - internal static int MapProbedSchemaVersion(bool hasConfigControlPlane, bool hasAlertDeliveryOverride, bool hasAnalysisState, bool hasAlertTuningKnobs, bool hasDefaultTraceEvents, bool hasIndexObjectStatsLatestIndex, bool hasCollectionLogHypertableOrPlainPg, bool hasJobHistory, bool hasAgentStatus, bool hasGenericWebhook, bool hasDeadlocksDatabaseName, bool hasQueryStoreReplicaRole, bool hasLongQueryCompletions, bool hasWebDashboardConfig, bool hasCustomViews, bool hasServerTags, bool hasConnectionRefireKnobs = false, bool hasAgCollectors = false, bool hasAgAlertKnobs = false, bool hasAgLatencyColumns = false, bool hasAgDisconnectRefire = false, bool hasPayloadDimensions = false, bool hasDimFloorIndexes = false, bool hasBlockingWaitThreshold = false, bool hasQueryStoreIntervalIdentity = false, bool hasPagerDutyWebhook = false, bool hasPagerDutyProxy = false, bool hasCollectorState = false, bool hasPlanCorrection = false, bool hasPvsStats = false, bool hasPvsPressureKnobs = false, bool hasDatabaseStateAlert = false, bool hasServerTagColour = false, bool hasQueryStatsHostObject = false, bool hasFindingDrillDown = false, bool hasStoreMetrics = false, bool hasPlanDimGzip = false, bool hasSelfAlertKnobs = false, bool hasJobMetricsColumns = false, bool hasJobCadenceKnob = false, bool hasBackfillSwitch = false, bool hasCollectorMemoryKnobs = false, bool hasDatabaseStateEdgeMemory = false, bool hasIncidentOccurrences = false, bool hasPlanXmlCompressionKnob = false, bool hasMonitoredServerEngine = false, bool hasPgBlockingEdges = false, bool hasQueryStorePlanMap = false, bool hasPgStatementText = false, bool hasQueryStoreText = false, bool hasPlanContentRetentionKnob = false, bool hasQueryStoreHealth = false, bool hasQueryStoreTextHash = false, bool hasComposeTimeoutKnob = false, bool hasFileGrowthAlert = false, bool hasCollectionLogFanoutRollup = false, bool hasTempDbMaxSize = false, bool hasServerEngineKind = false, bool hasPgDatabaseStats = false, bool hasPgIndexUsageStats = false, bool hasPgTableBloatStats = false, bool hasPgSessionStates = false, bool hasPgPlanCaptureReadiness = false, bool hasPgWriteStats = false, bool hasPgExtensionAvailability = false, bool hasPgLockStats = false, bool hasPgColumnStats = false, bool hasPgReplicationStats = false, bool hasPgBufferUsage = false, bool hasPgIndexBloat = false, bool hasPgPerDatabaseAttribution = false, bool hasPgWaitSampling = false, bool hasPgKernelStats = false, bool hasPgPredicateStats = false, bool hasPgPlanCapture = false, bool hasPgMajorVersion = false, bool hasPg18IoBytes = false) + internal static int MapProbedSchemaVersion(bool hasConfigControlPlane, bool hasAlertDeliveryOverride, bool hasAnalysisState, bool hasAlertTuningKnobs, bool hasDefaultTraceEvents, bool hasIndexObjectStatsLatestIndex, bool hasCollectionLogHypertableOrPlainPg, bool hasJobHistory, bool hasAgentStatus, bool hasGenericWebhook, bool hasDeadlocksDatabaseName, bool hasQueryStoreReplicaRole, bool hasLongQueryCompletions, bool hasWebDashboardConfig, bool hasCustomViews, bool hasServerTags, bool hasConnectionRefireKnobs = false, bool hasAgCollectors = false, bool hasAgAlertKnobs = false, bool hasAgLatencyColumns = false, bool hasAgDisconnectRefire = false, bool hasPayloadDimensions = false, bool hasDimFloorIndexes = false, bool hasBlockingWaitThreshold = false, bool hasQueryStoreIntervalIdentity = false, bool hasPagerDutyWebhook = false, bool hasPagerDutyProxy = false, bool hasCollectorState = false, bool hasPlanCorrection = false, bool hasPvsStats = false, bool hasPvsPressureKnobs = false, bool hasDatabaseStateAlert = false, bool hasServerTagColour = false, bool hasQueryStatsHostObject = false, bool hasFindingDrillDown = false, bool hasStoreMetrics = false, bool hasPlanDimGzip = false, bool hasSelfAlertKnobs = false, bool hasJobMetricsColumns = false, bool hasJobCadenceKnob = false, bool hasBackfillSwitch = false, bool hasCollectorMemoryKnobs = false, bool hasDatabaseStateEdgeMemory = false, bool hasIncidentOccurrences = false, bool hasPlanXmlCompressionKnob = false, bool hasMonitoredServerEngine = false, bool hasPgBlockingEdges = false, bool hasQueryStorePlanMap = false, bool hasPgStatementText = false, bool hasQueryStoreText = false, bool hasPlanContentRetentionKnob = false, bool hasQueryStoreHealth = false, bool hasQueryStoreTextHash = false, bool hasComposeTimeoutKnob = false, bool hasFileGrowthAlert = false, bool hasCollectionLogFanoutRollup = false, bool hasTempDbMaxSize = false, bool hasServerEngineKind = false, bool hasPgDatabaseStats = false, bool hasPgIndexUsageStats = false, bool hasPgTableBloatStats = false, bool hasPgSessionStates = false, bool hasPgPlanCaptureReadiness = false, bool hasPgWriteStats = false, bool hasPgExtensionAvailability = false, bool hasPgLockStats = false, bool hasPgColumnStats = false, bool hasPgReplicationStats = false, bool hasPgBufferUsage = false, bool hasPgIndexBloat = false, bool hasPgPerDatabaseAttribution = false, bool hasPgWaitSampling = false, bool hasPgKernelStats = false, bool hasPgPredicateStats = false, bool hasPgPlanCapture = false, bool hasPgMajorVersion = false, bool hasPg18IoBytes = false, bool hasPgServerConfig = false) { /* V71 (the PostgreSQL blocking-edges rung): a table-existence sentinel and now the newest-first arm. A collector table would ordinarily get no arm at all — see the V63-V69 note below — but the TOP @@ -810,8 +811,16 @@ The three comment blocks that used to stack here were moved down onto the arms t (#2530 → V82, #2539 → V83, #2542 → V85). They had drifted upward as each new rung inserted its own block above them, which is how a comment ends up explaining an arm two rungs away from the one it was written for. Keep the block with its arm. */ - /* V101 (#2655): PostgreSQL 18's measured I/O byte columns. Column sentinel, and the TOP rung, so - it must map exactly or the connect-time gate refuses a store that is perfectly current. */ + /* V102 (#2658): the pg_settings snapshot. Table-existence sentinel, and the TOP rung, so it must + map exactly or the connect-time gate refuses a store that is perfectly current. */ + if (hasPgServerConfig) + { + return 102; + } + + /* V101 (#2655): PostgreSQL 18's measured I/O byte columns. Column sentinel. Was the top rung until + V102 landed and keeps its own arm, because a store stopped between the two is a real state + during an interrupted upgrade. */ if (hasPg18IoBytes) { return 101; diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs index 1ef1d7b9..d9c3b513 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerPostgresTabs.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -80,7 +80,7 @@ internal static class ViewerPostgresTabs ViewerServerTab.PgOverviewInnerTabIndex, "overview", "Overview", - new[] { "pg_extension_availability" }, + new[] { "pg_extension_availability", "pg_server_config" }, /* No collector of its own: it reports on every one of them, from collection_log joined to the catalog. That join is the point — a collector gated off for this engine writes NO collection_log row diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs index 3e445dcd..d1f0b1d9 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Postgres.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -156,6 +156,7 @@ private async Task LoadPgOverviewAsync() ViewerDataService.BuildPostgresCollectorHealth(_server, collectors, facts); await LoadPgExtensionsAsync(startUtc, endUtc); + await LoadPgServerConfigAsync(); } /// @@ -198,6 +199,52 @@ private async Task LoadPgExtensionsAsync(DateTime startUtc, DateTime endUtc) + "pg_extension is per-database while the server's offer is cluster-wide."); } + /// + /// The server's own configuration (#2658), under the extension axis because it is the same kind of fact + /// one layer in: extensions say what this server CAN do, settings say what it was told to do. + /// + /// Not scoped to the toolbar window, unlike every other panel on this tab. A configuration + /// is the state NOW rather than something that happened during an interval, and filtering it by the + /// window would return nothing for a server whose HOURLY collector last ran just outside it — which on + /// this screen reads as "this server has no configuration" rather than "widen the window". + /// + /// The note leads with pending_restart when there is one, because that is the only row here that + /// reports a DISAGREEMENT rather than a value: the file has been changed and reloaded, the running + /// server is still on the old value, and nothing else in the product would ever mention it. + /// + private async Task LoadPgServerConfigAsync() + { + if (PgCollectorIsGatedOff("pg_server_config")) + { + PgServerConfigGrid.ItemsSource = null; + PgServerConfigNote.Text = PanelNote("pg_server_config", 0, string.Empty); + return; + } + + var rows = await _dataService.GetPgServerConfigAsync(_server.ServerId); + + /* Non-default only in the grid: several hundred parameters sorted alphabetically is a dump, and the + ones somebody chose are the answer. The full set stays one MCP call away for anyone who wants it. */ + var chosen = rows.Where(r => !r.IsDefault).ToList(); + PgServerConfigGrid.ItemsSource = chosen; + + var pendingRestart = rows.Where(r => r.PendingRestart).Select(r => r.Name).ToList(); + + PgServerConfigNote.Text = PanelNote("pg_server_config", chosen.Count, + "This collector runs HOURLY, so a server added in the last hour has nothing here yet.") + + (chosen.Count == 0 + ? string.Empty + : $" {chosen.Count} setting(s) differ from the compiled-in default; the rest are omitted " + + "rather than truncated.") + + (pendingRestart.Count > 0 + ? $" {pendingRestart.Count} setting(s) are PENDING RESTART — " + + string.Join(", ", pendingRestart) + + " — meaning the configuration file has been changed and reloaded but the running " + + "server is still using the previous value. The file and the server disagree until the " + + "next restart, at which point behaviour changes with no deployment to explain it." + : string.Empty); + } + /// /// 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 diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml index 5098fd11..8376b2df 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.xaml @@ -3652,6 +3652,8 @@ + + + + + + + + + + + + + + + + + + + diff --git a/Lite.Tests/CrossAppMcpToolInventoryPinTests.cs b/Lite.Tests/CrossAppMcpToolInventoryPinTests.cs index 64d4030c..571414e7 100644 --- a/Lite.Tests/CrossAppMcpToolInventoryPinTests.cs +++ b/Lite.Tests/CrossAppMcpToolInventoryPinTests.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor Lite. @@ -73,6 +73,8 @@ cannot acquire one (the engine gate never dispatches a PostgreSQL definition the "get_pg_extensions", "get_pg_lock_stats", "get_pg_write_stats", + "get_pg_server_config", + "get_pg_server_config_changes", "get_pg_replication_stats", "get_pg_top_queries", "get_pg_plans", diff --git a/PerformanceMonitor.Collectors/CollectorCatalog.cs b/PerformanceMonitor.Collectors/CollectorCatalog.cs index 08e06897..7d47766a 100644 --- a/PerformanceMonitor.Collectors/CollectorCatalog.cs +++ b/PerformanceMonitor.Collectors/CollectorCatalog.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -70,6 +70,7 @@ kept honest by the engine gate in AppliesTo(definition, target). */ PgWaitStatsCollector.Instance, PgStatementStatsCollector.Instance, PgWraparoundStatsCollector.Instance, + PgServerConfigCollector.Instance, PgXminHorizonCollector.Instance, PgReplicationSlotsCollector.Instance, PgAutovacuumStatsCollector.Instance, diff --git a/PerformanceMonitor.Collectors/CollectorEngineCapability.cs b/PerformanceMonitor.Collectors/CollectorEngineCapability.cs index 8e81ce4f..293179a0 100644 --- a/PerformanceMonitor.Collectors/CollectorEngineCapability.cs +++ b/PerformanceMonitor.Collectors/CollectorEngineCapability.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -171,6 +171,7 @@ mechanism that engine does not have. */ ["pg_extension_availability"] = "the per-database extension inventory", ["pg_lock_stats"] = "the sampled pg_locks activity", ["pg_write_stats"] = "the checkpoint and WAL write counters", + ["pg_server_config"] = "the server's pg_settings configuration snapshot", ["pg_replication_stats"] = "the pg_stat_replication connected-replica states", /* Named for the LOG, because that is where the gap actually is: auto_explain writes plans nowhere else, and on Aurora and RDS there is no filesystem to read them from diff --git a/PerformanceMonitor.Collectors/CollectorScheduleDefaults.cs b/PerformanceMonitor.Collectors/CollectorScheduleDefaults.cs index 96f22901..bd6331c4 100644 --- a/PerformanceMonitor.Collectors/CollectorScheduleDefaults.cs +++ b/PerformanceMonitor.Collectors/CollectorScheduleDefaults.cs @@ -1,4 +1,4 @@ -/* +/* * Copyright (c) 2026 Erik Darling, Darling Data LLC * * This file is part of the SQL Server Performance Monitor. @@ -140,6 +140,14 @@ there is something to read. */ catalog. */ ["pg_wraparound_stats"] = new(5, 90), + /* Hourly, and the cadence IS the design (#2658). Configuration changes when a person changes it, + so polling it per minute buys nothing and writes 415 rows a minute forever; an hour is inside + anyone's window for "what changed this afternoon". 365 days because the question this answers is + almost always asked long after the fact — "this got slow sometime last quarter" — and a config + history shorter than the memory of the incident is no history at all. It is cheap to keep: the + rows are small, and the changes read only ever looks at consecutive snapshots. */ + ["pg_server_config"] = new(60, 365), + /* Per-minute, unlike its wraparound sibling: an xmin holder is the FAST-moving leading indicator, and the thing an operator wants is the session or slot that appeared minutes ago, before it has cost anything. At most five rows a cycle. 30 days matches the other diff --git a/PerformanceMonitor.Collectors/PgServerConfigCollector.cs b/PerformanceMonitor.Collectors/PgServerConfigCollector.cs new file mode 100644 index 00000000..1e9b5ffd --- /dev/null +++ b/PerformanceMonitor.Collectors/PgServerConfigCollector.cs @@ -0,0 +1,173 @@ +/* + * 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.Collections.Generic; +using System.Data.Common; +using System.Threading; +using System.Threading.Tasks; + +namespace PerformanceMonitor.Collectors; + +/// +/// The server's configuration, from pg_settings (#2658). SQL Server answers this three ways +/// (get_server_config, get_server_config_changes, get_database_scoped_config) and +/// PostgreSQL had no answer at all — nothing stored a setting, so "what is work_mem here" and +/// "what changed last Tuesday" were both unanswerable after the fact. +/// +/// The value alone would not have been worth a collector. What makes this one earn its place is +/// the three columns beside it: +/// +/// +/// source separates server configuration from session noise. pg_settings is a +/// per-BACKEND view, so it reports what THIS connection sees: on the rig application_name = psql with +/// source = client sits in the same result as auto_explain.log_min_duration from +/// command line. Storing them undifferentiated would record the collector's own session as the +/// server's configuration, and every snapshot would then "change" whenever the collector reconnected. +/// boot_val and reset_val give "differs from the default" without a hardcoded +/// table of defaults that would rot at every major. That is the difference between an answer and a dump: +/// 415 settings on the rig, 28 of them non-default. +/// pending_restart is a production trap with no symptom. Someone edits +/// postgresql.conf, reloads, and the file now says one thing while the running server does another — +/// until a restart months later silently changes behaviour, usually during an unrelated incident. SQL Server +/// has no equivalent column; PostgreSQL hands it over for free and nothing was reading it. +/// +/// +/// Everything is stored, nothing is filtered here. A client-source row is evidence about +/// the collector's own session and dropping it at collection time makes that unrecoverable; the READ decides +/// what counts as server configuration. The rule that matters is the one the read enforces: a session-scoped +/// row must never be presented as the server's setting. +/// +/// Core catalog only, no extension, readable by any login — the same cheap tier as +/// . pg_settings is cluster-wide, so one connection sees +/// everything and there is no per-database fan-out. +/// +public sealed class PgServerConfigCollector : PostgresCollectorDefinitionBase +{ + public static PgServerConfigCollector Instance { get; } = new(); + + private PgServerConfigCollector() + { + } + + public readonly record struct Row( + string Name, + string? Setting, + string? Unit, + string? Category, + string? Context, + string? VarType, + string? Source, + string? BootValue, + string? ResetValue, + string? SourceFile, + int SourceLine, + bool PendingRestart, + string? ShortDescription); + + /* pg_settings is a per-backend VIEW over the GUC table, not a shared catalog, so this reports what the + collector's own connection sees. That is not a defect to work around — it is why `source` is stored. + + No ORDER BY on the read's behalf: the read sorts by what it is answering (non-default first), and a + 415-row cluster-wide result is small enough that sorting here would only be a second sort. + + sourceline is 0 rather than NULL when a setting did not come from a file, matching what PostgreSQL + reports; the value is only meaningful alongside a non-null sourcefile. */ + private const string QueryText = @" +SELECT + s.name AS name, + s.setting AS setting, + s.unit AS unit, + s.category AS category, + s.context AS context, + s.vartype AS vartype, + s.source AS source, + s.boot_val AS boot_val, + s.reset_val AS reset_val, + s.sourcefile AS sourcefile, + coalesce(s.sourceline, 0) AS sourceline, + s.pending_restart AS pending_restart, + s.short_desc AS short_desc +FROM pg_catalog.pg_settings AS s"; + + public override string Name => "pg_server_config"; + + public override string TargetTable => "pg_server_config"; + + /// Core catalog only — every PostgreSQL target, Aurora or not. + public override bool AppliesTo(CollectorTargetInfo target) => true; + + public override CollectorQuery BuildQuery(CollectorContext context) => new(QueryText); + + public override IReadOnlyList PayloadColumns { get; } = new[] + { + new CollectorColumn("name", CollectorColumnType.Varchar), + new CollectorColumn("setting", CollectorColumnType.Varchar), + new CollectorColumn("unit", CollectorColumnType.Varchar), + new CollectorColumn("category", CollectorColumnType.Varchar), + /* postmaster / sighup / superuser / user — whether changing this needs a RESTART, a reload, or + nothing. Half of what an operator wants to know the moment they decide to change something. */ + new CollectorColumn("context", CollectorColumnType.Varchar), + new CollectorColumn("vartype", CollectorColumnType.Varchar), + /* The discriminator between a server setting and this connection's own state. Without it every + read of this table is a guess about which rows are real. */ + new CollectorColumn("source", CollectorColumnType.Varchar), + new CollectorColumn("boot_val", CollectorColumnType.Varchar), + new CollectorColumn("reset_val", CollectorColumnType.Varchar), + new CollectorColumn("sourcefile", CollectorColumnType.Varchar), + new CollectorColumn("sourceline", CollectorColumnType.Integer), + /* The file and the running server disagree, and nothing else says so. */ + new CollectorColumn("pending_restart", CollectorColumnType.Boolean), + new CollectorColumn("short_desc", CollectorColumnType.Varchar), + }; + + public override async ValueTask> ReadAsync(DbDataReader reader, CollectorContext context, CancellationToken cancellationToken) + { + var rows = new List(); + + while (await reader.ReadAsync(cancellationToken)) + { + rows.Add(new Row( + Name: reader.GetString(0), + Setting: reader.IsDBNull(1) ? null : reader.GetString(1), + Unit: reader.IsDBNull(2) ? null : reader.GetString(2), + Category: reader.IsDBNull(3) ? null : reader.GetString(3), + Context: reader.IsDBNull(4) ? null : reader.GetString(4), + VarType: reader.IsDBNull(5) ? null : reader.GetString(5), + Source: reader.IsDBNull(6) ? null : reader.GetString(6), + BootValue: reader.IsDBNull(7) ? null : reader.GetString(7), + ResetValue: reader.IsDBNull(8) ? null : reader.GetString(8), + SourceFile: reader.IsDBNull(9) ? null : reader.GetString(9), + SourceLine: reader.IsDBNull(10) ? 0 : reader.GetInt32(10), + PendingRestart: !reader.IsDBNull(11) && reader.GetBoolean(11), + ShortDescription: reader.IsDBNull(12) ? null : reader.GetString(12))); + } + + return rows; + } + + public override void WritePayload(Row row, ICollectorRowWriter writer, CollectorContext context) + { + /* No deltas. A setting is a LEVEL, and the interesting derivative — that it changed — is a + comparison between two snapshots of the level, which the changes read does by looking at + consecutive rows. A delta calculator here would have nothing to subtract: these are strings. */ + writer + .Value(row.Name) + .Value(row.Setting) + .Value(row.Unit) + .Value(row.Category) + .Value(row.Context) + .Value(row.VarType) + .Value(row.Source) + .Value(row.BootValue) + .Value(row.ResetValue) + .Value(row.SourceFile) + .Value(row.SourceLine) + .Value(row.PendingRestart) + .Value(row.ShortDescription); + } +}