Do not abort the server when a loader worker cannot be spawned

[groeneai]

Changelog category (leave one):

• Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fixes a server abort that happens when the global thread pool is momentarily saturated while tables are still loading. AsyncLoader now keeps running if it cannot spawn an extra loader worker and some worker will still drain the queued jobs, instead of calling std::terminate.

Description

AsyncLoader::spawn wrapped scheduleOrThrowOnError in NOEXCEPT_SCOPE, whose handler is unconditionally std::terminate(). A transient CANNOT_SCHEDULE_TASK from a saturated global thread pool therefore killed the server (Received signal 6) during table loading. Seen 4 times over 4 unrelated PRs and 4 build flavours in 30 days, e.g. Stress test (arm_tsan), whose stack is terminate_handler() -> std::terminate() -> AsyncLoader.cpp:908 spawn() -> :744 enqueue().

A spawn must succeed only when nothing else will drain the pool's ready queue, so it now tells three states apart:

• a worker that entered worker() and is not inside wait() drains the queue now;
• a worker inside wait() on another pool's job resumes draining once that job runs;
• neither of those means nobody ever will.

In the first two cases spawn uses the non-throwing trySchedule, rolls back its Pool::workers increment on failure, warns and returns false. Only the last case keeps the fatal path, byte for byte: the queued jobs would be stranded, and throwing is not an option because schedule marks that region non-exception-safe. Covering it needs rollback-and-retry across the loading DAG, out of scope here. All 4 observed rows are recoverable. BackgroundSchedulePool narrows the same way (929227464f9736b).

Two counters are added because Pool::workers also counts a worker still queued in the global pool, which drains nothing, and one blocked in wait(), which cannot take its own pool's jobs. A saturated pool is remembered so the failing thread creation happens once per episode, not once per queued job, counted by the new AsyncLoaderSpawnFailures event; entering wait() clears the memo.

Tests are gtests asserting which branch was taken, not just that the process survived. A SQL test cannot reach the line: on master the whole call sits under blockFaultInjections().

Related: #110174, which removed the mask that hid this exception.

Version info

• Backported to: 26.7.4.53

[groeneai]

Internal second-model review (5 rounds)

Each round: cold review of the tree, then an independent second-model review of the same diff.
Everything marked AGREE was fixed before this PR was published. Condensed to one table.

	Round	Finding	Verdict
