Repository navigation
The PostgreSQL poison-wait host holds on silence like the SQL Server engine does, and three presence-flat alerts grade their severity from the bars the health bands already measured (#3653 alert riders) - #3660
Conversation
…engine does, and three presence-flat alerts grade their severity from the bars the health bands already measured (#3653 alert riders) PG poison wait: the clear arm requires an observed window. Since #2694 a quiet poison event writes no pg_wait_stats row, so the rows cannot witness their own absence; the read now carries the pg_wait_stats collector's own collection_log SUCCESS count over the same window (the #3537 xmin witness) as PostgresPoisonWaitWindow, and the host clears only when Observed. An unobserved window holds every active flag and logs once per server at Debug. A8e engine half: Deadlocks Detected grades Warning/Critical on the #3368 rate tiers (plumbed through IAlertEngineSettings.DeadlockRateThresholds so Darling's V120 knob governs; Lite returns the shipped pair), High CPU grades Critical at the CPU health band's 95% bar, tempdb Space fires an explicit Warning with no Critical tier (no measured bar to cite). Every graded fire sets the tier on the context and the outcome so both SKUs persist it.
There was a problem hiding this comment.
LGTM — reviewed the PG poison-wait observed-window witness (SQL, adapter, host clear-gate, dictionaries) and the three engine-side severity grades (deadlock rate band, CPU band, tempdb explicit Warning), including all six IAlertEngineSettings implementers for Lite/Darling parity, the channel-builder blast radius for CPU's now-non-null empty context, and the README/AlertSeverity replay-arm updates. Logic, boundary conditions, and parity all check out; no substantive findings. Could not run a local build to verify compilation (sandboxed, no build-command approval), so relying on manual trace of the changed paths — the PR's own stated local test run should be trusted for the compile/behavior confirmation.
What does this PR do?
Two alert-engine riders from #3653, one per half of the same lie: an alert surface saying something it never measured. The PostgreSQL Poison Wait host stops announcing "Poison Waits Cleared" when the collector goes silent — it holds until a window it can prove was observed comes back under the bar, the contract the SQL Server engine adopted in #3593. And the three alerts that still fired with
Severity: null— Deadlocks Detected, High CPU, tempdb Space — now grade themselves at the fire site from bars the product already bands on, so the alert-history grids,get_alert_historyand every channel render the tier the alert actually fired at instead of the colour its NAME implies. Partial for #3653 (ticks "PG poison-wait host clears on an EMPTY read" and "A8e engine half"); references #3653, does not close it.Item 1 — the PostgreSQL poison-wait host clears on an empty read
Root cause
DarlingWorker.EvaluatePgPoisonWaitAsyncread the two poison events' accumulations over the ten-minute window, computed findings, then walked every active subject and cleared any not in the firing set. An empty read — zero rows — produced zero findings, and every standing alert cleared. #3593 fixed the SQL Server engine's identical arm ("unwatched ≠ quiet": an absent measurement holds the flag; the #3282 rule for a CPU reading that stops arriving) and flagged the PG host as the residual this PR takes.Why the SQL Server fix does not port as-is
The SQL engine's witness is the rows themselves:
wait_statslands a THREADPOOL row every cycle on any server old enough to fire the alert, soaccumulated.Count > 0means "the collector ran".pg_wait_statsdoes not behave that way. Since #2694 the collector skips a wait event whose waits delta is zero over a real interval — an idle event writes no row — so a poison event that genuinely went quiet for ten minutes and a collector that stopped delivering both read as "no rows for that subject". Holding on the rows alone would never clear a finished storm; clearing on them (today) announces a recovery on collector silence. The witness has to come from somewhere that records a cycle whether or not it stored a row.What changed
The read carries the witness.
DarlingPostgresAlertReadAdapter.PoisonWaitSqlgains acapturesCTE counting thepg_wait_statscollector's owncollection_logSUCCESS rows inside the same window — the exact source and reasoning the xmin-horizon read took for its denominator in #3537: the log gets a row per run including a zero-row run, so "looked and found nothing" counts as a look, while ERROR / ABANDONED / PERMISSIONS / YIELDED runs (a cycle that could not look) do not. It is joinedcaptures LEFT JOIN accumulated ON trueso the read returns exactly one witness-only row when no poison event wrote in the window and one row per event otherwise, every row carrying the count. The accumulation half is the #2711 text byte-for-byte.GetPoisonWaitPressureAsyncnow returnsPostgresPoisonWaitWindow(Waits, CapturesInWindow)withObserved => CapturesInWindow > 0 || Waits.Count > 0— a stored row is itself proof the collector ran, which covers the log write's documented failure-isolation (it can skip a row; the count can only undercount, which turns a clear into a one-sweep hold, the conservative direction).PostgresAlertEvaluator.EvaluatePoisonWaitsis untouched — it still takes the rows.The host gates the clear loop on
window.Observed. Unobserved: every active flag holds where it is, one Debug line per server explains why a Cleared the operator might expect has not arrived (the collector's own Collection Stopped self-alert is the loud channel for a dead collector; this line is context, not a page), and the once-flag (_pgPoisonWaitHoldLogged, keyed per server because the silence is the collector's) resets on the next observed window so a second outage logs again. Observed: a standing subject clears when it is no longer over the bar — including the subject having written no row at all, which on an observed window is exactly "quiet". The fire arm, the per-subject cooldown, the #2716 seed and the #2704 collection-time guard are unchanged.Sub-second by shape, under the alert pass's 10 s deadline (#3597). The accumulation half is what it was: one
server_id, ten minutes, two event literals, the uncompressed head of one chunk. The witness half is one server'scollection_logrows for ten minutes behindidx_collection_log_time (server_id, collection_time), filtered on name and status, counted — a few hundred rows at most on the busiest cadence, and the xmin-horizon read has run this exact CTE under the same deadline since #3537. No LIMIT (a limit on a count is an undercount), no ORDER BY, no join key beyond the constanttrue. Executed against a throwaway PG18 + TimescaleDB 2.28.1 container:EXPLAINshows the witness as an Index Scan on the chunk'sidx_collection_log_timewith the two filters, chunk exclusion on both hypertables; the three planted scenarios returned(NULL…, 0)for nothing-at-all,(NULL…, 3)for log-only with the ERROR, out-of-window, other-collector and other-server log rows all excluded, andbtreepage 600083/300403+BufferIo 3154/8with3on each row for two events across both Aurora casings with out-of-window, other-server and non-poison rows excluded. Container torn down.Darling/README.md's engine-coverage table now says both hosts clear only on an observed window and names the PG witness.Alternatives rejected
pg_wait_statsrow for the server as the witness. The collector excludes idle-class waits and a quiet instance can legitimately write nothing for ten minutes; the log is the only source that records a run that stored nothing.observed_intervalsto the PG row like SQL Server'sPoisonWaitAccumulation. A per-subject count is zero for exactly the quiet subject the clear is about; the witness has to be collector-level.Item 2 — A8e's engine half: three alerts fire with
Severity: null#3635 made the grids and
get_alert_historyrender whatever tier the fire site persisted, and named the three metrics whose fire sites persist nothing: every Deadlocks Detected row rendered red (one deadlock and a hundred identically), every High CPU row amber (a 100% fire indistinguishable from an 80% one while the card beside it was red), every tempdb Space row amber by name. Grading is engine work, and the rule was: existing measured bars only, no new constants without lineage.Deadlocks Detected — graded on the #3368 rate tiers, the same instrument the health band uses. The count knob (
DeadlockCountThreshold) still decides whether it fires, unchanged. The fire is then graded byServerHealthClassifier.DeadlockSeverityover the engine's ownRollingCountWindowHours(one hour — so the count IS the hourly rate and the band's one-hour minimum window is met by construction): Critical atDeadlockCriticalPerHour(shipped 20/hr — placed inside the measured empty interval [16, 89] of 14,448 production server-hours), Warning otherwise (the band's Healthy maps to Warning too: a delivered alert the operator asked for is never rendered as nothing). The pair is plumbed, not hard-coded:IAlertEngineSettingsgainsDeadlockRateThresholds, which Darling'sDarlingAlertSettingsalready exposed from its V120 store knobs (deadlockWarnPerHour/deadlockCriticalPerHour,update_alert_settings) — an operator who raised the card's Critical tier to 31/hr must see the alert grade Warning at 25 exactly as the card bands that hour. Lite has no such knob and returnsDeadlockRateThresholds.Default, the pair its own card bands on; the pass-through census pins that as deliberate. Not a new knob: an existing one, honoured by one more surface. The four test fakes that implement the interface gained the member (no default implementations, by rule).High CPU — Critical at the CPU health band's Critical bar, and what its lineage is, plainly. The product has two CPU ladders: the analysis scorer's
CPU_SQL_PERCENT(75, 95), a scoring formula with no stated measurement, andServerHealthThresholds.CpuWarningPercent/CpuCriticalPercent(80, 95), the ONE ladder the fleet card,get_fleet_overview,/api/fleetand the Performance Calendar's high-CPU day count band on, pinned across both SKUs by #3539 A2. Neither is a fleet percentile the way the deadlock tiers are, and the code says so — this is not a claim that 95% is measured; it is the claim that the alert row and the card must not disagree about the colour of the same minute. The quantity is the engine'salertCpuValue, SQL Server's ring-buffer percent of a FIXED host (this arm never sees a PostgreSQL target;cpu_utilizationhas no row for one, and the PI/ACU reading is the Darling host's own CPU arm). High CPU passed no context before; it now passes one whose only member is the grade —Detailsempty,DetailTextthe same hand-built block byte-for-byte — because Lite's deliverer persists only the context, so a tier on the outcome alone would never reach a Lite history row. An operator whose threshold is 97 sees every fire Critical, which is a coherent reading of a knob above the bar (the deadlock record's Critical tier is likewise not floored at its Warning tier).tempdb Space — an explicit Warning, and only Warning, with the reason stated. The candidates for a Critical tier were checked and declined at the fire site: the target-volume family's
DiskCriticalFreePercent/Gbare operator knobs about a VOLUME's free space (and the tempdb volume is already that alert's business); Store Disk Pressure's 10% + 50 GB floor (#3528) is a fire bar for the monitor's own disk, not a tier; and this alert's percent has two denominators (#2515 — the growth ceiling where one exists, the allocation where not), so a single "95% reserved" would mean two different distances from failure. Inventing one is exactly the folklore constant the lineage rule refuses. What changes is that the row now says Warning rather than leaving the by-name map to imply it — so when a measured bar exists it lands as a second tier with nothing else to unpick.Every graded fire sets the tier on BOTH
AlertContext.SeverityOverrideandAlertOutcome.Severity— the #3593 Poison Wait pattern: Darling's deliverer folds the outcome's severity into the context (#2090), Lite's does not, so setting both makes the two SKUs persist the same row by construction.AlertSeverity.ForMetric's three by-name arms stay, re-commented as the replay arm for pre-#3653 rows (every such deadlock row WAS red, every CPU/tempdb row WAS amber — the faithful replay, not a default);AlertHistoryRowSeverity's class doc no longer lists the three as presence-flat. Both READMEs' alert tables state the grades.Alternatives rejected
DeadlockRateThresholds.Defaultinside the engine (no interface change). Diverges from Darling's card the moment an operator moves the V120 knob — the exact card/alert disagreement this item exists to end.Blast radius
IPostgresAlertReadAdapter.GetPoisonWaitPressureAsyncreturn type changed; one implementer (DarlingPostgresAlertReadAdapter), one caller (the host), no test fakes.IAlertEngineSettingsgains a member; implementersDarlingAlertSettings(already had it),AppAlertEngineSettings(new, returns Default), four test fakes.Detailsand noIncidents. Checked every channel builder:EmailTemplateBuilder,WebhookAlertService(Teams/Slack/PagerDuty/generic) iterateDetailsand readSeverityOverride;IncidentDeliveryFilter/PerEventNotificationact only onIncidents;AlertDetailText.Flattenreturns null for an empty context exactly as for a null one. Webhookcontext_jsonfor CPU goes from{}to{"Details":[],"Incidents":null,"Severity":"Warning"}, which is the point.AlertReadFailureSurfaceTestscensus: no catch block added or removed inDarlingWorker.csorAlertEngine.cs; every await in the poison try block is still followed byreadClock.Restart().EvaluatePgDeadlocksAsync,EvaluatePgBlockingAsync,EvaluatePgLongRunningQueryAsync,EvaluatePgCpuAsync) still fireSeverity: null— outside this lane'sDarlingWorkerboundary (another lane owns that file tonight); listed for the coordinator, and the README says the PG deadlock/CPU fires are not yet graded.What it does NOT do
CHANGELOG.mdedit (entry handed to the coordinator).get_alert_settings.poison_wait.threshold_ms, the PG host's other alert arms,AlertMetricClassifier's by-name map, the deprecated Dashboard's own engine, orPostgresAlertEvaluator.EvaluatePoisonWaits's grading.Test plan
All new and changed tests are compile-verified here (
Darling.Tests,Lite.Tests: 0 warnings,-p:EnableWindowsTargeting=true) and first EXECUTED in CI (net10.0-windows). Before trusting them, every engine scenario below was run on this machine against the realAlertEnginewithAlertEngineTests' own fakes in a throwaway console under a hostilede-DEculture — 90 assertions, all green, including the pure grades at every boundary, the operator-tier refire, the CPUDetailTextbyte-identity, the tempdb resolution string, thePostgresPoisonWaitWindow.Observedtruth table and the serialized-context round trip throughAlertHistoryRowSeverity. The PG SQL was executed as described under Item 1.Darling.Tests/AlertEngineTests:Deadlock_GradesWarningByDefault_AndCriticalAtTheBandsMeasuredRateTier(1/19 → Warning, 20/100 → Critical, on outcome AND context; count/knob numerics unchanged) — new.Deadlock_GradesOnTheOperatorsRateTiers_NotTheShippedPair(tiers 7/31: 25 → Warning, refire at 31 → Critical) — new.GradeDeadlockFire_IsTheBandsClassifier_OverTheEnginesOneHourWindow(boundaries; the record's floor on a default struct) — new.Cpu_GradesWarningAtTheKnob_AndCriticalAtTheBandsCriticalBar(80/94 → Warning, 95/100 → Critical; empty Details, null Incidents, DetailText byte-identical) — new.TempDb_FiresAnExplicitWarning_AndNeverGradesCritical(80%, 100% unlimited, 100% of a ceiling → Warning; the detail item intact) — new.Cpu_FiresOnlyOnTheThirdConsecutiveBreachingSample_ThenCooldownSuppressesRepeat— the "CPU passes no context" pin re-shaped to the graded context.Darling.Tests/PostgresAlertEvaluatorTests(new section):PoisonWaitWindow_IsObservedWhenTheCollectorRan_ByLogOrByRow— the witness truth table andUnobserved.PoisonWaitSql_CarriesTheCollectionLogWitness_OverTheSameWindow—collection_log/collector_name = 'pg_wait_stats'/status = 'SUCCESS', the LEFT JOIN, both binds used twice, the Postgres fleet (pgmon) has no per-target operational alerting — only tool-health and 3 outage-predictor alerts fire #2711 half intact, no LIMIT/ORDER BY, and the collector'sNameis the string counted.BothEngines_ClearPoisonWaitsOnlyOnAnObservedWindow— source pin: the SQL engine'saccumulated.Count > 0arm and the PG host'sif (!window.Observed)return ahead of the clear loop, the once-flag add and reset (the host is private and needs a live store to drive).Darling.Tests/AlertHistoryRowSeverityTests:AGradedDeadlockOrCpuRow_RendersTheTierItFiredAt_NotTheNames— new;APresenceFlatMetric_KeepsItsByNameColourrenamedARowWithNoTier_KeepsItsByNameColourwith the doc re-stated as the replay arm.Lite.Tests/LiteAlertForwardingTests: the CPU "passes no context" pin re-shaped;EngineSettings_PassThroughEveryMember_LivepinsDeadlockRateThresholdsas the one non-pass-through member.Builds, all zero warnings:
PerformanceMonitor.Notifications,PerformanceMonitor.Alerting,PerformanceMonitor.Darling.Service,Lite(-p:EnableWindowsTargeting=true),Darling.Tests,Lite.Tests.Which component(s) does this affect?
How was this tested?
See the test plan: compile-verified on macOS; engine scenarios executed locally against the real engine and fakes; the PG read executed against throwaway PG18 + TimescaleDB with EXPLAIN. No SQL Server was touched — the store-side read runs against the PostgreSQL store, and the engine changes are pure grading of values the engine already had.
Checklist
dotnet build -c Debug)