[erikdarlingdata]

Why

On an Availability Group, the Recommendations tab reports the same per-database finding once per node, secondary copies included. A database's settings and Query Store content replicate from the primary, so every node says the same thing about it. This change makes a node skip those findings for databases it holds only as a secondary copy. The primary reports them once. Findings that come from work done on the node itself (queries, blocking, waits, I/O, disks) stay on every node, and so do server-level findings.

How a secondary is decided

The rule uses data the monitor already stores (ag_replica_states and ag_database_replica_states). It adds no query on the monitored server, no collector change and no schema change. The engine's own definitions are in these pages:

• https://learn.microsoft.com/en-us/sql/relational-databases/system-dynamic-management-views/sys-dm-hadr-database-replica-states-transact-sql
• https://learn.microsoft.com/en-us/sql/relational-databases/system-functions/sys-fn-hadr-is-primary-replica-transact-sql

The stored snapshots are used instead of a live sys.fn_hadr_is_primary_replica call for three reasons. There is no new query. An AsOf pass and each compare_analysis window use the role at that time rather than now. And it fails open on Azure SQL Database, which has no such data.

A database is skipped only when all of these hold. Both snapshots exist and are within an hour of the window end. The database has a local row. The local replica's role for that database's group is explicitly SECONDARY. The role is read per group, so two groups with different primaries are handled. Missing, stale, NULL, RESOLVING, standalone and Azure SQL Database cases skip nothing.

What changes (done in this PR)

• Shared rule: AgReplicaScope in PerformanceMonitor.Common, next to AgAlertPolicy. It is pure, case-insensitive and fail-open.
• FactReplicaScope in PerformanceMonitor.Analysis is the one table that says which fact keys are replicated and which are node-local. Replicated keys are filtered on a secondary: DB_CONFIG and its sub-keys, FILE_AUTOGROWTH_PERCENT, PLAN_REGRESSION and ANOMALY_OBJECT_GROWTH. Everything else is kept, and an unknown key is kept.
• AnalysisContext.SecondaryReplicaDatabases is resolved at the pass door.
• Lite resolves it at all three doors of AnalysisService. Each compare window resolves its own set. The predicate is applied in the DB config, autogrowth and plan regression facts and drill-downs, and in the object growth anomaly.
• Darling has PgSecondaryReplicaScope (resolver and predicate) and the same predicates in the Pg fact, drill-down and anomaly twins. The audit-config and same-statement-pileup contexts set an explicit empty set because they read only node-local facts. Lite's same-statement-pileup context now does the same through SecondaryReplicaScope.NoneSkipped.

Display note on the findings surfaces

When this node holds a secondary copy of any database, the same one-sentence note (AgReplicaScope.SkippedNote) appears on every engine Recommendations surface.

• Lite Recommendations tab: a line above the list or the all-clear. It is hidden for the loading, still-collecting and nothing-collected states, which replace the whole area.
• Darling viewer Recommendations tab: the same line, same rule.
• Darling web Recommendations page: a notice strip from the secondary_replica_note field, shown with the list and with the empty list.
• get_analysis_findings in both Lite and Darling: a secondary_replica_note field on the payload (null when nothing is skipped). The empty answer carries the note in its message. An as_of read names the role at that time.

Finding alerts

Checked: no alert path re-reads facts. Both alert senders build their alerts from stored findings. The only code that calls a fact collector is the analysis service, which already runs behind the filter. So the alerts follow the filtered facts with no further change.

Test plan

• Lite: AgReplicaScopeTests, FactReplicaScopeCoverageTests and SecondaryReplicaFactTests (seeded DuckDB store), plus the new get_analysis_findings note test in McpAnalysisFindingsCommandTests. With the Census, Ratchet and Recommendations classes: 142 total, 0 failed.
• Darling, against a local PostgreSQL rig: SecondaryReplicaScopeLiveTests is the seeded twin of Lite's fact tests. It covers the DB config fact and drill-down counts, the autogrowth fact and drill-down, plan regression and object growth. It also covers the primary, no-rows and stale-rows cases, the AsOf role and the get_analysis_findings note. PgSecondaryReplicaScopeTests covers Apply (marker removed, array parameter numbering, WHERE keyword, node-local key, no set) and the fail-open ReadAsync and EnsureAsync. AnalysisContextSecondaryScopeSourceScanTests fails when a new AnalysisContext in Lite or Darling product code does not resolve the set. The web page has a Node harness scenario in AnalysisFindingsTabTests. Those classes: 30 total, 0 failed, 0 skipped.
• Darling Census, Ratchet, DocCommentHygiene, ViewerRecommendation, McpPayloadContract, AnalysisAsOfAnchor and the findings budget class: 634 total, 0 failed.
• dotnet build of Lite, Lite.Tests, Darling and Darling.Tests: succeeded, 0 warnings.
• Not run: the full suite and every Stage=Guard class (CI decides).
• No test drives the WPF windows themselves. The Lite and viewer view models are covered by tests, and the XAML wiring is not.

Left undone

• The FinOps Recommendations sub-tab (both products) is the other lane's work on this branch. It is not part of this section.

For the coordinator to double-check

• The freshness limit is one hour (AgReplicaScope.SnapshotFreshness). A custom AG collector cadence slower than that fails open.
• Both products' regressed-queries drill-downs now filter in SQL, ahead of the LIMIT 5, when the fact did not run (round 1).
• Lite.Tests/LatestValueLookbackTests.cs only had two seed helpers made public so the new tests can reuse them.

FinOps Recommendations

The FinOps Recommendations sub-tab now leaves out databases this server holds only as a secondary copy in an availability group. The primary reports them.

What is filtered, and what is kept

• Dropped for a secondary copy: idle databases, dev/test names, TDE blockers, and compression candidates.
• Kept: low IO latency. Each replica has its own storage, so its latency and cost belong to that replica.
• Nothing is skipped when the availability group data is missing, stale, null, resolving, standalone, or from Azure SQL Database. The set comes from the same stored snapshots as the rest of this PR.
• Lite filters the live-query results in C#. It adds no availability group query. For compression, the query reads only the connection's own database, so that database is skipped when it is a secondary copy.
• The Darling storage reader takes the set as an optional parameter. The Analysis project depends on Storage, so the caller (viewer or MCP tool) resolves it.
• If every TDE database on the node is a secondary copy, the edition audit stays silent. It does not claim "no Enterprise-only features".

Instance-level role check

• The edition note still follows the instance role (GetAgReplicaRoleAsync in Lite, server_properties.ag_replica_role in Darling). An edition is one decision for the whole group, so it is not a per-database finding.
• The per-database set replaces the role only for the TDE list. An instance that is primary for one group and secondary for another still lists the TDE databases it holds as primary.

Where the note shows

• It uses AgReplicaScope.SkippedNote. Lite shows it on the Recommendations sub-tab, the Darling viewer on its FinOps Recommendations sub-tab, and the web on the same page.
• The get_finops_recommendations payload carries it as secondary_databases_note. The key is written only when the node holds a secondary copy, so every other server's payload is unchanged.

Tests

• Lite: new FinOpsSecondaryReplicaTests (7 pass). A secondary copy drops from the idle list. A primary, stale data and no data change nothing. The low IO list is unchanged. The shared filter helper is covered.
• Darling: new PostgreSQL-gated FinOpsRecommendationsSecondaryReplicaLiveTests (5 pass on a scratch PostgreSQL). The cases are dropped from each filtered list, kept on the primary, kept on stale data, low IO unchanged, and all-TDE-secondary. They read through the storage reader, viewer and MCP tool. A new envelope test pins the payload with and without the note.
• Targeted Darling classes plus every Census and Ratchet class, and the source-scan classes that read touched files: 1010 run. Two guard pins were updated on purpose. The literal-LIMIT roster now names GetRecommendationsWithNoteAsync, and AgReplicaScope.IsFresh joins the known truncated ranges.
• Lite, Lite.Tests and Darling.Tests build with 0 warnings. I did not run the full suite.

Review round 1

• Lite reads the AG snapshots through v_ag_replica_states and v_ag_database_replica_states. These cover the hot tables and the Parquet archive, like every other Lite analysis read. An AsOf pass or compare window older than 7 days now resolves the role, and so does a store right after the 512 MB reset. A new test puts the AG rows only in Parquet and asserts the set. It fails with the bare tables.
• Lite AnalyzeAsync resolves the set inside its try, after IsAnalyzing = true. A cancelled or timed-out pass takes the existing abandon path, and the guard window is no wider. Darling's wrapper reads the set in its context initializer. There, an abandoned budget now yields an empty set instead of escaping AnalyzeAsync (DarlingAnalysisBudgetTests caught this). A new test checks that a cancelled pass returns an empty list without throwing.
• The AnalysisContext source scan now judges each new AnalysisContext on its own member, so a second context cannot borrow another's EnsureAsync. Lite's audit_config context (CollectConfigAuditFactsAsync) now sets NoneSkipped, like Darling's. Red plant: removing that one assignment made the scan fail on exactly that site. I restored it.
• analyze_server and compare_analysis, on Lite and Darling, carry secondary_replica_note. analyze_server carries the sentence for the set the pass used, from a new LastSecondaryReplicaNote on both analysis services. It is in the findings, empty and unavailable payloads and in the empty message. compare_analysis carries one note per window: baseline.secondary_replica_note and comparison.secondary_replica_note. In the both-windows-empty answer they are baseline_secondary_replica_note and comparison_secondary_replica_note under hints. Each comes from the role at that window's end. Both sides have payload tests.
• Darling's regressed-queries drill-down filters in the source CTE of both SQL variants, before LIMIT 5, matching Lite. The C# filter is gone. In the live test the secondary database holds the five worst regressions and the primary's two still come back. It fails without the filter.
• The newest-snapshot lookup has a lower bound of window end minus the one-hour freshness limit, on both sides. The answer is unchanged, since an older snapshot was already stale.
• One field name, secondary_replica_note, on every MCP tool, always present and null when nothing is skipped (the convention of truncation_note, persistence_note and the FinOps cost_reason). The FinOps tool's secondary_databases_note is renamed and now always written. Its tool text, web page and tests follow.
• Lite GetRecommendationsAsync is kept. The review said it had no callers, but Lite.Tests calls it in FinOpsTests, ArchivedHistoryReadersTests and TestDataSeeder.
• The two Lite test names now match their bodies. The primary, no-rows and stale cases are one test. The object-growth test seeds lock waits on the secondary database and asserts the contention anomaly still names it.
• AgReplicaScope remarks now say a distributed AG forwarder skips nothing, by design. Its database is the local primary of its own group, so it is kept (fail open).
• Guard stage run. It found six classes missing their Reads traits: three in Darling.Tests (Reads=Lite) and three in Lite.Tests (Reads=Darling). I added them.
• Done in round 2 (commit 3fad459): get_analysis_facts (Lite and Darling) now carries secondary_replica_note too, in the same shape as the other tools. It is read the way compare_analysis and get_analysis_findings read it, a separate read as of the window end, so two concurrent reads cannot swap notes. The field is in the facts payload, in the "facts but the window was never observed" answer and in the "no facts" answer, and the "no facts" message also names the note. That last answer used to be a bare status and message when no collection caveat applied. It now always carries a hints block with the field. New tests: two on each side (secondary node in all three envelopes, primary node null). Run, all passing: the new Darling class on a PostgreSQL rig (11). Guard stage (441 Lite, 1409 Darling). The Census, Ratchet, McpPayload, DocCommentHygiene and analysis classes (135 Lite, 600 Darling). The existing get_analysis_facts live classes (9).