❌	1	The drop predicate read Pool::workers, which counts submitted workers, so a spawn could be dropped with no loader worker actually executing.	AGREE, fixed: started_workers, incremented on entry to worker().
⚠️	1	SpawnFailureThenRecovers asserted only that 32 zero-work jobs completed, which one running worker satisfies; an implementation that declined every optional spawn passed.	AGREE, fixed: it now asserts two workers run concurrently.
⚠️	1	A saturated pool was re-probed once per enqueued job, each a real thread creation under the loader mutex.	AGREE, fixed: bounded per saturation episode; the warning names the cause.
💡	1	Changelog entry should be present tense.	AGREE, fixed.
💡	1	Suggested Critical Bug Fix.	DISAGREE: I do not self-assign that category. Happy to change it if you prefer.
❌	2	A worker blocked in a cross-pool wait() was still treated as a drainer: suspended_workers counts same-pool waits only, and prioritize refuses to promote when the target priority is not higher.	AGREE, fixed: any wait() counts as unavailable, plus a cross-pool regression test.
⚠️	2	No test distinguished started_workers from workers, so reverting round 1 left the suite green.	AGREE, fixed: a test drives the submitted-but-not-started state.
⚠️	2	ASSERT_LT(delta, 16) also accepted zero, so deleting the profile-event increment was undetected.	AGREE, fixed: exact assertion.
❌	3	The spawn attempt inside wait() was still under the same-pool conditional, so a cross-pool waiter was recorded unavailable with no spawn attempted.	AGREE, fixed: the attempt runs for any wait, gated by canSpawnWorker; the suspended_workers increment stays same-pool.
⚠️	3	Nothing failed if the retained fatal branch became non-fatal.	AGREE, fixed: a death test pins it.
⚠️	4	Round 3's hoist made the spawn reachable from a cross-pool wait, where the waiter counts as unavailable, so as the pool's only started worker it made the spawn mandatory and a failure fatal in two states master recovers from.	AGREE, fixed: a cross-pool waiter is a bounded future drainer, so it gets its own tier that attempts the spawn but stalls rather than aborting.
❌	4	With max_threads = 1 a cross-pool wait can leave a ready queue undrained.	DISAGREE: deadlocks identically on master, which reaches no spawn there at all. Pre-existing capacity accounting; the remedy would change canSpawnWorker, out of scope for an abort fix.
💡	4	The comment claiming the new counter is maintained under the loader mutex is inaccurate: only the increment is.	AGREE, fixed.
💡	4	Pool::workers was still documented as currently-executing workers.	AGREE, fixed.
❌	5	The counters stay incremented after the awaited job finishes, and prioritize can migrate the awaited job, so the fatal branch could be selected for a waiter that is about to drain.	DISAGREE, both halves. The fatal branch needs waiting == suspended, i.e. every waiter is a same-pool waiter, and in that state master aborts too: at the merge base every spawn is scheduleOrThrowOnError inside NOEXCEPT_SCOPE (no trySchedule anywhere) and wait() does reach it after incrementing suspended_workers. On migration, the wait path calls prioritize with its own pool as the destination and only when worker_priority < job_priority, so it converts cross-pool waits into same-pool ones, never the reverse, and the increment follows on the next line. The staleness is real but one-directional: a stale-high count can only move the decision toward attempting or requiring a spawn, never toward dropping one that was needed.
💡	5	The spawn declaration comment, a test helper comment and one test name still describe the two-state contract.	AGREE, not fixed here: naming and comments only. The three-state rule is stated at spawn()'s head, and the test's assertion is still correct and still fails under mutation. Will fold into the next push.

Noted, not blocking: the abort is narrowed, not removed. When no worker that can drain the queue
remains, the previous fatal path is kept byte for byte, because dropping the spawn there would
strand the ready queue and propagating is unavailable (schedule marks the region as not
exception-safe). A cross-pool wait cycle between two equal-priority pools can still stall both
queues; that is unchanged from master, which attempts no spawn there at all, and the three
production pools have distinct priorities so they never spawn concurrently.

[clickhouse-gh]

Workflow [PR], commit [4a6ff09]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR narrows one important AsyncLoader abort path by treating spawn failures as recoverable when another worker can still drain the pool, and the added gtests cover several previously-missed states. I found one remaining blocker in that new classification: a waiter that starts as same-pool and is later reprioritized to another pool is still counted as suspended, so the code can still abort on a recoverable CANNOT_SCHEDULE_TASK in a reachable state.

Missing context / blind spots

• ⚠️ The full PR CI is not available yet from the Praktika report: at review time only Build profile diff was published there, while most build jobs in gh pr checks were still pending. A green PR report or finished build matrix would close that gap.

Findings

❌ Blockers

[src/Common/AsyncLoader.cpp:825-831, 933-936] spawn() assumes waiting > suspended is equivalent to "some waiter is waiting on another pool", but suspended_workers is fixed when wait() begins and is never updated if the awaited job is later moved to a different pool. JobPrioritizedWhileWaited (src/Common/tests/gtest_async_loader.cpp:992-1035) already proves that reprioritization can happen while the waiter is blocked. In that state the waiter will resume once the other pool runs the awaited job, but waiting == suspended still makes spawn_is_required true, so a later global-pool saturation in this pool still reaches the fatal scheduleOrThrowOnError branch instead of the recoverable path this PR is trying to introduce.

