Darling measures its rollup floors once per store and in the background, so the first daily summary no longer waits on them (#4957)

[erikdarlingdata]

Closes #4957.

Why

On a 43-server store, a get_daily_summary call that finds the rollup-coverage cache cold costs about 13 s, and about 9.7 s of that is the coverage probe: each rollup's min(bucket) decompresses and sorts the batches of its oldest compressed chunk (query_store_stats_interval_daily alone is 6.3 s of the 9.7 s). The cache was one per data source, so the cold cost came back:

• on the process's first call, once for each data source the service opens on the store (collector, MCP host, web host);
• after a failed probe;
• every time a view's entry passed RollupFloorMaxReuse (1 h), inline, on whichever caller got there first.

What changes

The probe's SQL, RollupFloorMaxReuse (1 h) and the chunk-identity check are unchanged. What changes is who pays and how often.

1. One floor cache per store. Keyed on host, port and database from the connection string, not on the data source. Each entry also records the database's OID, read in the same round trip as the existing oldest-chunk catalog check (one extra column), and an OID that differs from the entry's counts as a changed identity, so a database dropped and recreated under the same name is measured again even when it reproduces the old chunk and hypertable names. The comment on the cache says how The rollup coverage probe writes about 170 MB of temp every five minutes by sorting two rollups' compressed batches #4539/Store: re-measure a rollup's coverage floor only when its oldest chunk changes (#4539) #4553's two goals still hold: no cross-store pollution (the key plus the OID) and nothing that grows without bound (one small entry per store the process has opened).
2. Warm after start. RollupCoverageWarmup.RunDelayedAsync (the delayed-start shape of QueryStoreIntervalWideBrinIndex.RunDelayedAsync) runs DetectRollupsAsync then DetectRollupCoverageAsync, the same two calls both production callers make, so it measures exactly the union of views they need. The worker launches it 30 s after migrations and drains it at shutdown. The Viewer is its own process and its first routed read does pay the probe inline, so it launches the same warm (3 s delay) once its store has answered the reachability, schema and seat probes. A failed warm logs once at Debug and changes nothing else; it deliberately does not go through the service's or the Viewer's cached availability answer, because a failure there would cache "no rollups" for five minutes. The Viewer has no ILogger and no Debug level, so its warm logs nothing.
3. Re-measure in the background, for up to two hours. A caller that finds an entry between one and two hours old (RollupFloorMaxReuse up to RollupFloorMaxServeAge) whose oldest chunk, hypertable and database are unchanged is served the cached floor (and ceiling) and starts one background re-measure for the store; callers that arrive while it runs, or after it lands, start none (the start re-checks the cache's current entries under the store's lock). An entry two hours old or older is measured inline by the caller, as before this PR, however long the store went without a caller. RollupFloorMaxServeAge is declared as RollupFloorMaxReuse * 2, right after it, not as a second number. A changed chunk, a different database or no entry still measures inline at any age. Staleness bound, written in the RollupFloorMaxReuse comment: at most two hours plus one probe's duration, whether or not callers keep arriving. No floor is served once its entry is two hours old; the probe is what the call that serves a floor may still spend measuring its other views before it returns.
4. A cycle writes back only the views it measured. Before, every cycle replaced the whole cache from a snapshot taken before its slow measure. With several data sources and a background writer that would let an older cycle undo a newer measurement, so the write-back now touches only the views the cycle measured (and drops views whose rollup is gone).
5. Lite parity: Lite keeps no TimescaleDB rollups. grep -rlE "continuous_aggregate|timescaledb|DetectRollup|RollupFloor" Lite Lite.Tests PerformanceMonitor.Common PerformanceMonitor.Collectors finds nothing. Nothing in Lite changed.

Pin updated deliberately: RollupCoverageCeilingLiveTests expected the first call past the hour to measure inline. It now expects that call to read the cached ceiling and waits for the background re-measure to advance it. RollupFloorsToMeasure keeps its meaning (every view that needs a measure of either kind), so its existing tests are untouched.

Decisions worth a look:

• Quiet spells. The first caller after a quiet spell no longer gets a floor of any age. A caller waits for the probe on a view only when it finds no entry, a changed chunk, or an entry two hours old or older, and then it pays the probe once, as it did before this PR. The cap also bounds the cached ceiling, which sits in the same entry and so has the same age. A retention delete inside the oldest chunk is the only way a cached floor is wrong while its chunk is unchanged, and that exposure is bounded as above.
• A call that arrives while the warm is still running measures inline, as it did before; the inline path is not single-flighted across data sources.
• The warm delays (30 s service, 3 s Viewer) are named constants on RollupCoverageWarmup.

Test plan

Tests first, at the tests-only commit for the first version of this change (the warm entry point is a compile-only placeholder there), against the old per-data-source cache:

Darling.Tests  Total: 7, Errors: 0, Failed: 6, Skipped: 0, Not Run: 0
RollupFloorCacheLiveTests.TwoDataSourcesOnOneStore_MeasureTheFloorOnce_AndTwoStoresNeverShareAFloor [FAIL]   Expected: 0  Actual: 1
RollupFloorCacheLiveTests.AfterTheStartUpWarm_TheFirstCoverageCallOnAnotherDataSource_RunsNoMinBucket [FAIL]   Expected: 0  Actual: 1
RollupFloorCacheLiveTests.APastTheHourFloorWithItsChunkUnchanged_IsServedFromTheCache_...                [FAIL]   Expected: 2026-09-29T02:00:00  Actual: 2026-09-29T04:00:00  (measured inline, not served from the cache)
RollupFloorCacheLiveTests.ADatabaseDroppedAndRecreatedUnderTheSameName_IsMeasuredAgain_...               [FAIL]   Expected: 2026-09-29T07:00:00  Actual: 2026-09-29T02:00:00  (the old database's floor)
RollupCoverageWarmupTests.TheWorker_LaunchesTheCoverageWarmAfterMigrations_...                           [FAIL]
RollupCoverageWarmupTests.TheViewer_WarmsTheCoverageOnceItsStoreHasAnswered_...                          [FAIL]

The one passing test there is the existing #4539 test. The changed-chunk-is-inline check sits inside the background-re-measure test (it is green on the old code too: that path is unchanged).

Tests first for the two-hour cap, at its own tests-only commit (RollupFloorMaxServeAge is declared there but the planner does not use it yet), against the planner as the branch had it:

RollupFloorStoreCacheTests  Total: 20, Errors: 0, Failed: 3, Skipped: 0, Not Run: 0
RollupFloorStoreCacheTests.Plan_AFloorAtOrPastTheServeCap_WithItsIdentityUnchanged_IsInline_NotServedFromTheCache(minutesPast: 0)      [FAIL]   Item not found in set (Inline)
RollupFloorStoreCacheTests.Plan_AFloorAtOrPastTheServeCap_WithItsIdentityUnchanged_IsInline_NotServedFromTheCache(minutesPast: 1)      [FAIL]   Item not found in set (Inline)
RollupFloorStoreCacheTests.Plan_AFloorAtOrPastTheServeCap_WithItsIdentityUnchanged_IsInline_NotServedFromTheCache(minutesPast: 43200)  [FAIL]   Item not found in set (Inline)
RollupFloorCacheLiveTests.AFloorPastTwiceTheHourWithItsChunkUnchanged_IsMeasuredInline_NotServedFromTheCache [FAIL]   Expected: 2026-09-29T04:00:00  Actual: 2026-09-29T02:00:00  (the stale floor served from the cache)

The other new tests pass on the old planner by design, and pin the rest of the rule: an entry just under two hours old is still served with one background re-measure, a changed oldest chunk, hypertable or database is measured inline at any age (5 minutes, just past the hour, just past two hours, a month), and RollupFloorMaxServeAge equals twice RollupFloorMaxReuse. Kept as they were: Plan_AFloorPastTheHourWithItsIdentityUnchanged_IsDueInTheBackground_NotInline and the live APastTheHourFloorWithItsChunkUnchanged_... (an entry just past one hour is served from the cache with one background re-measure, and a changed chunk is measured inline).

After the cap:

• The new and touched no-database classes, RollupFloorStoreCacheTests, RollupFloorCacheTests, DocCommentHygieneTests and TsqlConventionGuardTests: Total: 133, Failed: 0.
• Every test class that names PlanRollupFloorMeasurements, RollupFloorMaxReuse, MergeRollupFloors or DetectRollupCoverageAsync, plus RollupCoverageCeilingLiveTests, on a live TimescaleDB store with DARLING_TEST_PG set (26 classes, among them RollupFloorCacheLiveTests, RollupCoverageCeilingLiveTests, DailyStitchLiveTests, FrozenRollupLiveTests, IntervalHonestHourlyRollupLiveTests, RollupBackfillLiveTests, TopQueriesHourlyRoutingLiveTests): Total: 145, Errors: 0, Failed: 0, Skipped: 0, so the live tests ran rather than skipped. RollupFloorCacheLiveTests and RollupCoverageCeilingLiveTests alone, which hold the new live test: Total: 7, Failed: 0.
• Darling.Tests builds with 0 warnings, 0 errors.
• Full Darling.Tests at this head: not run locally; CI runs it on this PR.
• Lite.Tests not run: no Lite or shared code changed.

Before the cap, at the previous head:

• Targeted classes on a live TimescaleDB store: RollupFloorCacheLiveTests, RollupCoverageWarmupTests, RollupCoverageCeilingLiveTests, RollupFloorCacheTests, RollupCoverageRoutingTests, TsqlConventionGuardTests, DocCommentHygieneTests, QueryStoreIntervalWideBrinIndexTests, StartupCommandTimeoutTests, StorageCommandTimeoutTests, DailySummaryReadShapeTests, FleetGateSignalTests: Total: 215, Failed: 0.
• Full Darling.Tests, no DARLING_TEST_PG: Total: 19955, Errors: 0, Failed: 0, Skipped: 1324 (the live classes skip), Not Run: 1.
• Full Darling.Tests with DARLING_TEST_PG on a freshly created database: Total: 19955, Errors: 0, Failed: 1, Skipped: 98, Not Run: 1. The one failure, DarlingCliCommandsHostCheckTests.CheckSettingsAsync_ByoStore_NothingEverStale_ReturnsOk_TextAndJson_Gated (expected exit code 0, got 2), is a --check-settings verdict that reads nothing the rollup floors touch. That class alone on another freshly created database passes (Total: 7, Failed: 0), so it depended on state the full run leaves in the shared database. I did not check dev's own CI for it.

CHANGELOG

No entry in this PR; the release owner adds it under [Unreleased].

Proposed entry:

SECTION: Fixed
ENTRY:
- **The first daily summary after a start no longer waits on the rollup coverage probe** ([#4968]) - on a store with a long Query Store history the probe that measures how far back each rollup reaches took about 10 s, and every data source paid it on its own: at start, after a failed probe, and once an hour. The floors are now measured once per store (a database recreated under the same name is measured again), the service and the Viewer measure them in the background shortly after start, and a floor between one and two hours old is served from the cache while one background pass re-measures it. A floor two hours old or older is measured by the caller, so a floor is never served once it is two hours old
REF:
[#4968]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/4968

[claude]

Non-blocking notes (not the verdict)

Each of these is small and none of them is the reason for the changes-requested; the one substantive finding is the inline comment on StartRollupRemeasure.

1. The cache now has no eviction at all. RollupFloorCaches (TimescaleSupport.cs:9350) is a plain static ConcurrentDictionary and nothing ever calls TryRemove. The claim it replaces is true in the dimension the body means — cardinality is bounded by the number of host:port/database triples a process opens, and the service and Viewer each open one — but the other half of what the ConditionalWeakTable bought is gone: #4539/#4553's own words were "a disposed data source's entry is collected with it rather than leaking forever", and now a store's entry outlives every data source on it for the life of the process. For both shipping processes that is a handful of structs and genuinely does not matter; worth a sentence in the comment at :9338-9344 so the next reader does not have to re-derive that the lifetime change was deliberate rather than overlooked.

2. RollupStoreKey's parse fallback keys the cache on the raw connection string. TimescaleSupport.cs:9367-9370 returns connectionString — password included — when NpgsqlConnectionStringBuilder throws, and that string then lives as a dictionary key in process-lifetime static state. In practice this is unreachable: the string came off an already-built NpgsqlDataSource, so it parsed once. Still, the repo is careful about this elsewhere (the #4214 setting-scrub comment at DarlingWorker.cs:2290 exists specifically to keep primary_conninfo passwords out of logs), and a hash, or nameof-style sentinel, would cost nothing.

3. The Viewer's warm takes no cancellation token. MainWindow.xaml.cs:435 calls _ = _dataService.WarmRollupCoverageAsync() with the default CancellationToken, so a window closed inside the 3 s delay leaves the probe to run against a data source that is on its way out. RunDelayedAsync swallows the resulting ObjectDisposedException, so nothing breaks — but the Viewer does have a shutdown token available, and the service's call site passes stoppingToken (DarlingWorker.cs:2324) for the same reason.

4. using System.Runtime.CompilerServices; (TimescaleSupport.cs:15) is now dead. ConditionalWeakTable was its only consumer in that file and the diff removed it; nothing else in the file uses that namespace. No build impact — Directory.Build.props sets neither EnforceCodeStyleInBuild nor TreatWarningsAsErrors, so "builds with 0 warnings" still holds.

5. The two polling loops fall through to their assertions on timeout. RollupFloorCacheLiveTests.PollForFloorAsync:437-447 and RollupCoverageCeilingLiveTests:123-130 both exit on a 30 s deadline and then assert on whatever they last read, so a CI box slow enough to miss the window fails as "expected X, actual Y" rather than "the background re-measure never landed in 30 s". Not the #3676 shape — these bound a wait rather than measure an interval, and they do fail rather than pass falsely — but a message naming the timeout would save the next person reading a red build.

[claude]

Verdict: CHANGES REQUESTED

One finding: the inline comment on StartRollupRemeasure (TimescaleSupport.cs:9559-9570). Five non-blocking notes are in a separate top-level comment. All line references below are Darling/PerformanceMonitor.Darling.Storage/TimescaleSupport.cs unless named otherwise.

Claims

• VERIFIED — one floor cache per store, keyed on host/port/database rather than the data source — :9350, RollupStoreKey :9358-9372; pinned by RollupFloorStoreCacheTests.RollupStoreKey_IsTheSameForAnyUserPasswordOrSessionOption_OnOneHostPortAndDatabase + ..._DiffersWhenTheHostPortOrDatabaseDiffers.
• VERIFIED — each entry records the database OID, one extra column in the same round trip, and a differing OID is a changed identity — :9316, ordinal 4 at :9725, carried :9624, compared :9492; pinned by Plan_ADifferentDatabaseOid_IsInline_EvenWhenTheEntryIsPastTheHour and live ADatabaseDroppedAndRecreatedUnderTheSameName_IsMeasuredAgain_NotServedItsOldFloor.
• VERIFIED — the coverage probe SQL and RollupFloorMaxReuse (1 h) are unchanged — RollupCoverageProbeSql untouched; :9394 still TimeSpan.FromHours(1), doc comment only.
• VERIFIED — the warm follows QueryStoreIntervalWideBrinIndex.RunDelayedAsync and runs DetectRollupsAsync then DetectRollupCoverageAsync — RollupCoverageWarmup.cs:51-70 vs the precedent at QueryStoreIntervalWideBrinIndex.cs:82,169-195.
• VERIFIED — worker launches at 30 s after migrations, drains at shutdown — DarlingWorker.cs:2322-2324, :3823-3824; pinned by RollupCoverageWarmupTests.TheWorker_LaunchesTheCoverageWarmAfterMigrations_WithoutAwaitingItOnTheStartupPath.
• VERIFIED — the Viewer warms at 3 s once its store has answered reachability, schema and seat — MainWindow.xaml.cs:435 (after DetectReadOnlyAsync and after every early return in :400-432), ViewerDataService.RollupAvailability.cs:84-85; pinned by TheViewer_WarmsTheCoverageOnceItsStoreHasAnswered_AndDoesNotWaitForIt.
• VERIFIED — a failed warm logs once at Debug and bypasses both cached availability answers, where a failure caches "no rollups" for five minutes — RollupCoverageWarmup.cs:61-69; the caches are ComposeStoreAvailability.cs:59,218 and ViewerDataService.RollupAvailability.cs:34,66.
• VERIFIED — callers arriving during or just after the re-measure start none — guards :9549 and :9554; the second reads cache.ByView, not the caller's snapshot, which is what makes it correct.
• VERIFIED — a changed chunk, a different database or no entry still measures inline — :9487-9497; pinned by Plan_NoEntryYet_IsInline and the live inline arm.
• VERIFIED — a cycle writes back only the views it measured and drops views whose rollup is gone — :9512-9534 under the lock at :9834; Apply_ReplacesWhatThisCycleMeasured_DropsAnEmptyView_AndLeavesTheRestAsTheCacheHoldsThemNow pins the no-undo property exactly, Apply_DropsAViewWhoseRollupIsGone the eviction.
• VERIFIED — Lite parity, and RollupFloorsToMeasure keeps its meaning so its tests are untouched — ran the author's grep over Lite, Lite.Tests, PerformanceMonitor.Common, PerformanceMonitor.Collectors: no files, diff all under Darling/. :9451-9457 returns Inline union Background; RollupFloorCacheTests.cs still behaves because both new members are trailing parameters defaulting to 0. Pinned by Plan_RollupFloorsToMeasure_IsTheUnionOfBoth.
• REFUTED — "the chunk-identity check [is] unchanged" — :9492 adds entry.DatabaseOid != identity.DatabaseOid, :9414/:9418 gained the member. The body's next sentence describes the addition, so a loose sentence rather than a false account, and not a basis for the verdict.
• REFUTED — "the hour plus one probe's duration" as the staleness bound — not a bound when the re-measure does not land. :9495 sends an age-only entry to Background, :9736-9739 keeps it out of toMeasure, so no caller measures it inline again; the only refresher swallows every failure at :9565-9569 with _ = ex;, no log, no state. See the inline comment.
• UNVERIFIED — the 43-server numbers (13 s cold call, 9.7 s probe, 6.3 s interval-daily) — a live reading.
• UNVERIFIED — the test-plan totals and "0 warnings" — needs a build and a live DARLING_TEST_PG; every new live assertion is behind Assert.SkipWhen(string.IsNullOrEmpty(BaseConnectionString)), so a green run without it proves nothing. Directory.Build.props sets neither EnforceCodeStyleInBuild nor TreatWarningsAsErrors, so the 0-warnings claim is at least consistent.

Riskiest lines

1. :9565 — catch (Exception ex) { _ = ex; } is now the sole refresher for every past-the-hour floor, since :9736-9739 removes the inline path those entries used to take. A re-measure that keeps failing serves an over-claiming floor indefinitely and silently, with Failed: false upstream so #3905's GetMeasuredRollupsAsync guard routes the daily summary on it — UNPINNED.
2. :9554 — all that stops N concurrent past-the-hour callers each starting a re-measure once the first completes but before its write reaches their snapshots — pinned by RollupFloorCacheLiveTests.APastTheHourFloorWithItsChunkUnchanged_IsServedFromTheCache_AndRemeasuredOnceInTheBackground_AChangedChunkIsMeasuredInline, live-gated.
3. :9350 — a process-global cache replacing a per-data-source one: no eviction, and the availability sweep at :9530 lets any data source's narrower availability evict another's entries. Isolation pinned by TwoDataSourcesOnOneStore_MeasureTheFloorOnce_AndTwoStoresNeverShareAFloor; lifetime and that eviction UNPINNED.

Traps near this diff

• "A retention-tiered store answers a 7-day question from a 4-day raw tier and labels the missing days quiet unless the read routes by tier and says which tier served it (#3590; the viewer's charts in #3684)." — applies — :9565 plus the removed inline path at :9736-9739 lets a floor read earlier than the truth without limit, and ComposeStoreAvailability.cs:132-134 routes the daily summary on it because the probe reported success.
• "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit." — does not apply — Lite keeps no TimescaleDB rollups; the grep returns nothing and the diff is all under Darling/.
• "A test that reads the clock twice measures the CI scheduler (#3676). One anchor owns both ends of an interval." — does not apply — the new clock reads (RollupFloorCacheLiveTests.cs:437, RollupCoverageCeilingLiveTests.cs:123) bound a wait rather than measure an interval; no assertion compares two readings.
• "A pin that encodes the lie ... Ask what a pin would fail ON (#3699)." — does not apply — Apply_ReplacesWhatThisCycleMeasured_... fails if the write-back overwrites an unmeasured view, Plan_ADifferentDatabaseOid_IsInline_... if the OID leaves the identity. The two RollupCoverageWarmupTests are source-text IndexOf pins, weaker than behaviour but this repo's shape (Darling.Tests/RepoFile.cs:70).
• "ORDER BY 3 + 4 DESC is not 'ordinal 3 plus ordinal 4'" / "Integer division in a rate column" / "A percent whose denominator is the PAGE" — do not apply — the only SQL change is one appended projection at :9316: no ordering, no arithmetic, no denominator, and it lands at ordinal 4, so the reader at :9722-9726 still takes 0, 1 and 3.

Divergence (blind read vs PR body)

Close agreement on every mechanism — item 4's write-back split is implemented and pinned exactly as described. Two divergences.

1. The staleness bound. The body states it flatly and qualifies it only for the sporadic caller, under "Quiet spells". It never mentions the failing probe, which is where the bound stops existing: the inline past-the-hour measure is gone, so :9565-9569 is the only refresher left and nothing records or surfaces that it is failing. The body frames this as moving who pays; it also changes who finds out when nobody can pay, and #3905's GetMeasuredRollupsAsync arm was built on the old behaviour.

2. "nothing that grows without bound (one small entry per store the process has opened)." True as written, and cardinality is bounded for both shipping processes. But the comment it replaces claimed a second thing — "a disposed data source's entry is collected with it rather than leaking forever" — and the new cache removes nothing, ever. :9338-9344 does not say that half was traded away. Harmless at these sizes, so it is in the notes comment.

Reviewer checklist

• Run the four new live tests plus RollupCoverageCeilingLiveTests with DARLING_TEST_PG on a TimescaleDB store — everything new is behind Assert.SkipWhen, and the two 30 s polling loops are where a slow box tells you what a read-only tree cannot.
• Decide the failing-re-measure policy from the inline comment, and whether GetMeasuredRollupsAsync should treat a floor the re-measure could not refresh as measured.
• Confirm on the 43-server store that the 30 s service warm beats the first agent call; a warm firing while EnsureContinuousAggregatesAsync still runs measures a partial availability and logs nothing above Debug.
• Watch the Viewer's first tab load there with the 3 s warm — a pool-contention judgement no static read can check.
• Build Darling.Tests to confirm the 0-warnings claim survives the now-dead using at TimescaleSupport.cs:15.

[claude]

Notes from the re-review (non-blocking)

These are the observations that did not rise to the verdict. Three are inline (TimescaleSupport.cs:9531-9534, :9589-9593, the dead using at :15); these four are not anchored to one line.

1. The serve cap closes the previous review's finding, and the pin that matters runs without a rig. The earlier CHANGES_REQUESTED rested on there being no inline path left for an age-only entry: a background re-measure that failed every time would serve an over-claiming floor indefinitely and silently. RollupFloorMaxServeAge (:9407) plus :9512-9516 restores it — age >= RollupFloorMaxServeAge is Inline, so the staleness bound exists whether or not the re-measure ever lands. Worth saying explicitly that the pin for this is RollupFloorStoreCacheTests.Plan_AFloorAtOrPastTheServeCap_WithItsIdentityUnchanged_IsInline_NotServedFromTheCache at minutesPast 0/1/43200, which has no database and no Assert.SkipWhen — so unlike almost everything else new here, CI actually runs it. ServeCap_IsTwiceTheReuseWindow's second assertion (MaxServeAge > MaxReuse) is the one that would catch the static-initialization-order hazard the :9400-9406 comment names, and :9398/:9407 are in the right textual order.

2. The warm is a one-shot, so the first caller after a two-hour quiet spell still pays the full inline probe. Nothing re-warms: DarlingWorker.cs:2326 fires once at +30 s, MainWindow.xaml.cs:438 once per Viewer session. On a store that goes two hours without an MCP or web call, the next get_daily_summary is back to the ~10 s cold shape #4957 is named for — just once every two hours rather than once an hour per data source. The body's "Quiet spells" bullet says this plainly ("it pays the probe once, as it did before this PR"), so it is a stated tradeoff, not a gap. Flagging it only because the CHANGELOG entry's headline — "The first daily summary after a start no longer waits on the rollup coverage probe" — is true of a start and not of a quiet spell, and the release owner will be copying that sentence.

3. The lifetime half of #4539/#4553's rationale was traded away, and the new comment does not say so. The comment this replaces held two reasons for the per-data-source key: no cross-store pollution and "a disposed data source's entry is collected with it rather than leaking forever". :9338-9344 answers the first (key + OID) and reframes the second as cardinality ("one small entry per store this process has opened"), which is true and fine for both shipping processes — but the static ConcurrentDictionary at :9350 removes nothing, ever, and RollupFloorCache.Remeasure keeps the last task reference per store. A Viewer session that re-points at many stores accumulates one entry each for the life of the process. Harmless at these sizes; just no longer the property the old comment claimed, and the new one does not note the trade.

4. ApplyRollupFloorMeasurements' availability sweep now has a per-process blast radius, and I could not construct a reachable case. :9554 drops every cache key the calling cycle's availability does not have. With one cache per store, a data source holding a narrower availability than another's evicts the other's entries. I went looking for a way two data sources on one store disagree and did not find one in product code: DetectRollupsAsync is built on to_regclass, which :9075-9076 documents as needing no table privilege, so availability is role-independent; and all three product callers (ComposeStoreAvailability.cs:194, ViewerDataService.RollupAvailability.cs:59, RollupCoverageWarmup.cs:67) pass a freshly probed answer rather than a narrowed constant — only DailyStitchLiveTests.cs:255 passes RollupAvailability.WithoutIntervalDailies, and that is a test. The one live window left is the warm firing at +30 s while EnsureContinuousAggregatesAsync is still creating aggregates on a first start or an upgrade: that cycle measures a partial availability, and the cache is empty then so nothing is evicted, but it is the case I would watch on the 43-server store. Noting it as the thing I checked and could not break, not as a finding.

Also checked and clean, for the record: every live test that calls DetectRollupCoverageAsync mints its own GUID-named ScratchPostgres database (grep -L ScratchPostgres over all of them returns nothing), so moving the cache from per-data-source to a process-static per-store map does not create cross-test pollution in Darling.Tests. And the new database_oid projection at :9316 lands at ordinal 4, so the reader at :9747-9751 still takes 0, 1 and 3 for the columns it already read.

[claude]

Verdict: LGTM

Re-review at the serve-cap head. The earlier CHANGES_REQUESTED rested on one thing: an age-only entry had no inline path left, so a background re-measure that kept failing would serve an over-claiming floor indefinitely and silently. RollupFloorMaxServeAge fixes exactly that. Three inline comments and one top-level note carry the rest; all non-blocking. Bare line numbers are Darling/PerformanceMonitor.Darling.Storage/TimescaleSupport.cs.

Claims

• VERIFIED - one floor cache per store (host/port/database) instead of per data source, with the database OID as a third identity field - :9350, RollupStoreKey :9358-9372, used :9730; :9316 ordinal 4, read :9750, carried :9648, compared :9505. Pinned by RollupFloorStoreCacheTests.RollupStoreKey_*, Plan_ADifferentDatabaseOid_IsInline_EvenWhenTheEntryIsPastTheHour, live TwoDataSourcesOnOneStore_... and ADatabaseDroppedAndRecreatedUnderTheSameName_....
• VERIFIED - the coverage probe SQL and RollupFloorMaxReuse (1 h) are unchanged - RollupCoverageProbeSql :9266-9276 untouched; :9398 still TimeSpan.FromHours(1), doc comment only.
• VERIFIED - the warm runs DetectRollupsAsync then DetectRollupCoverageAsync, logs a failure once at Debug, and bypasses both cached availability answers (a failure there caches "no rollups" for five minutes: ComposeStoreAvailability.cs:151,218, ViewerDataService.RollupAvailability.cs:34,66) - RollupCoverageWarmup.cs:60-75. Worker launches it 30 s after migrations and drains it at shutdown (DarlingWorker.cs:2213 then :2326 then :3825 then :3827); the Viewer warms at 3 s after :408, below every early return at :401/:423/:432 (MainWindow.xaml.cs:438), logging nothing because logger: null. Pinned by RollupCoverageWarmupTests.
• VERIFIED - an entry one-to-two windows old with its identity unchanged is served from cache and starts one background re-measure; callers arriving during or after it start none - :9512-9521, :9871-9875, guards :9573 and :9578 (the second re-reads cache.ByView, not the caller's snapshot, which is what makes it correct). A changed chunk, hypertable, database or no entry measures inline at any age - :9499-9509.
• VERIFIED - the staleness bound really is two hours plus one probe whether or not callers keep arriving: an entry at or past twice the window is measured inline - :9512-9516, with :9398 before :9407. Pinned by Plan_AFloorAtOrPastTheServeCap_WithItsIdentityUnchanged_IsInline_NotServedFromTheCache (0/1/43200 min, no database, no skip gate, so CI runs it) and ServeCap_IsTwiceTheReuseWindow, whose MaxServeAge > MaxReuse arm catches the init-order hazard :9400-9406 names. The previous review refuted this claim; the cap makes it hold even when :9589-9593 swallows every re-measure forever. It bounds the cached ceiling too (carried :9672, answered :9862-9869), and a stale ceiling reads earlier, the safe direction at both consumers (QueryStoreTrendRouting.cs:161-162, DarlingDataReader.cs:1352).
• VERIFIED - write-back touches only the views the cycle measured and drops a view whose rollup is gone (:9542-9557 under the lock at :9857); RollupFloorsToMeasure keeps its meaning (:9465-9471, old fixtures match because DatabaseOid defaults to 0 at :9437/:9442); Lite parity holds (no rollup code under Lite/, diff all under Darling/); no cross-test pollution, since every live test calling DetectRollupCoverageAsync mints its own ScratchPostgres database.
• REFUTED - "the chunk-identity check [is] unchanged" (first line of What changes) - :9505 adds entry.DatabaseOid != identity.DatabaseOid. A wording slip, not a false account: item 1 describes the addition two sentences later, so not the verdict basis.
• UNVERIFIED - the 43-server numbers (13 s cold call, 9.7 s probe, 6.3 s interval-daily) - a live reading.
• UNVERIFIED - the test-plan totals and "0 warnings, 0 errors" - needs a build and a live DARLING_TEST_PG; dotnet build was unavailable here, and every new live test sits behind Assert.SkipWhen. The 0-warnings claim is at least consistent: nothing sets EnforceCodeStyleInBuild, so the now-dead using at :15 cannot fail it.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Storage/TimescaleSupport.cs:9512 - the age >= RollupFloorMaxServeAge arm is the whole staleness bound now that :9761-9764 keeps age-only views out of toMeasure - pinned by Plan_AFloorAtOrPastTheServeCap_WithItsIdentityUnchanged_IsInline_NotServedFromTheCache.
2. Darling/PerformanceMonitor.Darling.Storage/TimescaleSupport.cs:9578 - all that stops N concurrent past-the-hour callers each starting a re-measure once the first completes but before its write reaches their snapshots - pinned by RollupFloorCacheLiveTests.APastTheHourFloorWithItsChunkUnchanged_IsServedFromTheCache_AndRemeasuredOnceInTheBackground_AChangedChunkIsMeasuredInline, live-gated.
3. Darling/PerformanceMonitor.Darling.Storage/TimescaleSupport.cs:9350 - a process-global cache replacing a per-data-source one: no eviction ever, and :9554 lets one cycle's narrower availability drop another's entries - isolation pinned by TwoDataSourcesOnOneStore_MeasureTheFloorOnce_AndTwoStoresNeverShareAFloor; lifetime and that eviction UNPINNED.

Traps near this diff

• "A retention-tiered store answers a 7-day question from a 4-day raw tier and labels the missing days quiet unless the read routes by tier and says which tier served it (#3590)." - does not apply, though it did at the previous head - :9512 changed the answer: a floor reads earlier than the truth for at most RollupFloorMaxServeAge, after which the caller measures it.
• "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit." - does not apply - Lite keeps no TimescaleDB rollups; the diff is all under Darling/.
• "A test that reads the clock twice measures the CI scheduler (#3676). One anchor owns both ends of an interval." - does not apply - the new DateTime.UtcNow pairs (RollupFloorCacheLiveTests.cs:418-422, RollupCoverageCeilingLiveTests.cs:123-124) bound a 30 s wait rather than measure one, and the injected now the planner is graded against is fixed in both.
• "timescaledb_information.job_history is ownership-filtered while jobs and job_stats have no WHERE at all (#3585). A read of that view must state whose eyes it has." - does not apply to the new column - :9316 reads world-readable pg_catalog.pg_database and adds no timescaledb_information relation :9317-9327 did not already name; it is uncorrelated, carries AS d per CONTRIBUTING, and lands at ordinal 4 so the reader at :9747-9751 still takes 0, 1 and 3. That also disposes of the ORDER BY 3 + 4, integer-division and per-page-denominator traps: no ordering, arithmetic or denominator is added.

Divergence (blind read vs PR body)

Close agreement on every mechanism - I reached the same account of all five numbered items before reading the body, item 4's write-back split included. Two divergences, both wording rather than behaviour.

1. "The ... chunk-identity check [is] unchanged." It is not: :9505 compares a third field. Graded REFUTED above; the body's own item 1 corrects it two sentences later, so not the verdict basis.

2. Item 4's stated purpose reaches further than the code. The doc at :9531-9534 says the new write-back stops an older cycle undoing a newer measurement. It does so for views the slower cycle did not measure; for a view both measured, :9546 is still last-writer-wins by completion order, and :9587's backdated now means the loser's stamp can survive too. Concrete interleaving in the inline comment. Both effects sit inside the exposure :9393-9396 already documents, so this is an accuracy note, not a behaviour ask.

Worth underlining rather than disputing: the warm is a one-shot in both processes, so the first caller after a two-hour quiet spell still pays the full inline probe. The body's "Quiet spells" bullet says so; the proposed CHANGELOG headline is true of a start and not of a quiet spell.

Reviewer checklist

• Run the five new live tests plus RollupCoverageCeilingLiveTests with DARLING_TEST_PG on a TimescaleDB store - everything new except the two no-database classes is behind Assert.SkipWhen, and the two 30 s polling loops are where a slow box tells you what a read-only tree cannot.
• Build Darling.Tests for the "0 warnings, 0 errors" claim - the new tests reach RollupFloorPlan, PlanRollupFloorMeasurements, ApplyRollupFloorMeasurements and RollupStoreKey through InternalsVisibleTo, which only a compile proves.
• On the 43-server store, confirm the 30 s warm beats the first agent call, and watch a first start or upgrade where it can fire while EnsureContinuousAggregatesAsync is still creating aggregates: that cycle caches a partial availability and logs nothing above Debug.
• Watch the Viewer's first tab load there with the 3 s warm - pool contention is a judgement no static read can make.
• Decide whether a persistently failing background re-measure should leave any trace (:9589-9593). The cap bounds it to 2 h, so this is diagnosability, not correctness.

Resolved at 37985e5: RollupFloorMaxServeAge measures an entry past twice the reuse window inline, and the re-review at that head verifies it.