Round 1 tests (Lite.Tests and Darling.Tests built with 0 warnings): Guard stage 441 Lite and 1409 Darling, all passing. Census, Ratchet, McpPayload and DocCommentHygiene classes: 576 Darling and 60 Lite, all passing. Source-scan classes that read touched files: 2021 Darling (72 skipped, live) and 1106 Lite, all passing. Live Darling classes against a local PostgreSQL 55520 rig (SecondaryReplica*, AnalysisAsOfAnchor, FinOpsRecommendationsReadsLive, NewestSnapshotReadsLive): 35 passing. I did not run the full suite. CI decides.

Review round 2

All four Low findings are fixed in 5e86494.

• The compare note now comes from the sets the windows used. ComparePeriodsAsync returns both windows' secondary sets after the coverages (Lite/Analysis/AnalysisService.cs:671, Darling/PerformanceMonitor.Darling.Analysis/DarlingAnalysisService.cs:914), and compare_analysis builds both notes from them (Lite/Mcp/McpAnalysisTools.cs:458, Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpTools.cs:492). The second role read, and Darling's read with no logger, are gone. get_analysis_facts filters at read time, so it had the same gap. Its set now rides on the returned CollectionCaveatState.SecondaryReplicaDatabases (PerformanceMonitor.Analysis/AnalysisContext.cs:397) and both tools build the note from it (McpAnalysisTools.cs:313, DarlingMcpTools.cs:347). get_analysis_findings stays as it was: it reads findings the pass already stored, so there is no filter at read time to match. New tests: CompareAndFactsReads_CarryTheSetTheirContextFilteredWith_SoTheNoteCannotDisagree on Lite (DuckDB) and Darling (live PostgreSQL).
• Database names now match exactly (ordinal). Both sides come from the same catalog. The AG snapshot's database_name is sys.databases.name (PerformanceMonitor.Collectors/AgDatabaseReplicaStatesCollector.cs:133, d.name), and the replicated facts' names are d.name or DB_NAME() (DatabaseConfigCollector.cs:82, QueryStoreCollector.cs:180, IndexObjectStatsCollector.cs:344, FileIoStatsCollector.cs:80). Changed: the set builder and IsSkipped (PerformanceMonitor.Common/AgReplicaScope.cs), the DuckDB filter (PerformanceMonitor.Analysis/FactReplicaScope.cs:114,124) and the PostgreSQL filter (PgSecondaryReplicaScope.cs:147), which no longer wrap the column in lower(). The FinOps Recommendations path uses IsSkipped, so it matches exactly too. Two tests that pinned case-insensitive matching were updated on purpose (FinOpsSecondaryReplicaTests, FinOpsRecommendationsSecondaryReplicaLiveTests). The one limit (a case-only rename between a stored row and the snapshot shows a finding, never hides one) is in the AgReplicaScope remarks. New tests: a secondary SalesDb next to a separate salesdb on the pure rule, on the PostgreSQL predicate, and on a seeded DuckDB store.
• The source scan stops at the member end (Darling/Darling.Tests/SecondaryReplicaScopeTests.cs, the limit clamp before the }; search). Red-planted once: a temporary file with Foo(new AnalysisContext { ServerId = 1 }); in one method and a context that sets the property in the next. The old scan passed it, and the new scan failed it at the first method. The plant is removed.
• The drill-down test: both SQL variants (raw and interval table) are pinned to carry the marker exactly once, before LIMIT 5 (EachRegressedQueriesVariant_CarriesTheMarkerExactlyOnce_BeforeItsLimit, a two-case theory). I took the brief's second option rather than seeding the interval table for a live theory.

Round 2 tests (built with 0 warnings on both projects): Guard stage 441 Lite and 1409 Darling, all passing. McpPayload, Census, Ratchet, source-scan, analysis and FinOps classes on both sides pass. Two live classes ran on a throwaway PostgreSQL on port 55531 and passed. The rig is stopped. No full suite was run.

Fixes #5558