From 8bbf10e5082cdbae7a375f07940751abfa3dee57 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:00:37 -0400 Subject: [PATCH] Extend #2490's synthetic-slug convention to the spots it did not reach #2490 pinned fixture HOSTNAMES to invented placeholder slugs, and FleetIdentifierScrubTests matches only the full hostname shape, so two other kinds of reference to the same fleet stayed unpinned: - a display_name sitting beside an already-synthetic hostname on the same DarlingPeerDisclosureTests fixture row, plus the Assert.Equal that pins it - comments naming a server by its bare short name rather than a full hostname, across PerformanceMonitor.Collectors, the Darling service and Darling.Tests Thirteen locations, 14 occurrences, all now `omega` -- a slug that named no other server in any of the affected files, so no two servers collapse into one identity. Every measurement in the edited comments is unchanged: the 83 digit runs across those 13 lines hash identically before and after, and the lines differ only in the name token. Also adds pgmonitor to the allowlist. It is a role word like monitor, which is already on the list, so a legitimate reference to that host would otherwise trip the guard. Nothing depends on it today. No schema change, no behaviour change. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 6 ++++-- Darling/Darling.Tests/DarlingPeerDisclosureTests.cs | 4 ++-- Darling/Darling.Tests/FleetIdentifierScrubTests.cs | 5 +++-- Darling/Darling.Tests/QueryStorePlanFetchTests.cs | 4 ++-- Darling/Darling.Tests/QueryStorePlanSizeLearnTests.cs | 2 +- Darling/Darling.Tests/StatementSplitTimingTests.cs | 2 +- Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs | 2 +- PerformanceMonitor.Collectors/CollectorContext.cs | 2 +- PerformanceMonitor.Collectors/QueryStoreCollector.cs | 4 ++-- PerformanceMonitor.Collectors/QueryStorePlanXmlState.cs | 2 +- 10 files changed, 18 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bee4b01ae..cd4b7cf574 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **PostgreSQL wraparound Warning signals autovacuum losing the race, not routine proximity** ([#2693], closes [#2689]) - the Warning arm fired at 0.9x `autovacuum_freeze_max_age` unconditionally, and that is a ROUTINE operating point every healthy database climbs to and resets on every freeze cycle - one fleet database re-fired the identical alert every ~5-minute cycle at 9% of the way to actual wraparound, with the read tool's own severity reading `ok`. The collector already reports `freezing_is_keeping_up` (has the age come down from its own recent peak) but the alert never consulted it. The relative Warning arm now sits AT the setting (1.0x, the crossing point where autovacuum itself forces a vacuum) and is gated by FreezingIsKeepingUp per counter - resetting every cycle stays silent, stuck at or above it warns. Also adds an absolute early-Warning arm at 50% of the true 2^31 ceiling, mirroring the existing Critical ceiling arm, so a cluster tuned high enough that the relative arm is unreachable still gets an early signal. Critical arms (2x setting, 74.5% of ceiling) are unchanged. - **`pg_deadlocks` now works on Aurora and RDS** ([#2692]) - it was failing PERMISSIONS on every managed PostgreSQL target, 100% of the pgmon fleet, with no grant able to fix it: the collector's own doc comment already described a two-transport design where the route is "chosen at dispatch", and PgDeadlockLogParser already carried an Extract(rawLogText) entry point built for it, but the actual dispatch table never grew the Aurora/RDS branch `pg_plan_capture` has ([#2538]) - so every managed target fell through to the `pg_read_file` route, which Aurora cannot serve regardless of grants (no real local log directory, `pg_read_server_files` not grantable). Adds `RdsDeadlockIngestor`, mirroring `RdsPlanIngestor` exactly, with its OWN `RdsLogSource` instance - sharing plan capture's would consume the other's resume marker and starve whichever ingestor ran second in a cycle. The existing `RdsLogUnavailableException` handling at the cycle level needed no changes: it already degrades an authorization refusal to PERMISSIONS for any collector by exception type. - **`pg_statement_stats` stops re-shipping idle statements** ([#2691]) - `pg_stat_statements` has no server-side “changed since I last looked” filter, so at the 1-minute cadence every PostgreSQL collector runs, the collector re-wrote its ENTIRE tracked catalog every cycle whether or not a statement ran - across a 50-cluster fleet this drove `pg_statement_stats` to 176.9 GB of a 180 GB store within 36 hours of onboarding, +126 GB in a single day. A row now ships only when its calls delta is a CONFIRMED zero-activity interval's opposite: `CollectorDeltaCalculator` reports interval 0 on a first sighting, a counter reset, or a gap re-baseline, and all three are genuinely new information, so all three still ship; only a real interval with zero new calls - meaning the statement demonstrably did not run, and `pg_stat_statements` never advances any other counter without a call - is dropped. The decision moves to `ReadAsync` because the write loop starts a row's binary-import before `WritePayload` runs, so by the time a collector could refuse to write, the row already exists; the other two delta series stay unconditional on every row, including skipped ones, so an idle stretch cannot leave their baselines stale for a later real gap to falsely reset. No TOP-N cap - the measured driver was idle repeats, not a large active statement population. -- **query_store retires the runaway detector for a flat candidate-plan ceiling** ([#2690]) - [#2683]/[#2685] detected a store whose plan population churns faster than any pass can drain by watching for 24 consecutive count-clamped passes, then dropping the per-pass ceiling from 2048 to 512 with hysteresis. The 2026-08-29 peak verification showed this fail on the store it was built for: AYR logged ZERO clamped passes and ZERO runaway arms while plan_fetch still ran 38-73s across twelve straight passes, because the learned average happened to keep "wanted" just under the old 2048 ceiling, so the streak never advanced and the throttle never engaged. MaxCandidatePlans is now a flat 512 - always in effect, so this failure mode cannot occur - and the streak/hysteresis machinery is retired. The accepted trade: databases with the smallest measured plan sizes converge over more cycles instead of fewer, acceptable for query_store specifically because it serves historical analysis rather than in-the-moment troubleshooting. +- **query_store retires the runaway detector for a flat candidate-plan ceiling** ([#2690]) - [#2683]/[#2685] detected a store whose plan population churns faster than any pass can drain by watching for 24 consecutive count-clamped passes, then dropping the per-pass ceiling from 2048 to 512 with hysteresis. The 2026-08-29 peak verification showed this fail on the store it was built for: OMEGA logged ZERO clamped passes and ZERO runaway arms while plan_fetch still ran 38-73s across twelve straight passes, because the learned average happened to keep "wanted" just under the old 2048 ceiling, so the streak never advanced and the throttle never engaged. MaxCandidatePlans is now a flat 512 - always in effect, so this failure mode cannot occur - and the streak/hysteresis machinery is retired. The accepted trade: databases with the smallest measured plan sizes converge over more cycles instead of fewer, acceptable for query_store specifically because it serves historical analysis rather than in-the-moment troubleshooting. - **`plan_correction` binds on compatibility-level-100 databases** ([#2688]) - the recommendation-JSON shred used `TRY_CONVERT`, which is not a recognised built-in below database COMPATIBILITY_LEVEL 110 and raises Msg 195. plan_correction runs per-database through `[db].sys.sp_executesql`, so it executes in the TARGET database's compat context, and a compat-100 database on a 2017+ instance is legal - so the whole read threw for such a database. The doc comment's compat-100 reasoning had covered JSON_VALUE-over-OPENJSON but missed this. All eleven numeric extractions now use `TRY_CAST`, which binds at compat 100 (verified on the SQL 2025 rig); output is unchanged. House rule: always `TRY_CAST` over `TRY_CONVERT` unless a CONVERT style code is actually needed. - **`plan_correction` stages its recommendations before touching Query Store** ([#2687]) - the per-database read joined `sys.query_store_query`, `sys.query_store_query_text` and `sys.query_store_plan` straight off `sys.dm_db_tuning_recommendations`, a DMV with no statistics, so on a bloated Query Store the optimizer guessed its cardinality high and satisfied those joins by SCANNING the whole plan store - the 260s tail [#2673] could only backstop with a wall-clock budget. The recommendation set is invariably small (the engine keeps a handful and erases them on restart), so it is now materialised into a `#temp` with its details JSON already shredded, and the Query Store views are joined off that - a real row count the optimizer turns into a nested-loop SEEK per `query_id`/`plan_id`. The sp_QuickieStore rule again (see [#2680]): never naively join a Query Store view you can seek. Output is unchanged - the same 38 columns in the same order, the enablement-only row still survives a zero-recommendation database, and the joins stay LEFT so an aged-out plan cannot drop a live recommendation - verified against a real Query Store on SQL 2025. The seek-versus-scan flip is the DMV-cardinality mechanism, not a rig-measured delta, so the [#2686] per-item wall-clock budget stays as the backstop and `collector_cost` will show the real before/after. - **The live Query Store fetch stops joining a view it never reads** ([#2680]) - the per-item query joined `sys.query_store_query_text` even on the Darling path, where the text column is nulled and the text is pulled separately by-ids, so the join produced one probe per row and fed nothing. `query_store` is already the fleet's single heaviest collector - the #2674 self-metric now says so in numbers - and this is the sp_QuickieStore rule applied literally: don't join a Query Store view you don't consume. It is gated on the same flag that nulls the column, so Lite, which reads the text inline, keeps the join. Measured -12% (398ms -> 351ms) on a 40,388-plan catalog on the rig, provably non-filtering because `query_text_id` keys exactly one text row so the emitted rows are unchanged, and the join-less shape was run against a real Query Store to confirm identical output. @@ -101,6 +101,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **The web dashboard's server page gets the viewer's tabs: twelve sections, 61 of the 82 served reads, and a time range** ([#2475]) - the service dispatched **82** reads at `GET /api/read/{name}` and the built-in pages reached **23** of them; `#/server/{name}` was one scroll of seven panels against the desktop viewer's **65** `TabItem`s. The gap was never backend work - `panels.js` has been a generic renderer over those reads since #1562 - so this is descriptors over the unchanged `renderPanel` seam: twelve sub-tabs (Overview, Wait Stats, CPU, Memory, Blocking, File I/O, Queries, Configuration, Config Changes, Activity, System Events, Collection Health) carrying ~70 panels, reaching **61** reads. The header carries the WHY beneath the band badge - `Warning` has three unrelated causes (a real metric breach, a server awaiting its first collection, a collector error), so a badge reading "Warning" with no way to ask why is [#2422] rebuilt on a new surface; the fleet's own reason string and the fleet card's severity chips are rendered there, the chips through `fleet.js`'s own `metricBands` so there is one implementation rather than two, and neither is re-derived in the browser (R1). Every web grid that renders query text now puts it immediately right of its time/identity anchor ([#1949]) and the pin that enforced that on one array enforces it on all seven. The tab id rides in the hash (`#/server/{name}/{tab}`) so a section is deep-linkable and survives the 60s refresh, and an unknown or absent id resolves to Overview, which is what keeps every existing `#/server/{name}` link working. A page-level range picker (1h/4h/12h/24h/7d/30d) is the twin of the viewer's toolbar presets - **not** persisted, because a page that reopens on a 30-day window is slow for a reason the reader cannot see; panels whose read takes no window at all say "latest snapshot" rather than inheriting a label that would misdescribe them. Two reads were previously **unreachable from a browser** because they require a parameter no UI collected - `get_wait_trend` needs a `wait_type` and `get_perfmon_trend` a `counter_name` - and both now have a picker, the wait one seeded from the rows of the table directly above it so the picker and the table cannot disagree. **No fifth viz kind was added**: the property-grid shape that tempted one is served by `stat` (the reads returning a flat object) and `table` (the ones already returning rows), and a fifth kind that lived only in `panels.js` would be a page-only special case, while doing it properly means `KnownVizList`, `derive.js` and an editor config arm - composer surface this change does not need. What the browser genuinely cannot do is **stated in the tab where a reader goes looking for it** rather than left as a page that quietly lacks a feature: plan analysis, the query heatmap, cached-plan retrieval and actual-plan re-execution need a plan renderer and a command back to the monitored server, and the block-chain view and interactive deadlock graph need a graph viewer - the Blocking tab hands over the captured blocked-process-report and deadlock-graph XML verbatim instead of pretending. Every data panel supplies its own empty-state sentence and both helpers THROW without one. vizTable's generic "No rows in this window" reads as a fault on a collector that is off, opt-in, or daily; and the chart case was worse - `get_blocking_trend` and `get_deadlock_trend` answer an IDLE server with `trend: []` and no `{status,message}` envelope at all, so a perfectly healthy server was told its blocking chart did not have "enough data points to chart yet". `vizLine` now renders a descriptor's `emptyText` at exactly ZERO rows and still falls through at one (where the chart's own sentence is the true one) and when no `emptyText` was authored, so every stored view predating this is unchanged. A read feeding several panels on one tab is fetched ONCE (`fanout`) rather than per descriptor - `readTool` has no cache, so `get_collection_health`, which rolls up seven days of collector logs and computes sweep pressure, was running three times to open its own tab; six such duplicates existed across five tabs and a pin now refuses a seventh. Guarded by an invariant rather than by spot-checks: every read name the module mentions must exist in the shipped dispatch, every parameter key must be one its read actually binds (an unknown query key is silently ignored, so `limit` sent to a read binding `top` quietly returns the default), every viz must be in the shipped vocabulary, and no `get_pg_*` read may appear until the fleet payload can tell a PostgreSQL target from a SQL Server one - it carries `engine_edition`, not a `CollectorTargetEngine`, so a PostgreSQL panel today would render on all 42 SQL Servers, permanently empty. ### Changed +- **Fixture display names and short-name comment references now use synthetic slugs too** ([#2490]) - [#2490] pinned fixture HOSTNAMES to invented placeholder slugs, but `FleetIdentifierScrubTests` matches only the full hostname shape, so two other kinds of reference went unpinned: a `display_name` sitting beside an already-synthetic hostname on the same fixture row, and comments naming a server by its short name rather than a full hostname. Thirteen locations across `PerformanceMonitor.Collectors`, the Darling service and `Darling.Tests` now read `omega`, a slug that named no other server in any of those files; every measurement in those comments is unchanged. `pgmonitor` joins the allowlist as well - a role word like `monitor`, so a legitimate reference to that host cannot trip the guard. - **Darling's slice-repair collapse keeps staging the FULL projection, because on PostgreSQL the width is what makes the plan safe** ([#2876]) - the sibling question raised by [#2771], which rewrote Lite's collapse after pushing `query_plan_text` through a per-group aggregate raised a hard `Out of Memory Error` at 31,426 split intervals. Measured on PostgreSQL 18 rather than assumed to differ, at the same 31,426 groups and again at 51,426, carrying 3,033 MB of decompressed plan text. Three independent reasons it does not transfer: PostgreSQL SPILLS where DuckDB dies - the planner picks `GroupAggregate` over an external merge sort (4.3 s, 125 MB temp, negligible resident), and forced onto the `HashAggregate` that is Lite's failure shape it still bounded itself at 45 batches / 1.1 GB peak and completed; TOAST keeps the wide values out of the sort payload, so 3,033 MB of logical plan text spilled as 125 MB because the tuples carry out-of-line pointers that `min()` detoasts lazily; and the ~50 transition states per group are what make `GroupAggregate` win on cost, where a cut-down three-column form of the same query plans as an UNSPILLED `HashAggregate` at 1.6 GB. So narrowing this projection toward Lite's shape would move it TOWARD the risk. Independently, the shipped survey over the whole production hypertable returns zero split groups - [#1907] stopped the collector emitting them and the historical ones have aged past raw's 4-day edge. Pinned over the collector's own `PayloadColumns` so the narrowing cannot be made quietly, and the reasoning recorded beside `BuildCollapseSql` so the question is not re-opened. - **Lite's server-scoped watermark read stays UNBOUNDED, and the asymmetry with its per-database twin is now documented as deliberate** ([#2800]) - filed as the Lite half of [#2795], on the reasonable read that `GetLastCollectedTimeForDatabaseAsync` taking a `collectedSince` bound while `GetLastCollectedTimeAsync` does not is the same oversight [#2344] left on the Postgres side. Measured against DuckDB rather than inherited from Darling's numbers, it is not. **The failure [#2795] fixed cannot occur here**: its mechanism was Npgsql's undocumented 30 s default `CommandTimeout` cancelling a 40,743-50,560 ms read, the cancellation being swallowed, and the resulting null being indistinguishable from a first run - silently downgrading the collector to its fallback window. `DuckDBCommand.CommandTimeout` defaults to **0**, meaning no limit, so there is no ceiling to exceed and nothing to cancel. **And the bound does not pay.** Measured on DuckDB 1.5.5 (the version Lite ships) against this table's shipped generated DDL at 50M rows / 4.7 GB - far past any realistic Lite store - with the connection already open so the figure is query cost rather than connect cost: with ONE monitored server, the common Lite deployment where `server_id` selects everything, unbounded is **0.34 ms** and bounded is **0.56 ms**, a 65% LOSS, because DuckDB answers the unfiltered `MAX` from column zonemap metadata and the `collection_time` predicate forces real evaluation instead. With five servers the bound is a genuine 9.8x (8.26 ms to 0.84 ms) but saves ~7 ms once per five-minute cycle. **The per-database twin's bound is earned and stays**: 8.40 ms to 0.79 ms at one server and 10.42 ms to 0.93 ms at five, roughly 10x in BOTH shapes, because `database_name` is not in `idx_query_store_time(server_id, collection_time)` so that `MAX` genuinely scans and genuinely prunes. The two reads have different cost structures in a columnar engine; in Postgres both scanned every chunk, which is why the shapes match there and diverge here. Verified across 1M-50M rows, narrow and realistic payload widths (identical, as expected when the query reads three narrow columns), and correlated vs scattered `last_execution_time` - the conclusion holds on all of them. No behaviour change: the method's doc comment now carries the measurement and names `CommandTimeout`'s default as the load-bearing assumption to re-check. - **The SQL-Agent collectors no longer gate on msdb access, so running the GRANT we advise actually does something** ([#2559]) - `HasMsdbAccess` is a **grant**, not an engine capability, but it was probed once at connect and cached for the connection's life. Three collectors gated dispatch on it, so the sequence a user actually follows was broken end to end: read our advice, run the `GRANT`, and nothing happens until the service restarts, with nothing to indicate why. `running_jobs`, `job_history` and `agent_status` now attempt regardless and fail into `PERMISSIONS`, which is a first-class outcome here rather than a defect - error **916 is already in `SqlServerPermissionErrors`**, so the run is classified as a permission denial and never as an ERROR, and `CollectorHealthClassifier` bands a collector that has only ever been denied as `NO_PERMISSIONS`, a check that runs **before** FAILING and STALE. So a server that will never have the grant does not read as broken, raises no alert, and the grant now takes effect on the next cycle instead of the next reconnect. The cost this trades for is three fast-failing statements per cycle on such a server - a compile-time permission check with no execution behind it. The fleet number that made the call: across both stores over three hours, **all 84 servers already dispatch all three collectors with zero non-SUCCESS runs**, so `HasMsdbAccess` never fires here and removing it changes nothing on this fleet while fixing the journey for anyone it does fire on. `HasMsdbAccess` is kept and still probed - it is honest information for a connection surface - it just no longer decides dispatch. One pin was **deleted rather than left passing**: the PostgreSQL-leak guard asserted no SQL Server gate reads `HasMsdbAccess`, which is now true for a reason that has nothing to do with the filter it was written to test, so it could no longer go red and was reading as coverage it did not provide. @@ -153,7 +154,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **A wall-clock-budget-abandoned cycle no longer records as `SUCCESS`** ([#2801]) - a collector cycle the #2673 whole-server budget gives up on stores nothing and advances no watermark, but reached the log site by RETURNING rather than throwing, so it took the ordinary path and inherited that path's hardcoded `"SUCCESS"` in both hosts. Every one of the 36 abandonments in the use1 store's 17-day retention was `SUCCESS` with `rows_collected = 0`. Beyond the wrong label it claimed a collection that never happened: `ReadCollectionSignalsAsync` takes `last_success` and `recent_success` from `status IN ('SUCCESS', 'SKIPPED')`, so a collector abandoning every cycle read as perpetually fresh, and its message landed in the #1837 note channel whose whole claim is that the run succeeded. Now its own `ABANDONED` status, shared by both hosts through `EnumeratedCollectorDriver.ClassifyReturnedRun`. Not `ERROR` (that would page on a guard doing its job), not `YIELDED` (documented as the 1s `LOCK_TIMEOUT` guard and read as target lock contention), not `SKIPPED` (a healthy no-op that counts as success). Safe to add because every read buckets by explicit list rather than by complement, and `collection_log.status` carries no CHECK constraint, so no migration rung. The underlying slowness is NOT new - the affected server had 2-11 `procedure_stats` runs over 60s every day of the retention window, with its worst maxima (246-278s) predating #2673. - **Web dashboard: a pinned notebook panel cell showed no "Pinned" badge in the read view** ([#2788]) - the badge read only the nested #2735 `range` pin, so a cell pinned with the composer's per-panel "Time range" dropdown (a top-level `hours` override) rendered its pinned window but wore no badge, and the scope bar silently claimed a different range for that panel. The badge and the applied window now resolve through one shared `effectivePin`, so both pin shapes badge identically and the badge can never disagree with the window the panel actually renders. - **The server-scoped watermark read now carries #2344's chunk-pruning bound** ([#2795]) - `DarlingCollectorRunner.GetLastCollectedTimeAsync` ran `SELECT MAX() ... WHERE server_id = $1` with no predicate on `collection_time`, the partitioning column, so it scanned every chunk in retention for that server on every cycle. #2344 fixed exactly this, but only in the per-database sibling `GetLastCollectedTimeForDatabaseAsync`; query_store declares both `WatermarkColumn` and `PerDatabaseWatermarkColumn`, so it kept paying the unbounded cost here. Measured on a 62.5 GB `query_store_stats`: 40.7 s and 50.6 s cold against Npgsql's 30 s default `CommandTimeout`, so the read was cancelled mid-flight - the store's own log recorded 2,092 such cancellations in one day while the bounded sibling recorded 17. Each one returned null, which is indistinguishable from a first run, so the collector fell back to query_store's 60-minute window instead of the ~5-minute incremental one and re-collected what it already held. Bounded at `WatermarkPolicy.ReadFloor` the same read is 2.9-5.9 s. The timeout is now set explicitly rather than inherited, and a failed watermark read is logged instead of silently swallowed. -- **query_store plan and text fetch: force HASH JOIN so the in-memory Query Store TVF is read once** ([#2791]) - `sys.query_store_plan` unions the on-disk table with the TVF `QUERY_STORE_PLAN_IN_MEM`, and the optimizer has no statistics for it: on AYR it estimated 1,000 rows against 14,633 actual (1,463% off). That guess put the TVF on the inner side of a Nested Loops join, re-executed once per candidate plan_id up to the 512 `MaxCandidatePlans` cap, each execution scanning the whole in-memory Query Store before any plan was decompressed. sp_QuickieStore put these statements at **55,000-61,000ms CPU each**, top of the whole instance, while the `TOP (50000)` runtime-stats payload this was long assumed to be averaged ~2s. `OPTION(RECOMPILE, HASH JOIN)` reads the TVF once: **~60,000ms CPU to 0.508s** measured on AYR. The same shape and the same fix apply to the text fetch, which joins two TVF-backed views directly. Rowsets verified byte-identical with and without the hint at 16/128/512 candidates under the collector's own ARITHABORT OFF. +- **query_store plan and text fetch: force HASH JOIN so the in-memory Query Store TVF is read once** ([#2791]) - `sys.query_store_plan` unions the on-disk table with the TVF `QUERY_STORE_PLAN_IN_MEM`, and the optimizer has no statistics for it: on OMEGA it estimated 1,000 rows against 14,633 actual (1,463% off). That guess put the TVF on the inner side of a Nested Loops join, re-executed once per candidate plan_id up to the 512 `MaxCandidatePlans` cap, each execution scanning the whole in-memory Query Store before any plan was decompressed. sp_QuickieStore put these statements at **55,000-61,000ms CPU each**, top of the whole instance, while the `TOP (50000)` runtime-stats payload this was long assumed to be averaged ~2s. `OPTION(RECOMPILE, HASH JOIN)` reads the TVF once: **~60,000ms CPU to 0.508s** measured on OMEGA. The same shape and the same fix apply to the text fetch, which joins two TVF-backed views directly. Rowsets verified byte-identical with and without the hint at 16/128/512 candidates under the collector's own ARITHABORT OFF. - **Web dashboard: the composer's partial-window notice read "1 days" on a one-day store** ([#2785]) - the day count in BuildRetentionNotice was interpolated with a hardcoded "days" plural, so a store reaching back a single day read "reaches back about 1 days, but the requested window starts 1 days back". It now pluralises on the rendered number - "1" singular, "1.5"/"30" plural. - **PostgreSQL statement text was never stored on ANY Aurora server, because the Aurora fetch does not deduplicate `queryid`** ([#2786]) - `aurora_stat_statements` keys on `(queryid, userid, dbid, toplevel)` exactly as `pg_stat_statements` does, so one `queryid` returns once per user and database that ran it, and the `(server_id, queryid)` upsert meets those duplicates as `21000: ON CONFLICT DO UPDATE command cannot affect row a second time`. That aborts the STATEMENT, so the batch's non-duplicated rows are lost with it - one repeated id stores nothing at all - and it is deterministic, so nothing self-heals. Measured on the monitoring fleet before the fix: 50 of 50 Aurora servers failing, ZERO succeeding, and `get_pg_top_queries` returning `query_text: null` on every row of every server. [#2651] met this exact failure while adding the vanilla arm and fixed it THERE; this arm predates it ([#2284]) and was only renamed, so the defect survived behind a green suite - the pin it added named the arm it was written for. Both pins now run over EVERY arm `FetchSqlFor` can return, which is exhaustive rather than enumerated, and the shared write path deduplicates as a backstop. The cadence guard made it worse rather than containing it: `IsDueSql` COALESCEs a missing answer to TRUE, which is correct for a first fetch and indistinguishable from a server whose every write has failed, so a broken server re-fetched on every sweep instead of hourly - ~12,000 `showtext` calls a day against 50 production Aurora clusters, to store nothing. - **Web dashboard: an offline server no longer shows a green "Collectors OK"** ([#2779]) - the summary Collectors chip keyed its verdict on the FAILING count alone, and a stale collector counts as neither healthy nor failing, so a server that had stopped collecting kept a green "Collectors OK - N healthy - 0 failing" even while its own header read "no recent collection" and its Collection Health tab showed every collector STALE. The chip now reuses the reachability signal the card already carries (is_online, the same one that bands the card Offline): an offline server reads "Stale - no recent collection" in the neutral tone instead. One shared metricBands builder, so the fleet cards and the per-server detail header are both fixed at once - and no new stale-count threshold to over-fire on normally-infrequent collectors. @@ -3223,3 +3224,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#2876]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2876 [#2797]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2797 [#2860]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2860 +[#2490]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2490 diff --git a/Darling/Darling.Tests/DarlingPeerDisclosureTests.cs b/Darling/Darling.Tests/DarlingPeerDisclosureTests.cs index e7dc319964..5c1c21818a 100644 --- a/Darling/Darling.Tests/DarlingPeerDisclosureTests.cs +++ b/Darling/Darling.Tests/DarlingPeerDisclosureTests.cs @@ -382,7 +382,7 @@ private static JsonElement RenderedServerList(DarlingPeerDirectory.Snapshot peer { var rows = new List { - new(1, "prod-sql-use1-beta-01", "ayr", 16, new DateTime(2026, 8, 19, 12, 0, 0, DateTimeKind.Utc)), + new(1, "prod-sql-use1-beta-01", "omega", 16, new DateTime(2026, 8, 19, 12, 0, 0, DateTimeKind.Utc)), }; return JsonDocument @@ -410,7 +410,7 @@ public void ListServers_CarriesThePeerFleetsSummary() /* The existing payload is untouched — the disclosure is additive here too. */ var server = Assert.Single(root.GetProperty("servers").EnumerateArray()); Assert.Equal("prod-sql-use1-beta-01", server.GetProperty("server_name").GetString()); - Assert.Equal("ayr", server.GetProperty("display_name").GetString()); + Assert.Equal("omega", server.GetProperty("display_name").GetString()); } [Fact] diff --git a/Darling/Darling.Tests/FleetIdentifierScrubTests.cs b/Darling/Darling.Tests/FleetIdentifierScrubTests.cs index c64ab04de9..ed7ce2d259 100644 --- a/Darling/Darling.Tests/FleetIdentifierScrubTests.cs +++ b/Darling/Darling.Tests/FleetIdentifierScrubTests.cs @@ -25,11 +25,12 @@ spelled in capitals. */ public sealed class FleetIdentifierScrubTests { - /* Greek letters and role words -- nothing here names a real customer. */ + /* Greek letters and role words -- nothing here names a real customer. pgmonitor is a role + word too: it names one of the maintainer's own monitoring hosts, the same as monitor. */ private static readonly HashSet SyntheticSlugs = new(StringComparer.OrdinalIgnoreCase) { "alpha", "beta", "gamma", "delta", "epsilon", "zeta", "omega", - "monitor", "multi", "primary", "replica", "secondary", "reporting", + "monitor", "pgmonitor", "multi", "primary", "replica", "secondary", "reporting", "test", "sample", "example", "demo", "fake", "dummy", "placeholder", }; diff --git a/Darling/Darling.Tests/QueryStorePlanFetchTests.cs b/Darling/Darling.Tests/QueryStorePlanFetchTests.cs index cafb47a524..233b688860 100644 --- a/Darling/Darling.Tests/QueryStorePlanFetchTests.cs +++ b/Darling/Darling.Tests/QueryStorePlanFetchTests.cs @@ -309,7 +309,7 @@ public void CandidatePlanCount_SitsJustPastTheBudget_AtEveryMeasuredFleetPlanSiz /// /// The smallest measured fleet quartile (15 KB) wants ~1229 plans at a 12 MB budget — past the flat /// 512 ceiling (#2683/#2685's adaptive runaway detector was retired in favor of this: it failed to - /// engage during the 2026-08-29 AYR peak precisely because "wanted" stayed just under the old 2048 + /// engage during the 2026-08-29 OMEGA peak precisely because "wanted" stayed just under the old 2048 /// ceiling, so the throttle never armed). A flat ceiling applies unconditionally, so this database-shape /// now converges over more cycles instead of fewer — the accepted trade for query_store, which serves /// historical analysis rather than in-the-moment troubleshooting. @@ -438,7 +438,7 @@ private static string LiveSql(CollectorContext context) => /* --------------------------------------------------------------------------------------------- #2791: the fetch statements read TVF-backed Query Store views, for which the optimizer has no - statistics and uses a fixed guess (1,000 estimated against 14,633 actual on AYR, 1,463% off). + statistics and uses a fixed guess (1,000 estimated against 14,633 actual on OMEGA, 1,463% off). That guess put QUERY_STORE_PLAN_IN_MEM on the INNER side of a Nested Loops join, re-executed once per candidate id up to MaxCandidatePlans = 512, at 55,000-61,000ms CPU per fetch. The query-level hint is the only lever - the joins live inside the view definition. diff --git a/Darling/Darling.Tests/QueryStorePlanSizeLearnTests.cs b/Darling/Darling.Tests/QueryStorePlanSizeLearnTests.cs index 5b35bcf895..fec28fe551 100644 --- a/Darling/Darling.Tests/QueryStorePlanSizeLearnTests.cs +++ b/Darling/Darling.Tests/QueryStorePlanSizeLearnTests.cs @@ -121,7 +121,7 @@ public void TheDefaultEstimateMeansNeverLearned() /// /// #2683/#2685 tried an adaptive runaway detector here (a streak of clamped passes armed a reduced /// ceiling, with hysteresis to survive oscillation) and it failed at the moment it mattered: the - /// 2026-08-29 peak verification on AYR showed zero clamped passes and zero runaway arms while + /// 2026-08-29 peak verification on OMEGA showed zero clamped passes and zero runaway arms while /// plan_fetch still ran 38-73s across twelve straight passes, because the learned average happened to /// keep "wanted" just under the ceiling. MaxCandidatePlans is now a flat 512 instead, so the throttle /// applies unconditionally and cannot fail to engage. This pins that the ceiling used by diff --git a/Darling/Darling.Tests/StatementSplitTimingTests.cs b/Darling/Darling.Tests/StatementSplitTimingTests.cs index 5f4f31fd67..5ecc98506c 100644 --- a/Darling/Darling.Tests/StatementSplitTimingTests.cs +++ b/Darling/Darling.Tests/StatementSplitTimingTests.cs @@ -76,7 +76,7 @@ public void EveryPhaseAccountedFor_ThePartsNeverExceedTheWhole() /// /// #2312: the separate plan-XML and text fetches run INSIDE the driver's sql: stopwatch but are - /// their own queries against the Query Store catalogs — on ayr-01 a 0-row closed-only cycle still cost + /// their own queries against the Query Store catalogs — on omega-01 a 0-row closed-only cycle still cost /// 298s and the blended number could not say where. They must come out of drain exactly like the /// watermark phase, or drain silently absorbs the one cost this investigation needs isolated. /// diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs index dbfe72d469..b329b1c8a6 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs @@ -4905,7 +4905,7 @@ surfacing as an unobserved task exception. */ /* #2717: plan_correction gets the identical treatment for the identical reason. Its own SQL is already correctly seek-based (#2687) and averages ~1 second, but on a server whose Query Store carries the same leaflogix-class distinct-plan-population signature - already root-caused for query_store on multi-03/AYR, it can spike to 20+ seconds — the + already root-caused for query_store on multi-03/OMEGA, it can spike to 20+ seconds — the same bimodal shape, just a smaller worst case. Detached the same way, through the generic DetachedCollectorGate (#2717) rather than query_store's own gate, which has an orthogonal second job (excluding the backfill loop) this collector does not share. diff --git a/PerformanceMonitor.Collectors/CollectorContext.cs b/PerformanceMonitor.Collectors/CollectorContext.cs index af6bf6b755..5a4b3eb7e8 100644 --- a/PerformanceMonitor.Collectors/CollectorContext.cs +++ b/PerformanceMonitor.Collectors/CollectorContext.cs @@ -330,7 +330,7 @@ public sealed class CollectorContext /// runs one (Darling; Lite has no separate fetch and leaves it zero). The fetch is INSIDE the driver's /// per-item sql: stopwatch but is neither open nor drain — it is its own query against /// sys.query_store_plan, and on a database with a huge Query Store catalog it can dominate the - /// whole item (ayr-01: a 0-row closed-only cycle still cost 298s, and the blended number could not say + /// whole item (omega-01: a 0-row closed-only cycle still cost 298s, and the blended number could not say /// where). Measured so drain stops absorbing it, exactly the #2164 argument one seam further down. /// public long PerItemPlanFetchMs { get; set; } diff --git a/PerformanceMonitor.Collectors/QueryStoreCollector.cs b/PerformanceMonitor.Collectors/QueryStoreCollector.cs index 2d0b495b0f..1114bb2833 100644 --- a/PerformanceMonitor.Collectors/QueryStoreCollector.cs +++ b/PerformanceMonitor.Collectors/QueryStoreCollector.cs @@ -1270,7 +1270,7 @@ scope exit regardless. HASH JOIN on the fetch statement (#2791), and it is the whole fix rather than a tuning knob. sys.query_store_plan's view definition unions the on-disk table with the in-memory TVF QUERY_STORE_PLAN_IN_MEM, and the optimizer has NO statistics for that TVF - it uses a fixed guess. - Measured on AYR the guess is 1,000 rows against 14,633 actual, 1,463% off, which is what makes + Measured on OMEGA the guess is 1,000 rows against 14,633 actual, 1,463% off, which is what makes Nested Loops look cheap: the TVF lands on the INNER side and is re-executed once per candidate plan_id, up to MaxCandidatePlans = 512 times, each execution scanning the whole in-memory Query Store before a single plan is decompressed. sp_QuickieStore put this statement at 55,000-61,000ms @@ -1416,7 +1416,7 @@ SET NOCOUNT ON so the SELECT INTO emits no result set. #temp is scoped to this s out in the open: sys.query_store_query and sys.query_store_query_text are BOTH TVF-backed unions over their in-memory halves, driven by an IN list, which is exactly the shape that put the plan fetch on the inner side of a loop 512 times over. The plan fetch is the variant that carries the - AYR measurement; this one is the same defect treated the same way, and that distinction is stated + OMEGA measurement; this one is the same defect treated the same way, and that distinction is stated rather than blurred - the join-strategy change and the identical-rowset property are verified here, the 60s->0.5s number is not this statement's and is not claimed for it. diff --git a/PerformanceMonitor.Collectors/QueryStorePlanXmlState.cs b/PerformanceMonitor.Collectors/QueryStorePlanXmlState.cs index 889c4ffb65..d752082285 100644 --- a/PerformanceMonitor.Collectors/QueryStorePlanXmlState.cs +++ b/PerformanceMonitor.Collectors/QueryStorePlanXmlState.cs @@ -57,7 +57,7 @@ public static class QueryStorePlanXmlState /// issues detected a "runaway" store (one whose plan population churns faster than any pass can drain) /// by watching for 24 CONSECUTIVE passes clamped at a high ceiling (2048), then dropping to a low one /// (512) with hysteresis to survive the estimator's oscillation. Verified against the 2026-08-29 peak - /// window on AYR — the pathological store the detector was built for: the log showed ZERO "candidate cap + /// window on OMEGA — the pathological store the detector was built for: the log showed ZERO "candidate cap /// clamped" lines and ZERO RUNAWAY/Holding lines during a stretch where plan_fetch still ran 38–73s /// across twelve straight passes. The learned average happened to keep "wanted" just under the 2048 /// ceiling, so no pass ever clamped, the streak never advanced, and the throttle that existed