Skip to content

High CPU, Deadlocks, Poison Wait and Volume Free Space deliver Warning-to-Critical escalations at once (#5715, part A) - #5751

Merged
erikdarlingdata merged 8 commits into
devfrom
fix/5715-warning-to-critical-escalation
Oct 13, 2026
Merged

erikdarlingdata merged 8 commits into
devfrom
fix/5715-warning-to-critical-escalation

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Claude posting for Erik Darling.

Fixes #5715: part A (the four alerts), the backoff fix (all six alerts) and part B (the restart seed).

Why

High CPU, Deadlocks Detected, Poison Wait and Volume Free Space have a Warning tier and a Critical tier. A Warning that worsened to Critical inside the alert cooldown was held until the cooldown ended. #5709 built TierEscalation for Runnable Queue; this applies it to the four older alerts in the shared PerformanceMonitor.Alerting/AlertEngine.cs, which Lite and Darling both run.

What changes

Each alert gets its own TierEscalation instance, keyed like its cooldown dictionary. All four follow the Runnable Queue pattern: IsEscalation before the gate, the cooldown bypassed on an escalation, EscalationBypassesCooldown set on the context, Highest then Record before the send (muted fires too), Restore when every channel failed, and Close when the incident ends. suppressed, Poison Wait's fresh-collection guard and mutes are unchanged. No store reads were added.

Alert Gate before Gate after Tier (graded before the gate, same value used for the fire) Close
High CPU !suppressed && Cooldown !suppressed && (escalation || Cooldown) GradeCpuFire(value) the persistence gate's resolve edge, even when suppressed
Deadlocks Detected RollingCountAlertGate(count, threshold, watermark, Cooldown, suppressed) same call, with escalation || Cooldown as its cooldown input (its new-count test still applies; a Critical rate is a higher count than the Warning's, so it passes) GradeDeadlockFire(count, rate tiers) any sweep where the gate is not Active (window emptied or alert off)
Poison Wait !suppressed && freshCollection && Cooldown !suppressed && freshCollection && (escalation || Cooldown) findings[0].Severity (worst finding) the observed-and-quiet clear branch
Volume Free Space !suppressed && ShouldAlert && Cooldown !suppressed && (escalation || (ShouldAlert && Cooldown)); an escalation passes the worsening check too IsCriticallyLow(worst volume) the all-volumes-recovered branch

Other changes:

  • TierEscalation.s_optedIn now lists all six metric names; its doc comment, the AlertContext.EscalationBypassesCooldown comment and the engine field comment name them. The restart-gap paragraph is left for part B.
  • The email, webhook, IncidentCooldown, RepeatDeliveryBudget and Darling viewer toast cooldown already read the flag or IsOptedIn generically, so they needed no code change.

Calls to check

  • Volume Free Space is keyed per server, not per volume. The brief said per volume, but the alert's cooldown (_lastLowDiskAlert), its worsening level (_lastAlertedLowDiskPercent) and its tier (the worst volume) are all per server, and it fires one alert per server. The escalation uses the same key as its cooldown dictionary.
  • Lite's tray toast: Lite has no toast cooldown of its own (SystemTrayService / LiteAlertDeliverer show whatever the engine fires), so nothing to change there. Darling's AlertToastCoordinator keys on server and metric and reads IsOptedIn, so the four metrics are picked up through the list.
  • None of the four had a "no clear path" problem; each has one (above).

Test plan

New Darling/Darling.Tests/TwoTierEscalationTests.cs (the existing RunnableQueue* escalation tests live in Darling.Tests), each sequence run for all four alerts through the real engine:

Sequence Result
Warning, then Critical inside the cooldown delivered at once, flag set, clock restarts
Critical back to Warning (and Critical again) nothing
Repeated Critical held until the cooldown counted from the Critical, then delivered without the flag
Clear, then Warning, then Critical escalates again
Every channel failed on the Critical next evaluation delivers it as an escalation with the flag; the following Critical waits
Critical that opens the incident ordinary first fire, no flag
Muted Warning (High CPU) then Critical Critical delivered at once
Volume Free Space, free percent inside the 1-point worsening step escalation delivered anyway
s_optedIn pin exactly the six metric names

Existing pins updated deliberately in RunnableQueueEscalationTests: the opted-in list moved to the new pin, the IsEscalation( / EscalationBypassesCooldown = source counts went from 2 to 6, and the "viewer keeps its cooldown for other alerts" test now uses Blocking Detected instead of Poison Wait. AlertEngineTests.DeadlockRow became internal for the new tests.

  • Darling.Tests and Lite.Tests build, 0 warnings
  • TwoTierEscalationTests, RunnableQueueEscalationTests, RunnableQueueRound2Tests, RunnableQueueIncidentReplayTests, AttentionAlertTests, IncidentDeliveryFilterTests, AlertEngineTests: 365 passed, 0 failed
  • Darling -trait Stage=Guard: 1616 passed; *Census*, *Ratchet*, DocCommentHygiene, AlertReadFailureSurface: 668 passed, 1 skipped
  • Lite -trait Stage=Guard plus Census, Ratchet, AlertIncidentRender, WebhookCooldownSeed, AlertEngine classes: 67 passed
  • ci-select.py map-select not run: it needs the CI class-to-file map artifact
  • Reverting the engine change to show the new tests fail on the old shape was not done (the opt-in pin and the flag assertions fail by construction without the change)
  • Full suites left to CI

Proposed CHANGELOG entry (whole issue, once part B lands)

SECTION: Fixed
ENTRY: - A Warning that worsens to Critical is delivered at once for High CPU, Deadlocks Detected, Poison Wait and Volume Free Space ([#5715]) - these alerts held the Critical until the cooldown ended. They now deliver it immediately, in Lite and Darling, as Runnable Queue and Client Timeouts already did. All six alerts keep the escalation across a service restart by reading the last tier sent from alert history. An escalation whose every channel failed now waits out the usual 1, 2, 4, 8 minute retry backoff instead of retrying at every check.

Backoff fix: a failed escalation backs off like any other failed send (all six alerts)

Defect. #4752's failed-send backoff lives in AfterFire: when every channel failed it records a failure in _failedSends and back-dates the family's cooldown stamp, so CooldownElapsed reopens after 1, 2, 4, 8 ... minutes (capped at the cooldown). Each escalation site calls TierEscalation.Restore when every channel failed, so the next evaluation is an escalation again, and the gate escalation || CooldownElapsed(...) skipped the back-dated stamp. With every channel down, a Critical escalation retried at every evaluation with no backoff.

Rule. An escalation skips the cooldown only when its family and key have no open failure streak in _failedSends. While a streak is open it waits for CooldownElapsed (the back-dated stamp is the retry delay). The fire that goes still carries EscalationBypassesCooldown = true, so the email and webhook cooldowns let it through. Volume Free Space: while a streak is open the escalation waits for the cooldown but still skips the worsening check (ShouldAlert).

Code. One helper, AlertEngine.EscalationOpensGate(family, key, escalation, stamps, now, cooldown), backed by a new read-only FailedSendBackoff.HasOpenStreak(family, key, nowUtc) (open = last failure no more than twice the cap ago, the same lapse rule RecordFailure uses). All six sites call it with the family string they pass to AfterFire: Runnable Queue (RunnableQueueEvaluator.MetricName), Client Timeouts (AttentionEvaluator.MetricName), "High CPU", "Deadlocks Detected", "Poison Wait", "Volume Free Space". The escalation field comment now says Volume Free Space is one alert per server (worst volume) with a per-server key. The restart seed and the TierEscalation restart-gap paragraph are untouched (part B).

Tests. One new test per alert, each running the same six steps:

Step Check
1 A Warning is delivered
2 A Critical inside the cooldown, every channel failing, goes out with the flag set (failure 1)
3 Evaluations 30 s and 59 s later (before the 1-minute delay) send nothing
4 At 1 minute the Critical goes out again with the flag, and fails again (failure 2)
5 Evaluations at +1 minute and +1:59 send nothing; the second failure waited 2 minutes
6 At +2 minutes the Critical is delivered, which ends the streak; after a clear, a later Warning then Critical escalates at once again
  • TwoTierEscalationTests.AnEscalationWhoseEveryChannelFailed_BacksOffLikeAnyFailedSend_AndADeliveredSendEndsTheStreak (theory: High CPU, Deadlocks Detected, Poison Wait, Volume Free Space)
  • RunnableQueueEscalationTests.AnEscalationWhoseEveryChannelFailed_BacksOffLikeAnyFailedSend_...
  • AttentionAlertTests.Engine_AnEscalationWhoseEveryChannelFailed_BacksOffLikeAnyFailedSend_...

Red/green. With the helper reduced to the old escalation || CooldownElapsed, all 6 new tests fail (6 of 6 failed); with the fix they pass (6 of 6).

Ran (Darling.Tests / Lite.Tests, both builds 0 warnings). Darling: *Escalation*, *AlertEngine*, *FailedSendBackoff*, *ChannelFailure*, *Census*, *Ratchet*, *AttentionAlert*, *RunnableQueue* classes: 1092 total, 0 failed, 17 skipped; -trait Stage=Guard: 1616, 0 failed. Lite: the same escalation, engine, backoff, channel-failure, Census and Ratchet classes: 93, 0 failed; -trait Stage=Guard: 501, 0 failed. No whole suites (CI decides). Lite shares AlertEngine from PerformanceMonitor.Alerting, so no Lite code differs.

(The failed-escalation backoff sentence is folded into the ENTRY above.)

Part B: restart seed

After a restart the engine forgot that a Warning was delivered, so the first Critical was a plain first fire and the email and webhook cooldowns (seeded from history) held it. Now the tier is seeded from the alert history.

Design

  • IAlertHistoryStore.GetLastFiredTierAsync(serverId, metricName, resolutionMetricName, sinceUtc, countMuted = true, tierWhenRowCarriesNone = null) returns the tier of the newest FIRE row inside the window, or null. Implemented in DuckDbAlertHistoryStore (Lite) and PgAlertHistoryStore (Darling). The interface has a default body that answers null (many test doubles and the deprecated Dashboard store implement the interface); a test in each project pins that its store overrides it (EscalationSeedHistoryStoreTests.TheStoreOverridesTheInterfaceDefault in Lite.Tests, EscalationRestartSeedTests.ThePgStoreOverridesTheReader_NotTheInterfaceDefault in Darling.Tests, which checks only the PostgreSQL store).
  • Null when: no fire in the window; a resolution row (stored under its title, e.g. "CPU Resolved") is as new as the fire or newer (an exact tie goes to the resolution, shared in AlertHistoryEscalationSeed.Resolve); the newest fire carries no tier and no fallback was given.
  • Counted: muted fires (except for Client Timeouts, whose fire site does not record a muted fire's tier in process, so it passes countMuted: false), and throttled, unconfigured and folded fires. Not counted: a fire whose every channel failed (alert_sent false and send_error not null, the engine's own EveryChannelFailed). A legacy undelivered row cannot be told apart from a throttled one and is counted. Only a Warning seeds (round 1 below), so the cost is that a Warning from such a row lets the next Critical inside the cooldown go out at once, as an escalation would.
  • The tier is read from context JSON as AlertHistoryRowSeverity.FiredAt does. Found while testing: Volume Free Space writes a tier only for a Critical fire, so its Warning rows carry none. Round 1: the engine's family table decides the tierless fallback, not the metric name. Only Volume Free Space maps a tierless row to Warning; for every other family a tierless row seeds nothing (the old name arm, AlertMetricClassifier.IsCritical, made a tierless Deadlocks or Poison Wait row Critical, which cancelled those families' escalation).
  • Server identity uses AlertHistoryServerIdentity.SeedScope, as GetLastEmailSentUtcAsync does. Storage precision: tests use whole-microsecond instants; the window bound goes to PostgreSQL as naive UTC.
  • A failed read THROWS from the store (the cooldown seeds swallow and return null; this one must be distinguishable from "no history").
  • Engine: one new optional constructor Func (escalationSeedReader, built by EscalationSeed.ReaderFor(store)), wired in Lite/MainWindow.xaml.cs and DarlingWorker.BuildAlertEngine. EnsureEscalationSeededAsync runs at the start of EvaluateCoreAsync, before any family, once per (server key, family) per process; it Restores the tier only when the instance has no entry. The window is the alert cooldown. The six gate lines and FailedSendBackoff are untouched. A failed read logs, counts under "escalation tier seed" on the read-failure surface, seeds nothing, never blocks a fire, and is retried on the next sweep for that family only. The six resolution titles are now constants (EscalationSeed) used by the engine's AlertResolution sites, with a pin that no bare literal returns.
  • The TierEscalation restart-gap paragraph is rewritten to describe the seed.
  • Lite writes no resolution rows, so there a Warning whose incident cleared inside the window still seeds. Cost: a Warning seed from a closed Lite incident can make the next incident's first Critical inside the cooldown go out at once.
  • Census pins moved deliberately: AlertReadFailureSurfaceTests counted sites 41 to 42, and AlertEngine.cs counted 17 to 18 (the new swallowed-read catch).

Red/green. With the EnsureEscalationSeededAsync call removed, EscalationRestartSeedTests fails 7 of 14 (the 3 restart cases for High CPU, Volume Free Space and Runnable Queue, plus the throws, once-per-key, retry, six-metric and counted-failure tests). With it restored, 15 of 15 pass (one wiring pin was added after the probe). For High CPU the restart without the seed is even held outright by the in-engine cooldown that the persisted state re-arms.

Test Where Covers
EscalationSeedHistoryStoreTests (14) Lite.Tests, DuckDB newest fire wins, newer resolution gives null (a later fire seeds again), window edge inclusive, muted counts or is excluded, tie goes to the resolution, failed send skipped, throttled counted, tierless fallback, identity scoping vs GetLastEmailSentUtcAsync, store overrides the default; round 1: exclusive microsecond upper bound, a resolution after the bound still closes an earlier fire
EscalationSeedLiveStoreTests (11) Darling.Tests, live PostgreSQL the same cases on PostgreSQL, including the three upper-bound cases
EscalationRestartSeedTests (22) Darling.Tests restart then a Critical delivered at once with EscalationBypassesCooldown for High CPU, Volume Free Space and Runnable Queue; no-reader control; a Critical from history is not an escalation; the seed covers all six metric names with titles and window; title-literal pin; a throwing reader leaves the fire unchanged and is counted; reader called once per key and family over many sweeps; a failed read is retried for that family only; the seed stops once the process recorded the tier; both hosts wired; round 1: see the list below

Run (this branch, rebased onto the part A backoff commit). Darling.Tests: *Escalation* 75, *AlertEngine* 294, *AlertReadFailure* 27, *Census* 548, *Ratchet* 15, *FailedSend* 123, -trait Stage=Guard 1616, all passing; the 8 live PostgreSQL store tests passed on a throwaway cluster (since stopped). Lite.Tests: *Escalation* 11, *AlertReadFailure* 2, *Census* 67, *StoreRoundTrip* 12, -trait Stage=Guard 501, all passing. Both test projects build with 0 warnings. After the final rebase I reran the non-live classes only (the 9 live tests skipped, PostgreSQL already stopped). Not run: whole suites.

Double-check: the Volume Free Space tierless-row fallback (now the only one); legacy undelivered Warning rows counted; the Lite no-resolution-row case (a Warning seed from a closed incident, see the cost above); Client Timeouts passing countMuted: false; the engine start instant is taken at construction.

Round 1 fixes (review of 2026-10-13)

The bot found that a seeded Critical can never create an escalation (IsEscalation is tier > highest) and, because Record keeps the higher tier, can only cancel the NEXT incident's Warning-to-Critical escalation. Fixes:

  • Warning only. The engine seeds nothing unless the history tier is Warning (EnsureEscalationSeededAsync drops anything else; the reader passes the row's own tier through, and a comment on EscalationSeed.ReaderFor says so). A repeated Critical after a restart is a plain first fire, which the history-seeded email and webhook cooldowns already hold.
  • Tierless fallback per family. EscalationSeedQuery.TierlessRowIsWarning comes from the engine's family table: true only for Volume Free Space. Every other family's tierless row seeds nothing. The name classifier no longer decides it.
  • Only rows from before the engine started. The engine records its start from the injected clock at construction (_engineStartUtc). The query carries UntilUtc; both stores take untilUtc as an exclusive bound (alert_time < $4). Both bounds go through AlertHistoryEscalationSeed.TruncateToMicroseconds, so a bound and a stored row compare at the column's precision; the tie rule is unchanged, and the resolution lookup is unbounded (a resolution after the bound still closes an earlier fire).
  • Bounded retry. Seeding for a (key, family) ends at the first of: a read succeeds; this process Recorded or Closed that key (TierEscalation.IsTouched, because Highest(key) is null is also true after Close); or now - engineStart >= cooldown (every pre-start row is then outside the window). With the store unreachable, reads stop after one cooldown instead of failing on every sweep forever.
  • PR body. The stale part A sentence ("a failed Critical retries on every evaluation") is removed (the backoff section supersedes it); the "a Critical delivered sooner, never one held" cost statements are replaced with the true cost (a Warning seed from a closed Lite incident can make the next incident's first Critical inside the cooldown go out at once); the Darling override test is renamed ThePgStoreOverridesTheReader_NotTheInterfaceDefault (it ran over a one-element array under a BothRealStores... name).

Red/green (mutating the engine or store, then restoring; all in EscalationRestartSeedTests unless noted):

  • Accept any non-null tier, name-based tierless fallback and no upper bound: 4 of 22 fail (the Lite no-resolution-row case, the tierless-row case, the row-after-start case, the six-metric query pin).
  • Drop the touched check: 2 of 22 fail (a read that fails and succeeds after the process closed an incident; the seed stops once the process recorded the tier).
  • Drop the one-cooldown stop: 1 of 22 fails (reads stop after one cooldown with the store unreachable, counted per call).
  • Lite store without the upper bound: 2 of 14 fail (TheUpperBound_IsExclusive..., TheUpperBound_IsMicrosecondPrecise...). The PostgreSQL pair was not mutated; it passes live.
  • All restored: 22 of 22, 14 of 14, 11 of 11 live.

Run. Both test projects build with 0 warnings. Darling.Tests: *Escalation* 85 (includes the 11 live PostgreSQL store tests, run on a throwaway cluster, now stopped), *AlertEngine* 294, *AlertReadFailure* 27, *TwoTier* 31, *RunnableQueue* 139, *AttentionAlert* 38, *Census* 548, *Ratchet* 15, *AlertHistory* 120, *PgAlert* 54, *DocCommentHygiene* 78, *SlowClassCensus* 7, -trait Stage=Guard 1616, all passing. Lite.Tests: *Escalation* 14, *AlertReadFailure* 2, *RunnableQueue* 38, *Census* 67, *AlertHistory* 95, -trait Stage=Guard 501, all passing. AfterARestart_ACriticalFromHistory_IsNotAnEscalation and the Warning-seed theories stay green. Not run: whole suites.

@erikdarlingdata
erikdarlingdata marked this pull request as ready for review October 13, 2026 09:02
Comment thread PerformanceMonitor.Alerting/AlertEngine.cs Outdated
Comment thread PerformanceMonitor.Alerting/TierEscalation.cs Outdated
Comment thread Darling/Darling.Tests/EscalationRestartSeedTests.cs
@claude

claude Bot commented Oct 13, 2026

Copy link
Copy Markdown

Two notes on the body itself rather than the code, so whoever reads this PR from the top is not misled.

The part-A "Calls to check" bullet is stale. It says:

A failed Critical retries on every evaluation, not on the failure backoff. This is the same as Runnable Queue (Restore makes the retry an escalation, which bypasses the backoff-adjusted cooldown stamp).

The backoff section further down is the correction — EscalationOpensGate (PerformanceMonitor.Alerting/AlertEngine.cs:4061-4073) now refuses to open the gate while _failedSends.HasOpenStreak(family, key, now) holds, so a failed escalation does wait out 1, 2, 4, 8 minutes. Both halves describe the same six call sites and the first one is no longer true of the tree; worth deleting the bullet rather than leaving the reader to reconcile them.

The read-failure retry is unbounded, and it is the only one of the three store-read seeds that is. EnsureEscalationSeededAsync only records _escalationSeeded[(key, metric)] on success (AlertEngine.cs:641), so a store that cannot answer is re-asked for all six families on every sweep, for the life of the process, each one logging an Error and incrementing "escalation tier seed" on the #3013 surface. EnsureWatermarksSeededAsync beside it sets _seededServerKeys[key] in every case including the catch (AlertEngine.cs:604), and the cooldown seeds degrade to null. The doc comment calls the retry deliberate and the behaviour is right — "could not read" must not look like "no history" — but the asymmetry with its neighbour is worth one sentence saying so, because the window the seed can still change an answer in is one cooldown wide and the retries continue long past it. Capping the retry at, say, the first sweep after the window would keep the correctness and bound the log.

Everything else in both accounts matched the tree; the one substantive finding is on AlertEngine.cs:636.

claude[bot]
claude Bot previously requested changes Oct 13, 2026

@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.

Verdict: CHANGES REQUESTED

The finding is inline on PerformanceMonitor.Alerting/AlertEngine.cs:636, with two body-level notes in a separate comment.

Claims

  • VERIFIED — four own TierEscalation instances, keyed like their cooldown dictionaries — AlertEngine.cs:1496-1499, all keyed on snapshot.ServerKey (:460)
  • VERIFIED — the four gate rewrites are the body's table — AlertEngine.cs:1079, 1846, 2013, 2878-2880
  • VERIFIED — deadlocks keep the new-count test, a Critical rate being a higher count — GradeDeadlockFire (:3951) is monotone in the count; the watermark is the last fire's count (RollingCountAlertGate.cs)
  • VERIFIED — email, webhook, IncidentCooldown, RepeatDeliveryBudget, viewer toast needed no change — EmailSendCore.cs:155, WebhookAlertService.cs:261, IncidentDeliveryFilter.cs:181/256, AlertToastCoordinator.cs:169-172
  • VERIFIED — Lite has no toast cooldown of its own — IsOptedIn's only product-side reader is AlertToastCoordinator.cs:170
  • VERIFIED — HasOpenStreak uses RecordFailure's lapse rule — FailedSendBackoff.cs:132-134 vs :110-113
  • VERIFIED — all six sites pass their AfterFire family string, spelled alike — AlertEngine.cs:4067 vs :4093; families match AfterFire at 1174, 1917, 2068, 2251, 2425, 2913
  • VERIFIED — an exact fire/resolution tie goes to the resolution — AlertHistoryEscalationSeed.cs:49
  • VERIFIED — Volume Free Space persists a tier only for a Critical fire — AlertEngine.cs:2891-2899
  • VERIFIED — the seed runs first in EvaluateCoreAsync, once per (key, family), window = the cooldown, Restoreing only with no entry — AlertEngine.cs:473, 619-653
  • VERIFIED — a failed read logs, counts as "escalation tier seed", seeds nothing, retried per family — :647-652 plus the census bump 41→42 / 17→18
  • VERIFIED — both real stores override the default member, both hosts wire the reader — DuckDbAlertHistoryStore.cs:348, PgAlertHistoryStore.cs:175, Lite/MainWindow.xaml.cs:342, DarlingWorker.cs:7359
  • VERIFIED — Lite writes no resolution rows — Lite/MainWindow.AlertEngine.cs:445
  • VERIFIED — the six titles are constants with a no-bare-literal pin, the tier read as AlertHistoryRowSeverity.FiredAt reads it — EscalationSeed.cs:23-28, EscalationRestartSeedTests.cs:322-336, AlertHistoryEscalationSeed.cs:53 on AlertContext.cs:890
  • UNVERIFIED — "both projects build, 0 warnings" — needs a Windows build; no dotnet build here
  • UNVERIFIED — every test count (365 / 1616 / 668 / 67 / 1092 / 93 / 501 / 75) — needs a Windows run of both suites
  • UNVERIFIED — the red/green probes (6 of 6; 7 of 14 then 15 of 15) — needs a mutated tree, built and run
  • UNVERIFIED — "the 8 live PostgreSQL tests passed" — needs DARLING_TEST_PG; the class skips without it (EscalationSeedLiveStoreTests.cs:548)
  • REFUTED — "Cost: a Critical delivered sooner than the cooldown would have, never one held." — a seeded Critical can only hold. AlertSeverityLevel is {Warning, Critical} (AlertSeverity.cs:17-21), IsEscalation is tier > highest (TierEscalation.cs:72), and Record keeps the higher tier (:85), so a Critical seed survives the next incident's Record(key, Warning) and cancels that incident's worsening. Walked through on AlertEngine.cs:636
  • REFUTED — "A legacy undelivered row … is counted; the cost is a Critical delivered sooner, never one held." — same mechanism; such a row carrying Critical is kept by AlertHistoryEscalationSeed.cs:29 and blocks the next escalation
  • REFUTED — part A's "A failed Critical retries on every evaluation, not on the failure backoff." — EscalationOpensGate (AlertEngine.cs:4061-4073) refuses the bypass while a streak is open. The body's own backoff section is the correction: a stale sentence, not a tree defect

Riskiest lines

  1. PerformanceMonitor.Alerting/AlertEngine.cs:636 — decides what crosses a restart; a Critical here is pure downside, and Highest(key) is null is also true after Close, so a late-succeeding read can seed into an incident this process already finished — UNPINNED (AfterARestart_ACriticalFromHistory_IsNotAnEscalation pins the same-incident case and stays green either way)
  2. PerformanceMonitor.Alerting/AlertEngine.cs:4066 — one gate for all six families; misreading the streak either re-fires a Critical every 30 s with every channel down or drops the bypass — pinned by TwoTierEscalationTests.AnEscalationWhoseEveryChannelFailed_BacksOffLikeAnyFailedSend_AndADeliveredSendEndsTheStreak and its RunnableQueueEscalationTests / AttentionAlertTests twins
  3. PerformanceMonitor.Alerting/AlertEngine.cs:2878-2880 — an escalation now skips LowDiskAlertGate.ShouldAlert, the only brake on a standing breach re-paging — pinned by TwoTierEscalationTests.VolumeFreeSpace_AnEscalationPassesTheWorseningCheck_WhenTheFreePercentBarelyMoved

Traps near this diff

  • "A default interface member compiles a parity gap away … its members are required." (#3640) — does not apply — both real stores override GetLastFiredTierAsync (DuckDbAlertHistoryStore.cs:348, PgAlertHistoryStore.cs:175) under reflection pins (EscalationSeedHistoryStoreTests.TheStoreOverridesTheInterfaceDefault, EscalationRestartSeedTests.cs:323); the only other implementor is the deprecated JsonAlertHistoryStore
  • "A history row wearing the colour its alert's NAME implies rather than the tier it fired at." (#3635, #3680) — applies — EscalationSeed.cs:56-58 resolves a tierless row by NAME via AlertMetricClassifier.IsCritical, true for "Deadlock"/"Poison" names (AlertMetricClassifier.cs:100-106), so such a row seeds Critical and cancels those families' escalation
  • "An alert evaluated every ~30 s against an observation that changes once per COLLECTION re-fires on every cooldown expiry." (#3579, #3636) — does not apply — Poison Wait keeps hasFreshCollection (AlertEngine.cs:2004-2013), deadlocks the new-count test (1846-1848), High CPU a gate edge needing a fresh CpuSampleTimeUtc (896-898); Volume Free Space allows one escalation per incident (2887)
  • "An alert that fires on ONE breaching sample and resolves on the next … sit behind AlertPersistenceGate at K consecutive collections." (#3692) — does not apply — the gate is untouched and FireCpuAsync is reached only from a gate edge (AlertEngine.cs:928-933)
  • "Did I send is not was it delivered … Gate on a delivery stamp in the store." (#3633) — does not apply — part B is that fix: the tier comes from config_alert_log (AlertEngine.cs:619-653) and a fire whose every channel failed is dropped (AlertHistoryEscalationSeed.cs:29)
  • "Lite and Darling ship in parity … a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding." (#3594) — does not apply — shared engine, both stores on the shared predicate/resolver, both hosts wired (Lite/MainWindow.xaml.cs:342, DarlingWorker.cs:7359), both halves tested
  • "A test that reads the clock twice measures the CI scheduler … One anchor owns both ends of an interval." (#3676) — does not apply — every new test drives one injected anchor (TwoTierEscalationTests.cs:801, EscalationRestartSeedTests.cs:151, EscalationSeedLiveStoreTests.cs:516)

Divergence (blind read vs PR body)

Three places, all one direction — the body understates what a seeded Critical can do.

  1. The body twice bounds the seed's cost at "a Critical delivered sooner … never one held", and lists the Lite no-resolution-row case under Double-check. That case is a real hole: a Critical seed cannot create an escalation and can only cancel the next one, and on Lite the read's resolution arm is structurally dead, so a closed incident's Critical stays readable for a full cooldown.
  2. The AlertMetricClassifier.IsCritical fallback is presented only as what makes Volume Free Space's tierless Warning rows seed; it equally makes a tierless Deadlocks/Poison row seed Critical, the opposite effect, unmentioned.
  3. Part A's "A failed Critical retries on every evaluation" describes a tree this PR no longer ships; the backoff section is its own correction.

No behaviour attached: "a test pins that both real stores override it" is two tests in two projects, the Darling one named BothRealStores… over a one-element array.

Reviewer checklist

  • Build both test projects on Windows and run the cited counts — the new theories lean on AlertEngineTests.Harness members (h.Muted, h.Settings.RunnableQueueEnabled, the mutable VolumeFreeSpaceInfo.FreeMb at TwoTierEscalationTests.cs:1101) only a compiler can confirm
  • Re-run the part-B red/green probe after narrowing the seed to Warning-only, and confirm AfterARestart_ACriticalFromHistory_IsNotAnEscalation still passes
  • Run EscalationSeedLiveStoreTests with DARLING_TEST_PG set — the only check of the naive-UTC window bound (PgAlertHistoryStore.cs:196) and the microsecond ties on a real server
  • Add the uncovered case: no resolution rows, a Critical fire inside the window, then a fresh Warning-to-Critical incident, asserting the Critical still carries EscalationBypassesCooldown
  • Watch the "escalation tier seed" counter against a store unreachable at startup — the retry is unbounded (AlertEngine.cs:641 records only on success)

… started, per-family tierless rule, bounded retry (#5715 round 1)

A seeded Critical can never create an escalation and only cancels the next incident's. Seed Warning only; a tierless row seeds a Warning only for Volume Free Space; read only rows before the engine start (exclusive, microsecond) in both stores; stop seeding a key when a read succeeds, the process touches it, or one cooldown passes.
@erikdarlingdata
erikdarlingdata dismissed claude[bot]’s stale review October 13, 2026 10:17

Claude posting for Erik Darling. Round 1 finding fixed in 6b0a60e: the restart seed now applies only a Warning (a Critical seed could only hold the next escalation), only Volume Free Space reads a tierless row as Warning, only rows from before the engine started count, and seeding stops once a read succeeds, the key is touched, or a cooldown has passed since start. The body's cost statements, the stale part A sentence and the test name are corrected. The follow-up review on 6b0a60e hit the 30-turn cap; CI 38378999400 is green.

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