Suggested fix: update the waiter classification when prioritize migrates a waited job across pools, or compute the resuming-worker tier from live waited-job state rather than the stale same-pool counter; then add a regression that saturates the global pool after the reprioritization and enqueues another job in the original pool.

Final Verdict

❌ Needs changes: the recoverable/non-recoverable split is still wrong after a waited job is reprioritized to another pool, so the server-abort bug survives in a reachable path.

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	86.80%	86.80%	+0.00%
Functions	92.00%	92.00%	+0.00%
Branches	79.20%	79.20%	+0.00%

Changed lines: Changed C/C++ lines covered: 340/351 (96.87%) · Uncovered code

Full report · Diff report

[groeneai]

Pre-PR validation gate (click to expand)

#	Question	Answer
a	Deterministic repro?	Yes. taskset -c 0 build/src/unit_tests_dbms --gtest_filter='AsyncLoader.SpawnFailureFromFinishDoesNotTerminate' aborts on demand (exit 134). Not probabilistic: the test caps the global thread pool at 1 thread while a loader worker is running, so the next spawn always fails. The base-arm stack matches the reported one frame for frame (AsyncLoader.cpp:908 spawn -> :744 enqueue -> :658 finish -> :936 worker).
b	Root cause explained?	Yes. spawn wrapped scheduleOrThrowOnError in NOEXCEPT_SCOPE, whose handler is unconditionally std::terminate() (noexcept_scope.h:18-22). When the global thread pool is saturated, scheduleImpl throws CANNOT_SCHEDULE_TASK (ThreadPool.cpp:471, re-wrapped at :416), so a transient scheduling failure became a permanent server abort. The failing run's fatal.log shows threads=10000, jobs=10000, i.e. max_thread_pool_size/thread_pool_queue_size at their defaults.
c	Fix matches root cause?	Yes. It changes the policy at the one site that can tell whether losing this worker is survivable, distinguishing three states rather than two: a worker running now, a cross-pool waiter that resumes when its wait ends, and neither. The first two use the existing non-throwing trySchedule, roll back the Pool::workers increment, log a warning and return false; only the third keeps the abort, because there the queued jobs would be stranded. No bound widened, no test relaxed, no global pool limit raised (that would be mitigation, not a fix), and NOEXCEPT_SCOPE keeps its fatality everywhere else.
d	Test intent preserved / new tests added?	Yes. No pre-existing test's assertions changed, and the one loosened assertion of my own (ASSERT_LT(failures, 16)) was TIGHTENED to ASSERT_EQ(failures, 1). Eight new gtests: one per reaching path, a recovery case, two pinning the states in which dropping a spawn would leave nobody running (a worker submitted to the global pool but not started, and a worker blocked in a cross-pool wait()), one for the moment a running worker becomes a cross-pool waiter, and a death test pinning the abort that is deliberately retained. Gtests rather than SQL because spawn holds CannotAllocateThreadFaultInjector::blockFaultInjections() on the fatal branch, so no SQL-level lever reaches it; the tests saturate the real global pool instead, the technique the pre-existing ThreadPool.GlobalFull1/2 use. Every oracle is mutation-tested and each fails for exactly its own defect: permanently declining optional spawns fails the recovery test (observed_overlap 1 vs 2) and both injector-keyed tests; counting same-pool waits only fails the cross-pool test alone; counting submitted workers fails the not-yet-started test alone; deleting the AsyncLoaderSpawnFailures increment fails the coalescing test; leaving the spawn attempt inside the same-pool branch fails the new transition test alone (ran_during_wait false); making the retained fatal branch non-fatal fails the death test alone; deleting the cross-pool-waiter state (so its spawn failure is fatal again) fails only the new saturation test with exit 134; folding the retained fatal state into the recoverable one fails only the death test, proving the three-way split did not widen the recoverable set; and removing the spawn-failure memo reset at wait() fails only the two cross-pool tests. Because each mutation reddens a different subset, no oracle is merely asserting "nothing happened".
e	Both directions demonstrated?	Yes, per test, run separately since an abort ends the process. Without the fix the three saturating tests exit 134 with libc++abi: terminating ... CANNOT_SCHEDULE_TASK (threads=1, jobs=1), matching the reported stack; with the fix they pass, and the 8 spawn tests pass 400/400 under --gtest_repeat=50. The other three tests pass on the base arm by construction and say so explicitly: the unfixed binary has no droppable branch, so nothing can be dropped, and the death test asserts an abort master already performs. Their discriminating power therefore comes from the mutation arms, each of which reddens exactly one of them, not from the base arm. The newest test is not a survival check: it asserts the spawn was attempted AND failed in the cross-pool state (AsyncLoaderSpawnFailures delta 1 before the wait, 2 after), and a temporary per-tier probe confirmed both failures came from the intended branches, so it cannot pass by never reaching the state. Control filter NestedUtils.* (untouched by this diff) ran 2 tests and passed on both arms, so the filter is not vacuous, and the arms differ by Build ID.
f	Fix is general across code paths?	Yes, enumerated mechanically: exactly 1 of the 27 NOEXCEPT_SCOPE blocks in src/ contains a thread-pool schedule call, and it is this one. All five spawn call sites are now governed by the same predicate with none special-cased: the wait() site was the last one still gated on a narrower condition than the predicate it feeds, and hoisting it out of the same-pool branch means every reaching path re-evaluates spawning whenever a worker stops draining. The predicate itself counts waits into ANY pool, so no path measures a narrower property than it claims, and it now distinguishes the two ways a pool can lack a running worker instead of collapsing them, so no reaching path aborts on a state master recovers from. The symmetric subsystem BackgroundSchedulePool already handles a failed spawn this way (929227464f9736b); this brings AsyncLoader into line.
g	Fix generalizes across inputs (params/datatypes/wrappers)?	N/A for type wrappers: no user-supplied values reach this code. The equivalent matrix is pool state, covered by one predicate evaluated at every call site: started_workers vs waiting_workers, including the boundary where they are equal (fresh pool, and every started worker waiting in wait()), where the previous fatal behaviour is kept. Two states were measured explicitly rather than assumed, and each is why the predicate reads what it does: a worker submitted to the global pool but not yet started drains nothing, so submitted workers are not counted; and a worker waiting on a job of another pool drains nothing now but resumes when that pool runs the awaited job, which is why it is treated as a future drainer rather than as nobody, and why all 9 ordered pool pairs declared in PoolId.h were enumerated (3 are neither promoted by priority inheritance nor counted as suspended). Underflow is impossible: the predicate is a <= comparison, and the only subtraction runs on the branch where that comparison already established the operand order.
h	Backward compatible? (maintainer-approved exception only)	Yes. No setting, no serialization format, no public API change, so no SettingsChangesHistory.cpp entry is needed. Behaviour only becomes more permissive: a case that used to abort either aborts as before or keeps running. The one additive surface is a new AsyncLoaderSpawnFailures profile event.
i	Invariants and contracts preserved?	Yes. Pool::workers is rolled back under the same held lock, so canSpawnWorker, canWorkerLive and hasWorker stay truthful; a leaked increment would have made hasWorker lie and suppress the honest wait() failure on "jobs pending, no worker", and one of the new tests drives exactly that state. Pool::started_workers is written only under AsyncLoader::mutex and read only at the decision point, which holds the same mutex, so it is consistent and not merely atomic. Pool::waiting_workers is incremented under that mutex too, and its guard is declared before the suspended-worker guard so it is released last, keeping it a superset of suspended_workers at every instant including during unwinding; both wait() throw sites precede the guard being armed. Balance is asserted by a temporary destructor probe that aborts on any non-zero residual: silent across the whole AsyncLoader suite, and proven live by a negative control (neutering the waiting_workers decrement makes it report PROBE_LEAK ... waiting=1); the probe also asserts that suspended_workers never exceeds waiting_workers. Widening the count can only make more spawns mandatory, never fewer. The decrement is guarded by a local flag and runs once, so no double decrement and no size_t underflow. The spawn-failure memo is cleared in wait() under the same loader mutex the following spawn attempt already requires, so no lock ordering changed; it grants one attempt per wait entry rather than per queued job, and the coalescing test still asserts a delta of exactly 1 across 64 queued jobs. trySchedule is called with the default wait_microseconds = 0, so it cannot block while the loader mutex is held. Logging stays inside NOEXCEPT_SCOPE with ALLOW_ALLOCATIONS_IN_SCOPE, required because worker() runs under DENY_ALLOCATIONS_IN_SCOPE. No on-disk format, so there is no crash or restart re-entry path to walk.

