Skip to content

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

Merged
erikdarlingdata merged 1 commit into
devfrom
fix/3653-alert-engine-riders
Sep 18, 2026
Merged

erikdarlingdata merged 1 commit into
devfrom
fix/3653-alert-engine-riders

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

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_history and 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.EvaluatePgPoisonWaitAsync read 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_stats lands a THREADPOOL row every cycle on any server old enough to fire the alert, so accumulated.Count > 0 means "the collector ran". pg_wait_stats does 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.PoisonWaitSql gains a captures CTE counting the pg_wait_stats collector's own collection_log SUCCESS 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 joined captures LEFT JOIN accumulated ON true so 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. GetPoisonWaitPressureAsync now returns PostgresPoisonWaitWindow(Waits, CapturesInWindow) with Observed => 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.EvaluatePoisonWaits is 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's collection_log rows for ten minutes behind idx_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 constant true. Executed against a throwaway PG18 + TimescaleDB 2.28.1 container: EXPLAIN shows the witness as an Index Scan on the chunk's idx_collection_log_time with 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, and btreepage 600083/300403 + BufferIo 3154/8 with 3 on 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

  • Hold whenever the subject has no row (the literal SQL Server port). Never clears a storm that ended — see the collection health: distinguish STOPPED (gone dark) from FAILING (still erroring) #2694 skip above.
  • Any pg_wait_stats row 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.
  • Add observed_intervals to the PG row like SQL Server's PoisonWaitAccumulation. A per-subject count is zero for exactly the quiet subject the clear is about; the witness has to be collector-level.
  • A second read for the count. One more alert-path read per PG server per sweep for a number the same window bind already scopes; the CTE join costs one aggregate row.

Item 2 — A8e's engine half: three alerts fire with Severity: null

#3635 made the grids and get_alert_history render 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 by ServerHealthClassifier.DeadlockSeverity over the engine's own RollingCountWindowHours (one hour — so the count IS the hourly rate and the band's one-hour minimum window is met by construction): Critical at DeadlockCriticalPerHour (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: IAlertEngineSettings gains DeadlockRateThresholds, which Darling's DarlingAlertSettings already 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 returns DeadlockRateThresholds.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, and ServerHealthThresholds.CpuWarningPercent/CpuCriticalPercent (80, 95), the ONE ladder the fleet card, get_fleet_overview, /api/fleet and 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's alertCpuValue, SQL Server's ring-buffer percent of a FIXED host (this arm never sees a PostgreSQL target; cpu_utilization has 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 — Details empty, DetailText the 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/Gb are 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.SeverityOverride and AlertOutcome.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

  • Grade deadlocks on DeadlockRateThresholds.Default inside 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.
  • Use the scorer's (75, 95) for CPU. No lineage, and it is not the ladder any card renders.
  • Leave High CPU Warning-only. Defensible under the strict reading of "measured", but it keeps the card red and the row amber for the same 100% minute, which is the lie A8e named; the comment states exactly what the 95 is and is not.
  • A tempdb Critical at "≥ 95% of a measured ceiling". Unmeasured, and blind on the unlimited-files arm.

Blast radius

  • IPostgresAlertReadAdapter.GetPoisonWaitPressureAsync return type changed; one implementer (DarlingPostgresAlertReadAdapter), one caller (the host), no test fakes.
  • IAlertEngineSettings gains a member; implementers DarlingAlertSettings (already had it), AppAlertEngineSettings (new, returns Default), four test fakes.
  • High CPU now delivers a non-null context with empty Details and no Incidents. Checked every channel builder: EmailTemplateBuilder, WebhookAlertService (Teams/Slack/PagerDuty/generic) iterate Details and read SeverityOverride; IncidentDeliveryFilter/PerEventNotification act only on Incidents; AlertDetailText.Flatten returns null for an empty context exactly as for a null one. Webhook context_json for CPU goes from {} to {"Details":[],"Incidents":null,"Severity":"Warning"}, which is the point.
  • Deadlocks/tempdb: the builder's context (never null when the gate fires) gains the tier; details untouched.
  • AlertReadFailureSurfaceTests census: no catch block added or removed in DarlingWorker.cs or AlertEngine.cs; every await in the poison try block is still followed by readClock.Restart().
  • The PG host's other arms (EvaluatePgDeadlocksAsync, EvaluatePgBlockingAsync, EvaluatePgLongRunningQueryAsync, EvaluatePgCpuAsync) still fire Severity: null — outside this lane's DarlingWorker boundary (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

  • Does not close Brains-review campaign: deferred structural residue (from #3538 / #3539 / #3540 / #3541) #3653; two checklist items only.
  • No migration rung, no new knob, no CHANGELOG.md edit (entry handed to the coordinator).
  • Does not touch 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, or PostgresAlertEvaluator.EvaluatePoisonWaits's grading.
  • Does not change when any of the three alerts FIRES — only the tier the fire carries.

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 real AlertEngine with AlertEngineTests' own fakes in a throwaway console under a hostile de-DE culture — 90 assertions, all green, including the pure grades at every boundary, the operator-tier refire, the CPU DetailText byte-identity, the tempdb resolution string, the PostgresPoisonWaitWindow.Observed truth table and the serialized-context round trip through AlertHistoryRowSeverity. 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 and Unobserved.
  • 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's Name is the string counted.
  • BothEngines_ClearPoisonWaitsOnlyOnAnObservedWindow — source pin: the SQL engine's accumulated.Count > 0 arm and the PG host's if (!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_KeepsItsByNameColour renamed ARowWithNoTier_KeepsItsByNameColour with the doc re-stated as the replay arm.

Lite.Tests/LiteAlertForwardingTests: the CPU "passes no context" pin re-shaped; EngineSettings_PassThroughEveryMember_Live pins DeadlockRateThresholds as 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?

  • Lite
  • Darling
  • Lite Tests
  • Darling Tests
  • SQL collection scripts
  • Documentation
  • Full Dashboard (deprecated)
  • CLI Installer (deprecated)

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

  • I have read the contributing guide
  • My code builds with zero warnings (dotnet build -c Debug)
  • I have tested my changes against at least one SQL Server version (store-side and pure-engine change; verified against the store engine it reads — PG18/Timescale — and the real engine in-process)
  • I have not introduced any hardcoded credentials or server names

…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.
@erikdarlingdata
erikdarlingdata enabled auto-merge (squash) September 18, 2026 23:12

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant