Collection Health: qualify a persistently-empty enumeration against the target's user databases

[erikdarlingdata]

Closes #1852.

What was missing

#1837 shipped the qualifier that says HOW MUCH of the window an enumerated collector spent empty - enumeration yielded 0 items ... (all 96 runs) - and deliberately stopped there. It could not tell apart:

• a target that is legitimately empty (no user databases, nothing matching the collector's filter), which must stay quiet and stays HEALTHY, from
• a target that has user databases and is still enumerating zero of them, which is the interesting case: a login that cannot enter any of them, an exclusion filter that swallowed everything.

Answering that needs a per-server inventory the collection_log aggregate does not carry. This adds it.

What ships

The three collectors that enumerate user databases now render a persistent empty-enumeration note as ... (all 96 runs, target has user databases) when the store has seen user databases on the same server inside the same window. All four Collection Health surfaces inherit it from the one shared CollectorHealthClassifier.FormatCollectionNote - Lite's grid, the Darling Viewer's grid, both get_collection_health MCP tools, and the web dashboard's table behind them. Both WPF grids already bind NoteFormatted and the web table already binds the server-composed note_summary, so no XAML and no JavaScript changed: the audit the design asked for came back "they inherit it".

The MCP payload also carries the raw target_has_user_databases, so a caller diagnosing an empty collector gets a boolean instead of parsing it out of a sentence.

The banding is untouched, which is the ceiling #1854 pinned. No new status value, nothing new reaches Classify, and both suites re-assert the neutrality test against the new input - two collectors identical except for the note AND the inventory flag band identically. A monitoring tool that reddens on a filter nobody minds gets ignored.

The mapping, and why it is only user databases

CollectorHealthClassifier.ExpectsUserDatabases is an explicit name set - query_store, database_scoped_config, index_object_stats - kept that way for the same reason OnLoadCollectorNames is: so the classifier stays free of a dependency on the collector catalog. It is not trusted on faith. Both suites walk CollectorCatalog.All, invoke BuildEnumerationQuery on an on-prem target, and assert the resulting set is exactly those three AND equals the classifier's set, so a collector cannot start or stop enumerating without a decision landing here.

That pin is also what settled the AG half of the design sketch. No AG collector enumerates: the empty-enumeration note is written in exactly one place (EnumeratedCollectorDriver.BuildNote, reached only from the enumerated path), and AgReplicaStatesCollector / AgDatabaseReplicaStatesCollector are plain single-query collectors. A mapping for them could never fire. Independently, the Darling store has no v_ag_replica_states view - Lite creates a v_ view for every catalog table, Darling's list is hand-maintained and omits it - so joining one needs a schema migration with a SchemaVersion bump. Filed as #1867 with what shipping it would take, rather than half-wiring a mapping whose signal is never supplied.

Staleness, and the four gates

The qualifier appears only when all four hold, and each has its own test:

1. the note is the empty-enumeration one - a probe-failure note already names its own cause, and appending "target has user databases" would restate what was just read (the combined "empty AND probes failed" note still qualifies, because it is an empty enumeration);
2. every run in the window carried it - the "all N runs" branch. A sometimes-quiet collector is normal however much inventory the target has;
3. the collector enumerates user databases - the flag is a property of the SERVER so it reaches every row, and only the mapping keeps it off the others;
4. there is inventory. Absent, aged out, another server's, or simply not collected all read the same: no qualifier. Silence over a false alarm - a server that genuinely has no user databases is the ordinary install this must never nag, and the unqualified rendering is asserted byte-identical to what Enumerated collectors that yield zero items log SUCCESS indistinguishable from healthy #1837 shipped rather than merely similar.

The inventory window is the health read's own second parameter - the same trailing seven days the grid summarizes. No second knob, and an inventory that aged out is not evidence about the target today, so it yields nothing rather than a claim the store can no longer support.

Why database_size_stats and not database_config

database_config is the intuitive inventory and the wrong one, for three reasons found while building this:

• it is an on-load collector (frequency 0), run once per server connect in both hosts, so on a long-running service its rows can age past the window while collection is perfectly healthy;
• it has no index in either store (PgSchemaGenerator.CreateIndex returns null for it, mirroring Lite);
• it reads sys.databases, where database_size_stats reads sys.master_files - so the size collector still sees databases the monitoring login cannot enter, which is exactly the fault being diagnosed.

database_size_stats runs every 60 minutes, is indexed on (server_id, collection_time), and screens on database_id > 4 - the collector takes every ONLINE database, tempdb included, so a bare row check would be true on every server alive. That screen is not decoration: dropping it to > 0 is one of the mutations below.

Cost, measured

The subquery is uncorrelated, so Postgres evaluates it once per query as an InitPlan rather than per row. Measured on PG 18.4 + TimescaleDB with the product's own settings (work_mem 16MB, shared_buffers 1GB), 200 servers x 4,000,000 collection_log rows over 7 days plus 3,360,000 database_size_stats rows:

case	probe	whole read
target HAS user databases	0.068 ms, 4 buffers of 20,043	83 ms median (10 warm runs)
before this PR	-	98 ms median (10 warm runs)
worst case: only system databases	7-12 ms warm, scans and rejects all 16,800 rows	~120 ms median

In the common case the two shapes are indistinguishable - the run-to-run spread is wider than the difference. The worst case is the legitimately-empty target, where the probe walks that server's whole window and finds nothing; 7-12 ms on a read that already costs ~100 ms, well inside the per-server budget. The plan confirms the shape outright:

InitPlan 1
  ->  Index Scan using idx_database_size_stats_time on database_size_stats (actual time=0.068..0.068 rows=1.00 loops=1)
        Index Cond: ((server_id = 42) AND (collection_time >= ...))
        Filter: (database_id > 4)
        Buffers: shared hit=4

The fleet rollup pays nothing, holding #1863's line. FleetCollectionHealthSql projects CAST(NULL AS integer) AS has_user_databases for the ordinal MapHealthRow shares and no value, for #1863's two reasons plus one of its own: this read groups server_id INTO its result, so there is no single server to probe an inventory for. A truthful fleet version would be a cross-collector join across every enabled server on a query the status bar re-runs on every aggregate-tab refresh. With last_note already NULL there, a real value could not change one rendered character. Pinned at source AND behaviorally, against live data that DOES qualify on the per-server read.

Test plan

Paired suites in both apps (EmptyEnumerationInventoryTests / DarlingEmptyEnumerationInventoryTests), each holding the formatter gates, the catalog pin, the banding neutrality, and live-store behavior - real DuckDB and real PostgreSQL 18.4 through BOTH Postgres readers (the Viewer's and the MCP service's), because a source pin cannot tell an inventory subquery that matches from one that silently never does.

Covered behaviorally: the qualifier appears only with a persistent empty note AND inventory; system databases alone are not inventory; an inventory older than the window says nothing; another server's says nothing; no inventory at all leaves the note byte-identical to #1837's; an unmapped collector is never qualified however much inventory there is; a probe-only note is not qualified; the combined note is; the fleet read still carries none while the per-server read on the same rows does.

Two shared contracts get their own pins: EnumeratedCollectorDriver.EmptyEnumerationMessage must start with CollectorHealthClassifier.EmptyEnumerationMarker (the marker is one duplicated substring, because Common deliberately does not reference Collectors - inverting that boundary would drag the collector layer into every consumer of Common), and the expectation set must equal the catalog's actual enumerators.

Three existing pins were strengthened rather than merely updated: both apps' "the property delegates to the shared formatter" tests now assert it on a row where the new inputs CHANGE the answer, so they would fail if NoteFormatted dropped them; the fleet ordinal-parity pin now also pins the deliberate NULL and that v_database_size_stats appears in the per-server read and NOT the fleet one; and the MCP note_summary pin is whitespace-collapsed so it survives the wrapped argument list instead of pinning one formatting of it.

Watched red - six mutations, each reverted after:

mutation	Lite	Darling
qualifier never applied	5	5
database_id > 4 to > 0	1	0 then 1
inventory window dropped	1	1
fleet read claims inventory	-	2 (source pin + live)
collector-mapping gate dropped	3	3
empty-enumeration marker gate dropped	1	1

The second row is why this was worth doing: the Darling suite originally seeded system and user databases together, so it stayed green with the screen removed. It now has a system-databases-only case, and goes red.

dotnet test Lite 1713 passed / 0 failed; Darling 3826 passed / 0 failed / 9 skipped against a live store (dropped and recreated first). dotnet build PerformanceMonitor.sln -t:Rebuild: 0 warnings, 0 errors. Installer.Tests not run.

Filed rather than shipped

• Collection Health: no Availability-Group inventory qualifier - no AG collector enumerates, and the Darling store has no v_ag_replica_states view #1867 - the Availability-Group mapping, with both reasons and the three things shipping it would take.

Note for the reviewer

The one methodological trap worth recording: an early portability probe created stub collection_log / database_size_stats tables in the same scratch database the live tests then migrated, and CREATE TABLE IF NOT EXISTS no-opped over them - so the first live run failed on a column that exists everywhere real. dropdb && createdb and it was gone. The suite numbers above are from a database recreated fresh.

🤖 Generated with Claude Code

[claude]

Review summary

Went through the full diff (11 files, +1152/-35) plus the surrounding call sites in both apps. This is a tightly scoped, well-tested change — no blocking issues found.

Correctness

• FormatCollectionNote's branch rewrite (noteCount >= totalRuns && totalRuns > 0 → early-return on the negation) is a faithful De Morgan transform; the unqualified path is still byte-identical to the pre-Enumerated collectors that yield zero items log SUCCESS indistinguishable from healthy #1837... er, pre-Collection Health: qualify a persistently-empty enumeration against whether the target HAS user databases #1852 rendering, and the new suites pin that explicitly.
• All four gates (empty-enumeration marker present, persistence == totalRuns, collector in ExpectsUserDatabases, inventory flag true) are independently required and independently tested, including the "probe note already names its cause" exclusion and the combined empty+probe case.
• Verified the $1/$2 binding order against the actual GetCollectionHealthAsync parameter-add code in both DarlingDataReader.cs and ViewerDataService.CollectionHealth.cs (and Lite's DuckDB equivalent) — the new EXISTS subquery's server_id = $1 AND collection_time >= $2 lines up correctly with serverId/window-start in all three readers, and the new has_user_databases column lands at ordinal 13 consistently everywhere it's read.
• v_database_size_stats does exist in both stores (Postgres view in PgMigrations.cs, and it's already used elsewhere in Lite's DuckDB queries), so the new subquery isn't reaching for a view that doesn't exist.
• The fleet-vs-per-server split holds up: DarlingFleetReader.cs's separate FleetCollectionHealthSql (used by the MCP fleet tool) never surfaces last_note/note_count at all, so it correctly needed no changes — it's a genuinely different query from ViewerDataService's fleet read, not a missed parity spot.

Lite/Darling parity

• All four production call sites (Lite/Mcp/McpHealthTools.cs, Lite/Services/LocalDataService.CollectionHealth.cs, Darling/.../DarlingMcpDataTools.cs, Darling/.../ViewerDataService.CollectionHealth.cs) were updated in lockstep — the MCP JSON payload shape, the NoteFormatted property, and the SQL comment blocks are essentially byte-identical between apps (only a single cosmetic word differs: Darling's comment says "long-running service", Lite's says "long-running install" — harmless, matches each product's own vocabulary).
• CollectorHealthClassifier.ExpectsUserDatabases/EmptyEnumerationMarker/HasUserDatabasesQualifier live once in PerformanceMonitor.Common and both apps' test suites pin the same expectation set against CollectorCatalog.All, so a collector starting/stopping enumeration can't silently drift out of sync with the mapping.
• Confirmed Classify(...)'s signature is untouched and never receives the new flag — banding neutrality claim checks out in the code, not just the tests.

Security

• No dynamic SQL / string concatenation introduced — the new EXISTS subquery is a static, parameterized string using the query's existing $1/$2 placeholders. No new attack surface.

Performance

• The subquery is uncorrelated (no reference to the outer GROUP BY's columns), so it evaluates once as an InitPlan/single execution rather than per row on both engines, matching the PR's measured numbers. No missing-index concerns to raise here since it's reading an already-indexed internal store table, not a monitored-server DMV pattern.

Nice test coverage (live DuckDB + live Postgres through both readers, mutation-tested per the PR description). Nothing to change from my end.