Repository navigation
Add a blocking alert on total blocked wait time (#1839) - #1840
Conversation
The existing blocking gate counts blocked-process events in a rolling window, which cannot distinguish one session blocked for an hour from one blocked for a second - the count is 1 either way. This adds a second, independent gate on the SUM of wait_time_ms across the latest dmv_blocking_snapshots snapshot. BlockingWaitSecondsThreshold (int, 0 = off, OFF BY DEFAULT in both SKUs) sits beside BlockingCountThreshold under the same BlockingEnabled toggle. With the threshold at 0 the adapter read never happens at all. A snapshot sum, not a windowed one: the alert answers "how much aggregate blocked wait is happening right now", and summing a window would accumulate blocking that has already ended and could never clear while the window still held it. Level-triggered on the High CPU mechanism (fire -> cooldown re-fire while still above -> resolve on the way down) rather than the count gate's rolling-window edge trigger, so standing blocking keeps announcing itself instead of going quiet after one alert. Reports under its own "Blocking Wait Time" metric so mute rules, cooldowns and history rows never tangle with the count gate's, but carries the same blocked-process incident content, built from the rows the sweep already fetched rather than a second query. Resolution title "Blocking Wait Cleared". A stale snapshot neither fires it nor holds it active (#1812's rule, at the blocking collector's cadence): a stopped collector leaves a "latest" snapshot that would otherwise read as NOW, and a level-triggered alert on frozen rows would re-fire every cooldown forever. Staleness RESOLVES here rather than latching - the opposite of the running-jobs treatment, and deliberate: for a level condition the worse failure is staying latched on data of unknown age. Documented at CurrentBlockingWaitResult. Numerics travel as real numbers (#1830) - NumericCurrentValue is the total in seconds, never parsed back out of the display prose. Wired end to end on both SKUs. Darling's alert settings round-trip through the Postgres control plane and ApplyToConfig replaces config.Alerts wholesale, so a column absent from that round-trip would reset the knob to 0 on every worker start and the alert could never fire whatever darling.json said - hence store migration V40 (one additive NOT NULL DEFAULT 0 column), the seed/read pair, the Viewer settings window, the MCP reader, and the V40 probe sentinel + map arm. Two adjacent corrections found while building this: - DuckDB widens SUM(BIGINT) to HUGEINT, which DuckDB.NET materializes as System.Numerics.BigInteger - a type Convert.ToInt64 cannot convert because BigInteger does not implement IConvertible. The read threw at alert-check time; both stores now cast in SQL. Caught by the new round-trip test, not by review. - Lite's MCP get_alert_settings reported the blocking COUNT under the key threshold_seconds, a name copied from the deprecated Dashboard whose blocking threshold genuinely is seconds. Now threshold_count beside the new wait_threshold_seconds, which would otherwise have collided. Tests: 10 engine tests covering off-by-default (no read, no fire), the inclusive fire with correct numerics and content, cooldown suppress and re-fire while above, resolution below threshold, stale neither firing nor latching, absent snapshot, the BlockingEnabled toggle, distinctness from the count gate, and muted delivery; 3 real-DuckDB round-trip pins on the latest-snapshot sum (newest snapshot only, distinct sessions, null for an unknown server); plus the settings-forwarding and defaults parity pins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| App.AlertBlockingThreshold = blocking; | ||
| /* #1839: >= 0, not > 0 like its siblings — 0 is this setting's OFF value, so rejecting it would | ||
| make the gate impossible to turn back off once enabled. */ | ||
| if (int.TryParse(AlertBlockingWaitSecondsBox.Text, out var blockingWait) && blockingWait >= 0) |
There was a problem hiding this comment.
Lite's new AlertBlockingWaitSecondsBox is wired into LoadAlertSettings/save/JSON-export here, but three sibling call sites elsewhere in this same file that every other alert control gets were not updated for it, unlike Darling's SettingsWindow.xaml.cs, which got all three:
UpdateAlertControlStates()(~line 777) sets.IsEnabled = enabledfor every other blocking/deadlock/etc. textbox but never touchesAlertBlockingWaitSecondsBox— so unchecking "Alerts Enabled" grays out every threshold box except this new one, which stays editable.RestoreAlertDefaultsButton_Click(~line 707-728) resets every other threshold box (e.g.AlertBlockingThresholdBox.Text = "1") but never resetsAlertBlockingWaitSecondsBoxto"0"— clicking "Restore Defaults" leaves a stale/enabled value in this one field.UpdateAlertPreviewText()(~line 738-762) never adds a"blocked wait >= Xs"part, even though Darling'sSettingsWindow.xaml.cswas updated to append exactly that (if (... blockingWaitPreview > 0) parts.Add($"blocked wait >= {blockingWaitPreview}s");). Lite's preview text will silently omit the new gate even when it's configured and enabled.
None of these break the underlying alert logic (App.AlertBlockingWaitSecondsThreshold itself still round-trips correctly), but they're real, user-visible Lite/Darling parity drift and should probably be fixed to match Darling's SettingsWindow.xaml.cs behavior.
| blocked-process events — the seconds name was copied from the Dashboard, whose | ||
| blocking threshold really is seconds. Leaving it would now collide with the real | ||
| seconds threshold below. */ | ||
| threshold_count = App.AlertBlockingThreshold, |
There was a problem hiding this comment.
This rename fixes the threshold_seconds naming bug described in the PR body, but it introduces a fresh key-naming mismatch with Darling's equivalent MCP payload. Darling's DarlingMcpAlertTools.cs reports the same field as count_threshold (and uses that same count_threshold convention for deadlocks too):
blocking = new { enabled = s.BlockingEnabled, count_threshold = s.BlockingCountThreshold, wait_threshold_seconds = s.BlockingWaitSecondsThreshold },
deadlocks = new { enabled = s.DeadlockEnabled, count_threshold = s.DeadlockCountThreshold },Lite now emits threshold_count for the same value. An MCP client (or any tooling/prompt) written against one app's get_alert_settings schema and pointed at the other will see a differently-named key for the identical concept. Since this key is being renamed in this very PR anyway (to fix the threshold_seconds collision), it seems worth picking count_threshold here to match Darling's existing convention instead of introducing a new, second inconsistency.
Review summaryThis adds a second, level-triggered blocking alert on total blocked wait time (sum of Correctness / SQL: Both the Postgres ( Migration (V40): Additive Two Lite/Darling parity issues found (left as inline comments):
Minor/non-blocking: the PR notes the Postgres side of the No missing-index or other DMV-folklore recommendations here — nothing in this PR touches that surface. |
…d: program names match by PREFIX, logins exactly, the two lists default to the SQL Agent job-step prefix and the two NT AUTHORITY service logins, and the card counts what each arm removed in sessions (#3653 A5, Q5 addendum) The 7-day read of one large production store (monitor seat, authorised under Q5) split the long-running population into four classes: (1) SQL Agent job-step programs - program_name prefix "SQLAgent - TSQL JobStep", ~460 sessions/week across 11 single-server jobs, medians 35-62 min; (2) the NT AUTHORITY\SYSTEM / NT AUTHORITY\NETWORK SERVICE logins - the permanent multi-DAY background (~70 sessions across 42 servers, medians 4.8-8.6 days, 300K+ snapshots, CDC-capture shaped); (3) the application's admin login - carries the job wave but ALSO real ad-hoc long-runners, so excluding it would blind the alert to what it is for; (4) named humans - 3 sessions/week, the page's whole purpose. That read changes the knob's shape from the previous commit: - Program names match by PREFIX (case-insensitive), because job-step names embed the job id; logins match EXACTLY (a prefix would let svc swallow svc_owner). The trailing-* grammar is gone: an entry is the literal prefix / value, % and _ escaped before LIKE, * a character. The member is LongRunningQueryExcludedProgramNamePrefixes (settings.json alert_long_running_query_excluded_program_name_prefixes) so the name says the rule. - SEEDED DEFAULTS: LongRunningQueryExclusions.DefaultProgramNamePrefixes = ["SQLAgent - TSQL JobStep"], DefaultLogins = ["NT AUTHORITY\SYSTEM", "NT AUTHORITY\NETWORK SERVICE"]. Lite: the App statics initialise to the seeds and the settings.json reader replaces a list only when its key is PRESENT - so a fresh install / a file without the key evaluates with the seeds, and an operator who empties a box (the window always writes both keys) has cleared that arm for good; Restore Defaults puts the seeds back, not empty boxes. Darling: DarlingAlertSettings returns the seeds - not an empty knob - until the V135 rung gives the knob its store home, so both SKUs evaluate the same population from day one. - excluded_count SPLITS: both reads return excluded_by_program_prefix and excluded_by_login as two scalars over the candidates CTE, counted in SESSIONS (COUNT(DISTINCT session_id) - inside one collection of one server the (server_id, session_id, tran_start_time) identity collapses to session_id; a MARS session with two request rows is one session), the login count taken AND NOT the program flag so a session matching both arms counts once, under the prefix. The card's knob item shows Excluded Count, Excluded By Program Prefix, Excluded By Login and the entries. - Docs: IAlertEngineSettings, the Lite Settings window, settings.sample .json, README.md and Darling/README.md each state the four classes, the two seeds with their grounding, and why the admin login is absent. - Lite's Settings boxes are AlertLrqExcluded*Box so the #1840 wiring census sees them; they are gated on the master switch and reset to the seeds. The census's collection exception names the two new statics. Executed: the Darling read against a live PostgreSQL 18 store (the E2E's four-class snapshot: prefix arm 2, login arm 1 despite two rows, both arms once; mutation to COUNT(*) without AND NOT fails 3 != 1); the exact Lite DuckDB SQL text on DuckDB 1.5.5 (same fixture, plus the %/_ escaping and the literal-* cases); 347 Darling.Tests in the mactest harness. Partial #3653 - item 6 (Q5). Does not close it.
…utive collections, not on every snapshot; Long-Running Query gets an opt-out knob seeded from the production read — job-step program prefix, the two NT AUTHORITY logins — applied ahead of the row cap and counted by arm (#3653 A5, Q4 + Q5) (#3734) * Blocking Wait Time fired on one snapshot and cleared on the next: one snapshot at 3x the bar still fires at once, anything less holds for 3 consecutive collections through the shared AlertPersistenceGate (#3653 A5, Q4) The Blocking Wait Time alert fired on the FIRST fresh blocking snapshot whose summed wait_time_ms reached the configured bar and resolved on the first one under it. Measured on one production store class (43 servers): 102 episodes, 97 of them a SINGLE snapshot at the collector's ~72 s cadence - including the p99, 1,509 s of summed blocked wait across 145 sessions, gone by the next collection - so a plain K-consecutive gate (the tempdb shape from #3692) would have dropped exactly the episodes an operator most needs to hear about. Erik ruled Q4 on #3653: fire on a single snapshot at N = 3x the configured threshold; otherwise K = 3 consecutive collections through the shared AlertPersistenceGate. Both arms ride ONE gate: the bar handed to ObservePersistenceAsync is one sample when the snapshot alone clears the multiple and BlockingWaitBreachSamples otherwise, so a 3x fire opens the incident on the same persisted record the consecutive arm would have opened it on and the clear that follows is the same clear. The fire prepends a gate item whose `Fired By` field carries the arm token (single_snapshot_3x / consecutive_k3) so the card, the history row and get_alert_history's context_json can say why. What makes this arm unlike tempdb's: dmv_blocking_snapshot writes rows only while something is blocked, so a quiet cycle writes nothing and "the latest snapshot" simply stops advancing. Two breaching snapshots with a quiet cycle between them are two episodes, so the arm restarts a partial streak when a breaching snapshot lands more than 1.5 collector cadences after the last one counted (CurrentBlockingWaitResult now carries the cadence both adapters already resolved for the freshness verdict). An OPEN incident is not restarted by a short gap. A STALE snapshot is a clear here (the #1812 rule this arm always had - it is the collector saying the blocking ended), observed per sweep; a NULL read freezes the gate (the #3692 rule); disabling the gate is not a clear (the CPU arm's rule). The in-memory _activeBlockingWaitAlert flag is gone; the persisted Firing bit under the alert's own metric name in the existing (server_id, metric_name) table is the one "incident open" fact (no rung), seeded at startup with the cooldown stamped so a restart does not re-announce. Lite and Darling both route this alert through the shared engine, so both SKUs get the gate. Long-Running Query (Q5) is a separate commit. Partial #3653 - item 6 (Q4). Does not close it. * Long-Running Query gets an opt-out knob by program_name / login, applied in the read ahead of the row cap, and the card counts what it removed (#3653 A5, Q5) - engine + Lite; Darling's store home is a rung away Measured on one production store class: 191 distinct sessions over the 30-minute Long-Running Query bar in 7 days, the p90 of them seen in 6,192 snapshots. These are permanent background requests - brokers, replication readers, agent loops, pollers - that are over any duration threshold forever. No persistence gate can help (they persist by definition); coverage is the fix, and which programs and logins are the background on an estate is per-operator knowledge, so it is a knob. Erik ruled Q5 on #3653: the knob is allowed; the "what are they" measurement that would seed it is the monitor seat's read. IAlertEngineSettings gains LongRunningQueryExcludedProgramNames and LongRunningQueryExcludedLogins. The rule is spelled ONCE in the new LongRunningQueryExclusions (Alerting): case-insensitive, whole-value, a trailing * matches any suffix, % and _ literal, null matches nothing, a bare * refused; From/Normalize give a Settings-window string and an MCP array one meaning. An excluded session is NOT EVALUATED - not read into the decision, not counted, not fingerprinted - which is the opposite of a mute rule, and is why excluding a paging program resolves the open incident instead of silencing it. Applied IN THE READ, before LIMIT, on both SKUs. The read is longest- elapsed-first capped at maxResults (5), and the sessions this knob is for are the longest-running on the server by construction, so a post- read drop (the excludedDatabases shape) would let five permanent requests fill the cap on every sweep and hide the real query behind them - the alert made blind by the knob meant to make it accurate. Both reads gain a candidates CTE that flags each over-threshold row with the shared ILIKE ... ESCAPE predicate (BuildSqlPredicate, operands from $5), keep the unflagged rows under the cap, and return the flagged count as an uncorrelated scalar over the same CTE: one statement, one snapshot. IAlertReadAdapter.GetLongRunningQueriesAsync returns LongRunningQueryReadResult(Sessions, ExcludedCount); login_name is now projected. The High CPU card's maintenance probe passes None: a backup run by an excluded service login is still what is burning the CPU. The card appends an "Excluded Count" item - only when the knob is set - so an operator can see the knob working; a setting whose only effect is a page NOT arriving is one nobody can verify. Lite: two settings.json arrays (documented in settings.sample.json), two boxes on the Settings window's Alerts tab, AppAlertEngineSettings forwards them. Darling: DarlingAlertSettings returns EMPTY for both, with the reason in the comment - the store row is authoritative for every alert knob (ApplyToConfig replaces config.Alerts wholesale), so a darling.json-only member would read as configured and be dead; the honest home is two text[] columns on config_alert_settings, which is a migration rung this lane may not write (cross-seat fence). The MCP twins publish the knob on neither SKU yet for the same reason (McpAlertSettingsKeyTests pins the two payloads to one shape). The Darling store plumbing follows as its own commit, gated on that rung. Partial #3653 - item 6 (Q5). Does not close it. * The Long-Running Query opt-out knob is seeded from the production read: program names match by PREFIX, logins exactly, the two lists default to the SQL Agent job-step prefix and the two NT AUTHORITY service logins, and the card counts what each arm removed in sessions (#3653 A5, Q5 addendum) The 7-day read of one large production store (monitor seat, authorised under Q5) split the long-running population into four classes: (1) SQL Agent job-step programs - program_name prefix "SQLAgent - TSQL JobStep", ~460 sessions/week across 11 single-server jobs, medians 35-62 min; (2) the NT AUTHORITY\SYSTEM / NT AUTHORITY\NETWORK SERVICE logins - the permanent multi-DAY background (~70 sessions across 42 servers, medians 4.8-8.6 days, 300K+ snapshots, CDC-capture shaped); (3) the application's admin login - carries the job wave but ALSO real ad-hoc long-runners, so excluding it would blind the alert to what it is for; (4) named humans - 3 sessions/week, the page's whole purpose. That read changes the knob's shape from the previous commit: - Program names match by PREFIX (case-insensitive), because job-step names embed the job id; logins match EXACTLY (a prefix would let svc swallow svc_owner). The trailing-* grammar is gone: an entry is the literal prefix / value, % and _ escaped before LIKE, * a character. The member is LongRunningQueryExcludedProgramNamePrefixes (settings.json alert_long_running_query_excluded_program_name_prefixes) so the name says the rule. - SEEDED DEFAULTS: LongRunningQueryExclusions.DefaultProgramNamePrefixes = ["SQLAgent - TSQL JobStep"], DefaultLogins = ["NT AUTHORITY\SYSTEM", "NT AUTHORITY\NETWORK SERVICE"]. Lite: the App statics initialise to the seeds and the settings.json reader replaces a list only when its key is PRESENT - so a fresh install / a file without the key evaluates with the seeds, and an operator who empties a box (the window always writes both keys) has cleared that arm for good; Restore Defaults puts the seeds back, not empty boxes. Darling: DarlingAlertSettings returns the seeds - not an empty knob - until the V135 rung gives the knob its store home, so both SKUs evaluate the same population from day one. - excluded_count SPLITS: both reads return excluded_by_program_prefix and excluded_by_login as two scalars over the candidates CTE, counted in SESSIONS (COUNT(DISTINCT session_id) - inside one collection of one server the (server_id, session_id, tran_start_time) identity collapses to session_id; a MARS session with two request rows is one session), the login count taken AND NOT the program flag so a session matching both arms counts once, under the prefix. The card's knob item shows Excluded Count, Excluded By Program Prefix, Excluded By Login and the entries. - Docs: IAlertEngineSettings, the Lite Settings window, settings.sample .json, README.md and Darling/README.md each state the four classes, the two seeds with their grounding, and why the admin login is absent. - Lite's Settings boxes are AlertLrqExcluded*Box so the #1840 wiring census sees them; they are gated on the master switch and reset to the seeds. The census's collection exception names the two new statics. Executed: the Darling read against a live PostgreSQL 18 store (the E2E's four-class snapshot: prefix arm 2, login arm 1 despite two rows, both arms once; mutation to COUNT(*) without AND NOT fails 3 != 1); the exact Lite DuckDB SQL text on DuckDB 1.5.5 (same fixture, plus the %/_ escaping and the literal-* cases); 347 Darling.Tests in the mactest harness. Partial #3653 - item 6 (Q5). Does not close it.
… adds two text[] columns to config_alert_settings whose DEFAULT is the production read's seeds, wired through the service seed/read, the Viewer's Settings window and select/upsert, the viewer probe's top arm, and both SKUs' get_alert_settings / update_alert_settings twins (#3653 A5, Q5) (#3736) #3734 gave both SKUs the knob - program_name PREFIXES and exact logins whose sessions the Long-Running Query alert does not evaluate, seeded from the 7-day production read (SQLAgent - TSQL JobStep; NT AUTHORITY\ SYSTEM and NT AUTHORITY\NETWORK SERVICE) - editable on Lite, and on Darling the seeds and nothing else, because a Darling alert knob has no rung-free home: the store row is authoritative (ApplyToConfig replaces config.Alerts wholesale), so a darling.json-only member is dead, and McpAlertSettingsKeyTests pins both SKUs' get_alert_settings to one shape, so neither could publish it until Darling could store it. This is that store home, one rung after #3730's V134. V135: long_running_query_excluded_program_name_prefixes and long_running_query_excluded_logins, text[] NOT NULL, DEFAULT the two seeded arrays - the one knob rung on this row that ships a non-empty default, so a pre-rung row reads as exactly what DarlingAlertSettings returned before the rung and what Lite ships, and the two SKUs and the two sides of the rung evaluate one population. An operator's cleared list is stored as an explicit '{}' and honoured: present-and-empty is a decision, and nothing re-seeds it. AlertsConfig and the Viewer's AlertSettingsRow initialise to the same seeds (pinned equal to the rung's DEFAULT and to LongRunningQueryExclusions.Default*). No reload beacon (V17's trigger), no GRANT (table-level), IF NOT EXISTS. Plumbing, all at the appended ordinals: StoreConfigProvider seeds them as $69/$70 and reads them at 67-68 (a column selected but not read resets the knob on every worker start); the Viewer's select/upsert bind them as $68/$69 and read 67-68, its Settings window gets two boxes (AlertLrqExcluded*Box, so the #1840 wiring census sees them: gated on the master switch, Restore Defaults puts the SEEDS back); the viewer probe gains the V135 sentinel as the top arm (V134's top-rung claims move here, as V134 moved V133's); DarlingAlertReader reads 67-68 for get_alert_settings; update_alert_settings accepts long_running_query.excluded_program_name_prefixes / .excluded_logins through the shared normaliser (empty array = cleared arm); Lite's get_alert_settings publishes the same two keys. Every description states the two rules, the four classes, the seeds' grounding and why the admin login is not a default. TimeHonestyRungLivePostgresTests' simulated climb drops V135's columns and stamp too and expects two rungs: the applier skips anything at or below MAX(version), so a "one rung short" store must be short of the TOP, which this rung now is. Executed on this Mac: the real Darling.Tests.dll (WindowsDesktop stripped) against PostgreSQL 18 in Docker - 402/402 across the rung, probe, seed/ read, viewer bind, MCP and engine classes (the seed -> read scratch round-trip carries a cleared login list back empty and a padded prefix list back normalised; the store's column DEFAULTs read back from the catalogue as the seeds); full suite 11,451 with 191 platform reds (PresentationFramework / SqlClient ConfigurationManager / Windows paths), the same baseline as before this change. Partial #3653 - item 6 (Q5). Does not close it.
Summary
Closes #1839.
The existing blocking gate counts blocked-process events in a rolling window, which cannot tell one session blocked for an hour from one blocked for a second - the count is 1 either way. This adds a second, independent gate on the sum of
wait_time_msacross the latestdmv_blocking_snapshotssnapshot.BlockingWaitSecondsThreshold(int,0= off, off by default in both SKUs) sits besideBlockingCountThresholdunder the sameBlockingEnabledtoggle. At 0 the adapter read never happens at all.Design points, all as settled with the reporter:
NumericCurrentValueis the total in seconds; the display text ("745s across 3 blocked session(s)") is prose no parser could recover.CurrentBlockingWaitResult.Deadlocks and the existing blocking count gate are untouched.
Why the Darling surface is wide
Darling's alert settings round-trip through the Postgres control plane, and
StoreConfigProvider.ApplyToConfigdoesconfig.Alerts = view.Alerts- a wholesale replacement. A setting absent from that round-trip would be reset to 0 on every worker start and the alert could never fire, whateverdarling.jsonsaid. So the wiring includes store migration V40 (one additiveNOT NULL DEFAULT 0column), the seed/read pair, the Viewer settings window, the MCP reader, and a V40 probe sentinel + map arm with its cap-at-39 pin.Two adjacent corrections
SUM(BIGINT)widens to HUGEINT, which DuckDB.NET materializes asSystem.Numerics.BigInteger- a typeConvert.ToInt64cannot convert, becauseBigIntegerdoes not implementIConvertible. The read threw at alert-check time. Both stores now cast in SQL. Caught by the new round-trip test, not by review.get_alert_settingsreported the blocking COUNT under the keythreshold_seconds, a name copied from the deprecated Dashboard whose blocking threshold genuinely is seconds. Nowthreshold_countbeside the newwait_threshold_seconds, which would otherwise have collided with it.Test plan
Both suites build with 0 warnings, 0 errors (
-t:Rebuild); Installer.Tests deliberately not run.Darling.Tests: 3583 passed, 0 failed (191 skipped - the live-Postgres gated set).Lite.Tests: 1626 passed, 0 failed.New coverage:
AlertEngineTests): off by default (never reads, never fires), fires at/above threshold with correct numerics + metric name + context, below threshold silent, cooldown suppresses then re-fires while still above, resolves with "Blocking Wait Cleared", stale snapshot neither fires nor holds active, absent snapshot, follows theBlockingEnabledtoggle, is a distinct metric from the count gate when both trip in one sweep, and muted alerts still deliver flagged.BlockingWaitSnapshotReadTests): sums only the newest snapshot rather than history, counts distinct blocked sessions rather than rows, returns null when the server has no snapshot.Not run: any live-Postgres validation of the V40 migration (no store available in this environment) - it is a single
ADD COLUMN IF NOT EXISTS ... NOT NULL DEFAULT 0following V37's shape exactly.🤖 Generated with Claude Code