AG reads: share one floor-bounded newest-snapshot read across Viewer and Service (#4228)

[erikdarlingdata]

Closes #4228.

Why

Both Availability Group reads (replica states and database-replica states) found each server's newest
snapshot by joining to SELECT server_id, MAX(collection_time) ... GROUP BY server_id with no bound on
collection_time. That inner aggregate reads every retained row for every server before it can answer.
TimescaleDB cannot exclude a single chunk: 2,803 ms and 441k buffers for 398 rows on a production store
(#4228's own measurement). The same shape appeared in four places: the Viewer's AG tab, get_ag_health, /api/ag, and Lite's own
alert read (found in this pass, see G3).

What changes

• New Darling/PerformanceMonitor.Darling.Storage/DarlingAgStatesReader.cs: one shared reader, the same
  precedent as DarlingQueryStoreClutterReader. The Viewer's AG tab, /api/ag, /api/ag/count and
  get_ag_health all call it now instead of carrying their own copy of the statement text.
• Each grain is two steps. Step one finds every server's newest instant with an ordered LATERAL descent
  (ORDER BY collection_time DESC LIMIT 1), driven from servers. It uses an index range scan per server,
  not an unbounded aggregate. Step two is the shipped statement's exact shape (inner aggregate, join back on
  the found instant, the servers ... AND is_enabled join). It adds collection_time >= $floor to both the
  inner aggregate and the outer scan.
• Staleness horizon: one chunk day (TimescaleSupport.ChunkIntervalDays). A server whose newest AG snapshot
  is older than that does not feed the shared floor. It is read on its own instead, with its own instant as a
  single-server floor.
• No is_enabled filter on step one, on purpose. Every step-two statement keeps the exact
  JOIN servers AS s ON s.server_id = ... AND s.is_enabled the shipped statements always had, unchanged. A
  disabled server is excluded from the output exactly as it is today.
• Bare table names, the Storage-layer convention (search_path = collect, config, public).
• The database-grain statement now selects the Service's superset column list (last_hardened_lsn,
  last_commit_lsn).
• Fixed the /api/ag/count nav-probe aggregate too (GetAvailabilityGroupCountAsync), the identical
  unbounded shape one function away from the two named in the issue.
• Lite's own AG alert read got the same fix (G3, this pass). See below.
• Every existing SQL-text pin moved to DarlingAgStatesReaderTests.cs. Added a source pin asserting no
  public SQL constant runs MAX(collection_time) ... GROUP BY server_id unbounded, and pure unit tests for
  the staleness split.

G2 verification

Ran on a PostgreSQL rig (port 55975, UTC, TimescaleDB), against darlingtest, after G1's PR sat with every
live/EXPLAIN box unchecked.

