Darling: split query_store off the sequential per-server collection body

[erikdarlingdata]

Fixes #2700.

Problem

`RunDueCollectorsAsync` runs every due collector for a server sequentially in one `foreach`, awaited inline, inside `ProcessServerSweepAsync`. The outer launch loop will not relaunch a server's sweep body while the previous one is still running (INV-2, "one body per server").

`query_store`'s per-run duration is bimodal: mean ~5-35s, but a heavy batch runs 100-230+ seconds. Because it was awaited inline in the same sequential foreach as every other due collector (query_stats, procedure_stats, wait_stats — all 1-minute cadence), and because the whole body can't relaunch until it returns, a single heavy query_store run stalled that server's ENTIRE collection for its full duration, not just its own row. On use1 today this pushed 11 servers' `last_collection` stale enough to false-trip the fleet's 15-minute Offline threshold, with zero actual collector failures — confirmed per-server via `get_collection_health`'s existing `BODY_OVERRUN` sweep-pressure diagnostic, which already names the fix in its own generated text: "moving or splitting that collector so it stops sharing a cycle."

Fix

`query_store` is now fired detached rather than awaited inline. It reuses the existing per-server `QueryStoreServerGate` (#2165) for mutual exclusion — a still-in-flight previous tick skips rather than overlapping — and its window is watermark-driven (#1960), so a detached run that outlives the sweep resumes correctly from its own boundary on the next tick. Every other due collector in the foreach now runs on schedule regardless of how long query_store takes, and the body itself returns promptly so the outer launch loop keeps relaunching normally.

A small RunDetachedQueryStoreAsync wrapper exists only to keep a shutdown-time OperationCanceledException from surfacing as an unobserved task exception — RunOneAsync's own catch-all already contains every other fault.

Test plan

• dotnet build (project + full solution) clean, 0 errors
• Could not run Darling.Tests locally (this Mac lacks the WindowsDesktop runtime the net10.0-windows TFM needs) — relying on CI
• Per this file's own documented testing philosophy (top of DarlingSweepSchedulingTests.cs): the stateful scheduling body has heavy dependencies and is asserted by full-diff review rather than unit tests; the change is a minimal, surgical inline-await → detached-fire-and-forget swap reusing an already-tested gate, not new scheduling logic.

🤖 Generated with Claude Code

[claude]

The detached query_store task is invisible to the #1553 shutdown drain.

Every other fire-and-track unit in this file is deliberately handed to something shutdown awaits:

• server.InFlightSweep is drained via Task.WhenAny(Task.WhenAll(inFlightSweeps), Task.Delay(s_shutdownDrainBudget, ...)) (~line 1582).
• The Query Store backfill loop is drained with await backfillLoop; (~line 1604).
• The baseline backfill task is drained the same way (~line 1611).

_ = RunDetachedQueryStoreAsync(...) here is the only fire-and-forget in the file that isn't assigned to anything a shutdown path awaits. Because RunDueCollectorsAsync (and therefore ProcessServerSweepAsync/InFlightSweep) now returns as soon as it fires this off instead of waiting on it, the shutdown drain's Task.WhenAll(inFlightSweeps) resolves almost immediately — it no longer represents "is this server's collection still running." ExecuteAsync can return, and the host can proceed to tear down _postgres and other shared resources, while a 100–230s query_store run is still mid-flight and still calling DarlingObservability.LogCollectionAsync(_postgres!, ...).

RunDetachedQueryStoreAsync's own catch (OperationCanceledException) only fires once the underlying command actually reacts to cancellationToken cancellation, which takes real time — time the drain budget existed to provide, and which this task no longer gets on the shutdown path (only what remains of the process/host lifetime).

Given the codebase's own established convention for every other fire-and-track task, this looks like a real gap rather than an intentional exception: consider tracking these detached tasks (e.g., a ConcurrentBag/list per server or fleet-wide, drained the same bounded way as inFlightSweeps) so shutdown still gets a chance to let an in-flight query_store run wind down before storage is torn down.

[claude]

Reviewed the diff (single file: Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs; this is Darling-only fleet-scheduling code, so no Lite counterpart applies here — Lite's CollectionBackgroundService doesn't have this per-server sequential-sweep-body architecture at all, so there's nothing to keep in parity for this change).

Main finding (posted inline): the new _ = RunDetachedQueryStoreAsync(...) fire-and-forget isn't tracked by the shutdown drain. Every other fire-and-track task in this file (InFlightSweep, the Query Store backfill loop, the baseline backfill task) is assigned to something the #1553 shutdown path explicitly awaits with a bounded budget. This one isn't assigned to anything, and because RunDueCollectorsAsync now returns as soon as it fires the detached task instead of awaiting it, InFlightSweep completing no longer means "this server's collection is done" — the shutdown drain can resolve near-instantly while a 100–230s query_store run is still writing to _postgres in the background.

Things I checked and consider fine:

• QueryStoreServerGate mutual exclusion ([PERF] Tick QS collection and QS backfill run concurrently against the same server — serialize per server #2165) correctly prevents the detached tick run and a concurrent snapshot_now/backfill run from overlapping against the same server — the interlocked TryAcquire is a real mutex, not just advisory.
• Each collector run opens its own connection from ConnectionString (provider.CreateConnection(...)) rather than sharing a live connection object, so running query_store concurrently with the next tick's other due collectors doesn't corrupt a shared SqlConnection/NpgsqlConnection.
• server.NextDue writes only happen on the synchronous sweep-body thread (never inside the detached task), so no data race there.

Lower-confidence note, not filed as a blocking finding: since RunDueCollectorsAsync can now return (and "one body per server" relaunch) while a previous tick's detached query_store is still in flight, server.Runtime = null (set from RunOneAsync's connection-fatal catch, ~line 4793) can now be written concurrently with a later tick's sweep body that's mid-loop against the same server. Each sweep body captures its own runtime local up front, so this doesn't corrupt in-progress work directly, but it does widen an existing eventual-consistency window (stale-runtime usage / delayed reconnect detection) from "duration of one sweep body" to potentially "duration of a lingering query_store run." Worth a second look, but I'm not confident it's a practical bug given how much of this scheduler already tolerates this kind of raciness by design.

No T-SQL in this diff, so the CONTRIBUTING.md T-SQL style items don't apply.