Skip to content

Darling: file growth, long-running job, failed agent job, database state and forced plan alerts are tried again after every channel failed (#4752) - #4804

Merged
erikdarlingdata merged 5 commits into
devfrom
fix/4752-retry-more-families
Sep 29, 2026
Merged

erikdarlingdata merged 5 commits into
devfrom
fix/4752-retry-more-families

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Part of #4752.

Why

An alert whose every channel failed (an HTTP 429 or 5xx, a timeout, an unreachable mail server) already retries after 1, 2, 4 ... minutes for nine SQL Server alert families instead of waiting out its cooldown. Five families still stamped their cooldown before delivery and stayed silent for the whole cooldown when nothing was delivered: Database File Growth, Long-Running Job, Failed Agent Job, Database State and Forced Plan Failing. Four of them also keep a second "already told" marker (written before delivery for file growth, failed job and forced plan; written after it, whatever the result, for database state), so a back-dated cooldown alone would have found nothing new on the retry sweep.

The failure streak had a gap of its own: only a delivery ended it, so a failure long after the condition cleared inherited the old count and started at the longer delay.

The failed-job marker is a saved watermark, and it was saved before delivery. When a fire that no channel received had no earlier watermark, the in-memory entry was removed but the saved row kept the new value. A restart inside the retry delay then read that failure as already announced, and the alert was lost for good.

What changes

  • New FailedSendBackoff (PerformanceMonitor.Alerting/FailedSendBackoff.cs) owns the streak bookkeeping AlertEngine.AfterFire did in a private dictionary: EveryChannelFailed(AlertDelivery?) (the rule, moved here), RecordFailure(family, key, nowUtc, cap) (counts a failure, returns ChannelFailureRetryDelay(failures, cap)) and RecordDelivered(family, key). It keeps the time of the last failure with the count, and a failure more than twice cap after the previous one starts again at one minute. Twice, because a live streak's gap is a little over one cap. Thread-safe, in memory only.
    • Two additions beyond that surface: a RecordFailure(..., out int failures) overload so AfterFire keeps its log line, and TrackedCount. Streaks older than twice their own cap are dropped once the table passes 1024 pairs, which changes no answer (such a streak already restarts) and stops the per-run Long-Running Job keys from growing it forever.
  • AfterFire uses one FailedSendBackoff instance. Its log line and back-dating are unchanged. ChannelFailureRetryDelay stays public static in AlertEngine. The private EveryChannelFailed now calls the shared rule.
  • The five families capture var delivery = await FireAsync(...) and call AfterFire, and each makes sure the retry sweep still has something to fire on when every channel failed:
    • Database File Growth: each breached file's observation stamp goes back to its prior value, or is removed if it had none. Without this a file whose rise is under the level gate reads as "not newer" on the retry.
    • Long-Running Job: nothing to put back. The cooldown is per run, and the stale-entry pass drops a back-dated stamp exactly when the retry is due.
    • Failed Agent Job: the in-memory watermark still moves to the newest failure before the fire, and goes back to the prior value (or is removed) when every channel failed. The saved watermark is now written after the fire, and skipped only when every channel failed. A muted fire attempts no channel, is not "every channel failed", and still saves, as before. A failed fire therefore never reaches the saved row, so a restart inside the retry delay reads the failure as not yet announced, and the old re-save of the prior value is gone because there is nothing to put back. The doc comment on IAlertStateStore.SaveFailedJobWatermarkAsync says the same. The change is in the shared engine, so Lite and Darling both get it; no store code changes.
    • Database State: SaveDatabaseStateAlertedAsync is not called when every channel failed, otherwise the retry of an edge-triggered state (a parked OFFLINE) reads as already announced. The comment above it now says a fire whose every channel failed is retried.
    • Forced Plan Failing: the plan's last-alerted observation goes back to its prior value.
  • The trade for the failed-job alert: it is now at-least-once. A crash after a send and before the save can send the same failure again after the restart. Before, the opposite crash lost it: the save came first, so a crash between the save and the send dropped a failure that no channel had received.
  • Comments that described the streak or which families retry were updated. Nothing else changes: the nine families already covered keep their markers, and Lite's deliverer reports nothing, which reads as delivered.

Test plan

  • Red first: the new tests were committed before the implementation, against the engine as it was on dev and a FailedSendBackoff that throws. 17 of the 19 new test cases fail there (AlertEngineTests: 17 failed of 230). The other two are partial-failure cases (one channel delivered, no early retry) that pass either way by design.
  • Each of the five families: every channel failed, no fire at +30 s, a fire at +61 s for the same condition, a second failure waits two minutes, and a delivered fire waits the full cooldown. File growth and forced plan also cover a newer observation after a delivered one; the failed job covers both a prior watermark and none; database state runs for OFFLINE (edge-triggered) and SUSPECT (repeats on the cooldown).
  • Failed job, saved after a delivery. Red first again: these tests were committed before the change and run against the branch as it was. 4 of the 10 FailedJobs_ tests failed there, and all 10 pass after the change:
    • a restart before the retry: every channel fails on the first fire with no earlier watermark, a new engine over the same saved state fires again on its next sweep (2 announcements expected, 1 seen before the change), then saves once;
    • the first-fire test, renamed FailedJobs_EveryChannelFailedOnTheFirstFire_NeverSavesTheWatermark_AndTheRetryFiresInThisProcess, now asserts the saved state never received the failure's time (it used to assert the saved state kept it);
    • every channel fails with an earlier watermark: the saved watermark stays at the earlier value and no save call is made (the prior-watermark test also checks that only the two delivered fires ever saved);
    • a delivered fire saves exactly once, with the newest failure's run time (the newest row is deliberately not first), and the same failure on the next sweep saves nothing more;
    • a muted fire still saves.
  • Long-running job: the retry is for the same run. Database state: after a failed fire the state is not saved as announced.
  • Partial failure gets no early retry (failed job, database state).
  • FailedSendBackoff unit tests: 1, 2, 4 minutes then capped; RecordDelivered starts over; independent per family and key; a failure more than twice the cap later starts at one minute; one exactly twice the cap later continues; pruning; the delivery rule.
  • Through the engine: High CPU fails twice (1 minute, then 2), clears, and a failure eleven minutes later (more than twice the 5-minute cooldown) retries after 1 minute, not 4.
  • Darling.Tests and Lite.Tests build with 0 warnings.
  • Every test class in either project that names SaveFailedJobWatermarkAsync or FailedSendBackoff, run again after the last edit (a comment-only change): AlertEngineTests and DarlingSelfAlertTests, with DocCommentHygiene (559 tests, 0 failed, 3 skipped), and LiteAlertForwardingTests and StoreRoundTripTests (43 tests, 0 failed).
  • Full Darling.Tests (no DARLING_TEST_PG), run once on the change before that last comment-only edit: 17379 total, 0 failed, 1140 skipped (the live PostgreSQL classes), 1 not run. The count is the earlier 17375 plus the 4 new tests.
  • Full Lite.Tests was last run on the earlier head of this branch (5607 total, 0 failed); after the change only the two Lite classes above were run.
  • Live PostgreSQL classes not run (no database for them here).

CHANGELOG

SECTION: Fixed
ENTRY:

…y after every channel failed (#4752)

The tests for the file growth, long-running job, failed agent job, database
state and forced plan alerts, the FailedSendBackoff unit tests and the
reset-through-the-engine test. FailedSendBackoff is a stub that throws, so
these fail against the engine as it is.
…ate and forced plan alerts are tried again after every channel failed (#4752)

Adds FailedSendBackoff, which owns the failure streak the engine kept in a
private dictionary: 1, 2, 4 minutes, capped at the cooldown, dropped by a
delivery, and now also started over by a failure more than twice the cap after
the last one. AlertEngine.AfterFire uses it.

The five families that #4786 left out capture their delivery and call
AfterFire, and each puts back the marker it wrote before delivery so the retry
still counts as news: the per-file observation stamps, the saved failed-job
watermark, the database-state announcement and the forced plan's observation.
The long-running job needs none because its cooldown is per run.
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review September 29, 2026 11:40
@erikdarlingdata
erikdarlingdata merged commit 04b11d6 into dev Sep 29, 2026
17 of 19 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/4752-retry-more-families branch September 29, 2026 13:13
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