Repository navigation
[PERF] Tick QS collection and QS backfill run concurrently against the same server — serialize per server #2165
Description
Activity
Updating this with what the #2164 measurement changed, because it makes this issue more relevant, not less — and because the naive fix is worse than the problem.
What changed. I originally framed this as two 64MB drains competing for the same link. The measurement killed that framing: cutting the byte budget 5x moved the pass clock ~0%, so the dominant cost is server-side work before the first row ships (Query Store's own aggregation), not the streaming. That reframes the overlap: when the tick and the backfill run against one server simultaneously, they are not two drains sharing a wire — they are two independent heavy aggregations against the same Query Store catalog on the same instance. On a 4-core box serving live traffic that is strictly worse than a shared-bandwidth problem, because CPU and the catalog's own latches are the contended resource and neither participant yields.
So the serialization is justified independent of where the time goes, which also means it does not need to wait on the open/drain split to be designed.
The trap the design has to avoid. A single per-server gate held for the duration of whichever statement gets there first can starve the tick. Backfill slices are the long ones by construction (they exist to chew through history), so a naive mutex would let a slice block live collection for that server — inverting the priority the #2058 design deliberately chose, where backfill is explicitly the yielding party. Whatever lands here must be biased: the tick wins, backfill defers, and a backfill slice already in flight must not be able to make the tick wait behind it.
That points at try-acquire semantics rather than a blocking lock — the tick proceeds unconditionally and the backfill skips its slice for this server this cycle when the tick is active, which costs nothing (the watermarks make every skip resumable) and cannot starve anything.
Sequencing. No urgency while the backfill is off on our fleet, but it MUST land before it goes back on, or the first re-enable reproduces the original incident. Not starting the implementation tonight: a cross-loop concurrency primitive with a starvation failure mode deserves fresh attention, not the tail of a long session.
Shipped in #2272, merged to
dev. Implemented in both apps as proposed — Darling'sDarlingWorkerkeyed by server id, Lite'sRemoteCollectorServicekeyed by server — over one sharedQueryStoreServerGateprimitive sitting besideAbandonableStep.Three notes on how it differs from the proposal, all deliberate:
A
SemaphoreSlimturned out to be the wrong primitive. The gate never waits, so a semaphore buys nothing it uses (blocking acquire, timeouts, async waits) while costing one undisposed kernel-backed object per monitored server for the process lifetime — which is also what CA1001 flagged. It's an interlocked flag with a disposable lease instead. Leases are idempotent on dispose, because a stray secondDispose()would clear a flag the other loop had since taken and let both run at once: the exact condition being prevented, reached backwards.Nothing waits, in either direction. Both sides try-acquire with a zero timeout and skip. An in-flight slice runs to a 180-300s abandonment deadline, so a blocking acquire would let one slow server stall collection for the entire fleet — #2148's wedge arriving through a lock instead of a hang.
"Tick wins, backfill defers" is realized by cadence, not preemption. The loser is whichever loop arrives second, since stopping a statement already running on the monitored server would mean killing it. The bias comes from the tick retrying on its ~1-minute interval against the backfill's 5, so it recovers ~5× faster from a collision, and a slice is byte-budgeted so it's short in the healthy case. Documented on the type so nobody later "fixes" it into a blocking wait.
One thing worth flagging for future reuse: skipping is only safe because this collector's window is a watermark (#1960) — the next pass resumes from the same boundary, so a skipped pass defers rows rather than dropping them. The same gate around a wall-clock-windowed collector would silently lose data. That's called out in the type's docs.
The backfill takes the gate outside its
AbandonableStep, so an abandoned-but-still-wedged slice keeps the gate closed — that statement is genuinely still running on the server, so the tick must keep yielding to it.
Component
Both apps (Lite RemoteCollectorService + Darling DarlingWorker backfill loops)
What
The regular per-tick query_store collection and the #2058 first-contact backfill are independent loops with no per-server coordination. Dogfood evidence (2026-08-10, 4-core multi-tenant box mid-consolidation): a 64MB backfill slice for the freshly restored database ran concurrently with the tick's QS collection of a sibling database (12:50:58 backfill ship overlapping the 12:51:09 tick completion) — two heavy
PerformanceMonitorLitestatements at once, ~128MB of QS text extraction in flight on the box least able to afford it. The overlap window is precisely when a server is already drowning: big catalog arrivals are what trigger both the backfill AND budget-bound tick passes.Proposal
Per-server mutual exclusion between the two loops: backfill yields while the tick's QS collection for that server is running (and vice versa — tick skips the QS collector for a server mid-slice, or waits). The AbandonableStep per-server guards (#2149) already give both loops per-server identity; a shared per-server gate (SemaphoreSlim keyed like the guards) is the natural shape. Bias: tick wins, backfill defers — live freshness over historical drain.