Session id: cron:clickhouse-review-slot-9:20260814-193600

[groeneai]

cc @serxa @al13n321, could you review this? AsyncLoader::spawn wrapped scheduleOrThrowOnError in NOEXCEPT_SCOPE, so a transient CANNOT_SCHEDULE_TASK from a saturated global thread pool aborted the server during table loading; the spawn is now dropped instead when some worker will still drain the ready queue, and stays fatal only when none will.

[clickhouse-gh]

Once pool.spawn_failed is set, this early return suppresses every later droppable spawn() before it can call trySchedule again. That means a transient global-pool saturation can outlive the actual saturation episode for this pool: if another pool frees a thread and we later enqueue or release more ready jobs here while the current worker is still running, we still skip the respawn and keep draining serially until that worker exits or enters wait(). In other words, the memo currently coalesces failures by worker lifetime, not by global-pool saturation episode, which can turn a momentary CANNOT_SCHEDULE_TASK into a much longer table-loading throughput regression.

[groeneai]

Your description of the mechanism is right and I checked each part at the current head: the memo is a
Pool member (AsyncLoader.h:376), it is assigned at :971 from the attempt's own result, read at :938,
and unconditionally cleared at only two places (:838 in wait(), :1014 on worker exit), so a thread
freed by another pool does not clear it here. I disagree that this should change, and I measured the remedy the finding asks for.

