Skip to content

Query Store reads can come from a readable replica (#6254) - #6342

Merged
erikdarlingdata merged 72 commits into
devfrom
fix/6254-qs-replica-reads
Oct 22, 2026
Merged

erikdarlingdata merged 72 commits into
devfrom
fix/6254-qs-replica-reads

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Oct 21, 2026 •

Copy link
Copy Markdown
Owner

Closes #6254.
Closes #6279.

Claude posting for Erik Darling.

Draft. Built over several lanes on one branch; this is the consolidated body. Final-tree results are in ## Test plan.

Why

#6254: with an Availability Group, Query Store is read from the primary, which is the replica that can least afford it. A readable secondary holds the same Query Store data, so the collectors can read it there, as long as the read stays correct: a secondary lags the primary by the redo, and the flush interval decides how old a row can be when it first becomes visible. Reading a replica naively loses rows (a row that was not yet visible at the watermark is never read again) or duplicates them. #6279 is the half of that problem that exists with no replica set: a capturing secondary (FOR SECONDARY SET QUERY_STORE = ON) writes rows into the primary's Query Store late, so the watermark can step past rows that arrive afterwards.

What changes

  • Per-server setting. QS replica (Lite: servers.json QueryStoreReplica and an Add/Edit field; Darling: queryStoreReplica in darling.json, store column config.config_monitored_servers.query_store_replica, the Viewer Add/Edit dialog, the web admin edit form, MCP add_servers and edit_server). A host, host,port or listener name; empty keeps today's behaviour exactly. SQL Server only; an Azure SQL Database never reads a replica. Protocol prefixes (np:, lpc:, admin:) and UNC or path values are refused; tcp:host,port is allowed.
  • How the collector reads it (shared layer QueryStoreReplicaRouting, QueryStoreShipGuard, QueryStoreCapturingSecondaryTracker in PerformanceMonitor.Collectors, used by DarlingCollectorRunner and Lite's RemoteCollectorService). Per database and per run it decides primary or replica. The replica is used only when it is the readable non-capturing secondary of the same group, the database's redo state is known and not behind, the versions match, and the flush interval is 900 s or less. Otherwise the database is read from the primary, with a stated reason (replica_is_primary, not_mirror, flush_too_slow, no_redo_state, wrong_database, version_mismatch, redo_behind, replica_unavailable). A replica read re-reads a look-back window of the watermark minus L (L is sized from the flush interval; 25 minutes is the measured bound at 900 s), a ship guard drops rows already shipped so the overlap never double-stores, and a replica read never advances the Query Store backfill contends with the live path on busy replicas — yield the slice when live collection is failing #2111 shrink counter or stages the open-interval stamp. A replica enumeration, version probe, redo read or per-database read that fails falls back to the primary the same run (replica_unavailable), never to a run error. A read cut by the row cap or byte budget records a hole and the next read starts at the watermark. Backfill always reads the primary.
  • Query Store collectors: rows a capturing secondary sends may land below the watermark #6279 (no replica set). A capturing secondary is detected from the rows the collector already reads (a row with a non-primary replica_role); a database with one gets the same look-back and ship guard on the primary, so late rows are no longer lost. The tracker lapses after 24 hours and is seeded from the store at start (Darling) or on the first Query Store run (Lite).
  • Replica password binding (Darling, security). The replica host is a field of ServerConnectionIdentity; PasswordBinding.ForServer binds it as an 11th field only when a replica is set, so every existing seal opens unchanged. A password sealed for one replica does not open for another or with the replica removed, so a stored login is never sent to a host nobody approved. Every seal and identity site (service, MCP edit core, Viewer seal, Viewer dialog, test-connect) carries it. A stored env:/file: reference goes to a replica only when darling.json declares it. The builder takes the replica from the identity the seal opens under, and returns null for PostgreSQL and Azure SQL Database. The MCP agent-route guard (MCP tag tools can change which servers an agent route covers #6281) counts a replica change like a host change, and add_servers runs the replica host through the same held-machine check as the host. The diagnostics bundle aliases a store-set replica host.
  • read_source says where a Query Store run read from, for example replica 12, primary 2 (redo_behind: Sales) (names capped at 3, non-AG databases left out, NULL when no replica is set and for every older row). Column collect.collection_log.read_source (Darling) and collection_log.read_source (Lite DuckDB v78). It shows in: Darling MCP get_collection_health and get_collection_log (per server and fleet; a run with none carries no key, so a no-replica page is byte-for-byte unchanged), the web Collectors table, the Viewer collection health and its two collection-log grids, Lite collection health, Lite get_collection_log and the Lite log grids (appended to the message column as <message>; read source: <source>).
  • Store migration V213 (query-store-replica; dev's V212 is MCP add_servers and edit_server recognise one machine under another name (#6285) #6340): ALTER TABLE ... ADD COLUMN IF NOT EXISTS for the two nullable columns (catalog-only, no backfill) and CREATE OR REPLACE VIEW collect.v_collection_log (the view freezes its column list). config.edit_monitored_server becomes 18 arguments (p_query_store_replica); the 17- and 15-argument forms stay as wrappers. A replica change counts as a connection change, so a replica-only edit asks for the password again; the password-rules trigger counts it as a move. Store-migration obligations done: probe sentinel and MapProbedSchemaVersion arm, GetBoolean list (index 188) and ViewerSchemaVersionGateTests, RungRewindArtifacts.Ladder, the Scripts[^1] tests, the README rollback line for V213 (an older build ignores both columns; nothing to do before rolling back), and query_store_replica in the Viewer's non-secret column list.
  • Bring-your-own stores must re-run Darling/tools/provision-roles.sql after the upgrade. The service replaces the three edit_monitored_server functions itself when its login owns them, but the 18-argument one is a new function that only the script grants to the viewer and mcp roles, and only the script grants them the new query_store_replica column. Until then a web or MCP server edit fails with permission denied. Compose and managed stores do it on their own. The README says so beside the V203 line.
  • Lite parity. Lite has the setting, the dialog field, the replica connection (same login), the same routing, ship guard, tracker and look-back, the DuckDB v78 column (archive views use union_by_name, so older parquet reads NULL), and its own health, log and grid display. Lite docs: docs/lite-guide.md. Darling docs: README server-settings table.
  • Also fixed on the way: the replica probe admits actual_state 0 or 1 with readonly_reason & 8 = 8 (a non-capturing secondary reports 0 on SQL Server 2025 CU9, 1 on 2022), never 4 (a capturing secondary, routed not_mirror); on SQL Server 2022 a non-capturing secondary reports its own default flush row, so the larger of the primary's and the replica's flush value gates and sizes L.

Test plan

Final-tree results (rig 55504, Postgres runtime zip, merged with origin/dev; this lane)

  • Merged origin/dev (7d10470 and later). One conflict, DarlingEmptyEnumerationNoteTests: dev (Darling runner: check whether a collector note is dropped before the success return #6356) deleted the runner-note source pin and added a behaviour test; took dev's side. No migration collision (dev's newest is V212, this branch stays V213). Restored the UTF-8 BOM that an earlier edit had dropped from PgMigrations.cs.
  • Build: Darling.Tests and Lite.Tests, 0 Warning(s), 0 Error(s) (checked on the build's own summary line, not a pipe's exit code).
  • Brief classes on the rig (*QueryStoreReplica*, *PasswordBinding*, *Census*, *Ratchet*, *Migration*, *ViewerSchemaVersionGate*, *RungRewind*, *DarlingMcpServerAdmin*, *McpMachineIdentity*, *ServerIdentityFromStore*, *PostgresMajorVersionRegistry*, *ViewerDataServiceTests*, *CollectionCycleRow*, *DarlingRunnerCollectionNote*): Total: 1014, Errors: 0, Failed: 0, Skipped: 3 (the 3 are the AG-fixture live tests).
  • Lite *QueryStoreReplica*, *Census*, *Ratchet* plus the local-classes list: Total: 192, Failed: 0.
  • ci-select.py local-classes list (260 Darling classes, 40 Lite classes): the Lite 40 ran, 0 failed. The Darling 260 ran for 40 minutes and I stopped it at the lane's wall clock before it finished (killed by PID). In the part that ran it logged 2 failures, both this branch's and both fixed in commit 3892954: PostgresTargetConfigTests.EveryRoundTripCriticalServerPropertyHasAStoreColumn (the QueryStoreReplica property had no entry in the round-trip list) and AlertNotebookTableCatalogTests.EveryFieldKeyInTheCatalog_IsAFieldTheReadsOwnPayloadBuilds (read_source is written into the serialized payload by two helpers, so the scan now names them). Re-run after the fix with QueryStoreReplicaSettingTests: Total: 94, Failed: 0. 56 tests skipped in that partial run (live tests that need SQL Server or a second PostgreSQL).
  • Guard (-trait Stage=Guard), both suites: [guard-darling] Darling.Tests Total: 4442, Errors: 0, Failed: 0, Skipped: 0; [guard-lite] Lite.Tests Total: 1196, Errors: 0, Failed: 0, Skipped: 0.
  • bash .github/scripts/pre-push.sh was NOT run on the final tree (its build plus Guard plus class selection did not fit the lane's 60 minutes). Earlier tips ran it; see Gates below. CI decides the rest.

Measurements behind the design (SQL Server 2025 CU9 and 2022 CU26 docker AG fixture, flush 60 s)

  • A non-capturing readable secondary reports actual_state = 0 (2025 CU9) or 1 (2022 CU26) with readonly_reason 8; with FOR SECONDARY SET QUERY_STORE = ON it reports 4, reason 8. sys.query_store_replicas cannot detect a capturing secondary (a static four-row roster), so detection uses the rows already read.
  • A capturing secondary's unflushed rows show on its own side with replica_group_id = 2 and a negative interval id. Redo rows (sys.dm_hadr_database_replica_states) read on both sides: same group_database_id, last_commit_time within about 0.01 s, redo queue 0. The 25-minute L for capturing databases is a measured bound (18.5 minutes at F = 900), not a proof.
  • The generated enumeration and redo statements were run against ag1 (primary), ag2 (secondary), a recreated 2025 fixture and sql2019 (non-AG): every result set parses through its matching reader (QueryStoreReplicaReadsLiveSqlTests, gated on DARLING_TEST_SQL*).
  • Live AG: Lite on ag1 with QueryStoreReplica = ag2 gives read_source replica 1, primary 0 with rows under ag1's server id; Darling QueryStoreReplicaAgFixtureLiveTests (the real runner, real connector) the same. Both passed on the fixture in earlier lanes; the fixture is torn down and they skip without it.
  • Browser check on a local rig service (playwright-cli, about 14 commands, 0 screenshots): the web admin edit form shows "QS replica:" with the stored host, and the query_store Collectors row shows "Read source: replica 12, primary 2".

Behaviour tests and revert proofs (each break made once, test failed, restored)

  • Routing and ship guard (QueryStoreReplicaRoutingTests, QueryStoreShipGuardTests): no L subtraction in ComputeCutoff fails T1 and T1c; probe admitting state 4 fails NotAReadableNonCapturingSecondary; eviction at W instead of W - L fails 6 guard tests; WasCut ignored fails T4; FailureCountsTowardShrink true fails 2; tracker never lapsing fails its test; replica flush used alone fails PrimaryFlushOver900....
  • Lite runner (QueryStoreReplicaRunnerTests, through RunCollectorAsync): zero look-back fails T5 and the capturing test; skipping the guard filter fails the same 2; T6 (setting empty: 0 replica connections, cutoff equals the watermark, read_source NULL) and enumeration failure fail 2 and 1 when reverted; per-database fallback (catch disabled), the clamp hole (record dropped), the row-cap cut (record dropped, WasCut ignored) each fail. The Azure check is dead code behind RunsPerDatabase, so no revert fails it; it is a guard only.
  • Lite setting: v78 AddedColumns entry and bump removed plus builder keeping the primary host fails 5 of 8.
  • Password binding: appending the 11th field disabled fails 2 of 7 (A seal for replica A does not open for B, does not open once a replica is set).
  • MCP and builder (security round 1): MovesConnection => AddressChanged plus no replica check fails both live MCP tests; stored-reference check removed fails A_stored_reference_goes_to_a_replica_only_when_the_file_declares_it; free property instead of identity and no Azure gate fail 2; Refusal returning null fails all 9 refusal cases; replica read returning no rows fails the bundle alias test.
  • Runner commit order: commit call removed fails on the final cache value; commit before the flush fails while blocked (TheRunnerCommitsTheDatabaseWatermark_OnlyAfterTheFlush_AndDoesCommit).
  • Health and log display: Darling health wiring emptied fails Health_ShowsTheNewestQueryStoreRunsReadSource_OnThatRowOnly; per-server and fleet log page read sources nulled each fail Log_CarriesTheReadSourceOnTheRunsThatHaveOne...; Viewer reader and MessageFormatted reverted each fail their test; Lite health, per-server log, fleet log, fleet reader, drill reader, grid reader and MessageFormatted reverted each fail theirs.
  • Query Store backfill contends with the live path on busy replicas — yield the slice when live collection is failing #2111 identity-clearing on an 18-argument edit (MCP add_servers and edit_server recognise one machine under another name (#6285) #6340 block): disabling it fails AnEditThatMovesTheAddress_ClearsTheStoredIdentity.
  • Live store tests (passed on the rig): a store at V212 with a sealed server row and a log row climbs to V213, both columns read NULL, the password is untouched, re-applying changes nothing (QueryStoreReplicaStoreLiveTests); a replica-only edit answers password_needed through the viewer role, managed and self-managed text; a store holding the old edit function gets the current one at the service start.

Tests deleted or re-spelled (#5833; each had a spelling rule, not a behaviour one)

  • Deleted: three SQL-text spelling tests of the enumeration builders (the live SQL test replaces them with behaviour); QueryStoreDatabaseWatermarkWiringPinTests.TheCommit_IsCalledOnce_... (source positions; behaviour covered by DatabaseWatermarkCacheRunnerLiveTests and the commit-order test above); DeadlocksTelemetryCursorTests.BothHosts_LandStagedItemState_... (source order; DeadlocksAzureLoopStateLiveTests runs the loop); the substring block of QueryStoreAzureTextAndPlanFetchTests.BothReadSitesShareOneFetchMethod_...; DarlingEmptyEnumerationNoteTests.Runner_Carries_The_Note_Onto_The_Success_Return_Too (dev replaced it with the behaviour test DarlingRunnerCollectionNoteLiveTests in Darling runner: check whether a collector note is dropped before the success return #6356; the merge took dev's side).
  • Moved (rule holds): PasswordBindingCensusTests allowlists ConnectionIdentity (the binding itself, reason in the test); CollectionLogPhaseSplitStoreTests tripwire 32 to 33 parameters; PostgresTargetConfigTests.EveryRoundTripCriticalServerPropertyHasAStoreColumn gained QueryStoreReplica; AlertNotebookTableCatalogTests names the two helpers that write read_source into the serialized payload; UnpooledDropConnectionCensusTests allows the live test's scratch-database drop.

Gates (earlier tips; the final tree's gate is in Final-tree results)

  • Lite part: [guard-darling] Total: 4441, Failed: 0, [guard-lite] Total: 1196, Failed: 0, GATE STOP: selection is the whole suite (darling 2247, lite 707).
  • ui-finish tip (merged with dev, rig 55481): [guard-darling] Total: 4441, Failed: 0, [guard-lite] Total: 1196, Failed: 0, GATE STOP (whole-suite selection); *CollectionHealthReadSource*, *ViewerSchemaVersionGate*, *CollectionLog*, *Census*: 651 run, 0 failed, 1 skipped; Lite *CollectionHealthReadSource*, *CollectionLog* 0 failed.
  • final-merge tip: [guard-darling] Total: 4442, Failed: 0, [guard-lite] Total: 1196, Failed: 0, GATE STOP.

Not run, parked, or still open

  • The AG-fixture live tests are skipped tonight (no fixture): Darling.Tests.QueryStoreReplicaAgFixtureLiveTests, Lite.Tests.QueryStoreReplicaAgFixtureLiveTests, QueryStoreReplicaReadsLiveSqlTests. They passed on the fixture in earlier lanes, before the merge with dev and the later edits.
  • bash .github/scripts/pre-push.sh on the final tree.
  • The rest of the 260-class local-classes list for Darling (the part that ran is above), and the full suite once.
  • Lite Add/Edit Server dialog opened in a window (XAML built, not opened). Viewer Add/Edit dialog's "QS replica" box not opened either.
  • Revert proof for ViewerQueryStoreReplicaTests (the old code passed null, so the first two facts fail by construction; not run). Revert proofs for the V213 live tests (a) drop the ADD COLUMN, (b) drop the replica term from v_connection_changed: not run (the R9-18 test is its own before and after).
  • The tracker seed in Lite runs on the first Query Store run per server per process, not at app start (same effect). The first look-back read after a restart that finds nothing writes its window once more (the guard is empty then).
  • The primary host field has no protocol-prefix or UNC check (only length and control characters), unlike the new replica field, so an np: pipe path is accepted as a host over MCP, the web and the Viewer. A different field with wider reach; needs its own change. No issue filed tonight.
  • Please double-check: the 25-minute L for a capturing secondary is a measured bound (18.5 minutes at F = 900), not a proof.

Security round fixes (L1-L3) and #6357 alignment

Claude posting for Erik Darling. Final security round on this PR: no Blocking, High or Medium; three Lows and a nit, plus aligning the replica with the host rule that #6357 (#6352) merged to dev. Merged origin/dev (one conflict, in the Viewer's AddServerDialog.xaml.cs; kept both the host refusal and the replica refusal).

  • L1 (Darling, DarlingMcpServerAdminTools.Edit.cs). The in-transaction recheck of a scoped agent route read plan.AddressChanged && ..., so a replica-only edit skipped it. It now reads plan.MovesConnection (address or replica). Test: ServerEditLiveTests.ARouteSetUpDuringTheConnectionTest_RefusesTheEditAndTheAdd_AndWritesNothing gained a replica-only edit during whose probe a scoped route is inserted for the server; the edit is refused and no replica is stored.
  • L2 (Darling, QueryStoreReplicaSetting.Refusal). The host part is now judged by ServerHostSetting.Refusal, which trims the protocol word the way SqlClient does, so np :... and np<tab>:... are refused (also tcp:np<tab>:x). Kept the replica-specific checks (empty after tcp:, a second tcp:) and the replica sentences. A bare IPv6 literal is now accepted as the host rule accepts it. Tests: QueryStoreReplicaSettingTests.A_replica_with_a_protocol_prefix_or_a_path_is_refused gained np :, np<tab>: and tcp:np<tab>:.
  • Darling replica connection. MonitoredServerConnection.BuildQueryStoreReplicaConnectionString sets the Data Source through ServerHostSetting.TcpDataSource: a remote replica gets tcp:, a local one and one that already names a protocol keep what they have. Tests: A_remote_replica_gets_tcp_in_front_and_a_local_one_does_not; the two older tests that read the Data Source now expect tcp:.
  • L3 (Lite had no replica rule). ServerManager.ReplicaRefusal holds the replica to the host rule. AddServer refuses a bad replica; UpdateServer refuses a changed one and lets an unchanged one through, using a new _savedReplicas snapshot kept beside _savedHosts (loaded, added, updated, imported and removed together), so it also holds when the edit dialog has changed the live object before the call; ImportServersFromFile skips the entry and logs the refusal, counted as skipped like a host refusal. The Add/Edit dialog refuses on Test and on Save (an edit that keeps its replica is not held). ServerConnection.WithQueryStoreReplicaHost takes its Data Source through TcpDataSource. Tests in Lite.Tests/ServerHostRefusalTests.cs for add, update (parameter object and live object), a legacy replica staying editable, and an imported Windows-auth entry with np:\\attacker\pipe\sql\query; QueryStoreReplicaSettingTests (Lite) expect tcp:.
  • Nit. The V213 summary comment in PgMigrations.cs moved from between V209's summary and V209Sql to sit above V213Sql. Comment move only; no migration change.
  • Test fix found on the way. ServerEditLiveTests.AsTheMcpRole_AddServers_HoldsTheReplicaHost_AndRefusesAProtocolPrefix listed tcp:attacker as a refused replica, but the Query Store collectors: a setting to read from a readable replica that mirrors the primary #6254 ruling allows tcp:; it now lists tcp:tcp:attacker (a double prefix, which is refused).

Not changed: the Lite dialog wiring itself has no UI test (it calls ServerManager.ReplicaRefusal, which the behaviour tests cover).

Test lines

  • Revert proof: with L1, L2, the Darling TcpDataSource change reverted to 38929546f, ARouteSetUpDuringTheConnectionTest... fails and QueryStoreReplicaSettingTests shows 8 failures (the np :, np<tab>:, tcp:np<tab>: cases and the tcp: Data Source cases); with Lite's WithQueryStoreReplicaHost reverted, 3 Lite QueryStoreReplicaConnectionStringTests fail. The Lite ServerManager refusals were not reverted separately (their tests do not compile without ReplicaRefusal).
  • bash .github/scripts/pre-push.sh: [guard-darling] Total: 4442, Errors: 0, Failed: 0, Skipped: 0; [guard-lite] Total: 1196, Errors: 0, Failed: 0, Skipped: 0; classes: darling 204, lite 65; ended GATE STOP: selection is the whole suite (CI's map answers FULL), so no selected class ran from the gate.
  • Own classes on a throwaway PostgreSQL rig (port 55516): Darling *QueryStoreReplica* *ServerHost* *ServerEdit* *Census* *Ratchet* *DarlingMcpServerAdminTools* *Migration*: Total 1603, Failed 0, Skipped 4. Lite *ServerHostRefusal* *QueryStoreReplica* *ServerManager* *Census* *Ratchet*: Total 88, Failed 0, Skipped 1.
  • Not run: the full suites (CI decides), the Lite dialog by hand.

Test Connection checks the replica; runner read-source test

Claude posting for Erik Darling. Two gaps the finish lane parked.

Gap 1: Test Connection now opens the Query Store replica

A server that names a replica is tested on two connections. The replica one is built by the same builder collection uses (MonitoredServerConnection.BuildQueryStoreReplicaConnectionString for Darling, ServerConnection.WithQueryStoreReplicaHost for Lite: replica host, ApplicationIntent=ReadOnly, the server's own login and auth) and opened with a SqlConnection, the connection the SQL Server target provider gives collection. A replica that will not open is a warning with its error. The test, the save and the add still succeed, because collection reads the primary.

  • Service (DarlingServerConnector.ProbeAsync): after the primary connects it calls the new ProbeQueryStoreReplicaAsync; ConnectionProbeResult gets trailing QueryStoreReplica and QueryStoreReplicaError. No replica, PostgreSQL targets and Azure SQL Database open nothing. DescribeQueryStoreReplica words the line once.
  • test_connect command (the Viewer's dialogs and the MCP test_connection tool, which already queues the stored queryStoreReplica): the reply adds queryStoreReplica, queryStoreReplicaError and queryStoreReplicaNote; status stays connected. Viewer: both Add-server dialogs append the note (ViewerDataService.ProbeReplicaNote), so the result reads "Connected - ... . Query Store replica h: connected." or "... could not connect (error). Collection reads Query Store from the primary until it can."
  • --test-connection CLI: the PASS line is followed by "Primary: connected. Query Store replica h: ..." and a failing replica is [WARN], never [FAIL] (exit code unchanged).
  • MCP add_servers / edit_server, and the web admin edit: the add detail and a new query_store_replica_test field on the edit reply carry the replica result. Over MCP and web the reply has a fixed sentence and the driver text goes to the service log (redacted, bounded) as ConnectFailureReply does; the --add-server verb shows the text. admin.js interpretEdit appends it to the "Saved" notice.
  • Lite Add/Edit dialog: Test now shows "Primary: connected." and "Query Store replica h: connected / could not connect (error)" in one message box, a warning icon when the replica fails (QueryStoreReplicaConnectionTest, device-code rendezvous handled). Only the Test button handler in the dialog changed, plus one method.
  • The web admin has no Test Connection action (no button, route or script for one; only the edit's "tested before saving" notice, covered above).

Gap 2: ReadSource through the real runner's success return

QueryStoreReplicaRunnerLiveTests already runs DarlingCollectorRunner.RunAsync(QueryStoreCollector.Instance, ...) on a fake target and asserts result.ReadSource: "replica 1, primary 0" for a replica-routed database, and "replica 0, primary 1 (replica_unavailable: <db>)" for a replica that cannot be opened or whose read fails; null with no setting. So no new test was needed. Revert proof: changing the success return to ReadSource: null fails 3 of the 7 (AReplicaRead_StoresALateRow..., AReplicaThatCannotBeOpened_..., AReplicaReadThatFails_...); restored, 7 of 7 pass (rig 55517).

Tests

  • Darling.Tests.QueryStoreReplicaConnectionTestTests (12): the probe opens the string the builder gives (replica host, read-only intent), an error is returned and not thrown, a real closed port gives an error, none/PostgreSQL/Azure open nothing, cancellation propagates, the CLI line, the test_connect reply read by the Viewer helper, add and edit replies (fixed sentence, driver text and password only in the log or, for --add-server, redacted). AdminServerEditBehaviourTests gets the interpretEdit notice fact (Node harness). Lite.Tests.QueryStoreReplicaConnectionTestTests (4).
  • Gate (bash .github/scripts/pre-push.sh, before the last merge of the issue branch): [guard-darling] Total: 4442, Failed: 0, [guard-lite] Total: 1196, Failed: 0, then GATE STOP (whole-suite selection, CI's map answers FULL).
  • Own classes on rig 55517 (merged with origin/dev and the issue branch): *QueryStoreReplica* *Census* *Ratchet* *AdminServerEdit* *DarlingCommandExecutor* *ConnectionTestReply* *DarlingCliCommands* *DarlingMcpServerAdmin* *ServerEdit* *AddServer*: 1432 run, 1 failed (ServerEditLiveTests...HoldsTheReplicaHost, because the setting now allows tcp:; the other lane fixed the same line, taken at merge). After the merge: the new classes (Darling 12, Lite 4), ServerEditLiveTests, the runner live tests and the executor/CLI/reply classes pass. Two assertions that the data source equals the host were changed to expect the tcp: prefix the builders add.
  • Not run: the Lite and Viewer dialogs opened by hand (the Test button's message box); the full suite (CI decides); Test Connection against a real unreachable replica on a live server (the real SqlConnection path is covered with a closed local port only). A test with a replica adds up to one more connect timeout to the test (about 15 s), so a slow replica can push the Viewer's 30 s per-row bulk limit.

CI fix (run 39697777426)

  • BYO script on a store behind V213 (AgentLevel2StoreGrantLiveTests.TheBringYourOwnScript_GrantsTheSameMatrix_AndSkipsOnAStoreBehindTheRung, 42703 s.query_store_replica): the config.edit_monitored_server 18-argument function read s.query_store_replica and wrote query_store_replica = ... unguarded, so a viewer edit on a store without the column failed. It now reads the column through to_jsonb(s) ->> 'query_store_replica' (the way the V203 columns are read) and picks one of two UPDATEs by a pg_attribute check, so the replica is left out of the write on a store without the column. Same change in DarlingManagedRoles.cs. The test_connect request function now reads it through to_jsonb in the managed SQL, and the script's copy now carries queryStoreReplica too (it was missing from the script).
  • Lite DeadlocksTelemetryCursorLiteTests.BothHosts_LandStagedItemState_OnlyAfterTheItemsWrite_AndDropItBeforeTheRead: the rule still holds, only the matcher missed the new shape. ReadQueryStoreItemAsync (the replica attempt and the primary read a failed replica read falls back to) calls context.DropStagedItemState() as its first statement (DarlingCollectorRunner.cs ~3867), before BuildPerItemQuery(item, context), and readItem (~4042) reaches it on both the replica path and the fallback path, so every attempt drops first. The pin now starts its search at the helper (and asserts readItem calls the helper before onItemComplete); Lite's host keeps the old shape. Not a spelling pin, so it was moved, not deleted.
  • Not touched: Darling PG tests (2), job 119561364612 (WPF STA runner stall in ViewerJobHistoryColumnBehaviourTests); git diff --stat origin/dev...HEAD -- '*JobHistory*' '*StaTestThread*' is empty.
  • Tests (rig 55534): *AgentLevel2Store* *ProvisionRoles* *ManagedRoles* Total 92, Failed 0, Skipped 0; *ServerEdit* *ServerAddViewer* *StoreServerPasswordRules* *AwsPerServerRole* *ProvisioningSerialization* *QueryStoreReplica* Total 758, Failed 0, Skipped 4; Lite *DeadlocksTelemetryCursor* Total 10, Failed 0.
  • Gate: [guard-darling] Total: 4442, Failed: 0, [guard-lite] Total: 1196, Failed: 0, classes: darling 222, lite 57, then GATE STOP (CI's map answers FULL); CI decides the rest.

CHANGELOG entry

SECTION: Added
ENTRY: - Read Query Store from a readable replica, and stop losing late rows from a capturing secondary ([#6254], [#6279]) - A SQL Server target can name a readable Availability Group secondary (a host, host,port or a listener name) as its Query Store replica, in darling.json as queryStoreReplica, in the Viewer and web admin edit forms and the MCP add_servers and edit_server tools, and in Lite's Add Server dialog as "QS replica". The collector then reads Query Store from that replica for each database that is safe to read there (a readable non-capturing secondary of the same group, redo not behind, same version, flush interval 900 seconds or less) and from the primary for the rest, or when the replica cannot be reached. A replica read re-reads a short look-back window and drops rows it already stored, so the replica's lag cannot lose or duplicate rows. The collection log records where each Query Store run read from (read_source, for example replica 12, primary 2 (redo_behind: Sales)), shown in get_collection_health, get_collection_log, the web Collectors table, the Viewer and Lite collection health and log grids. The replica host is part of the saved password's binding, so a password saved for one replica is never sent to another; a replica change asks for the password again. A capturing secondary (FOR SECONDARY SET QUERY_STORE = ON) is now detected even with no replica set, and its late rows are no longer skipped. Azure SQL Database ignores the setting. Darling store version 213 and Lite's DuckDB version 78 add one nullable column each.
SECTION: Important
ENTRY: - Upgrading Darling to the Query Store replica release runs store migration V213 on the first start, and a bring-your-own store needs one step after it ([#6254]) - V213 adds two nullable text columns, config.config_monitored_servers.query_store_replica and collect.collection_log.read_source, with no default and no backfill, so each is a catalog-only change that rewrites no table, and it rebuilds the collect.v_collection_log view so readers see the new column. An older build ignores both columns, so there is nothing to do before rolling back. A store whose roles you provisioned yourself ("bring your own") must run Darling/tools/provision-roles.sql again after the upgrade. The service replaces the three config.edit_monitored_server functions at each start when its login owns them and warns once, naming the script, when it does not; but the new 18-argument function and the viewer and mcp grant on query_store_replica come only from the script. Until it is re-run, editing a server from the web dashboard or the MCP edit_server tool fails with permission denied; collection and reads keep working. The compose store and managed provisioning do this on their own.
REF: [#6254]: #6254
REF: [#6279]: #6279

…nother name (#6285)

The connection test reads SERVERPROPERTY('ServerName') on the same connection, and it is compared with the server_name collected for each registration, beside the host-text match.
…QL identity, storage-key skip, multi-subnet edits, tests (#6285, #6339)
…b admin, MCP add/edit), read_source in the collection log tool (#6254)
…fix/6254-qs-replica-reads-darling-setting-ui
…ads-testconn

# Conflicts:
#	Darling/PerformanceMonitor.Darling.Viewer/AddServerDialog.xaml.cs
#	Lite/Windows/AddServerDialog.xaml.cs
…fix/6254-qs-replica-reads-testconn

# Conflicts:
#	Darling/Darling.Tests/ServerEditLiveTests.cs
#	Darling/PerformanceMonitor.Darling.Viewer/AddServerDialog.xaml.cs
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review October 22, 2026 00:07
@erikdarlingdata
erikdarlingdata enabled auto-merge (squash) October 22, 2026 00:07
…eeding the V213 column; deadlock cursor pin follows the read helper (#6254)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant