Repository navigation
Connection alerts: announce already-down servers and re-alert during standing outages (#1659) - #1674
Merged
Merged
Conversation
…standing outages (#1659) Connection alerts were pure edge detection - correct in isolation, wrong for the reporter's webhook-driven auto-heal loop: an app/service started mid-outage never announced the outage (no edge existed), and a standing outage alerted exactly once however long it lasted. Two OPT-INS, both default-off (classic behavior untouched), one shared definition (ConnectionAlertPolicy in PerformanceMonitor.Common - replaces Lite's ConnectionEdgeDetector and the inline machine in Darling's DarlingSelfAlertEvaluator; pinned from both suites, the SqlErrorClassification discipline): - Alert at first sight: a server already down on its first-ever observation announces instead of silently baselining. - Re-alert every N minutes while still down (0 = off). Re-fires deliver under the SAME "Server Unreachable" metric name so metric-keyed webhook automation re-triggers; the detail text marks the flavor. The re-fire clock stamps on DELIVERY only, so a decision suppressed by the notify toggles never consumes the window - and after a mid-outage restart, re-fire alone re-announces (no recorded down alert = due immediately) even with the startup opt-in off. Lite: settings.json + Settings window knobs beside the connection toggle. Darling: V33 columns on config_alert_settings (read live like the V20 toggle, refire clamped 0-1440), seeded by StoreConfigProvider, editable in the viewer's Settings window; schema-gate probe gains the V33 sentinel arm. No ACL/provisioning change - config_alert_settings carries table-level grants. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
erikdarlingdata
enabled auto-merge
July 26, 2026 12:31
…n-alert-refire # Conflicts: # CHANGELOG.md
erikdarlingdata
added a commit
that referenced
this pull request
Jul 26, 2026
Four store-polled self-alerts over the AG collectors' latest snapshot, evaluated on the existing per-server self-alert sweep: AG Failover (Warning) role_desc changed since the previous sweep AG Replica Disconnected (Critical) connected_state_desc crossed DISCONNECTED AG Replica Reconnected (resolved) the recovery notice for the above AG Sync Fell Behind (Warning) secondary_lag_seconds or redo_queue_size AG Database Suspended (Warning) is_suspended false -> true, with the reason State machines live in DarlingSelfAlertEvaluator behind injectable Func seams, keyed per AG grain (ag+replica, ag+database+replica) rather than per server, so two lagging databases on one host track and recover independently. Every alert still fires under the real server_id as its serverKey, so per-server delivery overrides, mute rules and history correlation are unaffected. First sighting of any replica or database is a silent baseline (the ConnectionAlertPolicy discipline), and a NULL state string is skipped rather than read as a transition - WSFC quorum loss nulls the AG catalog views wholesale, and treating that as an edge would spray alerts across the fleet at the worst possible moment. JudgeAgSync returns three states, not a bool, and that is the load-bearing design decision. secondary_lag_seconds reads 0 - not NULL - while data movement is SUSPENDED, so the seconds trigger has to abstain on a suspended row. If abstaining were a plain "not behind", the caller would read it as recovery: a lagging database that got SUSPENDED, or whose columns went NULL under quorum loss, would have emitted "AG Sync Recovered - has caught up with the primary" in the same sweep that reported it suspended. Only a database this sweep MEASURED as caught up resolves a standing alert. That also makes cross-server resolution structurally impossible rather than something a prefix check has to catch. Store-backed settings (V35 on config.config_alert_settings, all NOT NULL DEFAULT): notify_ag_health (true), ag_lag_alert_seconds (300, clamp 0-86400), ag_redo_queue_alert_kb (0 = off, clamp 0-1073741824). Wired through DarlingConfig, DarlingAlertSettings, StoreConfigProvider, ViewerDataService and the viewer Settings window on the #1674 pattern; read live, so an edit takes effect on the next sweep with no restart. Each grain is read by its own command and gated on its OWN snapshot time. The two were briefly read as two statements over one command to save a round trip; that cannot work. Npgsql only splits multi-statement text into a batch when it parses the SQL for NAMED placeholders - with the POSITIONAL ($1) parameters every read in this file uses, it sends one extended-protocol Parse and PostgreSQL rejects it with "cannot insert multiple commands into a prepared statement". Failure-isolated as that path is, the whole family would have been silently dead with one logged error per server per sweep. Per-grain gating is also correct on its own merits: the AG collectors are scheduled independently, so a healthy replica-grain timestamp must not vouch for stale database-grain rows. The read is skipped entirely when the master switch is off, so an AG-free fleet pays nothing either way. Also fixes four pieces of adjacent drift found while wiring this up: - AlertMetricClassifier.IsResolution recognized only Cleared/Resolved/Restored, so Darling's "Collection Resumed", "Agent Restarted" and "Compression Job Recovered" recoveries have been shipping styled as live actionable alerts in both apps' Alert History grids. Added Resumed/Restarted/Recovered/Reconnected rather than letting the AG recoveries become a fifth blind spot; no actionable metric name in either app contains those words. Both hand-maintained SQL copies of that suffix list (Darling's DailySummarySql, Lite's LocalDataService.DailySummary) are widened to match - they drive the Daily Summary actionable-alert counts in BOTH apps. - Registered all five AG metric names in the shared AlertSeverity map so a renderer reaching it without an explicit severity override does not fall through to INFO-blue (the #1136 gap). - get_alert_settings (MCP) stopped at 36 columns while the store grew to 41, so an MCP client could not see the V33 connection opt-ins at all. Extended to the full 41. ViewerControlPlaneStage3bTests - the parity test that pins the column list against the upsert placeholders, the bind order and the reader ordinals, i.e. the guard for the highest-risk defect class in this plumbing - had drifted the same way; extended, and its placeholder loop now runs off Columns.Length so the literal cannot drift again. - The Darling suite had a real ~1-in-6 intermittent failure, and it was not the AG work: ViewerTimeHelper keeps the display mode and UTC offset in process-wide statics, three test classes must mutate them, and their shared "viewer-time-statics" collection had NO CollectionDefinition - so it grouped its members without constraining them, and xUnit ran it in parallel with every other collection. Any viewer test rendering a timestamp could observe a swapped mode mid-assertion; the victim differed run to run (ViewerSystemEventsTests, then ViewerWave3DisplayTests), which is the shape that reads as "flaky, just re-run it". Added the definition with DisableParallelization, which constrains the three mutators instead of chasing an open-ended set of readers. Verified with 12 consecutive full-suite runs. TestResults/ is gitignored - trx logs from that investigation would otherwise land in commits. The two migration pin tests are re-pinned by identity rather than by ordinal and literal count - the same fact stated twice made every stacked branch collide on them. Suites green: Darling 3194, Lite 1494, Dashboard 768. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 21, 2026
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.
Closes #1659 (split out of #1535). The issue was "blocked on one answer" only to size the minimum fix — Erik's proposed shape from the #1535 thread covers both cases, and this implements both, opt-in, defaults off.
The gap
Connection alerts were pure edge detection: an app/service started mid-outage never announced the outage (no edge existed), and a standing outage alerted exactly once — which silently broke the reporter's webhook-driven auto-heal loop the day the app restarted while the server was down.
The shape
ConnectionAlertPolicyinPerformanceMonitor.Commonreplaces Lite'sConnectionEdgeDetectorand the inline machine in Darling'sDarlingSelfAlertEvaluator(theSqlErrorClassificationdiscipline — the two implementations are how these rules would drift). Pinned by identical test matrices in both suites.Server Unreachablemetric name deliberately — webhook automation keyed on the metric re-triggers, which is the point — with the detail text marking the flavor.Surfaces
settings.jsonkeys + two Settings-window controls beside the existing connection toggle.config_alert_settings(NOT NULL DEFAULT, additive) read live like the V20 toggle; seeded byStoreConfigProvider; editable in the viewer Settings window; the viewer's connect-time schema probe gains the V33 sentinel arm (mid-state pinned: tags-but-no-knobs reads 32). No ACL/provisioning change — the table carries table-level grants, no column carve.Tests
Lite 1479/0, Darling 3114/0 locally. New: policy matrix ×2 suites, evaluator startup-fire + interval re-fire + stamp-on-delivery-only (via the existing
utcNowseam), V33 migration pins, probe-arm pins.Field note: deploying this to DARLING01 applies V33 on service start (deploy sweep queued after merge).
🤖 Generated with Claude Code