Each per-pool ThreadPool is created with unlimited max_threads and queue_size (:61-63), so a
failure here always means the global pool is exhausted, and recovery is not cheaply observable:
available_threads and remaining_pool_capacity are private with no accessor (ThreadPool.h:211,218).
The only way to learn the pool recovered is to attempt a thread creation, and every attempt runs under
the held loader mutex. Re-probing per queued job is what the memo exists to prevent.

I implemented your suggestion as an arm (clear the memo at enqueue) and ran the suite:

arm	AsyncLoaderSpawnFailures for 64 queued jobs	suite
as shipped	1	32/32 pass
memo cleared per enqueue	64	SpawnFailureWithRunningWorkerDoesNotTerminate fails

So it turns one failed thread creation per saturation episode into 64 under the loader mutex, in the
state where threads cannot be created. SpawnFailureWithRunningWorkerDoesNotTerminate:1355 asserts that
delta is 1 deliberately.

The window is also bounded and already covered. The memo clears at the first of the running worker
entering wait() or exiting, and a worker re-evaluates after every job, so it lasts at most one job
execution rather than the whole saturation. SpawnFailureThenRecovers:1362 restores the limits and then
requires two jobs to run concurrently, which only happens if the pool spawns again; it passes.

Measured on bfd345b, unit test build id 773c8e7c2251fec8604b49e5fc5caf97de97c932.

[clickhouse-gh]

Build profile diff (arm_release)

Comparing 4a6ff09a0 with master 6d0a17390 (stripped binary size, per-symbol sizes and ThinLTO time; compile times per translation unit against the most recent warmup build that recompiled it).

✅ No significant changes.

Binary sizes

Binary	Master	PR	Δ
programs/clickhouse-stripped	693.26 MiB	690.25 MiB	-3.01 MiB (-0.43%)

