Repository navigation
Darling's analysis pass has the same wedged-server defect Lite just fixed, but its shutdown classifier blocks the same fix #2430
Description
The Lite half is fixed in #2419 (issue #2412). Darling has the same defect in the same shape, and porting the fix is not mechanical — the reason is the interesting part.
The shared defect
DarlingWorker.cs, the scheduled analysis pass:
if (!_analysisInFlight.TryAdd(serverId, 0)) { return new AnalysisPassResult(AnalysisPassStatus.Skipped, ...); }
var analyzeTask = analysisService.AnalyzeAsync(serverId, storageName, hoursBack: 4, stoppingToken);
/* Clear the in-flight marker only when the task truly finishes — not
when the timeout below moves us on — so a hung server is not relaunched. */
_ = analyzeTask.ContinueWith(completed => _analysisInFlight.TryRemove(serverId, out _), TaskScheduler.Default);
var finished = await Task.WhenAny(analyzeTask, Task.Delay(s_analysisTimeout, stoppingToken));AnalyzeAsync receives only stoppingToken, which means shutdown — never a timeout token. So the timeout abandons the wait without cancelling the work, and the marker is released only on true completion. An analysis that never finishes leaves that server's marker set for the life of the process, and every later cycle skips it silently. The comment is correct about what it is doing and does not anticipate the case where "truly finishes" never happens.
Why the Lite fix cannot simply be ported
DarlingAnalysisService classifies its abandon arm through AnalysisShutdown.IsShutdownAbandon(ex, context.CancellationToken). That is truthful only while the token means shutdown. Arm it with CancelAfter and every ordinary timeout on a healthy running service gets logged as "abandoned at shutdown", at Information — which is the same class of mistake as #2419's first review finding, where a when (token.IsCancellationRequested) filter would have relabelled any fault as abandonment because the token now fires during normal running.
So Darling needs its classifier to distinguish timeout from stop before the token can be armed. AnalysisShutdownResidueTests pins that call site's argument list, so the change is visible rather than silent — which is good, but it does mean the classifier, the call site and its test move together.
Two residues from the Lite work that apply here too
Threading the token into the individual store reads. The Lite fix ships checkpoints ahead of each store-touching stage — abandonment between reads, not during one. Measurements taken during #2419 show DuckDB genuinely honours an interrupt (a token cancelled at 2 s aborted a 15.2 s query at 2,004 ms), so the deeper wiring would work; it is 138 call sites across 15 files on the Lite side. Darling's equivalent is Npgsql, which honours cancellation natively, so this may be cheaper here than it was there.
DuckDbInitializer.AcquireReadLock() takes no timeout, unlike its AcquireWriteLock(TimeSpan?) sibling, so a pass queued behind a long archival is uninterruptible by any token. That one is Lite-only, but it is the reason #2419's observability half is load-bearing rather than a fallback, and Darling should not assume cancellation alone closes the permanent-skip symptom either.
Suggested order
Classifier first (separate timeout from stop, with its test), then the token, then the reporting. The reporting is what actually closes the user-visible symptom — a server silently skipped forever with one log line from the cycle that first timed out — so it is worth doing even if the token work stalls.
Activity
Put the new enum where it does not strand the class's own docs
Merge pull request #2438 from erikdarlingdata/fix/2430-darling-wedged…
erikdarlingdata commentedon Aug 21, 2026
Fixed on
devby #2438 — all three parts, and the measurement went against my guess in the issue.I wrote that Npgsql honouring cancellation natively might make token-threading cheaper here than the 138 DuckDB call sites on the Lite side. It is not: 167 of 209 async store calls in the analysis pipeline pass no token at all, including the entire 93-call fact-collection phase. Npgsql would honour one; the call sites simply never hand it one. That is more than Lite's 138, not fewer. Spot-checked independently — every matched call in
PgFactCollector,PgDrillDownCollectorandPgFindingStoreis the no-argument form.So abandonment is between stages rather than inside a read, exactly as on Lite, and the reporting is what covers the gap: the in-flight marker carries a start time and report count, and a server past 3× its budget is reported on a doubling backoff with minutes stuck and cycles lost. Split out as #2439.
The classifier moved first, as it had to.
IsShutdownAbandonbecameIsExpectedAbandonacross 16 sites, plus a three-wayClassify(ex, shutdownToken, passToken), withAnalysisContextcarrying the stopping token separately. The timeout arm is deliberately narrower —OperationCanceledExceptiononly — so a disposed data source or a 57P0x keeps the ERROR #2299 gave it. Without that, arming the token would have logged every ordinary timeout on a healthy service as "abandoned at shutdown", which was the whole reason this could not be a straight port.Verified for real rather than pinned:
PerformanceMonitor.Darling.Analysistargetsnet10.0, so the classification tests run offline on macOS in milliseconds. dev: 1 passed / 8 failed. Branch: 17/17. dev's line for the failing case is the exact symptom —Information: Analysis abandoned at shutdown for four-args, on a service that is not stopping.The review finding is worth recording because it is the same trap as #2419's: inferring the outcome from
findings.Count == 0 && cts.IsCancellationRequestedproduced a duplicate Warning per timeout and mislabelled a genuine fault landing after the budget asTimedOut, burying its ERROR. The scheduler reads the pass's own classification now instead of guessing from symptoms.One behaviour change worth knowing:
analyze_nowshares the pass, so it is now genuinely cancelled at 120 s rather than reporting a timeout while the work continued and persisted anyway. Its answer was alreadyTimedOut; it is true now.erikdarlingdata commentedon Aug 21, 2026
Correction to my comment above: the token-threading follow-up is #2443, not #2439. I wrote the reference before creating the issue and the number I guessed turned out to belong to a PR in another lane (#2439, the viewer settings-file fix). Apologies for the misdirection.
Make Darling's analysis budget mean something, and stop a wedged serv…
Say what a cancelled persist means, and make the answer "nothing was …