Skip to content

Database File Growth's rise arm fires once per hourly observation, not once per cooldown: the adapter returns the collection that produced the growth and the engine remembers which one it already reported per file (#3636) - #3639

Merged
erikdarlingdata merged 1 commit into
devfrom
fix/3636-lane
Sep 18, 2026

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

The mechanism

CheckFileGrowthAsync has two gates on one read. The LEVEL gate — a file at ≥ X% of its volume — is a standing level: it is true on every pass whether or not new data has landed, and it re-fires on the cooldown by design. The RISE gate is not. Its input is a stored delta between two collections of database_size_stats — the newest sample and the oldest inside the lookback window — and that collector runs hourly. The row becomes true the instant the hourly collection lands and then reads byte-identical until the next one does. The engine re-asks every ~30 s; the cooldown is 5 minutes; at pass granularity "still growing" and "no new data yet" are indistinguishable. Nothing carried the observation's identity from the adapter to the engine — the adapter's CTE selected c.collection_time for the window-width arithmetic and never projected it — so the engine took the second for the first, once per cooldown expiry: up to twelve cards for one growth event before the next collection replaced the observation.

This is #3579's mechanism exactly, one condition over, at a worse ratio (60-minute cadence against a 5-minute cooldown, versus 5–15 against 5). #3628's census named it as the one sibling with the identical shape whose fix was not the two-line guard because the stamp was not available to the engine. This PR plumbs it through both adapters and the info type exactly as #3628 did for forced plans, and applies the same observation-keyed guard.

The evidence

The #3579 census, verified at source: DatabaseFileGrowthSql (Darling) and Lite's DuckDB twin both compute growth_mb from two rows and project fourteen columns, none of them a clock; DatabaseFileGrowthInfo carries no stamp; the engine's only per-server state for the condition is a cooldown DateTime and an active bool, and its DTO says so in as many words — "no per-file state to keep, survive a restart, or leak." That sentence was true and is why the condition could not tell one hour's observation from the next pass's re-read of it. Replayed on a timescale/timescaledb:2.28.1-pg18 rig below.

Why this shape

The adapter returns the observation's identity. DatabaseFileGrowthInfo.ObservedAtUtc is the newest sample's collection_time, appended as the fifteenth column observed_at in both SKUs' SQL — appended, not inserted, so the fourteen ordinals both readers already bind do not move. It is the CTE's own collection_time, which current_files / windowed already selected; no new source column, no change to the window bound or the access pattern. Read back Kind-Unspecified (naive-UTC storage) and stamped Utc, as #3628's is. Lite's inline statement is lifted to a public const DatabaseFileGrowthSql beside ForcePlanFailuresSql so it can be pinned; the method body is otherwise unchanged.

The engine fires the card when at least one breached file is news. The card is per server and carries every breached file, so the guard is a question about the SET: a file breaching the level gate is always news (a standing level, untouched); a file whose only breach is the rise gate is news only if its stamp is newer than the one this file last fired on. "Not newer" rather than "not equal", so the contract reads as stated — once per NEW observation — and an older stamp (nothing produces one today) is not mistaken for news. A null on either side never matches: a stampless row keeps the pre-#3636 cooldown-repeat rather than degrading to silence, and a file that has not fired yet is always eligible.

