Darling monitors PostgreSQL and Amazon Aurora PostgreSQL - #2213
Merged
Conversation
docs/how-collection-works.md described only the deprecated Full/Dashboard edition (SQL Agent + T-SQL procs) and an out-of-date view of Lite in which the DMV queries lived in RemoteCollectorService partials. Neither matches the code: the 41 collector definitions live in the shared PerformanceMonitor.Collectors library, Lite's partials are thin delegations to those definitions, and Darling was not covered at all. Rewritten around the actual shape - one collection brain, two storage engines - documenting the definition model and its opt-in members, the three registration tables, Darling's sweep loop and error/health derivation, the store's column and partitioning conventions, schedule override precedence, and the three retention mechanisms. Full/Dashboard is now called out as deprecated with pointers to its own docs. Also corrects counts that had drifted from the catalog: - README.md and Darling/README.md said 38 collectors; the catalog and CollectorScheduleDefaults both hold 41 - README.md's collector table was missing database_states (1 min, feeds the database offline/unhealthy alert) - Darling/README.md said schema v29; StorageVersion.SchemaVersion is 59. Points at the generated migration-ladder fixture as the complete schema rather than enumerating every rung, since that list is what drifted - Darling/README.md's background-worker sizing said 41/52 for 39 hypertables; HypertableCount is CollectorCatalog.All + 1 = 42, so the derivation gives 44 and 55 - llms.txt said 32 collectors and named Full + Lite as the two editions Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Groundwork for monitoring PostgreSQL targets. Nothing about the current behaviour changes: both the definition side and the target side default to CollectorTargetEngine.SqlServer, so every one of the 41 definitions and every target the probes classify today gate exactly as before. - CollectorTargetEngine: SqlServer / PostgreSql. Deliberately about SQL dialect, not hosting - Azure SQL DB, Managed Instance, and RDS for SQL Server are all SqlServer, since their differences are already carried as flags on CollectorTargetInfo and they run the same T-SQL. - CollectorTargetInfo.Engine: what the target actually is. - ICollectorSchemaInfo.TargetEngine: what dialect the definition speaks. A default interface implementation rather than a required member, so the two test doubles that implement this interface directly, and all 41 definitions, need no change. - CollectorCatalog.AppliesTo(definition, target): the composed gate - engine match AND the definition's own AppliesTo. The runners now call this instead of AppliesTo directly, which is what makes it impossible to dispatch a T-SQL definition at a non-SQL-Server target. Individual definitions stay free to reason only about hosting flavour and version floors within their own engine, and the 27 existing AppliesTo overrides are untouched. Verified: PerformanceMonitor.Collectors, Darling.Service, Darling.Tests, and Lite all build clean. The test suites target net10.0-windows and cannot be executed on macOS (no WindowsDesktop runtime), so they were compile-verified only and still need a run on Windows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…the runner Two gaps in the engine discriminator, both found by asking what a second engine in one catalog actually does to the sweep. 1. A gated collector was still being logged. The runner's AppliesTo check returns zero rows, and RunOneAsync records that as SUCCESS - fine for the handful of Azure-gated collectors, but with two engines every target would log a fake success per foreign collector per cycle, most of them at a 1-minute cadence. That floods collection_log (60-day retention) and feeds phantom successes to the health bands and analysis, which key on status rather than row count. RunDueCollectorsAsync now drops wrong-engine collectors before the due-check via CollectorCatalog.EngineMatches: no dispatch, no log row, no NextDue churn. This is Darling's equivalent of Lite's pre-dispatch SKIPPED path. 2. TargetEngine defaulting to SqlServer is right for the 41 existing definitions but a silent footgun for the next one: a Postgres definition that forgot the override would advertise itself as T-SQL and be dispatched at SQL Server targets, failing every cycle. PostgresCollectorDefinitionBase seals TargetEngine to PostgreSql, so the dialect is structural rather than a line to remember, and a drift-guard test asserts every catalog definition is SqlServer today and names any that isn't. Tests added in Lite.Tests/CollectorTargetEngineGateTests.cs: the drift guard, the target default, both directions of the cross-engine gate, that the composed gate still honours within-engine gating (agent_status on RDS), and that an unknown name is not filtered. Full solution builds clean. The test projects target net10.0-windows and were compile-verified only - execution still needs Windows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Postgres The collector definitions were already engine-neutral - they return query text plus parameters and read through DbDataReader, and the collectors library has zero PackageReferences. What was missing was the runner: it constructed SqlConnection directly and mapped parameters to SqlDbType, so nothing but SQL Server could ever execute. - ITargetProvider (in the collectors library, System.Data.Common only, so its zero-dependency property holds): CreateConnection, CreateCommand, Classify. - CollectorTargetFault names failures the way the collection loop reasons about them - Permissions, LockTimeoutYield, SessionMissing, ObjectMissing, FeatureDisabled, CommandTimeout, ConnectionFatal - instead of in one engine's error numbers. Unclassified is the default so an unexpected failure stays loud. - SqlServerTargetProvider is a lift of the existing inline code; the parameter mapping and its throw-on-unmapped-type are unchanged, and Classify reproduces the error numbers the existing catch filters use. It deliberately does NOT produce SessionMissing: whether a 297 means "XE session gone" or "permission denied" is collector context, and the worker already raises its own exception type for that case. - PostgresTargetProvider classifies by SQLSTATE, not message text. Every code was observed while probing our Aurora fleet: 42501 from a function needing rds_replication, 42P01 from pg_stat_statements in a database where the view was never created, 0A000 from pg_stat_wal (Aurora blocks it outright), 55-class from a feature that raises rather than returning empty when disabled. Npgsql was already a dependency - it is the store driver - so this needed no new package. DateTime2 maps to timestamp WITHOUT time zone to match the store's naive-UTC convention; timestamptz would make Npgsql reject the DateTimeKind.Unspecified values this product uses everywhere. The two engine-agnostic runner paths now go through the provider. The Azure per-database and master-enumeration paths stay SqlConnection-typed because they are SQL Server features by definition, but they share the one parameter mapping so a type cannot be mapped two ways. Error-classification catch sites are NOT rewired here - that is a separate change where catch ordering has to be reasoned about carefully. Until then a Postgres failure falls through to the generic handler and is recorded as ERROR, which is honest if coarse. Tests: provider resolution for every declared engine, correct driver types, the parameter mapping is total on both engines, null becomes DBNull, wrong-engine connections are rejected, timeouts apply, and the Postgres SQLSTATE table. Full solution builds clean; the test projects target net10.0-windows so they were compile-verified only and still need a run on Windows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes the path from config to a probed target, which is what actually
makes the engine discriminator fire: nothing set CollectorTargetInfo.Engine
to PostgreSql before this, so no Postgres collector could ever dispatch.
- MonitoredServer.engine ("sqlserver" default, or postgres/postgresql/pg/
aurora-postgresql/aurora). This is configuration rather than something
probed because it has to be known BEFORE connecting - it decides which
driver builds the connection string and which detection query runs. An
omitted or misspelled value resolves to SQL Server rather than throwing,
so every darling.json in the field keeps its exact present behaviour and
one bad entry cannot stop the service from starting.
- MonitoredServer.port, for Postgres targets on a non-default port. SQL
Server keeps its host,1433 convention; nothing there changes.
- A Postgres branch in the connection-string builder holding the same
posture as the SQL Server path: 15s connect, 60s command, TLS
fail-closed, and an application name visible in pg_stat_activity. MARS,
ApplicationIntent, and MultiSubnetFailover are absent because the
concepts do not exist - a Postgres read replica is its own endpoint, so
point the entry at the reader's host. TrustServerCertificate maps to
Require rather than disabling TLS, since the case it covers is Aurora
presenting an RDS CA a stock trust store does not know. Integrated auth
is rejected loudly instead of producing a string that cannot
authenticate and failing further from the cause.
- A Postgres detection query built only from surfaces a pg_monitor login
can read, verified against live Aurora 16.11 and 17.7: server_version_num
(a division, not version() text parsing - that formatting has changed
across releases), pg_is_in_recovery(), and a pg_proc lookup for
aurora_version so stock PostgreSQL reads as "not Aurora" instead of
failing the probe.
- Four Postgres facts on CollectorTargetInfo. PostgresMajorVersion gates
the real 16->17 breaks; PostgresVersionNum exists because some gates are
minor-level (aurora_stat_resource_usage needs 16.9+/17.5+ and is absent
on 17.4, so a major-only check would call a function that is not there);
IsAurora gates the proprietary surface, most importantly the cumulative
wait counters core PostgreSQL does not have; IsInRecovery marks a reader,
which on Aurora is a distinct monitoring identity with its own statistics
rather than a shadow of the writer.
The SQL Server-only facts stay at their defaults on a Postgres target
because no Postgres definition consults them, and the engine check keeps
every T-SQL definition away regardless.
Tests: the SQL Server default when engine is absent, every accepted
spelling, typo fallback, the built connection posture, port handling,
the two TLS relaxations, both auth rejections, shared storage identity,
and that no T-SQL leaked into the Postgres detection query.
Full solution builds clean. Test projects target net10.0-windows so they
were compile-verified only and still need a run on Windows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cumulative Aurora wait counters with deltas computed on write - the Postgres counterpart of wait_stats, and the signal Postgres monitoring normally cannot provide at all. Core PostgreSQL has no cumulative wait accounting in any version: only the instantaneous pg_stat_activity.wait_event, with accumulation proposed and rejected twice on overhead grounds. The usual workaround is a sampling extension, and Aurora permits exactly thirteen preloadable libraries, none of which are pg_wait_sampling, pgsentinel, or pg_stat_kcache. Aurora instead exposes aurora_stat_system_waits() as a built-in, so on Aurora it is not the convenient source, it is the only one - hence the gate is IsAurora rather than a version. Signatures in the query are verified against live 16.11 and 17.7, and the AWS reference is wrong about one: aurora_stat_wait_event() returns THREE columns with type_id first, not the four documented. Aliasing it wrong does not error - the join matches nothing and every event name returns NULL, which is how it was found. LEFT JOIN throughout, never the NATURAL JOIN the AWS example shows, because the documented type list omits type 2 and Limitless adds 12; an unnamed wait is still a wait worth recording, and the tests pin that such rows survive. Columns are cast explicitly so reader types are deterministic - Npgsql throws on GetInt64 over an int4 column. wait_time_us carries its unit in the name because the AWS docs contradict themselves (microseconds for system_waits, milliseconds for backend_waits). Settled by measurement instead: read as milliseconds, the observed totals imply tens of thousands of concurrent waiters against a max_connections of 5,000. The name means nobody relitigates it. Deltas key on the numeric event_id, NOT the event name, because wait-event name casing differs between Aurora majors - AutoVacuumMain on 16.11 versus AutovacuumMain on 17.7 - so a name-keyed delta would break its own history across an upgrade, reading as one series ending and another beginning. Same 300-second gap policy as wait_stats. A type-level ignore list drops Activity, Client, and Timeout. Measured on prod, Client:ClientRead alone accumulated 565,758,023 seconds and every Activity event grows at ~1 second per second of uptime forever; left in they are over 99% of the chart. Filtering by type rather than by event name means a new background worker in a future release is excluded automatically. An undecodable type is deliberately kept. Registered in all three tables (catalog, schedule at 1 min / 30 days matching wait_stats, dispatch) plus migration V60 and a StorageVersion bump to 60. Verified with the ladder generator that the generated fresh-store table is column-for-column identical to the hand-written rung, index included - the invariant that otherwise silently breaks the binary COPY on an upgraded store. The catalog-count pins move 41 -> 42, and the engine drift guard is rewritten to key on the pg_ naming convention rather than asserting every definition is SQL Server, so it now catches both directions of mistake. Also fixes generate-ladder-fixture.csproj, which has been unloadable since it was added: its XML comment contained the CLI argument separator, and two consecutive dashes are illegal inside an XML comment. The release-cut tool could not build at all. The frozen v3.3.0 fixture is deliberately NOT regenerated - it represents the previous release, and V60 applying on top of it is exactly what the upgrade test should exercise. Full solution builds clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The viewer/MCP element that ships with the collector, per the one-with-each rule. A separate tool from get_wait_stats rather than a widened one, because the two engines' wait models do not line up: PostgreSQL has a two-level type/event taxonomy where SQL Server has one flat name, has no signal-wait concept at all, and reports microseconds where SQL Server reports milliseconds. Folding them together would mean either lying about a unit or emitting mostly-null columns. DarlingPgWaitReader aggregates the DELTA columns, never the raw cumulative ones - summing cumulative counters across snapshots multiplies the whole history by the snapshot count, which produces a plausible-looking number that is wrong by orders of magnitude, and a test pins it. Microsecond -> millisecond conversion happens once here so no consumer has to remember the stored unit, and the division is float so sub-millisecond events do not collapse to zero. HAVING excludes events that did not move in the window, which otherwise pad the result with every event the instance has ever seen. Unnamed events are surfaced, not filtered. wait_type and wait_event are nullable because their lookups are LEFT JOINed in the collector, and an event Aurora reports but cannot name is exactly the new-wait-type case worth seeing, so it gets a synthetic label from the numeric ids rather than being swallowed by the GROUP BY. The tool reports each event's share of window wait time alongside the absolute figure, since the absolute number alone does not say whether an event is the story or a rounding error. An empty result is explained rather than reported as "no waits": it means either no data, or that the server is not an Aurora PostgreSQL target, and the message says which tool to use instead. Registered in the MCP host's explicit tool-type chain as well as the web endpoint catalog and dispatch tables. Missing the host registration would have left the tool working over HTTP but invisible to every MCP client, which the catalog/endpoint parity test now covers. Full solution builds clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Postgres counterpart of query_stats, reading Aurora's extended
aurora_stat_statements(). Two column groups have no SQL Server equivalent
at all:
- The Aurora I/O source split (storage_blks_read / orcache_blks_hit and
their times) decomposes an otherwise opaque block read into "came from
the distributed storage volume" versus "hit the local NVMe Optimized
Reads tier". This is why a cache-hit ratio computed the community way
is arithmetically misleading on Aurora: a "read" may have been a fast
local hit.
- total_exec_peakmem_bytes / max_exec_peakmem_bytes are the nearest
thing PostgreSQL has to memory-grant data, which core PostgreSQL has
no concept of. Verified populated on prod with no config change.
Per-query wal_bytes is likewise a signal no SQL Server DMV offers.
The query is built per major version rather than SELECT *-ed, because our
fleet spans both and the columns genuinely differ: 16.11 has blk_read_time
/ blk_write_time where 17.7 has shared_blk_read_time /
shared_blk_write_time. A SELECT * would not error on either version - it
would silently shift every ordinal, which is exactly how several
monitoring tools shipped broken PG17 collectors. Casts pin the reader's
types since wal_bytes is numeric and Npgsql's checking is strict.
Delta key is the full (queryid, dbid, userid, toplevel) identity, not
queryid alone: the same normalized statement run by a different user or
against a different database is a separate pg_stat_statements entry with
its own counters, so keying on queryid alone would interleave several
series and produce nonsense. queryid is not stable across major versions,
so a mass reset after an upgrade is expected and the existing
counter-regression handling covers it.
NO query text column, and that was a correction mid-change rather than an
omission. Text belongs in the shared query_text_dim rather than inline -
inline payload was 94% of a 250 GB field store - but registering a new
dim-feeding table cannot be done from a rung this late. V38 is GENERATED
from PayloadDimensions.All, so adding an entry made V38 emit
"ALTER TABLE pg_statement_stats ADD COLUMN query_text_digest", and on an
upgraded store V38 runs long before V61 creates that table: the ALTER
would hit a nonexistent table and fail the whole migration, bricking the
upgrade. Caught by running the ladder generator and diffing its output
against the hand-written rung. Retrofitting a dim-feeding table needs
either an existence-guarded V38 or a rung-aware registry - a design
change, not a drive-by. queryid is the identity meanwhile, which is the
join key anyway, and text is better served by a dedicated low-cadence
collector storing each statement once rather than once per snapshot.
Verified with the generator that the fresh-store emission is
column-for-column identical to V61 (34 columns each) and that no stray
digest ALTER remains. StorageVersion 61, catalog pins 42 -> 43.
Full solution builds clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The viewer/MCP element paired with the collector, per the one-with-each rule. Separate from get_top_queries_by_cpu because the two engines report different things: no signal-wait split, an I/O source breakdown SQL Server has no concept of, and identification by queryid rather than text. The read is deliberately honest about a limitation of the collector's first cut. Only calls, total time and rows have per-interval deltas in the store, so those are SUMmed; the block, WAL and peak-memory columns are cumulative-only and are therefore read as MAX - the latest reading, which is at least a true number - rather than summed across snapshots, which would be arithmetically meaningless. The tool states that distinction in its own output so a caller cannot mistake a cumulative figure for a windowed one, and a test asserts SUM() never appears over a cumulative column, since that is the mistake most likely to be made when someone extends this query. Surfaces Aurora's I/O split as a ratio, not just raw counts: orcache_hit_pct_of_reads separates cheap local NVMe hits from network round trips to the cluster volume. That distinction is the whole reason the collector reads aurora_stat_statements instead of the vanilla view, and it is what makes a community-style cache-hit ratio misleading on Aurora. Grouped by (queryid, database_id) to match the collector's identity, and HAVING excludes shapes that did not execute in the window - pg_stat_statements retains an entry long after its last execution, so without that the list is padded with idle shapes showing zero. The empty-result message names the likeliest real cause rather than just saying no data: on some of our clusters pg_stat_statements exists only in the application database, not in postgres, so a collector pointed at the wrong database returns nothing while looking healthy. Registered in the MCP host's tool-type chain plus the web endpoint catalog and dispatch tables. Full solution builds clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first Tier 0 outage predictor, and the first PostgreSQL collector that is not Aurora-only: it reads core catalog surfaces, so it populates on any PostgreSQL target. Gating it on Aurora would have silently dropped the single most consequential PostgreSQL signal everywhere else, and a test pins that it applies to non-Aurora targets. PostgreSQL transaction ids are 32-bit and wrap, and the consequences escalate on a documented ladder: forced anti-wraparound autovacuum at autovacuum_freeze_max_age (200M default), failsafe mode abandoning cost limits and index cleanup at 1.6B, warnings near 40M ids remaining, and at 3M remaining the server refuses to assign new transaction ids - writes and DDL stop while reads continue. That last state is a write outage no failover fixes, because every replica shares the condition. MultiXact ids are collected as a first-class second counter because they are independent and separately fatal, and are the thing almost nobody monitors. They are consumed when a row is locked by several transactions at once, so a SELECT FOR UPDATE-heavy or foreign-key-heavy workload burns them far faster than plain transaction ids: a server can look comfortable on XID age while being in trouble on MultiXact age. Both live on pg_database, so collecting one and inferring the other would be a choice to be wrong. Two percentages per counter, against deliberately different denominators: distance to an emergency vacuum (autovacuum_freeze_max_age) and distance to the wraparound ceiling (~2^31). Conflating them would make a routine anti-wraparound vacuum read as an imminent outage - about a tenfold overstatement at defaults - and a test pins both denominators. An unreadable setting yields 0 rather than manufacturing a percentage, and therefore cannot manufacture an alert. Percentages are STORED rather than computed on read because their denominators are per-server settings: recomputing later against whatever autovacuum_freeze_max_age happens to be then would silently rewrite history the moment someone tunes it. Uses age()/mxid_age() rather than arithmetic on the raw xid, since both handle the modular wrap that makes naive subtraction wrong precisely near the boundary - the only region where being wrong matters. Reads the shared pg_database catalog, so no per-database fan-out. No deltas: age is a distance from a wall, not accumulated work, and it falls when autovacuum freezes. V62, StorageVersion 62, catalog pins 43 -> 44. Verified with the ladder generator that all three PostgreSQL tables now match their rungs column-for-column (pg_wait_stats 12, pg_statement_stats 34, pg_wraparound_stats 16). Per-relation attribution - which table is holding the freeze floor - is a per-database read and belongs in its own collector; this one answers how much time is left, which is the alerting question. Full solution builds clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Read as a LEVEL, not a rate — the opposite discipline from the wait and statement readers, and deliberately so. Freeze age is a distance from a wall, not accumulated work: averaging it would blur the only number that matters and summing it would be meaningless. So this takes the latest reading per database (DISTINCT ON) plus the window PEAK for each counter, and a test asserts SUM/AVG never appear over an age column. The peak is what makes the pair useful. A current age below the window peak means freezing has clawed age back at least once - the healthy sawtooth. Equal to the peak means age has only ever climbed within the window, which is the shape that ends in a write outage. The tool reports that comparison as freezing_is_keeping_up rather than leaving a reader to eyeball two numbers. Severity comes from the documented escalation ladder, not from round numbers, because each boundary is a real behaviour change: vacuum_failsafe abandoning cost limits and index cleanup around 1.6B ids, server warnings near 40M remaining, writes stopping near 3M remaining. Crucially, a database past its emergency-vacuum threshold but far from the ceiling is classified INFO, not warning - a forced anti-wraparound vacuum is normal operation, and alerting on it is exactly how wraparound monitoring earns a reputation for crying wolf. At defaults that state is 100% of autovacuum_freeze_max_age and only ~9.3% of the way to the ceiling. Both independent counters are surfaced with their own percentages and remaining-headroom figures, since MultiXact exhaustion is separately fatal and a server can look fine on transaction IDs while being in trouble on MultiXacts. The response leads with the worst database and its severity, because one database hitting the wall stops writes for the whole instance - the worst database IS the server's state. The thresholds themselves are included in the payload so a consumer can see what the percentages mean without consulting documentation. Registered in the MCP host chain plus the web endpoint catalog and dispatch. Full solution builds clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reason autovacuum can run, report success, and reclaim nothing. Four unrelated causes present IDENTICALLY from the symptom side - dead tuples accumulate, autovacuum logs success, the table never shrinks - and the remedy is completely different for each: kill a session, drop a replication slot, disable standby feedback, or resolve an orphaned prepared transaction. Doing the wrong one is at best useless. So attribution is the deliverable, not a nicety. The collector emits the oldest holder PER SOURCE and stamps which source is actually setting the horizon, rather than a single aggregate age that would leave a reader exactly where they started. Five sources, and two of them come from pg_replication_slots on purpose: a slot's xmin holds back ordinary row cleanup while its catalog_xmin holds back CATALOG cleanup specifically - what a logical decoding slot pins - and the two can differ by a lot. Collapsing them would misattribute one as the other. A test asserts every source is queried, because missing one does not degrade the answer, it inverts it: the collector would crown a different winner and send someone to fix the wrong thing. DISTINCT ON bounds output to the oldest holder per source, at most five rows a cycle. pg_stat_activity can carry hundreds of backends with an xmin, and storing all of them every minute would be a great many rows saying one thing. is_winner is stamped at collection, not derived on read, so a stored row names the winner as of the moment it was measured. Deriving it later would depend on which rows a query happened to select, and a filtered read could crown a holder that never held the horizon. Exactly one winner is stamped even on a tie, so a stored row is never ambiguous. Zero rows is the HEALTHY state and must never read as a collection failure, so nothing throws or synthesizes a placeholder. Every branch is independently empty-tolerant, which also covers Aurora: pg_stat_replication is expected to be empty there, since replicas read the same storage volume rather than streaming WAL. Per-minute cadence, unlike its wraparound sibling at five: an xmin holder is the fast-moving leading indicator, and the useful catch is the session or slot that appeared minutes ago, before it has cost anything. Not Aurora-gated - core catalog surfaces only. Worth noting the two Tier 0 collectors compose: a pinned horizon also blocks freezing, so an unattended xmin holder is an upstream CAUSE of the wraparound risk pg_wraparound_stats measures. V63, StorageVersion 63, catalog pins 44 -> 45. All four PostgreSQL rungs verified column-for-column against the generator (12, 34, 16, 9). Full solution builds clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reports the current holder per source joined to that source's PERSISTENCE across the window, because presence alone does not tell you what to do. A slot that won 58 of 60 samples is a standing problem someone must own; a session that won twice was a query that ran long and finished. Reporting only current state makes those identical; reporting only the window hides which cause holds the horizon right now. Each source carries its own concrete remedy, which is the entire payoff of attributing the cause: kill or bound a session versus drop an abandoned slot versus chase a stalled logical consumer versus fix a replica query versus resolve a two-phase transaction by gid. Tests assert all five remedies are genuinely distinct rather than one message with the noun swapped, and that an unrecognized source says so instead of guessing. The no-holder case gets a real finding rather than an empty envelope. An operator reaches this tool BECAUSE bloat is growing, so "nothing is holding the horizon" is informative: it redirects the investigation from "vacuum is blocked" to "vacuum is not being triggered", and the response says exactly that instead of leaving a dead end. The response leads with the winning source, its holder, and the recommended action, since that triple is the whole answer. Registered in the MCP host chain plus the web endpoint catalog and dispatch. Fixed a missing System.Linq using in the new test file. Full solution builds clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Worth its own collector even though pg_xmin_horizon already reads slots, because an abandoned slot has a SECOND, independent failure mode that the horizon view cannot see. A slot retains every WAL segment its consumer has not confirmed, and with max_slot_wal_keep_size at its default of -1 that retention is UNBOUNDED: the slot holds WAL until the volume fills, and a full WAL volume stops the server. Same slot, two outages on different axes - disk exhaustion and vacuum starvation - so both xmin counters are recorded here as well and slot state is legible without a join. retained_wal_bytes is COMPUTED from restart_lsn rather than read from safe_wal_size, because safe_wal_size is NULL whenever max_slot_wal_keep_size is -1. Depending on that column would mean reporting nothing on a stock server, which is precisely where retention is unbounded. -1 is stored as the not-applicable sentinel so a consumer cannot mistake "no limit configured" for "no data collected". The LSN reference switches on pg_is_in_recovery(), because pg_current_wal_lsn() ERRORS on a standby and Aurora readers are legitimate targets - without the switch the whole collection would fail on a reader rather than degrading. A test pins it. inactive_since and invalidation_reason are PG17+, conflicting is PG16+; on older majors the collector substitutes NULL/false so the payload shape is identical across a mixed-version fleet and a chart does not change shape at an upgrade. A test asserts both versions select the same number of expressions. inactive_since is the column that turns "this slot is inactive" into "this slot has been inactive for three weeks" - the difference between a consumer between polls and an orphan. Per-minute, 90-day retention: retained WAL grows at whatever rate the server generates WAL, which on a busy writer fills a volume in hours rather than days, and the question after an incident is how long the slot was orphaned, which has to outlive the incident. V64, StorageVersion 64, catalog pins 45 -> 46. All five PostgreSQL rungs verified column-for-column against the generator (12, 34, 16, 9, 20). Full solution builds clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes the fifth vertical slice. The read pairs the latest state per slot with that slot's earliest reading in the window, because retained WAL is only actionable as a trend: a slot holding 45 GB steadily is a consumer that is behind but keeping pace, while one that grew from 2 GB to 45 GB in an hour is a volume filling in front of you, and a single current figure cannot tell them apart. Severity turns on that conjunction — WAL retained because of this slot, nobody consuming it, and the pile still growing — rather than on size. Also fills the documentation debt the last five commits accumulated. The Darling README's collector count, schema version, and the TimescaleDB background-worker derivation were all stale, and the worker numbers are load-bearing: an unmanaged store sized to 44/55 while carrying 47 hypertables starves the policies that compress and expire data. Verified against the catalog rather than extrapolated (HypertableTables is CollectorCatalog.All, so it is 46 collector tables plus collection_log). The new PostgreSQL Targets section documents what the seam actually does, including the pg_monitor grant and the two Aurora-gated collectors, and states plainly what is still missing (per-database fan-out, alerting, analysis). Writing it surfaced a real gap: the README claimed integrated auth on a PostgreSQL target is rejected at config validation, but it was only rejected where the connection string is built — which for a service means the misconfiguration appears in a log after deployment instead of in --test-connection before it. Validate() now catches it, and range-checks the optional port while it is there. Clears the five build warnings this branch introduced.
The fan-out seam first. Per-database collection on SQL Server has two possible shapes — switch the connection's catalog, or stay put and prefix the query with EXECUTE [db].sys.sp_executesql — and PostgreSQL has only the first, because a connection there is bound to one database for its lifetime. So ITargetProvider gains the one shape both engines support (WithDatabase) plus BuildDatabaseListPlan, which packages "where to ask for the database list" with "what to ask": SQL Server hops to master, PostgreSQL reads pg_database where it already is. The Azure enumeration now goes through that same plan, so the sys.databases query lives in exactly one place instead of two. What deliberately did NOT move into the provider is the failure policy. An inaccessible master on Azure SQL DB has a real fallback (collect the one connected database) and a re-probe throttle; a PostgreSQL login that cannot read pg_database cannot monitor the server at all, so inventing a fallback there would turn a permissions problem into a quiet partial collection. pg_autovacuum_stats is the collector that exercises it. Dead-tuple counts are the standard PostgreSQL autovacuum metric and they are close to useless alone, because autovacuum fires at a threshold derived from the table's own row count: 500,000 dead tuples is routine on a 50-million-row table and urgent on a 10,000-row one. This computes each table's threshold and stores it alongside the count, honouring per-table reloptions overrides rather than only the GUCs — those overrides are common on exactly the big hot tables where the global default is wrong, so reading the GUC alone would report a threshold the server is not using. The read surface ranks by that ratio, and sorts a table with autovacuum switched off above everything regardless of its count. Two traps worth naming. The activity filter needs the insert clause, not just dead tuples: an append-only table has no dead tuples at all, so a dead-tuple filter drops precisely the tables that never get vacuumed and therefore never get frozen. And reltuples is -1, not 0, on a never-analyzed table, which unfloored yields a negative threshold and makes such a table read as permanently overdue. Cadence is hourly rather than per-minute because on PostgreSQL a per-database collector means one connection per database per cycle. Verified V60-V65 against the ladder generator: all six identical, 24 columns for the new rung. Fleet numbers in the README re-derived rather than incremented (48 hypertables, so 50/61 for an unmanaged store).
This read was mixing two time bases in one row. calls, total_exec_time_ms and rows_returned came from the stored delta columns and covered the requested window, while the block, storage/orcache and WAL figures were MAX() — the latest LIFETIME cumulative reading, since the last pg_stat_statements_reset(), possibly weeks earlier. Nothing in the output distinguished them, so a consumer reading total_exec_time_ms for the last hour beside shared_blks_read since forever would derive per-call I/O ratios that are pure nonsense. The old note admitted the limitation, which does not help: a caller that reads the note still cannot recover the windowed number. Fixed at read time rather than by storing more deltas. The data is already there, sampled per minute, so the difference is computed with a window function instead of adding eight delta series per query shape to the store. GREATEST(value - LAG(value), 0) is what makes that safe, and the reason a plain last-minus-first would not do. A counter reset — an explicit reset, an eviction and re-entry, or a major-version upgrade, since queryid is not stable across majors — makes one interval negative, and last-minus-first reports that as a large negative figure. Clamping each interval at zero drops exactly the reset interval and keeps the rest, which is the same rule the stored delta machinery already applies. The LAG partition is the full series identity (queryid, database_id, user_id, toplevel), matching how the stored deltas are keyed, with the roll-up to (queryid, database_id) happening after. Differencing at the coarser grain would interleave separate pg_stat_statements entries and produce garbage intervals. max_exec_time_ms and max_exec_peakmem_bytes stay MAX: they are high-water marks, not counters. Verified against real Aurora PostgreSQL 16.11 and 17.7 (stage, read-only), not just by text assertion — probe_validate_reader_sql.py substitutes a synthetic VALUES table for the store table, keeps the query body byte-identical, and checks the arithmetic. It confirms the reset case sums to 100 rather than -10, and that a series with one sample in the window reports 0 rather than its lifetime total. The same harness covers the autovacuum reader and proves its ranking: a table 50x past its threshold with 50k dead tuples sorts above one 2.5x past with 500k, which is the inversion the ratio ordering exists to fix.
Two defects found by probing live Aurora before building the next collector, neither of which the build or a text assertion would have caught. The replication-slot collector's PG17+ branch selected inactive_since bare, and that column is `timestamp with time zone`. Npgsql 10 maps a timestamptz read to DateTime with Kind=Utc and refuses to write a Kind=Utc DateTime into the store's `timestamp without time zone` column, so collection would have failed at COPY time on any Aurora 17 target holding a slot that had ever gone inactive — which is to say on exactly the servers the collector exists for. The PG16 branch substituted NULL::timestamp and was correctly typed all along, which is what hid the asymmetry. The autovacuum collector used `x::timestamp` on four timestamptz columns. That form converts, but it renders the instant in the SESSION's TimeZone before dropping the offset, so it agrees with UTC only while every parameter group says UTC. Verified that all probed instances do say UTC today — which is precisely what would keep the bug invisible until one of them didn't. Both now use AT TIME ZONE 'UTC', the only form that is correctly typed AND timezone-independent, pinned by test. The bigger find: pg_autovacuum_stats now gates off standbys. Not for permissions or availability — pg_stat_user_tables reads fine on an Aurora replica and reports ALL ZEROS. Same cluster, same database, same 15 tables, on 17.7: the writer reported 13,654,458 dead tuples and 150,790,506 live tuples while the reader reported 0 for n_dead_tup, n_mod_since_analyze, n_ins_since_vacuum and n_live_tup. Those are the writer's stats-collector numbers and they are not replicated. Ungated, a replica target returns no rows, the activity filter reads that as "nothing has pending work", and the tool reports perfect autovacuum health for a cluster 13 million dead tuples behind. A confidently wrong healthy answer is worse than no answer. The same reasoning applies more weakly to slots, so get_pg_replication_slots' empty result now says it is per-instance and points at the writer, instead of claiming neither WAL retention nor pinned vacuum is possible. Verified the fixed queries execute on live 17.7 and that no store column declared naive comes back carrying a timezone. Ladder regenerated and confirmed byte-identical: this changes SQL and a gate, not schema.
pg_stat_io attributes I/O to a (backend_type, object, context) triple rather
than to a file, so "the database is doing 40k reads/sec" becomes "autovacuum
workers are reading relations in the vacuum context". The context dimension has
no SQL Server counterpart and is the one that changes the remedy: it separates
ordinary buffer-pool misses, where more shared_buffers or a better index helps,
from sequential scans that deliberately bypass the pool through a small ring
buffer, where neither will. High bulkread volume looking like memory pressure and
not being memory pressure is the standard misreading of this view, so the tool
says so per row.
NULL is preserved end to end and never coalesced, which drove most of the design.
PostgreSQL uses NULL for "this counter does not apply to this combination" — the
checkpointer performs no reads or hits, bulkread never extends, the normal
context has no ring buffer to reuse — and on Aurora the ENTIRE write side is NULL
because backends there do not write data files, the storage layer does. Probed
before writing any of it: on 17.7 writes/write_time/writebacks/writeback_time/
fsyncs/fsync_time all come back NULL, and on 16.11 writebacks and fsyncs do. A
zero in any of those places would claim a measurement nobody took, and a consumer
averaging write latency would divide by it. So this is the one Postgres collector
here that uses NULL rather than a -1 sentinel: -1 suits a level a consumer reads
directly, but these are cumulative counters that get differenced, and -1
differenced against a real value is a garbage interval.
The read therefore reports write_counters_tracked alongside the numbers, so a
caller can tell "no writes happened" from "writes are not measured here". Differencing
uses the same clamped positive-interval rule as the statement read.
Two things the probe settled that a guess would have got wrong. The enum values
differ between majors — 17.7 showed a walreplay context and Aurora-specific
backend types ('aurora cache receiver process', 'aurora wal replay process',
'slotsync worker') that 16.11 did not — so nothing filters on them; a whitelist
would silently drop rows. And PG18 REMOVED op_bytes, so it is substituted there
rather than left to fail with "column does not exist"; the replacement per-operation
byte counters are deliberately not added speculatively, since they measure something
different and deserve their own columns decided against a real PG18 target.
Per-minute cadence, unlike the autovacuum collector: this is cluster-wide, so one
connection and no fan-out, and it returned 25-37 rows per snapshot on the fleet —
the same order as pg_wait_stats.
Verified rather than assumed. The collector SQL executed on live stage 16.11 and
17.7 returning 37 and 25 rows with stats_reset arriving naive. The reader SQL ran
on both majors against a NULL-bearing fixture and computed every case: the reset
clamp summing to 100 instead of -10, NULL writes reporting tracked=false, the
checkpointer's real writes reporting tracked=true at 600, bulkread's NULL extends
summing to 0 while its reuses summed to 150, and an idle combination filtered out
rather than returned as zeros. All seven PG rungs diffed identical to the ladder
generator (22 columns for this one).
…R forever The engine seam has had PostgresTargetProvider.Classify since the first commit on this branch, but nothing in the worker consulted it — every SQLSTATE-bearing failure fell through to the general handler, which logs ERROR and records ERROR. That is worst for the conditions that never resolve on their own. pg_statement_stats against a database where the extension was never created raises 42P01 on every cycle; a source Aurora does not implement raises 0A000 on every cycle; a feature gated off in the parameter group raises 55006 on every cycle. At a one-minute cadence each of those is 1,440 identical errors a day, which is how a real finding becomes noise nobody reads. These are the exact PostgreSQL analogue of the 8189 sys.traces denial that already degrades to PERMISSIONS, and for the same stated reason: a legitimate least-privilege or platform reality should not scream every cycle. The store has five statuses and none of them means "this feature is not installed", so those cases take the non-fatal-degradation bucket and the MESSAGE carries the truth — following the AzureDmvPermissionHint precedent, and saying explicitly "NOT a missing grant" plus the fix (CREATE EXTENSION, or the parameter group), because PERMISSIONS on its own would send someone hunting a GRANT that cannot help. Two discriminations worth calling out. A statement_timeout (57014) is an ERROR but is NOT connection-fatal, so it does not trip the reconnect-and-reprobe path — dropping the connection over a slow query would turn a tuning problem into a reconnect storm. And the general handler's reconnect trigger now recognises a Postgres connection failure (08 class, 57P0x) where before it only knew about SqlException, so a dead socket on a PG target went unnoticed and poisoned every subsequent collector on that server. An unrecognized SQLSTATE stays loud, deliberately: the quiet bucket is for conditions we have identified, not a catch-all. Verified against the real shipped code path rather than a copy — a console harness referencing the service project ran the full SQLSTATE truth table through DarlingWorker.PostgresFaultOutcome and PostgresTargetProvider.Classify: 12 states mapped as intended, every emitted status one the store already understands, the yield branch reachable for a collector that opts in, and 57014 confirmed distinct from the five connection-fatal codes.
The three Tier 0 outage predictors are collected, stored and readable, and nothing pages anyone about them. That is the biggest remaining gap for replacing DBM, so this writes down what the work actually is instead of leaving it as "add alerting". The finding is that it is NOT another vertical slice. The previous seven collectors were purely additive; the alert engine is SHARED WITH LITE and is what SQL Server monitoring alerts through today, so one new PostgreSQL alert changes a contract two SKUs implement, four test files reference, and the viewer's Settings window exposes — plus a migration for the new settings columns. Highest blast radius on the branch. That surfaces a real architecture question rather than a coding one: does a PostgreSQL-only signal extend the shared IAlertReadAdapter, forcing Lite to implement three methods for an engine it cannot monitor, or sit behind a separate adapter the engine consults only for PostgreSQL targets? The note recommends the second, for the same reason the collector seam gates by engine instead of having every definition claim every target — but it touches the shared brain, so it is Erik's call, and starting the implementation before that is decided would mean guessing at a seam and rewriting it. The note also records the thresholds each alert should use, derived from what the collectors already measure rather than invented: wraparound against autovacuum_freeze_max_age instead of a raw XID count, xmin against the winning holder's age AND persistence (the collector already attributes the cause, and the four causes need different fixes), slots against wal_status plus whether retained WAL is growing. Each maps to a severity the read surface already computes, so the engine's job is threshold, edge-trigger and dedup — not re-deriving the finding. And the trap an alert would otherwise inherit: anything reading autovacuum state must keep the standby gate, because pg_stat_user_tables reports all zeros on an Aurora reader.
The three Tier 0 predictors were collected, stored and readable, and silent. Now they page. Wraparound risk, a blocked vacuum horizon and replication-slot retention are evaluated on the alert cadence and delivered through the SAME deliverer, history and mute rules as every SQL Server alert, so they land in the same places and obey the same suppression rather than becoming a second notification path nobody configured. Option B as chosen: a separate IPostgresAlertReadAdapter consulted only for PostgreSQL targets. Extending the shared IAlertReadAdapter would have forced Lite — which has no PostgreSQL target and no PostgreSQL collectors — to implement three methods that can only ever return empty, leaving permanent dead code in a shipping SKU to satisfy a contract it has no stake in. This mirrors what collection already does: CollectorCatalog.AppliesTo gates by engine instead of having every definition claim every target. B turned out cheaper than the scoping note estimated. AlertEngine was not touched at all — the evaluator is a pure function of (rows, settings) beside it, and the host calls it after the shared sweep behind an engine check. So Lite, IAlertReadAdapter, IAlertEngineSettings and all four existing alert test files are untouched, and the blast radius collapsed to new files plus one gated call site. Failure-isolated too: a broken PostgreSQL read must not cost a server its CPU or blocking alerts, so it cannot. The thresholds are derived from PostgreSQL's own mechanics rather than picked, which is what makes them defensible as constants for now. Wraparound grades against the SERVER'S OWN autovacuum_freeze_max_age, not a fixed number: warning at 90% of it (before autovacuum force-starts its own prevention vacuum, while a planned one is still an option) and critical at 2x. That scaling matters — 400 million transactions is critical on a stock 200-million server and completely unremarkable on one tuned to 1.5 billion, and a constant would either never fire for the second or constantly for the first. A missing or non-positive setting silences instead of firing, because every derived threshold would otherwise be zero and alert on every database forever. xmin gates on persistence as well as age, which is the whole difference between a chronic holder and a report that ran long — without it this fires on any slow query, which is how an outage predictor earns a mute rule and stops being one. The message carries the remedy for the specific cause, since the five causes are indistinguishable by symptom and need completely different fixes. Slots fire at any size for lost/unreserved (failures that have already happened) and grade the inactive-plus-growing-plus-over-the-line conjunction as the disk-fill emergency, with each part alone a warning. Thresholds are NOT configurable yet: no new config_alert_settings columns, no migration, no Settings-window work. Deliberate first cut, and the design note says so plainly — the moment someone wants a different number, that is the work. Verified against the real shipped evaluator (harness kept alongside the probes): 29 checks over the boundary values, the scaling behaviour, the silencing cases, all five xmin remedies, and the slot grading conjunction. Every boundary asserted on both sides.
Settling the decisions before writing code, because doing it that way for alerting is what made that implementation fast — and this one has more traps than it looks. The load-bearing ones. pg_blocking_pids() takes ShareLock on the lock manager partitions per call, so calling it per row of pg_stat_activity on a 5,000-connection instance makes the monitoring query the incident; filter to wait_event_type = 'Lock' first, which is the only population that can have blockers, so the filter costs nothing. Store the edge list rather than a rendered tree, since root blocker, chain depth and fan-out are all cheap over edges and expensive to recover from a string. Capture the BLOCKER's own state and not just its pid — a chain rooted in "idle in transaction" is an application bug and one rooted in a long query is a tuning problem, and the pid alone does not say which, which is the most common gap in homegrown PostgreSQL blocking monitoring. Also written down: this is a SAMPLING collector, not a blocked-process-report equivalent. PostgreSQL has no ring buffer and no server-side threshold that materialises a report, so blocking shorter than the cadence is invisible. That belongs in the collector's own docs or it will be mistaken for the SQL Server surface it resembles. Two gates NOT to inherit: it should run on standbys (recovery conflicts are real blocking and pg_stat_activity reports the standby's own backends — the autovacuum collector's IsInRecovery gate exists for a reason that does not apply here), and it should not declare YieldsOnLockTimeout, since reading pg_stat_activity takes no table locks and the branch could never fire. And the trap that has already bitten twice on this branch: pg_stat_activity's timestamps are timestamptz, so AT TIME ZONE 'UTC' or store server-computed durations instead.
--test-connection is the deployment gate, and against a healthy Aurora cluster it
printed "SQL major version 0, Unknown (0), msdb access: yes". Every field in that
line is a SQL Server fact a Postgres target does not have: the major version and
edition are zero because nothing probed them, and HasMsdbAccess is true only
because that is its default. A PASS that reads like a misconfiguration is worse
than a FAIL on the one verb whose job is to be believed.
The probe already knew better -- ConnectPostgresAsync fills in the major, the
version_num, Aurora detection and pg_is_in_recovery -- but ProbeAsync dropped all
four on the floor, so nothing downstream could see them.
So carry them, and branch on engine. A Postgres target now reports version, writer
vs reader, Aurora vs not, and then the number that actually answers "will this
target give me what I expect": how many of the seven PostgreSQL collectors clear
the gate, naming the ones that do not.
[PASS] aurora-reader: PostgreSQL 17 (server_version_num 170007), reader (in
recovery), Aurora - 6 of 7 PostgreSQL collectors apply (skipped:
pg_autovacuum_stats)
That count is computed by asking CollectorCatalog.AppliesTo the same question the
runner asks, via ConnectionProbeResult.ToTargetInfo(), rather than by keeping a
parallel list that can rot. A stock-PostgreSQL 15 reader clears three of seven,
and finding that out at pre-flight is the difference between "this is configured"
and "this will collect" -- otherwise the first symptom is an empty table someone
has to explain weeks later.
The two format sites that had each grown their own copy of this string -- the CLI
PASS line and the add_servers MCP detail -- now call one describer, so they cannot
drift; that also settles the small existing divergence in their msdb wording. The
new facts ride alongside the old ones in the test_connect result_json rather than
replacing anything, so an existing consumer keeps working, plus a ready-made
`facts` string for the Viewer dialogs when they get there.
The PostgreSQL fields are trailing optional record parameters, so every existing
construction site still compiles and still means "a SQL Server target".
Verified: solution builds clean; harnesses/probecheck (new) exercises the real
describer against the four target shapes a real fleet has -- Aurora 16/17 writer,
Aurora reader, stock PG 15 reader -- and independently recomputes each count from
the catalog gate. 7/7, 6/7, 7/7, 3/7, all matching. The xUnit assertions are in
DarlingCliCommandsTests and remain unexecuted on macOS.
Writing the first-target runbook found the defect the runbook would have died on at step 5: none of this worked at all against a real store. config.config_monitored_servers is the AUTHORITATIVE server list once seeded -- darling.json seeds it when empty and is ignored afterwards, which is deliberate and documented. Every MonitoredServer field had a column there except Engine and Port. So a PostgreSQL entry was written without its engine, read back as the "sqlserver" property default, and connected to with SqlConnection. Not on a later restart -- on the FIRST start, because SeedIfEmptyAsync is immediately followed by the LoadViewAsync whose result replaces the file's list. Nothing failed to compile, no test covered the round trip, and the seven collectors, the fan-out, the fault classifier and the alerting on top were all built above a target that could never connect. V67 adds both columns, NOT NULL with defaults that make every existing row mean exactly what it means today; the seed writes them, the read restores them. Engine is stored as the raw string the operator wrote, alias and all, so MonitoredServer.TargetEngine stays the single place that interprets it. add_servers can now onboard one, which matters more than it sounds: it is the ONLY path into an already-seeded store, so without this a PostgreSQL target was unaddable to every existing install regardless of the columns. It validates engine STRICTLY, unlike the file parser that resolves anything unrecognized to SQL Server -- the parser's leniency stops one bad line from taking a fleet down at startup, while onboarding is a single deliberate act where "postgress" silently becoming a SQL Server target yields a connection failure against 5432 with nothing naming the cause. Then the pins. Three "the newest rung is N" assertions were still at 59, so the seven PostgreSQL rungs had been breaking DarlingObservabilityTests, PvsStatsStoreTests and StoreSelfMetricsTests since V60 and could not say so on a Mac. Four more in PgSchemaGeneratorTests were literals where the test's own name states an invariant: uniqueness of tables and names asserted as 45 while the real figure tracked the catalog count, and "emits every table" asserted as 46 of 48, which had quietly become a subset check. Those four now assert against CollectorCatalog.All.Count, so they cannot rot again; the catalog count itself stays a deliberate literal. harnesses/pincheck is new and exists because of that class of bug: it evaluates every literal pin against the real assemblies and prints pinned vs actual. It is the only thing here that catches a number a test file three directories away asserts, since the test projects cannot execute on this machine. Also: --test-connection's PASS line and add_servers' detail now report the probe's PostgreSQL facts through one shared describer (a Postgres target read as "SQL major version 0, Unknown (0), msdb access: yes"), and both name how many of the seven collectors clear the gate for that target -- 7 for an Aurora writer, 6 for a reader, 3 for a stock PostgreSQL 15 reader. docs/postgres-first-target-runbook.md is the procedure with a proof point per step, and says plainly that it has never been run end to end. Verified: solution builds 0 errors, 17 warnings all pre-existing (the CA1859 on _alertDeliverer was mine and is fixed). Ladder generates 66 rungs, top V67, with the ALTER ordered after the CREATE it depends on. All six harnesses pass, incl. pincheck's 15 pins. avcheck/iocheck no longer abort after passing when SQLOUT is unset. Nothing here has run against a live store or a live Aurora instance.
dev had moved 53 commits and taken V60 for database-state-edge-memory (#2166) while this branch was using V60 for pg-wait-stats. Two different rungs with the same version number: the applier would run one and skip the other depending on where a store already was, and PgMigrations.Scripts would carry a duplicate version. So dev's V60 keeps its number and the eight rungs here shift up one: 60 database-state-edge-memory (dev, unchanged) 61 pg-wait-stats 62 pg-statement-stats 63 pg-wraparound-stats 64 pg-xmin-horizon 65 pg-replication-slots 66 pg-autovacuum-stats 67 pg-io-stats 68 monitored-server-engine SchemaVersion is 68. The three "newest rung is N" pins conflicted three ways -- dev said 60, this branch said 67, the answer is 68. Nothing else conflicted semantically. dev's #2188 per-database collector_state prune looked like it might: it keys the existence list on database_states, which is a SQL Server collector gated off PostgreSQL targets, so an empty list could have retired pg_autovacuum_stats' per-database state every cycle. It cannot -- the prune is scoped to QueryStorePerDatabaseState.PrunableKeys and runs once per query_store cycle, query_store never runs on a PostgreSQL target, no PostgreSQL collector writes collector_state at all, and the SQL no-ops on an empty snapshot anyway. Checked rather than assumed because it is exactly the shape of thing that would have looked like a collector bug months later. Verified on the merged tree: builds 0 errors / 16 warnings, all pre-existing. Ladder generates 67 rungs, top V68, strictly ascending, no duplicate versions, and dev's V60 ALTER lands before pg_wait_stats' CREATE. All six harnesses pass, including pincheck's 15 pins now reading 68. Still nothing run against a live store or a live Aurora instance.
The connect-time gate refuses a store BELOW RequiredStoreSchemaVersion, which is StorageVersion.SchemaVersion. MapProbedSchemaVersion's newest arm was V60, so a fully-migrated v68 store probed as 60 and the viewer would have refused it with "older than this viewer -- upgrade/restart the service so it migrates the store" on a store that was already current. Every store this branch touches. dev's own V60 arm spells out why the arm has to exist even when nothing in the viewer would 42703: the invariant is that a fully-migrated store maps to EXACTLY SchemaVersion. So V68 gets an arm sensing config_monitored_servers.engine, plus the probe column and reader ordinal that go with it. V61-V67 get no arms on purpose -- they add tables in `collect` that no viewer read names, so a store sitting between them is only ever transient mid-migration, and the invariant that must hold is about the top rung. Three more pins the merge moved: RequiredStoreSchemaVersion is derived from SchemaVersion on dev, so the literal 60s in CollectorMemoryKnobTests, PvsStatsStoreTests and StoreSelfMetricsTests all read 68 now. pincheck grew three checks for this, because the probe is a second place a version has to be updated and nothing links the two: the newest arm must equal SchemaVersion, the reader ordinals must be contiguous from 0, and the probe SQL's top-level column count must equal the ordinal count. It reads the viewer's source rather than referencing the project -- the Viewer is net10.0-windows + WPF and cannot be referenced from a net10.0 console app, which is noted in the harness so nobody retries it. Verified: builds 0 errors / 16 pre-existing warnings; pincheck 18/18.
…ase note PostgresFaultOutcome was inserted directly above IsPermissionError's doc block in a8a98bd, which pushed that block up onto PostgresFaultOutcome and left IsPermissionError undocumented. DocCommentHygieneTests forbids exactly this (#1745, #2190) and would have failed the nightly. Per that test's own instruction, the fix is to MOVE the block back rather than delete the first summary -- seven of the eight found in #1745 were displaced blocks whose real member had been left undocumented, and deleting them lost the documentation instead of deduplicating it. IsPermissionError has its block again and nothing was rewritten. Verified by reproducing the detector's StackedSummaryRuns scan over every .cs in the tree: 1 offender before, 0 after. While in the same sweep, reproduced the reflection-based MCP parity test the same way: 99 tools, 80 /api/read endpoints plus 19 documented exclusions, zero tools without an endpoint, zero endpoints without a tool, and all seven PostgreSQL tools present in the dispatch AND the descriptor catalog. (My first pass at this reported 73 false positives because it sliced from BuildReadDispatch's first MENTION rather than its definition -- worth knowing, since the wrong answer looked alarming.) CHANGELOG gets the release note this branch never had, which for a nightly is the only place any of it is user-visible. No issue number: nothing on the branch cites one, and filing is not mine to do.
The status note said no instance had been monitored end to end, which read as "no Aurora instance has been touched" and undersold what is actually proven: every collector's generated SQL and every MCP reader's SQL has been executed against live Aurora 16.11 and 17.7, confirming shape, no timezone on naive columns, and correct windowed differencing. The real gap is one layer: the service has never done connect probe -> dispatch -> COPY into a store -> read back out. Naming the layer matters more than the disclaimer, because that is where the last defect was -- a PostgreSQL target could not survive its own registration while all of its SQL was correct and live-proven.
First real run of the suite: 561 of 4760 failed. Five causes, and the first is the one worth reading. 1. The collector table was named pg_replication_slots -- which is also pg_catalog.pg_replication_slots, the system view it reads. pg_catalog is searched implicitly and FIRST, ahead of every search_path entry, so an unqualified reference to that name resolves to the system view whatever the store holds. Two failures with very different characters: LOUD: V1's generated schema emits an unqualified CREATE INDEX ... ON pg_replication_slots. Indexing a view is 42809, so MigrateLockedAsync threw, the store never came up, and 531 tests failed behind the collection fixture. QUIET, and much worse: DarlingPgSlotReader and DarlingPostgresAlertReadAdapter both did FROM pg_replication_slots against the STORE. That would have returned the monitoring store's OWN slot list -- normally empty. get_pg_replication_slots would have reported "no slots" forever and the retention alert would never have fired. A silently muted outage predictor is worse than none, and it is exactly what the alerting note says must not happen. Renamed the table to pg_replication_slot_stats; the collector keeps the name pg_replication_slots after the view it reads, which is established practice here (query_store -> query_store_stats, and three more). Schema-qualifying every reference instead would have fixed the loud half and left the quiet half one forgotten qualifier away. Verified on live Aurora 16.11 and 17.7 (probe_catalog_name_collisions.py, new) that pg_replication_slots is the ONLY one of the seven that collides and that pg_replication_slot_stats is clear on both majors. A live-store test now asserts no collector table shadows a catalog object, checked against the real catalog rather than a hardcoded reserved list, with a teeth test so it cannot pass by matching nothing. 2. CI's throwaway cluster is sized off HypertableCount, and seven new tables moved 44/55 to 51/62. build.yml and nightly.yml carry those literals -- a 14th registration point I did not know about. pincheck now checks both workflows and the README, so the next collector cannot miss them. 3. ViewerCollectorCoverageTests: the seven PostgreSQL tables have no WPF reader. Allow-listed as UNBUILT UI with the reason -- they are read through MCP and /api/read today, and the viewer's surfaces are SQL-Server-shaped. One per line so removing one is a one-line diff. 4. RequiredStoreSchemaVersion_TracksTheBuildSchemaVersion_AndTheProbeCoversIt passed 43 hand-counted `true`s; my V68 sentinel was the 44th and defaulted to false, so "a fully-migrated store" quietly meant "one rung short" and the assertion failed on the version instead of on the call site. Now built from the method's own arity by reflection, which cannot drift from the signature. 5. My own TargetProviderTests handed one connection string to both engines; SqlConnectionStringBuilder rejects Host=. Split into a Theory with an engine-appropriate string each, plus a separate every-engine-has-a-provider case for the part that genuinely must loop the enum. Verified: builds 0 errors / 16 pre-existing warnings. Ladder 67 rungs, top V68, with the renamed table created and moved correctly on the fresh-store path. Six harnesses pass incl. pincheck's now-23 pins. Doc-hygiene scan 0 offenders, MCP parity 99 tools / 80 endpoints / 19 excluded / 0 unregistered.
erikdarlingdata
added a commit
that referenced
this pull request
Aug 14, 2026
…adder dev moved 10 rungs (V61 -> V71) while this sat, so the collision the V61 doc comment predicted actually happened -- and it surfaced exactly as that comment said it would, as a conflict on the migration list rather than as a silent gap. Renumbered to V72, immediately above dev's newest, no gap: the runner skips any rung at or below the store's stamped version, so a gap is skipped SILENTLY on every upgraded store while a collision is loud. Merged rather than rebased: same result, no force-push over 26 commits, and the 10-file conflict surface gets resolved once instead of per commit. Resolutions: - StorageVersion.SchemaVersion 61 -> 72. - The five conflicting test files take DEV's side wholesale. The branch had only bumped literal version pins in them; dev independently replaced those literals with the invariant form (Assert.Equal(StorageVersion.SchemaVersion, ...)) and made ViewerDataServiceTests build its all-true probe call by REFLECTION so the arity tracks the signature. That is strictly better than what the branch was reaching for -- the branch's own comment called the literal form "a recurring four-test failure". - ViewerDataService: dev's four new sentinels kept, the plan-map probe appended as #48, and its ladder arm returns 72 ABOVE dev's V71 arm because arms evaluate newest-first. The arm's comment deliberately does NOT name the table in prose -- dev's V71 comment records that a prose mention exempts a table from ViewerCollectorCoverageTests' ratchet, which strips information_schema probe lines but cannot strip a comment. One semantic conflict git could not see: DarlingCollectorRunner.cs auto-merged cleanly but left the #2210 plan-fetch call passing `sqlConnection`, which #2213's provider seam renamed and re-typed to a provider-neutral DbConnection. Resolved as `targetConnection is SqlConnection planFetchConnection`, which narrows the type the signature needs and gates the engine in one expression -- the enumerated path serves PostgreSQL targets now, and while query_store declares TargetEngine = SqlServer, relying on the catalog for that would put the invariant somewhere else. Full solution builds clean, 0 warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
erikdarlingdata
added a commit
that referenced
this pull request
Aug 15, 2026
The storage name was host + database + read-only intent only, so a SQL Server and a PostgreSQL instance on ONE host collided into a single server_id and interleaved their histories -- as did two PostgreSQL instances differing only by port, both of which #2213 made first-class configuration. The interesting constraint is what could not change. Lite derives server_id FRESH at runtime from the shared BuildStorageName, everywhere, with no stored-id fallback: GetServerNameForStorage hashes it on every read. So altering what that returns for an existing server re-keys it in Lite and orphans all its history, silently -- the same harm as #2158 from the other direction. That refutes simply adding the fields. So the parameters are OPTIONAL and append nothing at their defaults, keeping Lite's three-arg call byte-identical. Verified against a re-statement of the pre-change rule rather than hand-written strings, so it holds for any input. A SQL Server entry passing them is unchanged too, which is what lets Darling pass Engine and Port unconditionally: Engine folds to no token for SQL Server, and Port is PostgreSQL-only -- SQL Server carries a non-default port inside the host as host,1433 and is already discriminated there. Darling's registered PostgreSQL targets do not re-key either, for a different reason: their id comes from the store and is only ever derived for an entry with no row yet. That is why #2158 was a prerequisite for this and not a sibling. Every spelling folds to one token (postgres/PostgreSQL/pg, any casing), so a colleague's capitalisation cannot mint a second identity for one instance -- a split history nothing downstream could diagnose. An unrecognised engine appends nothing rather than being interpolated raw, so a typo cannot either. Suffix order is fixed (engine, port, :RO) so two callers with the same facts cannot produce two names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
erikdarlingdata
added a commit
that referenced
this pull request
Aug 15, 2026
… (#2278) * Carry engine and port in the server identity, re-keying nothing (#2218) The storage name was host + database + read-only intent only, so a SQL Server and a PostgreSQL instance on ONE host collided into a single server_id and interleaved their histories -- as did two PostgreSQL instances differing only by port, both of which #2213 made first-class configuration. The interesting constraint is what could not change. Lite derives server_id FRESH at runtime from the shared BuildStorageName, everywhere, with no stored-id fallback: GetServerNameForStorage hashes it on every read. So altering what that returns for an existing server re-keys it in Lite and orphans all its history, silently -- the same harm as #2158 from the other direction. That refutes simply adding the fields. So the parameters are OPTIONAL and append nothing at their defaults, keeping Lite's three-arg call byte-identical. Verified against a re-statement of the pre-change rule rather than hand-written strings, so it holds for any input. A SQL Server entry passing them is unchanged too, which is what lets Darling pass Engine and Port unconditionally: Engine folds to no token for SQL Server, and Port is PostgreSQL-only -- SQL Server carries a non-default port inside the host as host,1433 and is already discriminated there. Darling's registered PostgreSQL targets do not re-key either, for a different reason: their id comes from the store and is only ever derived for an entry with no row yet. That is why #2158 was a prerequisite for this and not a sibling. Every spelling folds to one token (postgres/PostgreSQL/pg, any casing), so a colleague's capitalisation cannot mint a second identity for one instance -- a split history nothing downstream could diagnose. An unrecognised engine appends nothing rather than being interpolated raw, so a typo cannot either. Suffix order is fixed (engine, port, :RO) so two callers with the same facts cannot produce two names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Update the tests whose derivation copy went stale (#2218) CI caught three tests holding their own copy of the identity rule, which stopped matching the moment the derivation grew engine and port. All three are legitimate expectation changes -- PostgreSQL identities intentionally changed -- not papering over: - PostgresEngineGateBehaviorTests hashed PG hosts with the 3-arg form while its MonitoredServer says Engine = "postgres", so it computed an id the product never uses and the gate returned "server not monitored" instead of the arm under test. Its helper now takes the engine, defaulted to null so the SQL Server call sites are untouched. - PostgresTargetConfigTests pinned the storage name for a PG server; ":pg" now sits between the database and ":RO". Extended to cover the port too, since that string IS the documentation of the rule. - ServerIdentityFromStoreTests hashed (host, database, readOnlyIntent) by hand for a server that carries Engine = "postgres" and Port = 6432. Now derived from the server's own StorageName, so it cannot drift again -- the old shape failed as "the seed wrote the wrong id" rather than "the test's copy of the rule is stale". Swept the rest of both suites: every other BuildStorageName caller uses a default engine and port, so their expectations are byte-identical and unaffected. SharedCollectorDefaultsPinTests' four pins are exactly the no-re-key cases and still hold, which is the guard proving the change is backwards compatible. Verified against the real helper: the two updated strings, that the seeded PG id genuinely differs from the stale hash (so the fix was required rather than cosmetic), the four no-re-key pins, and that sqlserver-default entries are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Merge the stacked doc block my last edit created (#2218) DocCommentHygieneTests caught it: inserting a doc comment above ServerIdFor pushed the block that was already there into a stack, so XML docs took the last one and only a human reading the file would have seen two. MERGED rather than deleted, per the test's own warning. The original point -- derive through the same helper the worker uses, so the test cannot drift from the lookup -- is the reason the helper exists at all, and #2218 is an instance OF that drift rather than a replacement for it. Deleting the first block would have lost the general rule and kept only the example. The detector I have for this only helps if it runs on the files a commit actually changed, not the ones I remember touching -- it is now driven off git diff --name-only, which reports clean across all six files here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
dphugo
pushed a commit
to dphugo/PerformanceMonitor
that referenced
this pull request
Aug 19, 2026
…61, Lite v54 The seam is two methods with REPLACE-THE-SET semantics, not upsert-each-row, because absence carries meaning: a fingerprint with no events left in the window has a finished incident, and leaving its row behind makes the next incident on that fingerprint read as a continuation of the old one. An empty map is therefore the clear, so there is no separate clear method to forget to call. Its own table in both stores, for two reasons that hold independently. The key is wrong for the watermark row — watermarks are per (server, metric), these are per (server, metric, FINGERPRINT), and two deadlocks over different object sets are different incidents whose totals must not pool. And Lite writes that row with INSERT OR REPLACE over a PARTIAL column list, which resets every unlisted column to its default: a counter living there would zero itself on every fired alert, i.e. exactly when it is read. V61 is max(dev)+1. erikdarlingdata#2211 and erikdarlingdata#2213 also claim 61+, which makes this a LOUD rebase conflict — the safe failure. A gapped rung would merge quietly and then silently skip every rung the other branches still intend to fill, because the ascent test is `version <= current`. Required interface members rather than defaulted ones, so the compiler found all three test fakes. A default implementation would have left them silently returning nothing, which is the same class of miss as a new probe parameter defaulting to false. Also the full rung recipe: nine literal pins across four test files in THREE forms (Scripts[^1].Version, StorageVersion.SchemaVersion, and RequiredStoreSchemaVersion), the probe sentinel, reader ordinal 43, a newest-first map arm, and the 44th explicit `true` in the full-sentinel pin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dphugo
pushed a commit
to dphugo/PerformanceMonitor
that referenced
this pull request
Aug 19, 2026
) A rung filled after a store has stamped past it is skipped silently and forever (the applier ascends with <=), so a NEW gap is a field incident waiting on a merge order. Density above the sanctioned V45 hole makes the gap fail at authoring time; with the existing no-duplicates pin, every wrong merge order of the current in-flight ladders (erikdarlingdata#2211/erikdarlingdata#2221 both at V61, erikdarlingdata#2224 at V62, erikdarlingdata#2213 stacked above) now fails CI instead of shipping a stranded-store window through a nightly. Watched-red on the real instance: the V62-carrying branch with no V61 fails Expected [...,60,61] / Actual [...,60,62]. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dphugo
pushed a commit
to dphugo/PerformanceMonitor
that referenced
this pull request
Aug 19, 2026
…arlingdata#2230) The three gates from erikdarlingdata#2213's round-3 fix had source-scanning pins and a live rig run, but nothing drove a PostgreSQL runtime through a gate and asserted the short-circuit. A source scan cannot tell a gate that returns early from one that falls through and happens to write the same text. analyze_now is the tractable one, as erikdarlingdata#2230 says: its observable is a PRESENCE — a row in analysis_state carrying the engine tombstone. The reconcile and snapshot_now gates observe an ABSENCE (no connection attempted), which still wants a counting seam. The regression guarded is specific and was real: clicking "Generate now" against a PostgreSQL target used to run the full SQL-Server-shaped pass, find nothing, and persist the GENERIC insufficient_data message, overwriting the honest tombstone the scheduled arm had written — so the Recommendations tab regressed from "does not apply, use the PG reads" back to "still collecting" the moment an operator pressed the button. The assertion that matters is therefore not "insufficient_data is true" but that the MESSAGE is the engine one, compared against the shared DarlingWorker.PostgresAnalysisNotApplicable constant. Second test is the discriminator, and without it the first is nearly worthless: a SQL Server target must NOT take the gate. A presence-assertion alone passes just as happily on a gate that fires unconditionally. Reflection for ServerLoopState because it is a PRIVATE nested class inside DarlingWorker — including its List<T>. Deliberately not widened to internal: the repo already reaches private worker state this way (CollectorMemoryKnobTests' gate tests), and changing production accessibility to observe behaviour reflection can already reach is the wrong trade. CommandOutcome is public, so no reflection there. Live-store gated on DARLING_TEST_PG, which CI's "Darling PostgreSQL tests" job sets; the gate's entire effect is a write through _postgres, so there is nothing to observe without one. Cleanup runs on CancellationToken.None per the LiveStoreCleanup convention and deletes only this test's own synthetic server_id. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dphugo
pushed a commit
to dphugo/PerformanceMonitor
that referenced
this pull request
Aug 19, 2026
…lingdata#2230's three The third dispatch loop, and the one that got neither engine gate in erikdarlingdata#2213's first round. Its regression is phantom SUCCESS rather than a crash: dispatched SQL Server collectors early-return in their own AppliesTo, yield zero rows, and RunOneAsync then writes SUCCESS to collection_log — so one operator snapshot against a PostgreSQL target produced ~40 rows saying collection worked. Neither of the tricks the other two gates needed applies here, and neither is required: the loop reports collectorsRun in its outcome JSON, and every run it makes writes itself to collection_log, so the gate's effect is a count and a set of rows. Schedule overrides disable everything except wait_stats — SQL-Server-only, and dispatched through the same loop — so the two arms differ in the ENGINE and nothing else: same collector, same overrides, same store, one dispatch decision instead of 49. - PostgreSQL target: collectorsRun is 0 and collection_log is EMPTY. Asserting absence rather than "not SUCCESS" is deliberate — a gate that dispatched and then failed would also avoid SUCCESS while having connected to a PostgreSQL host as SQL Server. - SQL Server target, same overrides: collectorsRun is 1 and the wait_stats row is there. Without this arm the gate assertion passes just as well against a snapshot that stopped dispatching anything, since "ran zero collectors" is also what a broken loop reports. Real DarlingCollectorRunner, not a stand-in: a fake would have to reimplement the dispatch the test exists to observe. Unresolvable hosts with a one-second connect timeout so the dispatched collector fails immediately; the status is not asserted, only that it was dispatched at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 tasks
This was referenced Aug 21, 2026
Closed
MisterZeus
pushed a commit
to MisterZeus/PerformanceMonitor
that referenced
this pull request
Aug 24, 2026
The viewer's startup dialog could name the file and the parse position and nothing else. That is not a wording problem: it round-trips a whole object, so when JsonSerializer.Deserialize threw there was no property in existence to report -- and every setting in the file reverted, not just the bad one. Per-property reads, the shape erikdarlingdata#2444 gave Lite, are the wrong answer here, and the reason is not the eighty-odd lines of typing the issue weighed. Lite's settings.json is built key by key on BOTH sides -- erikdarlingdata#2441 made every writer mutate one JsonObject -- so a per-key read is symmetric with a per-key write. These files are a whole-object round trip on both sides. Converting only the read half would break that symmetry: a property added to ViewerAppSettings afterwards would still be serialized on save and would silently never be read back. That is a worse defect than the one being fixed, and an invisible one. So the fix keeps the round trip and works on the exception instead. Measured rather than assumed, against the shipped BCL: JsonException.Path is "$" for a document fault and "$.AlertCpuThreshold" for a member's value, every time and for every fault class -- string-where-int, string-where-bool, number-where-string, number-where-list, a bad element inside a list, an overflow, a null. So the split between "the document is broken" and "one setting is the wrong shape" is a fact the reader already had in hand. SettingsFileGuard.Describe was throwing it away, deliberately: WithoutPathSuffix cuts " Path: ..." off the message because it duplicates the line and position, and Path is the one part that does not. ReadObject now names the member the deserializer stopped on, drops it, and runs the SAME deserialize again. Two consequences, and the second is the issue's title: a badly-shaped setting costs exactly its own setting, and the read can report the whole set rather than the first one -- which the path alone cannot, because a file with three bad members reports only whichever the reader met first. It is one judge, not two. erikdarlingdata#2213 spent a review round on a two-pass classify whose second pass judged by a different standard from the first; here every attempt is JsonSerializer.Deserialize<T> with the caller's own options over the caller's own type, and the only thing that changes between attempts is that one member is gone. Only a top-level member of a root JSON OBJECT is ever dropped, and that restriction is the load-bearing half. The viewer's server registry is a root ARRAY: "drop the element that would not deserialize" there means silently deleting a server the operator added -- the data loss erikdarlingdata#2434 exists to prevent, wearing a repair's clothes. The registry keeps its all-or-nothing behaviour, ViewerServerStore says so where the next reader will find it, and there is a control test for exactly that mistake because it was one line away. The state stays Unreadable for a partially recovered file rather than becoming some third thing, so PermitReplace still copies the original aside before the next save replaces it. Relaxing that would have destroyed the one setting the user actually got wrong, with no copy of it anywhere -- strictly worse than dev, where nothing was recovered and everything was preserved. The dialog gets two paragraphs, because there are two facts. A file that could not be read at all costs every setting in it; a file read after dropping named members costs only those. One sentence covering both would have to overstate one of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 31, 2026
Merged
Merged
pull Bot
pushed a commit
to ehtick/PerformanceMonitor
that referenced
this pull request
Sep 10, 2026
… option 2) A computed-on-read triage page for every alert firing, linked from all four webhook channels. GET /api/triage assembles the page from data the store already holds - the matching alert-history rows, the recent collection log, and alert-type-relevant reads, each running through the same /api/read dispatch the dashboard serves (zero SQL drift, nothing provisioned per alert, nothing to GC). The SPA renders it at #/triage?server=&metric=&at=&dedup= with derive.js auto-configs, and the Alert History page links each row to the identical page. The link base is the new web.publicBaseUrl (darling.json, file-authoritative; survives store config reloads which only overwrite Web.Enabled/Port). IAlertSettings grows a required TriageBaseUrl member so every adapter and fake states its answer; Lite serves no web page and answers empty. Unset base = every payload byte-identical to before. Per channel: Slack link button (actions block), Teams potentialAction OpenUri (real "@type" via dictionary keys), PagerDuty Events v2 links array + custom_details Triage row, generic {{triage_url}} token (documented in the Viewer settings placeholder line). Degrades, never 500s: server resolution, the history match, and each section are caught per-step and reported in the page body; unmapped metrics (self-alerts included) fall back to a summary + collection health page. A pinned test holds every mapped read to the dispatch table (the erikdarlingdata#2213 wiring lesson). Closes erikdarlingdata#2710
pull Bot
pushed a commit
to ehtick/PerformanceMonitor
that referenced
this pull request
Sep 10, 2026
…rikdarlingdata#2138) The orchestrator runs after each SCHEDULED analysis pass (analyze_now passes a null hook — an operator poking a server should not spend the bot's blast-radius budget), judges the pass's PLAN_REGRESSION targets through the shared gate, and journals the verdicts. It cannot write to a monitored server, and that is asserted three ways rather than promised. IPlanForceExecutor is declared with NO implementation; PlanForceBot holds no executor, no connection factory and no member that names the seam; and the compiled service assembly is searched byte-wise for sp_query_store_force_plan, sp_query_store_unforce_plan and FREEPROCCACHE and must contain none. Proven red by dropping the write path back in: all three go red, and they go red again when erikdarlingdata#2731 lands it — which is the point. Relaxing them is the reviewable act, not a side effect. Open all three gates on this build and the Force verdict journals as withheld, its own outcome: not 'failed' (nothing failed, and a failed force would cool the query down for a week) and not 'attempting' (which would surface as an orphaned intent owed a review of a force that never happened). Saying so out loud beats a silent downgrade to would-force — an operator who opened every switch believes the bot is live. Engine-gated at the boundary a connection would cross, not at the upstream fact (erikdarlingdata#2213's lesson), and failure-isolated at both seams so a bot fault can never reclassify a good analysis pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JgLaaxFenZFZs1SntKQDEg
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Teaches Darling to monitor PostgreSQL and Amazon Aurora PostgreSQL alongside SQL Server. A monitored
server declares
"engine": "postgres"and is collected by seven PostgreSQL collectors instead of the T-SQLones, into the same store, on the same naive-UTC contract and the same
server_ididentity. A mixed fleetis one store, one viewer, one MCP endpoint — nothing is partitioned by engine.
Every collector definition declares the engine it targets and is never dispatched against the other one, so
a PostgreSQL target is never sent T-SQL and a SQL Server target never sees
pg_stat_statements. A storemonitoring only SQL Server gains seven empty tables and nothing else changes.
What gets collected, and why each earns its place:
pg_wait_statsaurora_stat_system_waits()pg_stat_activity.wait_eventis an instantaneous sample, so there is nodm_os_wait_statsequivalent to read elsewherepg_statement_statsaurora_stat_statements()pg_wraparound_statspg_database,pg_classpg_xmin_horizonpg_stat_activity,pg_replication_slots,pg_stat_replication,pg_prepared_xactspg_replication_slotspg_replication_slotspg_autovacuum_statspg_stat_user_tables,pg_classreloptions— without the threshold the count is not actionablepg_io_statspg_stat_io(backend_type, object, context)rather than by file. PostgreSQL 16+, valid on a standbyThree of the seven are outage predictors rather than performance metrics, and they alert — graded against
the target's own settings (wraparound against that cluster's
autovacuum_freeze_max_age, not a constant),delivered through the same deliverer, history and mute rules as every SQL Server alert. That is deliberate:
PostgreSQL's most damaging failures are quiet, slow, fully predictable days ahead, and nothing in the engine
raises its hand about them.
Also here: the engine-execution seam (
ITargetProvider), per-database fan-out for PostgreSQL, faultclassification so an operator-actionable condition records a non-fatal skip with an explanation instead of
logging
ERRORevery cycle, and a read surface + MCP tool per collector.Permissions are one
GRANT pg_monitor. Nothing is created on the monitored instance — no Extended Eventssessions to provision, no server setting to bootstrap.
Two defects found while writing the runbook, worth calling out for review
config.config_monitored_servershad noengineorportcolumn. That registry is authoritative forthe server list once seeded, so a PostgreSQL entry round-tripped as
"sqlserver"(the property default)and the service opened a
SqlConnectionto port 5432 — on the FIRST start, since the seed is immediatelyfollowed by the load that replaces the file's list. Fixed by V68 plus the seed/read paths and
add_servers. A new property-driven test fails when any round-trip-criticalMonitoredServerpropertylacks a column.
MapProbedSchemaVersion's newest arm was V60.RequiredStoreSchemaVersionisStorageVersion.SchemaVersionand the connect-time gate refuses a store below it, so a fully-migratedstore would have been rejected as "older than this viewer". V68 gets an arm.
Also fixed seven stale literal pins in test files that the PostgreSQL rungs had been silently breaking since
V60, four of which are now asserted as the invariant the test's own name states rather than a literal.
Which component(s) does this affect?
PerformanceMonitor.Collectorsis shared with Lite, so Lite rebuilds — but no Lite behaviour changes. Litehas no PostgreSQL target, and the alerting deliberately rides a separate
IPostgresAlertReadAdapterconsulted only for PostgreSQL targets rather than extending the shared
IAlertReadAdapter, precisely so Liteis not left implementing three methods that can only return empty.
AlertEngineis untouched.How was this tested?
Please read this section rather than the checklist — the honest answer is more specific than the boxes.
Verified against live Amazon Aurora PostgreSQL 16.11 and 17.7 (stage):
(
pg_stat_io: 37 rows on 16.11, 25 on 17.7, carrying Aurora-specific enum values the community docs do notlist, e.g.
aurora cache receiver process).pg_replication_slots.inactive_sinceand the four autovacuum timestamps aretimestamptz, and Npgsql 10refuses to write a
DateTime(Kind=Utc)intotimestamp without time zone, so it would have failed atCOPY time in production. Every probed instance is
TimeZone=UTC, which is exactly what hides the wrongcast form.
pg_stat_iowrite side really is NULL, not zero — backends there do not write data files. Everycounter column is nullable on purpose and the read reports whether writes are TRACKED, so absent writes
cannot be misread as no writes.
VALUEStables substituted for the storetables, asserting the arithmetic: counter-reset clamping, NULL handling, single-sample → 0, and the ratio
ranking (a 10k-row table at 50× its threshold outranks a 50M-row table at 2.5×).
pg_autovacuum_statsis gated off standbys becausepg_stat_user_tablesreads fine on an Aurora reader andreports all zeros — measured on 17.7, same cluster and tables: writer 13,654,458 dead tuples, reader 0.
Ungated, a replica target would have produced a confident report of perfect autovacuum health.
Every migration rung was diffed against the ladder generator, since V1 is generated from the collector
catalog and a hand-written rung must match it column-for-column.
Not tested, and I would rather say so than have it found in review:
Darling.TestsandLite.Testshave never executed for this branch. They targetnet10.0-windowsandreference the WPF Viewer, so they compile on macOS and cannot run there. ~200 new xUnit assertions are
unexecuted. Local substitutes were used instead — the ladder-generator diff, standalone console harnesses
referencing the real projects, live SQL probes, and source-scanning reproductions of the reflection-based
MCP parity test and the
DocCommentHygieneTestsstacked-summary scan (the latter caught a real violationin this branch). CI is the first real run.
dispatch → binary COPY into a store → MCP read back out. The queries are proven; the wiring around them is
not.
docs/postgres-first-target-runbook.mdis the procedure for closing that gap, with a proof point atevery step, and it says plainly that it has not been executed.
dotnet build -c Debug→ 0 errors, 16 warnings, all pre-existing on dev (the one warning this branchintroduced, a CA1859 on a new field, is fixed).
Checklist
dotnet build -c Debug) — 0 errors and zero NEW warnings; 16pre-existing warnings remain, unchanged from dev. Not ticking a box I cannot honestly tick.
Aurora 16.11 and 17.7. No SQL Server behaviour changes; the shared-catalog changes are engine-gated.