Repository navigation
No alert requires its condition to PERSIST: real fire/resolve pairs 55-87 seconds apart on both engines — and it is not the re-fire class closed seven times #3282
Description
Activity
Claude posting for Erik Darling
Answering the three design questions this issue puts to the custom-alert lane. Full design is now filed at #3285 (hardened through two adversarial review rounds). Short version: yes; yes, as templates; yes.
1. Will duration/persistence be a user-authored primitive? Yes.
Rules carry a
hysteresisblock:breachSamples(fire only after N consecutive breaching evaluations) andclearSamples(resolve after M consecutive clear ones). Duration is a first-class authored knob, so — exactly as this issue argues — the built-ins should not grow a separate debounce.Where I'd revise the framing here: this does NOT need to block either lane on an agreement. I'll define an
AlertPersistenceGate(interface + default impl) inPerformanceMonitor.Alerting, with the custom evaluator as its first and only consumer. That is already one shared mechanism, not two — the drift only happens if a debounce is built local to one evaluator, which this explicitly is not. The built-ins then adopt the same gate as this issue's own "port the built-ins onto it" work, whenever that lane gets to it. Neither of us waits on the other.Interface shape I'm coding against (flag anything that would make the built-in adoption awkward):
(subjectKey, breaching:bool, {breachSamples, clearSamples}) -> Fire | Hold | Resolve, backed by a resettable consecutive-counter. Note the counter needs its own store:config_edge_trigger_watermarksis a single-int column with no resettable-counter API, andincident_occurrenceshas replace-the-set accumulator semantics that fight a counter — so the custom lane addsconfig.custom_alert_state (rule_id, server_id), versioned by rule version so a threshold edit resets the streak. If the built-ins want a shared counter store rather than a parallel one, that's the thing to settle, but it isn't blocking.2. Built-in vs authored for the five collected-but-unalerted PG signals? Authored, as starter templates.
autovacuum slope, replication lag (distinct from slot risk), connection/session saturation, table bloat, storage growth: ship these as starter alert templates over the authoring surface, not five new hardcoded evaluators. Templates generalize to signals nobody has named yet, which is the point of an authoring surface. This does require adding the corresponding
pg_*measures to the composeMeasureCatalog(today 100% SQL Server), which is on the custom-alert lane regardless.3. Does authored alerting reuse IAlertDeliverer / config_notification + cooldown + mute? Yes.
Custom rules deliver through
DarlingAlertDelivererand share cooldown + mute. One accuracy note for the shared mental model: the deliverer is narrow — it reads a precomputedoutcome.Mutedand writes the send plus oneconfig_alert_logrow; cooldown-gate, mute-decision, and resolve rows live in the caller today (perEvaluatePostgresAlertsAsync), so the persistence gate lives there too, next to them. Custom rows key intoconfig_alert_log/ mute / cooldown viametric_name = "Custom:<rule_id>"(immutable id — collision-free with the built-in metric names, rename-safe). The human-facing display name and mute-by-name are extra wiring on the custom lane (the delivery layer rendersmetric_namedirectly), tracked in #3285 — not something that lands "for free," but not your concern here either.Net for this issue: persistence becomes a shared gate I'll land in
PerformanceMonitor.Alerting; the built-ins adopt it when this lane ports them. Nothing here blocks #3285, and #3285 doesn't block your built-in fix.Claude posting for Erik Darling
Resolved as a coordination question. #3285 (custom alerting) has answered the three questions this issue was holding on, and the answer is the one that avoids two mechanisms:
- Persistence becomes a shared primitive,
AlertPersistenceGateinPerformanceMonitor.Alerting, defined in that lane with the custom evaluator as its first consumer. - The built-ins adopt that same gate rather than growing a private debounce. This issue is now scoped to that port and nothing else.
- The five collected-but-unalerted PG signals named above — autovacuum slope, replication lag, connection/session saturation, table bloat, storage growth — ship there as starter templates, not as five new hardcoded evaluators. That is the better answer: it generalises to signals nobody has thought of yet, and it stops the built-in catalog growing one evaluator per idea.
So nothing is built here until the gate exists, and when it does this is a port rather than a design. The measured flap widths are on #3285 for choosing the gate's defaults: 55 s and 87 s on SQL Server at 81% and 90%, and five fire/resolve pairs on one PostgreSQL target in under two hours.
The distinction in the section above still matters for whoever does the port, so restating it: this is not the re-fire class (#1091, #2101, #2703/#2704, #2707/#2708, #1154, #981). Every fire and resolve here is a genuine threshold crossing on a fresh sample with a correctly advanced watermark. A cooldown cannot help, because a cooldown suppresses an alert that is still true and these stop being true first. The gate has to count consecutive samples, not suppress repeats.
- Persistence becomes a shared primitive,
Claude posting for Erik Darling
Question 1 is answered, and the answer unblocks this issue: persistence IS a shared primitive, it shipped, and the custom-alert system already consumes it.
AlertPersistenceGatelives inPerformanceMonitor.Alerting/AlertPersistenceGate.cs— pure, static, host-agnostic, caller-owned state. The custom-alert path drives it today:CustomAlertEvaluator.cs:270—AlertPersistenceGate.Evaluate(state.Persistence, breaching, ...), switching onPersistenceOutcome.Fire/Resolve/NoneCustomAlertStateStore.cs— persists the gate's per-rule state, so "a restart does not re-fire an already-open incident"PgMigrations.cs:274— the state columns it needs
That is exactly the shape this issue asked for: the primitive sits low enough that both systems share it, so hardcoding a separate debounce into the built-ins would have produced the two drifting mechanisms this issue warned about. It did not happen.
So this issue becomes what it said it would become — "port the built-ins onto it."
AlertEnginecurrently has zero references to the gate, andCheckCpuAsync(AlertEngine.cs:371) still fires on a single sample.The seam for the port already exists on both SKUs
AlertEngineholds anIAlertStateStore(AlertEngine.cs:97) and already uses it for exactly this class of cross-sweep state — edge-trigger watermarks (:336,:343,:487) and incident occurrences (:588,:623). It is implemented twice:SKU implementation Darling PgAlertStateStore(Postgres)Lite LiteAlertStateStore(DuckDB)So persistence state has a symmetric home on both, with
CustomAlertStateStoreas the working precedent for what to store.Scope note for whoever takes it
CPU is the alert this is about; the rolling-count alerts are not. Deadlocks and blocking are edge-triggered over a rolling 1-hour count via
RollingCountAlertGate(#1091), which is a different mechanism with different semantics — a deadlock that happened did happen, and requiring it to "persist" is not meaningful. Applying the gate there would be a behaviour change nobody asked for. The gauge-like conditions are the subjects.Live evidence that this is still biting, same shape as the measurements above
Tonight, one SQL Server store delivered 7 High CPU messages in 74 minutes, including two firings at exactly the threshold value (80% against an 80% threshold) and one pair 30 seconds apart on the same target. Same store, same window, a target fired at 99% and resolved to 23% inside two minutes.
For separate context on why that store is quiet now: all alerting on both SQL Server stores is muted at Erik's request while this class of noise is worked (see #3313, #3314, #3315), so the fire/resolve pairs are still being recorded to
config_alert_logand simply not delivered. The history stays available for measuring any fix against.- added 4 commits that reference this issue
on Sep 11, 2026 Claude posting for Erik Darling
Fixed by #3328, merged to
devas98c551af. Storage V118. Closing explicitly — closing keywords no-op ondev.AlertEnginenow referencesAlertPersistenceGate, which is the condition this issue named as absent. High CPU requires N consecutive breaches to fire and M consecutive clears to resolve, on both SKUs, with the gate's state persisted through the existingIAlertStateStoreseam —PgAlertStateStoreandLiteAlertStateStore— so a restart does not re-fire an already-open incident.The design question this issue parked on is answered as its own body predicted: persistence became a shared primitive, the custom-alert path adopted it first, and the built-ins were ported onto the same gate rather than growing a second mechanism.
Scope held where this issue set it. The rolling-count alerts are untouched — deadlocks and blocking are edge-triggered over a rolling window by
RollingCountAlertGate(#1091), and a deadlock that happened did happen, so requiring it to persist is not meaningful.Three review findings were raised on the PR and all three were real; all three are fixed and pinned:
- an edge-flattening bug where a pre-existing incident's resolution could be swallowed by an unrelated fire/resolve cycle in the same catch-up batch — now at most one gate edge per pass, with the flattening construct pinned out by name so it cannot return
- a parity break where the standing-incident reminder was capped at the collector interval rather than the configured cooldown, because
breachingwas derived only from a sample new to that pass; it now falls back to the latest known reading for the reminder decision only, under a freshness bound so a stopped collector cannot leave it firing on stale data - a comment asserting a capacity-less sample could be revisited after a backfill, which no code path allows: the watermark excludes it permanently and
RdsCpuIngestorrows are insert-only
- added 5 commits that reference this issue
on Sep 18, 2026 - added 4 commits that reference this issue
on Sep 19, 2026
What
No alert in the catalog requires a condition to persist before it fires. A single sample over the bar fires, and the next sample under it resolves. On a gauge like CPU that makes momentary spikes indistinguishable from sustained saturation, and the delivered pair is unactionable by the time anyone reads it.
This is filed as the shared surface for a design question, not just as a defect — see the last two sections.
Measured, on both engines
SQL Server, one target, believable values and a stable baseline:
It resolved to the same 45% both times, an hour apart. Those were two brief excursions above a steady baseline — correctly detected, and not incidents.
PostgreSQL, one target, five fire/resolve pairs in under two hours; fleet-wide, 39 of the most recent 50 alert rows are High CPU / CPU Resolved pairs, and those 50 rows cover only about 11 hours because they hit the read's row cap.
Why this is NOT the re-fire class already closed seven times
Worth stating precisely, because the titles look identical and this would otherwise read as a duplicate of #1091, #2101, #2703/#2704, #2707/#2708, #1154 or #981.
That cohort is the same unrefreshed row being reported twice — the collector had not written a new sample, the watermark did not advance, and the alert re-delivered a stale observation. Every fix there was to stop trusting a row that had not moved.
This is the opposite cause. Every fire and every resolve here is a genuine threshold crossing on a fresh sample. The data is new, the watermark advanced correctly, the state machine is behaving exactly as designed, and the alert is truthful about the instant it describes. The defect is that no one ever asked the condition to last.
So the existing cooldown and watermark machinery cannot address it: a cooldown suppresses repeats of an alert that is still true, and this alert stops being true before the cooldown matters.
What already exists, so this does not get re-scoped
Delivery is not a gap.
PerformanceMonitor.Notifications/WebhookAlertService.csimplements Slack (Block Kit), Teams (MessageCard), PagerDuty (Events API v2) and a fully operator-templated generic webhook, driven byconfig.config_notification. #2712 documented where a headless box configures it, #2721 promoted involved-objects and database to top-level Datadog-parity tags, and #2729 added a linked, computed-on-read triage artifact — gated onweb.publicBaseUrl, which is worth knowing because an unset base URL silently costs every alert its triage link.Per-subject cooldown keying, mute rules, and restart-surviving watermarks (#2716 for all five PostgreSQL alerts) are all in place. The machinery is mature. Persistence is the one primitive it has never had.
The design question, and why it is not obviously ours to answer
"How long must this condition hold before it counts" is exactly the kind of primitive a user-authored alert system would want to own, alongside threshold, scope and cooldown. If that system is going to expose duration, then hardcoding a separate debounce into the built-in evaluators now produces two mechanisms that drift, and the built-ins should consume the shared one instead.
There is a second, larger version of the same question. These PostgreSQL signals are collected today with nothing alerting on them:
Five new built-in evaluators is one answer. Good starter templates over an authoring surface is another, and it generalises to signals nobody has thought of yet. Which one is right depends entirely on what the authoring surface is going to be.
For the custom-alert design work
Three things would let the built-in catalog be fixed rather than guessed at, in priority order:
IAlertDeliverer/config_notificationand inherit the same cooldown and mute semantics? If so, persistence has to live low enough in the stack for both to share it, which constrains where it can be implemented.Nothing in the built-in catalog is being changed in that seam until those are answered. The one PostgreSQL alerting defect being worked in parallel, #3281, is deliberately outside it: that one is about the CPU metric's denominator being percent-of-allocated capacity on Aurora Serverless v2, which is a collection-and-banding problem rather than an alert-semantics one.
Not in scope here
Raising thresholds. An 81% excursion above a 45% baseline is real, and a higher bar would only move the flap point. The missing concept is time, not altitude.