Per-file memory beside the per-server state, not a per-server stamp. Files enter and leave the breached set independently, and every row from one read need not share a stamp (DISTINCT ON / ROW_NUMBER takes each file's own newest row). A single per-server stamp would let a threshold lowered mid-hour silence a file that had never been reported. The memory is keyed database|file, the same key FileGrowthIncidents fingerprints on, so the memory and the occurrence accumulator agree about what a file is. Entries are pruned as files leave the breached set; the server's recovery clears the map. The gate predicates are split out of GetBreachedFiles into BreachesRiseGate / BreachesLevelGate and the breach list calls the same two, so the engine's classification cannot disagree with the list that put the file on the card. BreachesRiseGate takes the rate and lookback and applies #3631's FileGrowthRiseBarMb — this branch was rebased onto #3631 and the guard sits on its code.

Stamped on fire only, including a muted fire. LastAlerted, not LastSeen — #3628's lesson: a collection landing inside the cooldown must still fire when the cooldown elapses, and folding the two memories would have recorded it as seen and never fired it. Pinned. A muted fire stamps the observation as it stamps the cooldown: the operator muted the server's file growth, not the engine's memory of it.

Restart behaviour, stated. In-memory like the rest of this family and like the forced-plan memory it copies; a restart empties it, so the first pass after one may re-fire once for an observation still in the window — exactly what the cooldown clock, also emptied, did before this PR. The guard never makes a restart noisier than it was.

What it does NOT do

Tests

Executed locally (the mactest pattern — a net10.0 runner named Darling.Tests over the built Alerting/Service/Storage assemblies; Alerting is not WPF):

  • AlertEngineTests: all 121 pass (post-The File Growth threshold means one thing — megabytes per hour — at every surface that shows it, so a 10 GB bar stops meaning 10 GB per five minutes on one store and per day on another (#3539 A8c) #3631 count). Nine new pins on the file-growth condition:
    • the same hourly observation re-read across twelve cooldowns → one card; the read still happens on every pass (12 fetches); the next collection with no rise resolves it as before;
    • a newer observation with a rise → a second card carrying ITS growth, and that observation re-read past a cooldown is one card too;
    • a newer observation without a rise → no second card, the recovery is announced;
    • the LEVEL arm still re-fires every cooldown on the same stamp, and a rise-only file riding the same server's card does not silence it;
    • a new observation landing inside the cooldown fires once the cooldown elapses (seen ≠ alerted);
    • a muted fire stamps the observation, so lifting the mute does not replay it;
    • recovery forgets the observation, so a later episode fires;
    • the memory is per FILE: tempdev fires at 06:00, templog at 07:00 (no recovery, the server still breaches), and the 07:00 observation re-read is silent;
    • a stampless row keeps the pre-Database File Growth's rise arm re-fires every cooldown against the same hourly observation: up to twelve cards per growth event — #3579's mechanism, one condition over #3636 cooldown-repeat.
    • Negative control: with anyNewObservation removed from the fire condition, five of the nine fail (the four that pass on both are pins of preserved behaviour).
  • DatabaseFileGrowthReadTests (new, live-postgres collection): two ungated shape pins (observed_at is the 15th and last column and is c.collection_time; raw table, bound clock), and the gated round trip against a live timescale/timescaledb:2.28.1-pg18 rig (PG 18.4, TimescaleDB 2.28.1): store migrated from V1, two hourly collections of one file plus a single-sample neighbour seeded, the adapter returns growth 20,480 MB over 60 min with ObservedAtUtc tick-equal to the seeded newest collection_time, Kind Utc; the re-read returns the same stamp; a third collection moves it. Also replayed the shipped statement by hand on the same rig: observed_at = 2026-09-18 07:00:00, last column, the fourteen preceding columns unchanged. Rig torn down.
  • FileGrowthRiseUnitCensusTests (The File Growth threshold means one thing — megabytes per hour — at every surface that shows it, so a 10 GB bar stops meaning 10 GB per five minutes on one store and per day on another (#3539 A8c) #3631's nine-surface census) — all pass on this branch.
  • Lite.Tests.FileGrowthReadTests (new): executed here via the net10.0-windows runner with the WindowsDesktop framework entry removed from the runtimeconfig — the real DuckDB read returns the rise stamped with the newest collection to the tick, the re-read returns the same stamp, the next collection moves it. FileGrowthAlertTests all pass except the parity pin, which walks up to the repo root and cannot from an out-of-tree harness; its assertions were replayed in Python against the two source texts and hold.

CI is first execution for FileGrowthAlertTests.BothSkusFileGrowthReads_CarryTheObservationStamp_Last through the real test project (the walk-up), and for the net10.0-windows Lite.Tests assembly as CI builds it. Every touched project builds -c Release -p:EnableWindowsTargeting=true with 0 warnings: Alerting, Darling.Service, Lite, Darling.Tests, Lite.Tests.

Related: #3579, #3628, #3631.

Closes #3636

…t once per cooldown: the adapter returns the collection that produced the growth and the engine remembers which one it already reported per file (#3636)

The rise gate's input is a stored delta between two collections of database_size_stats, which lands hourly; the engine re-asked every ~30 s and re-fired on every 5-minute cooldown against the same two rows — up to twelve cards per growth event. #3579's mechanism, one condition over, at the worse ratio.

Both SKUs' reads project the newest sample's collection_time as observed_at, appended last so the fourteen bound ordinals do not move. DatabaseFileGrowthInfo gains a nullable ObservedAtUtc. The engine keeps, per (server, database, file), the stamp it last fired on beside the per-server cooldown clock and active flag, and declines to send the card when no breached file is news: a level breach is always news (standing level, re-fired by design); a rise-only file is news only with a stamp newer than the one it last fired on. Stamped on fire only, including muted fires; pruned as files leave the breached set; cleared on recovery. In-memory like the forced-plan memory — a restart may re-fire once, which the emptied cooldown clock already did.
@erikdarlingdata
erikdarlingdata enabled auto-merge (squash) September 18, 2026 19:14
@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown

Reviewed the diff (AlertEngine.cs, AlertContextBuilders.cs, DatabaseFileGrowthInfo.cs, both adapters, and the new tests).

  • Correctness: the once-per-observation guard mirrors Forced Plan Failing re-fires on every cooldown against the same unchanged observation until the next collection lands: one failure, six cards #3579's forced-plan pattern correctly. Traced the per-file memory lifecycle (stamp-on-fire-only, prune-on-departure, clear-on-recovery) against several edge cases (flapping between rise-only/level-only/both, muted fire, stampless fallback, cooldown-vs-observation interleaving) and didn't find a case where it double-fires or wrongly suppresses. The <= (not ==) comparison and null-never-matches fallback are the right calls for the stated failure modes.
  • Ordinal binding: verified both DatabaseFileGrowthSql texts append observed_at as column/ordinal 14 without disturbing the 14 pre-existing ordinals in either reader (DarlingAlertReadAdapter.cs and LocalDataService.FileGrowth.cs), and the new BothSkusFileGrowthReads_CarryTheObservationStamp_Last test pins both texts against each other.
  • Lite/Darling parity: both SKUs get the identical column, identical ObservedAtUtc stamping (naive-UTC read, tagged Utc), and the same engine/guard code (shared PerformanceMonitor.Alerting project), so there's no drift here.
  • Security: no new external input handling — the column is derived server-side from collection_time, no injection surface.
  • Style: the appended-not-inserted column and AS aliasing are consistent with the existing style in both SQL texts.

Minor, non-blocking observation: GetBreachedFiles/BreachesRiseGate now recomputes FileGrowthRiseBarMb per file instead of once per call, but it's a trivial multiply/divide so not worth a change.

No blocking findings.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — reviewed the once-per-observation guard, ordinal binding on both SQL adapters, and Lite/Darling parity; no correctness, security, or parity issues found.

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