Only the stripped binary is compared: the official master build keeps debug symbols while PR builds strip them, so the other binaries differ by construction.

Compile time of recompiled translation units

22 translation units recompiled, 201 s compile time in total, 22 of them have a recent master baseline.

Job report

[groeneai]

Bugfix validation (unit tests) was inconclusive on the previous head: the before-binary
compiled the overlaid test file and then failed to link it.

ld.lld-22: error: undefined symbol: ProfileEvents::AsyncLoaderSpawnFailures
referenced by gtest_async_loader.cpp:1322 ... referenced 15 more times

AsyncLoaderSpawnFailures is added by this PR, so the check could never reach a verdict, and a
build failure is deliberately not accepted as a reproduction. The event was the only symbol the
new tests needed from the non-test diff, so they now resolve it by name at runtime, as
ProgressTable and system.events already do.

Reproduced both directions locally by reverting only the non-test files to the merge base. The
before-binary now links, runs, and aborts with exit 134 inside AsyncLoader::spawn
(CANNOT_SCHEDULE_TASK), which is the bug itself. With the fix: AsyncLoader* 32/32, 80
iterations under --gtest_repeat=20, no undefined reference left in the test object file.

Also in this push, from my own review of the spawn tests:

• three asserted only that every job completed, which also holds if the spawn is never attempted.
  They now assert the attempt: an exact AsyncLoaderSpawnFailures delta of one on the finish()
  path, four submitted workers when none has started, and the queued job completing while the
  cross-pool wait is still outstanding. Each was verified with a mutation that skips the relevant
  spawn and reddens exactly that assertion, where the old ones stayed green.
• two tests named SpawnIsRequired* are renamed to SpawnIsAttempted*: a sole cross-pool waiter
  selects the resuming-worker tier, not the required one. Comments and the AsyncLoaderSpawnFailures
  description now name both kinds of drainer.
• the cited stress-test report was re-run green on 2026-08-14 at 22:21Z, which replaced its
  artifact. The body now links PR Add merge_use_batch_sorting_queue MergeTree setting for ordinary merges #108468's Stress test (arm_tsan), whose report still fails with
  the same stack. The CIDB row for the original run is unchanged.

Squashed to one commit.

[clickhouse-gh]

has_resuming_worker stops being accurate once a same-pool waiter is reprioritized after wait() starts. suspended_workers is incremented once at entry based on worker_pool == job->pool_id (wait() lines 825-831), but prioritize can later move the awaited job to another pool while the waiter is still blocked; JobPrioritizedWhileWaited already exercises that transition (gtest_async_loader.cpp:992-1035). After that migration the waiter will resume when the other pool runs the awaited job, yet waiting == suspended still makes spawn_is_required true here. A later CANNOT_SCHEDULE_TASK in this pool therefore still takes the fatal scheduleOrThrowOnError path in a state this patch is meant to recover from. Please either reclassify the waiter when prioritize moves the awaited job, or derive the resuming-worker tier from live waited-job state, and add a regression that saturates the global pool after the reprioritization.

[groeneai]

You are right, and my earlier reasoning on this was wrong. I reproduced the abort.

Constructing your scenario aborts with libc++abi: terminating ... CANNOT_SCHEDULE_TASK at
AsyncLoader.cpp:969 (spawn) -> :765 (enqueue) -> :406 (schedule). An otherwise
identical run without the prioritize() call survives, dropping the spawn and recording one
AsyncLoaderSpawnFailures, so the migration selects the fatal tier, not the saturation. My
earlier claim that canSpawnWorker's priority clause makes this unreachable was wrong:
current_priority is the minimum over active pools, and the old pool stays active while it
holds the waiter's worker slot and a non-empty ready queue.

It is not a regression: the pre-fix spawn() calls scheduleOrThrowOnError unconditionally,
so it aborts here too. This narrows the fix rather than breaking something new, as you say.

I am not pushing a fix this round, because both remedies you offer measure out unsound:

• Moving the count to the new pool also removes the pool's spawn allowance
  (workers < max_threads + suspended_workers), which the waiter still needs since it keeps
  occupying a worker slot. Result: no spawn is attempted at all (workers=2 max=2 susp=0).
• Deriving the tier from live waited-job state with a separate counter keeps the suite green,
  but a max_threads = 1 pool then hangs: when the only worker is the waiter, dropping the
  spawn leaves nobody to drain the queue.

The unanswered part: "the waiter will resume and drain" holds only if the awaited job can
progress in its new pool, and under global exhaustion it may not. So the tier cannot be
decided from pool-local counters alone, and dropping the spawn can trade a visible abort for a
silent hang. I would rather land the current strict improvement and treat this window as a
separate question with its own regression than widen the tolerated set on reasoning I cannot
back with a test.

[groeneai]

CI finish ledger for 4a6ff09

CI is fully finished on this head and there are no failures to attribute: 152 checks succeeded, 25 skipped, 0 failed. The praktika report for this head is OK with 0 dropped jobs, and CIDB agrees (0 FAIL/ERROR rows for this commit).

Check / test	Reason	Owner / fixing PR
-	no failures at this head	-
Sync	-	CH Inc sync (private, not actionable)

Session id: cron:our-pr-ci-monitor:20260815-180000

[serxa]

This is fantastic, Thanks!

[groeneai]

The residual I disclosed in this PR fired on the 26.7 release branch: Stress test (amd_debug) at 899a806, Received signal 6, AsyncLoader.cpp:943. The fix is present in that binary (backport #115205 merged seven days earlier), so this is the uncovered mandatory arm, not a regression.

Report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=26.7&sha=899a806f4539cb407a89cc662adedd8e1c66bfe6&name_0=ReleaseBranchCI&name_1=Stress%20test%20%28amd_debug%29

From that run's own clickhouse-server.err.log.zst: Cannot schedule a task: failed to start the thread: ... no free thread (timeout=0) (threads=10000, jobs=10000). Saturation was server-wide, with 54 such events also hitting the azure disks, executeQuery, TCPHandler and a background executor.

The tier is correct here, which is why I am not sending a patch. Failed to spawn a loader worker appears zero times in the whole log, so no spawn was ever dropped; the first failure landed straight on the mandatory arm. The abort is a foreground CREATE DATABASE over HTTP scheduling into a pool with no worker that can drain it, started=waiting=suspended=0, or a same-pool waiter at (1,1,1). In both states nothing runs the ready queue unless this spawn succeeds.

The three ways out all leave your file, so I would rather ask than guess:

1. Return false and rearm later. The only spawn retry is in wait(), inside if (current_load_job && ...). current_load_job is a thread_local set only in worker(), so for a foreground waiter it is null and the retry is skipped entirely. Dropping the spawn turns the abort into a permanent hang of the query, the same trade the max_threads = 1 measurement in this thread already rejected.
2. Let the exception reach the caller. schedule() states that the code past its DENY_ALLOCATIONS_IN_SCOPE is not exception-safe, and the throw happens mid pass 3: pool.workers is already incremented and never decremented, and jobs pulled in by gatherNotScheduled outlive any LoadTask, since remove() only cancels a task's own jobs. This means making schedule() exception-safe, not a local change.
3. Drain inline on the calling thread. Defensible for a foreground waitLoad, but current_load_job, executionPool(), priority accounting and the deadlock detector all assume jobs run on workers.

One occurrence in 30 days post-fix, on a release branch, none on master, so this is not urgent. I also checked #112928: it reworks this seam and drops the SpawnIsRequiredWhileNoWorkerHasStarted test, but its scheduleThreadOrThrow still throws and it does not touch AsyncLoader.cpp, so the mandatory arm still terminates there. I read it as a sequencing constraint rather than the answer.

Which of the three would you accept? I am happy to implement whichever you name, and I will leave this alone otherwise.