• Live row equality, new file Darling/Darling.Tests/DarlingAgStatesReaderLiveEqualityTests.cs.
  Seeded with 10 server shapes across several daily chunks. Six current servers. One stopped 2 days ago
  (staleness-horizon path). One stopped 2 hours ago (inside the horizon, still feeds the floor). One
  disabled with current rows. One removed from config_monitored_servers but still in servers.
  - AgStatesReader_MatchesTheOldUnboundedStatements_AcrossEveryServerShape_AgainstDevPostgres:
  compares GetReplicaStatesAsync, GetDatabaseReplicaStatesAsync, and GetReplicaGroupCountAsync
  row-for-row against the pre-The Availability Group read finds each server's newest snapshot by aggregating every retained AG row: 2.8 s and 441 k buffers for 398 rows; a two-step read returns the same rows in ~3 ms #4228 unbounded statement text. Runs both unfiltered (scoped to the
  test's own servers) and filtered per shape. Also drives the entry points directly:
  ViewerDataService.GetAvailabilityGroupsAsync, DarlingAgReader.GetAgHealthAsync,
  GetAvailabilityGroupCountAsync, and DarlingMcpAgTools.GetAgHealth with and without a server
  name. Every shape matches: disabled excluded, stale/inside-horizon/removed included at their true
  instants. The tie at each server's newest snapshot (2 replicas, 2 databases sharing one
  collection_time, the normal shape of any AG snapshot) round-trips identically.
  collection_time is NOT NULL on both AG tables (schema-enforced since V34). The tie is the only
  ordering edge case there is, and it is covered.
  - Proved the pin catches a regression, once. Temporarily neutralized the "stale" merge loop in
  GetReplicaStatesAsync (the foreach (var server in stale) block). A stopped-2-days-ago server's
  rows silently vanished from the read. Reran: the equality test failed
  (Assert.Equal() Failure: Collections differ). Reverted. git diff on the reader is empty.
  Both facts pass again.

  • The issue's EXPLAIN assertion, same file,
    AgStatesReader_StepTwoStaysBounded_ForCurrentAndStaleServers_AgainstDevPostgres: runs the real
    step one + DarlingAgStatesReader.SplitCurrentAndStale to get the production floor, then
    EXPLAIN (COSTS OFF) old vs. new. Made relative, not an absolute ceiling. darlingtest is shared
    with the rest of the suite. Another live class's own near-"now" seed can add a chunk this seed did
    not create. The same reasoning FleetReadsAreBoundedByTheFleetTests and
    EventWindowedReadsAreBoundedLivePostgresTests already use for the identical risk. Measured once by
    hand (forcing the assertions to fail momentarily to print the counts, then reverting. git diff
    confirms the file matches this PR's committed version):

    | plan | plan-line "chunk scan" count* | reduction |
    | --- | --- | --- |
    | old (unbounded), shared/current case | 12 | n/a |
    | new (floored), shared/current case | 2 | 10 |
    | old (unbounded), stale server's own read | 12 | n/a |
    | new (floored), stale server's own read | 6 | 6 |
    
    \* `PlanChunkScans.Count` counts scan-node lines. Each statement references `ag_replica_states`
    twice (the inner `MAX(...)` aggregate and the outer join), so the real chunk count is half the
    printed count. The shared/current case touches 1 real chunk here, inside the issue's
    "less than or equal to 2" bound.
    

  • A real finding: the stale server's own point read is not literally a single-chunk read for every case.
    Its WHERE collection_time >= floor has no upper bound. The shared table keeps gaining new chunks every
    day regardless of this one server's staleness. Its plan touches every chunk from its own floor's day
    through today (3 real chunks for a 2-days-stale server in this seed, not 1). It is still strictly
    cheaper than the unbounded oracle: bounded below by its own data, not scanning from the dawn of
    retention. Results are unchanged either way. This is a plan-shape and doc-precision note, not a
    correctness bug. The reader's code and doc comments are left alone per this lane's verify-only mandate.
    Worth a one-line doc tweak in a future pass.

G3 verification (this pass)

Picked up the three boxes G2 left unchecked. Rig reused at port 55975 (UTC, TimescaleDB), RIG_DIR
C:\GitHub\worktrees\rig-f1.

1. Lite parity: was wrongly dismissed, now fixed

G1's premise ("a Lite install monitors one server") is wrong. Lite can and does monitor several servers
(Lite/Services/LocalDataService.AvailabilityGroups.cs reads per server with WHERE server_id = $1). An
unbounded per-server MAX(collection_time) can scan that one server's whole retained history regardless of
fleet size. The cost driver is retention, not fleet size.

Seeded a Lite DuckDB (SharedDuckDbFixture pattern) at the real ScheduleManager defaults for both AG
collectors (ag_replica_states / ag_database_replica_states: 1-minute frequency, 30-day retention). Used 5
servers with rows interleaved by collection tick, 2 replicas and 2 databases per snapshot: 432,010 and
864,020 rows. This is the shape a multi-server Lite install actually writes, not clustered per server. Timed
GetLatestAgReplicaStatesAsync / GetLatestAgDatabaseReplicaStatesAsync for one server:

• Before: 47-93 ms steady state, over the ~20 ms parity bar. The query itself, isolated from connection
  overhead, was the dominant cost: 40-90 ms for the old correlated MAX subquery.

Gave both reads the same two-step newest-instant shape DarlingAgStatesReader uses. Step one is an
ordered-descent (ORDER BY collection_time DESC LIMIT 1, answered by the existing (server_id, collection_time) index). Step two is bound by that exact instant. Lite's read is already scoped to one
server per call, so step two uses a plain equality bound instead of Darling's >= floor aggregate. There is
no fleet-wide shared floor to protect here, so the simpler bound is correct and sufficient.

• After: the query itself dropped to 2-4 ms in the same seed, steady state: a reduction of over 90%,
  consistent with DarlingAgStatesReader's fix.
• A separate finding: the full method call (through LocalDataService.OpenConnectionAsync) still measures
  ~40-55 ms. Every single call opens a brand-new DuckDBConnection
  (_duckDb.CreateConnection(); await connection.OpenAsync();), and that alone costs 37-50 ms with no query
  at all. This is pre-existing across roughly 184 read call sites in LocalDataService, per
  SharedDuckDbFixture's own doc comment. It is not something The Availability Group read finds each server's newest snapshot by aggregating every retained AG row: 2.8 s and 441 k buffers for 398 rows; a two-step read returns the same rows in ~3 ms #4228 or this fix introduced. Fixing it is a
  much larger cross-cutting change: connection pooling or reuse, with a real design question about the
  existing ReaderWriterLockSlim locking. Filed Lite: every DuckDB read opens a fresh connection (LocalDataService.OpenConnectionAsync) #4262 rather than fixing it in this verify-only lane.

New Lite.Tests/AgAvailabilityGroupNewestSnapshotEqualityTests.cs
(AgAvailabilityGroupNewestSnapshotEqualityTests) pins the new two-step code against the OLD (pre-#4228)
correlated-subquery SQL text, row for row, for both grains. Each seed covers:

• a tie: two keyable rows sharing the newest collection_time, the normal shape of any AG snapshot.
• a NULL: a third row at that same newest instant with a NULL key column. This is the WSFC-quorum-loss
  shape (ag_name for the database grain, replica_server_name for the replica grain). Old and new code
  both drop that row without affecting which instant is "newest". collection_time itself is NOT NULL in the
  DuckDB schema, matching what G2 found on the Postgres side.
• an older, non-newest row and a different server's row, both of which must be excluded.

Both facts pass, 2 of 2. Proved the pin catches a regression, once. Loosened the new code's step-two bound
from collection_time = $2 to collection_time <= $2 temporarily, rebuilt, and reran.

The tie/NULL test failed (Assert.Equal() Failure: Collections differ, the stale row leaking in). Reverted
the change. git diff on the reader now matches this PR's committed shape. Both facts pass again.

2. Live EXPLAIN (ANALYZE, BUFFERS)

Seeded the rig's probe database (migrated, TimescaleDB-converted) with G2's exact same 10-server-shape
seed (DarlingAgStatesReaderLiveEqualityTests's shapes). Ran EXPLAIN (ANALYZE, BUFFERS) on today's
statements (the same pre-#4228 oracle text G2 verified against origin/dev) and on the new two steps.
Parameters were bound the way Npgsql binds them: NpgsqlParameter with an explicit
NpgsqlDbType/TypedValue, not string interpolation.

statement	planning ms	execution ms	buffers (hit+read)	chunk scans
OLD replica (shared/current, unbounded)	2.071	1.422	hit=319 read=0	12
NEW step one, replica (fleet)	0.947	0.250	hit=134 read=0	6
NEW step two, replica (shared/current)	1.385	0.351	hit=180 read=0	2
OLD replica (stale server's own read)	1.408	0.717	hit=216 read=0	12
NEW step two, replica (stale server's own read)	1.281	0.230	hit=110 read=0	6
OLD database (shared/current, unbounded)	15.841	0.751	hit=328 read=2	12
NEW step one, database (fleet)	0.852	0.402	hit=134 read=0	6
NEW step two, database (shared/current)	0.805	0.282	hit=167 read=0	2
OLD database (stale server's own read)	7.398	0.867	hit=216 read=0	12
NEW step two, database (stale server's own read)	0.823	0.183	hit=110 read=0	6

Chunk-scan counts match G2's COSTS OFF finding exactly: 12 to 2 for the shared/current case, 12 to 6 for
the stale server's own read. The ms/buffer deltas at this seed's scale are small in absolute terms. This is
the same 10-server seed G2 used for the chunk-count assertion, not a production-scale seed.

At that size most pages are already cache-resident (read=0 almost everywhere). Wall-clock and buffer counts
are dominated by fixed per-statement overhead rather than data volume. The structural win is chunk exclusion.

At production volume that is the difference between the issue's 2,803 ms and 441k buffers, and the few
milliseconds a bounded read costs. The probe database was not re-seeded at production scale. G2's shapes
were what this item asked for. The chunk-count story they produce is the mechanism, confirmed twice
(COSTS OFF and ANALYZE, BUFFERS) on two different seeds.

3. Full suite, merged with origin/dev

Merged origin/dev clean, no conflicts (84 files changed upstream, mostly FinOps, JobHistory and QueryStore
work unrelated to this PR).

• Darling.Tests, freshly recreated darlingtest: 13887 total, 3 failed, 47 skipped, 1 not run (799.8s).
  The 3 failures are CaptureDownChunkOrderTests, PgTargetAnomalyTests, and
  ServerListAndSummaryPlanShapeTests. These are in files this PR never touches (Lite and Darling AG code
  only). Re-ran all three, alone, on a freshly recreated darlingtest: 48 of 48 pass, 0 failed. That
  confirms they are the known shared-store, order-sensitive live-test flakes. Not caused by this change, and
  not filed, per the "passes fresh and alone" rule.
• Lite.Tests, full suite, since this pass touched Lite: 5344 total, 0 failed, 0 skipped (370.3s).

Rig stopped after this run (pg_ctl stop -m fast).

Test plan (from G1, still standing)

• dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Debug: Build succeeded, 0 Warning(s), 0
  Error(s) (reconfirmed by G2 and G3).
• DarlingAgStatesReaderTests, DarlingAgReaderTests, AgTopologyCardsTests,
  AvailabilityGroupCountReadTests, DarlingMcpAgToolsSurfaceTests, DarlingMcpAgToolsLivePostgresTests.
• McpPayloadContractCensusTests, StartupCommandTimeoutTests, StorageCommandTimeoutTests,
  DocCommentHygieneTests, DarlingWebEndpointsTests, McpToolGuideTests.
• Live row equality against today's statements (G2).
• Live EXPLAIN (chunk-count form) (G2).
• Live EXPLAIN (ANALYZE, BUFFERS) timing/buffer numbers (G3, table above).
• Lite parity: measured, was over the bar, fixed, pinned (G3).
• Full Darling.Tests suite once, merged with origin/dev, freshly recreated darlingtest: 13887
  total, 3 failed, 47 skipped, 1 not run (G3). The 3 failures pass alone on a fresh database and are
  confirmed unrelated.
• Full Lite.Tests suite once: 5344 total, 0 failed (G3).

CHANGELOG entry

SECTION: Fixed
ENTRY:
- **Availability Group reads: bounded the newest-snapshot lookup** ([#4228]). All five AG reads used an
  unbounded aggregate to find each server's newest snapshot. On a production Darling store that took 2.8
  seconds and 441k buffers for 398 rows. On a Lite install seeded at Lite's own defaults, each call
  took 47-93 ms. All five reads are now a shared two-step read. Step one finds each server's newest
  instant cheaply. Step two re-runs the original query with that instant as a floor. The bounded step
  touches about 1 chunk instead of every retained day. The Lite query dropped by over 90%. Results are
  unchanged for every server shape: disabled, stale, and removed-from-list-but-still-holding-history.
REF:
[#4228]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/4228

What to double-check

1. Lite parity is resolved. Measured over the bar, fixed with the same two-step shape, and pinned.
2. Full suite ran post-merge-with-dev. The 3 transient failures pass alone on a fresh database and touch
   files this PR never opens.
3. The "single-chunk" phrasing in DarlingAgStatesReader's type doc is optimistic for a server stale by
   more than a day. Still strictly cheaper and correct, but worth a one-line correction next time the file is
   touched.
4. Filed Lite: every DuckDB read opens a fresh connection (LocalDataService.OpenConnectionAsync) #4262 (Lite's per-call DuckDB connection overhead) as a separate, out-of-scope finding.

Generated with Claude Code

https://claude.ai/code/session_01FVjn4PBJN71NQXdFo6ZxNQ