Repository navigation
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
Conversation
…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
enabled auto-merge (squash)
September 18, 2026 19:14
|
Reviewed the diff (AlertEngine.cs, AlertContextBuilders.cs, DatabaseFileGrowthInfo.cs, both adapters, and the new tests).
Minor, non-blocking observation: No blocking findings. |
This was referenced Sep 18, 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
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.
The mechanism
CheckFileGrowthAsynchas 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 ofdatabase_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 selectedc.collection_timefor 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 computegrowth_mbfrom two rows and project fourteen columns, none of them a clock;DatabaseFileGrowthInfocarries no stamp; the engine's only per-server state for the condition is a cooldownDateTimeand an activebool, 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 atimescale/timescaledb:2.28.1-pg18rig below.Why this shape
The adapter returns the observation's identity.
DatabaseFileGrowthInfo.ObservedAtUtcis the newest sample'scollection_time, appended as the fifteenth columnobserved_atin both SKUs' SQL — appended, not inserted, so the fourteen ordinals both readers already bind do not move. It is the CTE's owncollection_time, whichcurrent_files/windowedalready 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 apublic const DatabaseFileGrowthSqlbesideForcePlanFailuresSqlso 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_NUMBERtakes 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 keyeddatabase|file, the same keyFileGrowthIncidentsfingerprints 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 ofGetBreachedFilesintoBreachesRiseGate/BreachesLevelGateand the breach list calls the same two, so the engine's classification cannot disagree with the list that put the file on the card.BreachesRiseGatetakes the rate and lookback and applies #3631'sFileGrowthRiseBarMb— this branch was rebased onto #3631 and the guard sits on its code.Stamped on fire only, including a muted fire.
LastAlerted, notLastSeen— #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
CHANGELOG.md.Tests
Executed locally (the mactest pattern — a
net10.0runner namedDarling.Testsover 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:anyNewObservationremoved from the fire condition, five of the nine fail (the four that pass on both are pins of preserved behaviour).DatabaseFileGrowthReadTests(new,live-postgrescollection): two ungated shape pins (observed_atis the 15th and last column and isc.collection_time; raw table, bound clock), and the gated round trip against a livetimescale/timescaledb:2.28.1-pg18rig (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 withObservedAtUtctick-equal to the seeded newestcollection_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 thenet10.0-windowsrunner 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.FileGrowthAlertTestsall 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_Lastthrough the real test project (the walk-up), and for thenet10.0-windowsLite.Testsassembly as CI builds it. Every touched project builds-c Release -p:EnableWindowsTargeting=truewith 0 warnings: Alerting, Darling.Service, Lite, Darling.Tests, Lite.Tests.Related: #3579, #3628, #3631.
Closes #3636