FinOps sizes: database-size latest-snapshot reads take a plan-time chunk bound (#4245)

[erikdarlingdata]

Closes #4245.

Why

DatabaseSizeLatestSql (viewer) and get_database_sizes (MCP) found the newest
database_size_stats snapshot through a correlated
collection_time = (SELECT MAX(collection_time) ... WHERE server_id = $1) subquery with
no bound of its own. TimescaleDB builds a subplan for every retained chunk before deciding
which holds the answer. The read pays for every chunk at plan time, regardless of how few
it needs. DatabaseSizeSummarySql and StorageGrowthSql's latest CTE had the same
shape. StorageGrowthSql's past_7d/past_30d CTEs had a partially-bounded version:
their inner MAX has collection_time <= $N, which only excludes chunks newer than the
cutoff, not older ones.

What changes

All four now resolve "the snapshot time" as its own round trip before the main read.
Two helpers do this, mirroring how DatabaseSizeLatestSql itself is duplicated between the
viewer and the MCP reader:

1. GetLatestDatabaseSizeSnapshotAsync (viewer) and GetLatestSnapshotTimeAsync
   (MCP, trimmed): for the three latest-snapshot reads. These serve GetDatabaseSizeLatestAsync,
   GetDatabaseSizeSummaryAsync, GetStorageGrowthAsync's latest CTE, and the MCP twin.
   A windowed probe (collection_time >= asOf - 2 days) lets the planner exclude every chunk
   outside the window. It falls back to an unbounded MAX only when the window is empty.
   That happens when collection stopped more than two days ago. Neither probe has an upper bound.
   A latest read has no "at or before" semantics. Bounding by the reader's clock is wrong.
   A snapshot the collector stamped a few minutes ahead is still the latest one. The first
   version of this fix dropped it. (See "Fixed after the first commit" below.) The fallback is
   the unqualified MAX(collection_time) the pre-fix code always ran. It has no failure mode
   to introduce.
2. GetDatabaseSizeSnapshotAtOrBeforeAsync (viewer only): for StorageGrowthSql's
   past_7d/past_30d CTEs. These mean "at or before a past cutoff." They keep the upper
   bound: collection_time >= asOf - 2 days AND collection_time <= asOf.
3. The main read, which now binds collection_time = <the resolved literal> instead of
   a subquery: the change that actually lets the planner exclude chunks.

Correctness argument: if the windowed probe finds any row, its MAX is the true unbounded
MAX. Nothing older than the window's start can be newer than something inside it. The
fallback only fires when the window is genuinely empty, never as an approximation. A server
with no database-size history returns null from both probes. The main read is skipped
entirely (same as the old behavior: equality against a NULL subquery result produced zero
rows).

DatabaseSizeSummarySql's topN moved from $2 to $3 (the resolved collection_time is
now $2). A pin (ViewerFinOpsSqlTests) checked LIMIT $2 and is updated to LIMIT $3.

Lite: not touched. Lite reads DuckDB, which has no chunks, so this TimescaleDB plan-time
cost has no DuckDB equivalent.

Not fixed here: IdleDatabasesSql's db_sizes CTE and TempdbSummarySql also read through
an unbounded correlated MAX. Same file, same bug shape, not named in the issue. Not measured
or touched.

Fixed after the first commit: latest reads must not upper-bound by the reader's clock

The first commit's GetDatabaseSizeSnapshotAtOrBeforeAsync bound every caller by asOf.
This included the three call sites that mean "the latest snapshot, period"
(GetDatabaseSizeLatestAsync, GetDatabaseSizeSummaryAsync, GetStorageGrowthAsync's
latest CTE) and the MCP twin's GetLatestSnapshotTimeAsync. A snapshot the collector
stamped after the reader's clock (clock skew, which the old unbounded MAX never cared
about) was invisible to both probes. The "latest" read skipped the actual newest row.

Ruled: a latest read takes no upper bound. Only the 7-day-ago and 30-day-ago comparison
points in StorageGrowthSql keep one: "at or before a past cutoff" is what they mean.
Split into GetLatestDatabaseSizeSnapshotAsync (viewer) and GetLatestSnapshotTimeAsync
(MCP). Proven with a live test. It fails on the first commit's shape and passes on this one. See below.

Diagnosis, confirmed with numbers

Seeded the rig's probe database: four server shapes (actively collecting, 20 days stale,
zero rows, and 5 minutes into the future). 35-75 days of database_size_stats per server,
1-day chunks, 76 total after ANALYZE. Measured EXPLAIN (ANALYZE, BUFFERS) through Npgsql
with bound parameters, matching how the product actually binds them. The codebase does not
set Max Auto Prepare anywhere (checked), so Npgsql's default applies (auto-prepare
disabled). These are the un-prepared numbers the product actually gets.

