Repository navigation
Never learn a transient state as a database's expected state (#2189) - #2202
Conversation
The database-state alert baselines each database's first observed state as its accepted normal. A database swept into monitoring mid-restore therefore learned RESTORING as expected and deviated forever by being ONLINE: 636 alerts in 24 hours from 5 databases on the 52-server production fleet, with no escape but an operator re-baselining by hand. Two halves, both stores. The seed's refusal list widens from the integrity states to include the transient ones (RESTORING, RECOVERING) via a shared DatabaseStateTokens constant, so a database mid-operation stays pending and silent until it settles into a state worth learning. That governs rows that do not exist yet, so the second half applies the same rule after the fact: an AUTO-seeded baseline recording a state the seed would refuse is not a baseline anyone chose, and once the database's effective state reaches ONLINE the steady state is learned instead. Already-poisoned rows repair themselves on the next sweep with no migration, and "reset to current" pressed mid-restore -- which records whatever it sees with no filter, and always will -- now un-writes itself. What the heal does NOT touch is as load-bearing as what it does, since being wrong here means silence: - A user override. #2166's composition contract depends on it: a database parked at expected OFFLINE stays quiet while parked and still alerts when it comes back ONLINE. - An OFFLINE or STANDBY baseline, even though both are inferred. They are steady states, and leaving one is real news -- a STANDBY secondary that turns up truly ONLINE has stopped being a secondary, so log shipping is broken, and healing it would swap that alert for silence and then fire when the operator FIXED it. An auto-OFFLINE database brought up for an hour and re-parked would come back deviating forever. - The raw state_desc. A standby secondary reports state_desc = ONLINE with is_in_standby set, so matching that column would re-baseline every log-shipping secondary and alert it forever for being STANDBY. A NORECOVERY secondary, permanently RESTORING, stays silent as before; it now gets there by never being baselined rather than by baselining RESTORING. Tested watched-red across the transition matrix: onboard-during-restore then ONLINE, permanent RESTORING secondary, operator override wins, ONLINE to OFFLINE still fires, standby not healed, re-park stays quiet. The seed and heal are pinned live against real Postgres, and the two Darling copies of the SQL are pinned to share the one state list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eline # Conflicts: # CHANGELOG.md
|
I had a competing implementation of this in #2196 and have closed it — yours is better and I've said why over there. Two specifics worth calling out, in case they're useful to whoever reviews this:
Two carry-overs from my live test that may already be implied by heal-in-place, offered rather than requested: an explicit assertion that a second pass changes zero rows (this runs on every evaluation of every server), and — if any DELETE path remains — that the heal is ordered so the steady state is re-learned in the same cycle rather than leaving the row absent for one. One factual note for the changelog line: the 636 figure was measured at 15:13 UTC; the box has since drifted to 721/24h, so if you want the number to age well it's worth phrasing as "636 in 24 hours when measured" rather than a standing rate. |
|
Reviewed the diff. This is a tightly-scoped, well-tested fix — no correctness bugs found, and Lite/Darling parity looks solid. What I checked:
No blocking issues found. Nice work isolating the heal to only the machine-inferred, seed-would-have-refused states — that exclusion list is doing the real work here. |
|
Green and mergeable. Not merging, per instruction.
The Darling PostgreSQL job is the one that matters most here: the seed and the heal are pinned by gated-live tests, and those statements decide which rows exist, which no engine stub observes. Local, against a real PostgreSQL 18.4 + TimescaleDB 2.28.1 cluster: Lite 2184/2184, Darling 4471 passed / 0 failed (10 pre-existing env-gated skips). Both apps build with 0 new warnings. One thing worth a reviewer's eye, since it changed mid-PR. The first implementation used the broad rule "ONLINE overwrites any baseline the machine inferred," which is the direction the issue gestures at. Review caught two regressions it caused, and both are now pinned watched-red:
The rule that shipped is narrower and symmetric with the seed: a baseline recording a state the seed would refuse to learn is not a baseline anyone chose, so those and only those are re-learned on ONLINE. It uses the same shared constant as the seed, which also means it picked up a case the broad rule and I both missed - "reset to current" pressed during a SUSPECT outage plants an auto-baseline of SUSPECT with no state filter, and that now self-corrects on recovery too. No migration; highest remains V60. Related deferral filed as #2203. |
|
Coordination note, no action needed from you: #2204 (mine, the #2203 Lite alerted-state memory) is green and CLEAN but I'm holding it behind this PR. We overlap in three files — The overlap looks adjacent rather than semantic — you change the seed's exclusion list and add the heal; I add the memory columns to the deviation read plus a store-derived clear that runs before the seed. Both want to be before the seed, so that's the one spot worth a glance when I rebase. I'll take the merge conflict rather than hand it to you. Flagging one thing in case it's useful while you're in there: my #2204 adds Lite schema v53. If your PR ends up needing a schema change too, we'd collide on the version number, and the initializer's ascent rule means a stale number silently skips rather than failing loudly. Doesn't look like it does from the file list, just naming it. |
1 participant
Fixes #2189.
The bug
The database-state alert baselines each database's first observed state as its accepted normal. A database swept into monitoring mid-restore learned
RESTORINGas expected, and then deviated forever by being healthy.On the 52-server production box that was 636 alerts in 24 hours from 5 databases, every one reading
Expected: RESTORING, Current: ONLINE, with no escape but an operator re-baselining by hand.This builds on #2182 (issue #2166), merged one day ago. That fix is untouched:
RepeatsAreNoisekeys on the current state, which is ONLINE here, so its edge trigger never engaged for this shape. The alert engine is not modified at all -- the whole fix is in the two stores' SQL plus one shared constant.The fix
Half one, the seed. Its refusal list widens from the integrity states to include the transient ones (
RESTORING,RECOVERING), via a sharedDatabaseStateTokens.NeverBaselinedSqlList. A database mid-operation stays pending and silent until it settles into a state worth learning; when the restore completes, ONLINE is what gets learned.Half two, the heal. The seed only governs rows that do not exist yet, so the same rule is applied after the fact: an AUTO-seeded baseline recording a state the seed would refuse is not a baseline anyone chose, and once that database's effective state reaches ONLINE the steady state is learned instead. Already-poisoned rows repair themselves on the next sweep -- no migration, no manual step -- and the route that stays open forever is covered too, since "reset to current" pressed during a restore records whatever it sees with no state filter at all.
What the heal deliberately does NOT touch
Being wrong on this side means silence, so the exclusions are the load-bearing part. A first pass used the broader rule "ONLINE overwrites anything the machine inferred"; review found two regressions it caused, both now pinned watched-red:
state_desc. A standby secondary reportsstate_desc = ONLINEwithis_in_standbyset, so matching that column would re-baseline every log-shipping secondary and then alert it forever for being STANDBY.The #1986 property survives: a NORECOVERY secondary, permanently
RESTORING, stays silent forever. It now gets there by never being baselined rather than by baselining RESTORING, and an operator who wants deviation coverage on one sets its expected state explicitly (which is an override, and therefore honoured).Test plan
Full transition matrix, each watched-red before being made green:
RESTORINGbaseline (planted directly, as no code path can write one now) heals to ONLINEAlertEngineTests.DatabaseState*unchanged and greenSuites: Lite 2184/2184, Darling 4456 passed / 0 failed (10 pre-existing env-gated skips). Both apps build with 0 new warnings.
The seed and the heal are pinned by gated-live tests against a real PostgreSQL 18.4 + TimescaleDB cluster (the
darling-pgrig), not just harness stubs -- these statements decide which rows exist, which no amount of engine stubbing observes. A source-text guard pins that both Darling copies of the SQL (service and viewer, which cannot reference each other) share the one state list, for the seed and the heal.No migration; highest remains V60.
🤖 Generated with Claude Code
Filed while working this, so it is tracked rather than living only in a changelog caveat: #2203 - Lite still has no persisted alerted-state memory, so every edge-triggered database state repeats forever there. That is #2166's deferred parity, but it gained teeth here: because this PR deliberately does NOT heal an auto-baselined OFFLINE row, the "park, bring up for maintenance, re-park" path alerts once in Darling and every cooldown forever in Lite.