Read	Before (plan+exec)	After (probe+main, total)	Ratio
Viewer latest (DatabaseSizeLatestSql)	9.6 + 1.3 = 10.9 ms	0.51 + 0.26 = 0.78 ms	~14x
MCP twin (get_database_sizes)	6.7 + 1.1 = 7.8 ms	0.59 + 0.24 = 0.83 ms	~9x
Storage growth (3 CTEs)	23.5 + 8.6 = 32.1 ms	3 probes (0.51+0.51+0.51) + main 0.68 = 2.2 ms	~14x

Numbers are one run on a local dev rig (Windows, PostgreSQL 18.6 / TimescaleDB 2.30.1).
Planning time on "before" varied 2-3x run to run (6-58 ms, likely background
TimescaleDB workers/checkpoints). The after shape was consistently under 1 ms per statement.
The improvement held in every run. The numbers are lower than the issue's field numbers
(148 ms / 71 chunks): the seeded rig has 76 mostly-empty chunks. The mechanism
(plan-time chunk exclusion vs. plan-time chunk walk) is the same either way and scales with
chunk count, not row count.

Also confirmed directly: the stale-20d server's windowed probe misses and falls back
correctly. The future-5m server's unbounded fallback returns a stamp after "now"
(is_after_now=true), the case the first commit's upper bound hid.

What changes (tests)

• ViewerFinOpsTests.cs: the FinOpsReads_PgDialect... theory gained the two new latest-only
  probe constants (DatabaseSizeLatestSnapshotWindowedProbeSql /
  ...FallbackProbeSql) alongside the pre-existing bounded ones.
• New DatabaseSizeLatestPlanShapeLiveTests ([Collection("live-postgres")], gated on
  DARLING_TEST_PG): drives all four reader methods for all four seeded server shapes.
  Compares every row against the pre-WPF FinOps latest-database-sizes read plans for 148 ms over every database_size_stats chunk to execute in 2.6 ms #4245 unbounded MAX(collection_time) oracle.
  The oracle has no upper-bound failure mode, so it is correct for the future-stamped server
  too. Regression proof: reverted to the first commit's upper-bound shape and reran.
  It failed on the future-stamped server's MCP rows. Restored the fix and reran clean.

Test plan

• dotnet build on PerformanceMonitor.Darling.Viewer, PerformanceMonitor.Darling.Service,
  and Darling.Tests: 0 Warning(s), 0 Error(s).
• Offline pin tests (no live Postgres): ViewerFinOpsSqlTests, DarlingMcpObjectStatsToolsSurfaceAndSqlTests,
  McpLatestSnapshotStampTests: 106 total, 0 failed.
• EXPLAIN (ANALYZE, BUFFERS) before/after, bound via Npgsql, on a 76-chunk seeded rig: see
  the numbers table above.
• Live regression test (DatabaseSizeLatestPlanShapeLiveTests): row-identity between
  the pre-WPF FinOps latest-database-sizes read plans for 148 ms over every database_size_stats chunk to execute in 2.6 ms #4245 unbounded-MAX oracle and the current two-step shape, all four server shapes
  including the future-stamped one. 1 total, 0 failed, against darlingtest.
• Proved the live test catches a regression: reverted to the first commit's shape, reran
  (failed on the future-stamped server), restored the fix, reran (passed).
• git merge origin/dev and one full Darling.Tests run against a freshly recreated
  darlingtest. Not done: this lane hit its context budget after finishing the items
  above. Every other item in the original checklist is now done. This is the remaining
  step before this PR leaves draft. The rig (C:\GitHub\worktrees\rig-f1, port 55975,
  UTC, PostgreSQL 18.6 + TimescaleDB 2.30.1) is stopped but intact, with darlingtest
  and probe still present.

What to double-check

• The no-upper-bound ruling for latest reads versus the kept upper bound on
  GetDatabaseSizeSnapshotAtOrBeforeAsync for the 7d/30d comparison points. The live test
  and the revert-and-rerun proof both back this. Worth a second look: it is a correction to
  this PR's first commit.
• The full suite run (git merge origin/dev + fresh darlingtest + full Darling.Tests)
  is still outstanding: someone needs to run it before this leaves draft.
• The 2-day window width (unchanged from the first commit) is a judgment call based on the
  collector's ~60-minute cadence, not a measured optimum.
• Whether IdleDatabasesSql/TempdbSummarySql (same file, same bug shape, not touched) are
  worth a follow-up issue.

CHANGELOG entry

SECTION: Fixed
ENTRY:

• Database Sizes and Storage Growth load without a multi-chunk planning tax ([FinOps sizes: database-size latest-snapshot reads take a plan-time chunk bound (#4245) #4252]): The latest snapshot reads had no bound on collection time. PostgreSQL had to plan a step for every retained chunk before running each read. Each read now resolves the snapshot timestamp in a separate step first. The main read then skips chunks it does not need. Clock skew between reader and collector does not drop the latest snapshot.
  REF:
  [FinOps sizes: database-size latest-snapshot reads take a plan-time chunk bound (#4245) #4252]: FinOps sizes: database-size latest-snapshot reads take a plan-time chunk bound (#4245) #4252

Generated with Claude Code

https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3