fix(harness): thread-spawn failure and worker panics deadlocked the search start barrier

[fcostaoliveira]

The bug

Every engine's search() synchronized its measured-window start with Barrier::new(parallel + 1) — a participant count fixed before the workers exist. Two ordinary failures make that count unmeetable, and both produce a permanent hang with no output rather than an error:

1. The OS refuses a thread. Scope::spawn panics on EAGAIN (ulimit -u, cgroup pids.max — i.e. any CI container with a large parallel). The panic unwinds into thread::scope's drop, which joins the workers already spawned; they are parked in ready.wait() waiting for parallel + 1 arrivals that can never happen.
2. A worker panics before the barrier — e.g. an out-of-bounds index in the "prime" query on a short query set. The coordinator then parks in ready.wait() forever.

--search-timeout defaults to 0.0 (disabled), so no watchdog breaks it.

The second half: a published number whose name lies

The barrier deadlock is the headline, but the same code had a quieter defect that needed no fault injection at all. Both cases below are a shipped config against a stock server, reproduced during review:

pgvector parallel: 100 (25 such points in pgvector-single-node.json) against Postgres 17 with max_connections=64:

	master	this branch
exit code	0	1
published	parallel: 100, rps: 23.5	(nothing)
server log	36 × FATAL: sorry, too many clients already	same
stderr	(silent)	only 64 of 100 pgvector-search workers reached the start gate — 36 failed setup: pgvector-search worker connect failed: FATAL: sorry, too many clients already

A 64-wide run labelled 100. The 36 failed workers crossed both barriers, returned empty, and had their errors discarded with no log line.

Redis maxclients=24, parallel: 64: master exits 0 and publishes parallel: 64, rps: 299.5, failed_queries: 0, mean_recall: 1.0 — measured by 24 workers. This branch exits 1 with only 24 of 64 redis-search workers reached the start gate — 40 failed setup … Refusing to report a run at parallel=64 that measured fewer workers.

Blast radius (independently derived)

The issue named 6 engines; its own follow-up comment had already raised that to 13. grep -rn 'Barrier::new' over src/ gives 15 harnesses across 14 engines — the extra being vertex, whose two harnesses spell it Barrier::new(workers + 1):

file	harness	shape
chroma, dragonfly, elasticsearch, kividb, milvus, mongodb, opensearch, pgvector, qdrant, redis, valkey, vectorsets	search	ready + go + OnceLock start cell
weaviate	search gRPC and GraphQL	one barrier pair shared by both branches
vertex	search and mixed	one barrier + OnceLock + spin

Checked for the same shape in other fixed-count primitives and found none: no mpsc channel awaiting N messages (the only channel is experiment.rs's watchdog, which uses recv_timeout and is disconnect-safe), no Condvar/latch counters, no WaitGroup. src/redisearch/ and src/vectorsets/ also contain barrier-free scopes but are not declared in lib.rs and are not compiled (confirmed against cargo's own dep-info).

23 s.spawn sites remain, in the upload / filter-only / mixed / turbopuffer paths. None has a fixed-count wait, so a refused spawn there panics loudly rather than hanging: 8 rely on an explicit h.join().unwrap() and 15 on thread::scope's implicit join, and either way the panic reaches the main thread. One of them — turbopuffer.rs:586 — is nonetheless inside a timed search harness, so it measures connection setup inside the window; that and seven other timed harnesses are #266.

The fix

New vector_db_benchmark::start_gate replaces both barriers and the OnceLock start cell with a gate whose wait is satisfied by ticket outcomes rather than a count:

• WorkerPool::spawn goes through thread::Builder::spawn_scoped, which returns io::Result instead of panicking, and mints the worker's ticket itself, handing it to the closure. A worker cannot exist without a ticket, or a ticket without a worker — that pairing is the whole deadlock-freedom argument, so it is an API property, not a convention. WorkerPool::new also takes the planned worker count, and start refuses a pool that spawned fewer than it plans.
• A ticket settles by arriving, by reporting a setup failure, or — via Drop — by being lost to a panic or to a thread the OS never started. Any terminal outcome satisfies the coordinator, so no arrival count is ever left unmet.
• Drop for WorkerPool aborts the gate, so an early ? anywhere in the scope closure releases parked workers. Weaviate's gRPC path fans out tokio tasks rather than scoped threads and drives a bare StartGate, so it holds an AbortGateOnDrop declared after the runtime — without it, a coordinator-future panic left tasks parked on a condvar and Runtime::drop joined them forever, which is thread-spawn failure deadlocks the fixed-count Barrier in 6 engines' search harness #214's own shape. INV-4c now requires that guard.

Vertex's open-loop path keeps its 100 ms scheduling lead via start_with. Every other engine's diff is mechanical.

Failure semantics

Per the settled policy — anything that changes the reported number is a hard error. A worker that never started, died before the gate, failed setup, or panicked mid-run means the run measured fewer workers than the parallel it reports, so WorkerPool::start returns Err naming what happened.

Scope of the "silent degradation" half. This closes it for the closed-loop search() of 14 engines only. search_mixed (4 engines), search_filter_only (3) and turbopuffer::search still publish a parallel from config with no guard — tracked in #266.

Mid-run panics are a diagnostic improvement, not a correctness fix. Master does not publish a wrong result here: all 14 engines used h.join().unwrap(), so a worker panic re-panics on the main thread and the process exits 101. The gain is a clean exit 1 with 1 of 8 redis-search workers panicked mid-run; discarding the run rather than reporting partial results, and a named thread (redis-search-4) where master prints thread '<unnamed>'.

Reproduction — before / after

Real RLIMIT_NPROC (ulimit -u set 8 above the current task count, parallel=64), against the pre-fix shape extracted verbatim into a standalone binary:

• unfixed: panics failed to spawn thread: Os { code: 11, kind: WouldBlock }, then hangs. SIGKILLed at 30 s in 2 of 3 runs; the third could not start because timeout itself could not fork. 30 s is the timeout bound — a floor, not a measured hang duration.
• fixed: errors in ~1–2 ms (1.04 / 1.78 / 2.20 ms over three clean runs) with could not start redis-search worker 7 of 64: Resource temporarily unavailable (os error 11). The OS refused the thread — lower parallel, or raise the thread/process limit (ulimit -u, cgroup pids.max). Single-machine numbers, and the extraction harness is not in this diff; an independent reviewer measured 142 µs–9.3 ms on other boxes, and one noisier box 17–27 ms. The claim reproduces qualitatively, not to the decimal.

In CI, deterministically, via a #[cfg(test)] thread-local spawn-failure seam (absent from a release build by both strings and nm — a test seam, not a runtime backdoor), plus real worker panics and real dropped tickets. Reverting the gate to the pre-fix semantics — fixed arrival count, no Drop settling, Scope::spawn's panic, failed workers crossing both barriers, no planned-vs-spawned check — turns 6 of the 13 start-gate tests red:

running 13 tests
test start_gate::tests::early_return_from_the_scope_closure_does_not_strand_parked_workers ... ok
test start_gate::tests::start_with_hands_every_worker_the_caller_chosen_instant ... ok
test start_gate::tests::worker_panic_after_the_gate_is_reported_not_swallowed ... ok
test start_gate::tests::zero_workers_is_not_a_hang ... ok
test start_gate::tests::all_workers_observe_one_shared_start_after_every_worker_warmed ... ok
test start_gate::tests::legacy_fixed_count_barrier_hangs_when_a_spawn_fails ... ok
test start_gate::tests::legacy_fixed_count_barrier_hangs_when_a_worker_panics ... ok
test start_gate::tests::spawn_failure_is_a_prompt_error_not_a_hang ... FAILED
test start_gate::tests::a_pool_that_spawns_fewer_workers_than_it_plans_is_an_error ... FAILED
test start_gate::tests::a_worker_that_settles_nothing_is_lost_not_hung ... FAILED
test start_gate::tests::abort_releases_workers_already_parked_at_the_gate ... FAILED
test start_gate::tests::worker_setup_failure_is_a_hard_error_not_a_quieter_run ... FAILED
test start_gate::tests::worker_panic_before_the_gate_is_a_prompt_error_not_a_hang ... FAILED

test result: FAILED. 7 passed; 6 failed; 0 ignored; 0 measured; 98 filtered out; finished in 5.01s

Those six only report rather than hanging forever because each wraps its call in a 5 s watchdog. With the watchdog raised to an hour, four of them are SIGKILLed at the 30 s timeout bound — spawn_failure_…, worker_panic_before_the_gate_…, worker_setup_failure_…, a_worker_that_settles_nothing_…. On this branch all 13 pass; the suite takes 2.00 s, which is the two legacy_* hang proofs deliberately watching a known-deadlocked shape for 2 s each. Every other test completes well under the 5 s bound it asserts (per-test timing is nightly-only, so no finer figure is quoted).

Tests and guards

• src/start_gate.rs — 13 unit tests. Two legacy_* tests replicate the pre-fix Barrier shape under both failure modes and assert it never completes, so "the new code returns Err" is evidence of something rather than a tautology. Reviewers confirmed they are non-tautological: neutering Drop for WorkerTicket alone produces exactly one failure, a full pre-fix revert produces six.
• tests/overhead_invariants.rs — this file had never run in CI. Every cargo test in the workflows is --lib --bins --release (which does not build tests/*.rs) or a named --test integration_<engine>, so INV-2 and INV-3 have been unenforced since they landed, and INV-4 would have been unfailable on day one. This PR adds cargo test --test overhead_invariants --release to the unit-test job.
• All guards now scan comment- and string-stripped source. This was a live hazard, not hygiene: INV-4 bans the Barrier type, and all 14 engines used to carry a comment describing the barrier it removes, so the guard forbade documenting its own subject.
• INV-4 bans the Barrier type anywhere under src/, not one spelling of the call — closing the alias, type-alias, UFCS and other-directory evasions at once — with a per-line // INV-4-ALLOW: <reason> opt-out so a future legitimate barrier is possible but reviewable.
• INV-4b derives its engine list from engine/mod.rs (new engines opted in by default; each exclusion carries a reason), requires one park per harness so ungating one of vertex's or weaviate's two is caught, bans let _ = ticket.arrive_and_wait(), and requires the park to sit below the first setup-failure arm — which catches hoisting it above client construction, i.e. connection setup back inside the measured window.
• INV-4c requires an AbortGateOnDrop wherever a bare StartGate is driven.

Mutation campaign — 10/10 as intended (8 realistic misses killed, 2 false-positive probes left alone):

mutant	verdict
let _ = ticket.arrive_and_wait() (redis)	KILLED
park hoisted above client construction (chroma)	KILLED
one of vertex's two harnesses un-gated	KILLED
use std::sync::Barrier as Rendezvous (kividb)	KILLED
type Gate = std::sync::Barrier + UFCS (milvus)	KILLED
AbortGateOnDrop removed (weaviate)	KILLED
verdict discarded at a second site (pgvector)	KILLED
brand-new engine file with Barrier::new	KILLED
Barrier::new in a comment	survived (correct)
legitimate barrier with INV-4-ALLOW	survived (correct)

The three API-level mutants reviewers used to break the ticket/spawn pairing are now unrepresentable: pool.spawn(\|\| {}) and pool.ticket() fail to compile (E0593, E0599), and minting N-1 for N planned fails at runtime with test spawned 3 workers but the run is labelled parallel=4.

Other review fixes

• WorkerPool::spawn's error read "worker k of k" (both operands were index + 1); it now names the planned parallel — worker 7 of 64.
• StartGate::wait_ready takes the harness label, so "N never reached the start gate" says which engine.
• pgvector's setup-failure message went through postgres::Error's Display, which is literally "db error"; it now reads as_db_error() and the source() chain, so the likeliest real trigger reads FATAL: sorry, too many clients already.
• A failing search point discarded the points that already succeeded. experiment.rs flushed pending_saves only after the whole phase, so a parallel 1, 2, 4 sweep in which only 4 fails wrote zero files. That contradicts the policy stated for --fail-on-dropped-queries ("results files are still written before the run fails, so the evidence survives"). The hard failure is now recorded and raised after the flush.

Behaviour change to be aware of

The shipped pgvector configs have 25 points at parallel: 100, and Postgres defaults to max_connections = 100 with 3 reserved for superusers — so on a stock server those points are one stray session away from a hard failure that master papered over. Mitigations here, none of which change a published parallel value:

• tests/docker-compose.test.yml starts pgvector with max_connections=200.
• pgvector warns before fanning out when parallel exceeds the live budget, with the arithmetic (max_connections=, superuser_reserved=, in use, available). Advisory only — it is racy by construction and meaningless behind a pooler; the gate stays the authority.
• --repetitions (default 3) already absorbs transient failures: a run whose rep 1 was refused but whose reps 2–3 succeeded still exits 0 with a valid result. Only deterministic failures go red.

Follow-ups filed (not fixed here)

• Eight timed fan-out harnesses still have no synchronized start #266 — eight other timed fan-out harnesses have no synchronized start (search_mixed ×4, search_filter_only ×3, turbopuffer::search).
• StartGate::wait_ready has no deadline: a worker that hangs *inside* setup still hangs the run #267 — wait_ready has no deadline, so a worker that hangs inside setup still hangs the run. Not a regression (the barrier was identical), but the module doc's "progress is therefore guaranteed" overstated it and has been softened.
• No runtime measurement invariant: nothing in CI defends the harness's own overhead #268 — no runtime measurement invariant exists. The review produced the numbers to add one: replacing the barriers cut dead time (stamp → last worker resume) from 5,644 µs to 1,744 µs at N=64 and from 205,527 µs to 26,703 µs at N=1024, because master stamped the start and then called go.wait() — a second thundering herd inside the window.
• --exit-on-error false publishes a truncated summary and concurrency_curve reports a knee computed from it #269 — --exit-on-error false publishes a truncated summary and concurrency_curve reports a knee computed from it.
• INV-4c is per-file, so a second bare StartGate in weaviate.rs slips through #276 — INV-4c is a per-file existence check, so a second bare StartGate in weaviate.rs would slip through (there is one today, and it is guarded).
• pgvector's connection-budget warning over-counts and warns spuriously #277 — pgvector's connection-budget warning counts background workers and subtracts superuser-reserved slots from a superuser connection, so it warns spuriously. Advisory only; it cannot fail a run.
• WorkerPool::start collects worker sample buffers inside the measured total_time #270 — WorkerPool::start merges sample buffers inside total_time. Sub-millisecond at 10k queries; also notes that pre-merge results carry a slightly larger dead-time penalty than post-merge ones, which matters because this repo publishes trend charts.

Note for whoever sequences this with #246

engine/opensearch.rs was flagged as off-limits for PR #246, but it is one of the six engines the issue names and leaving it would keep an identical hang alive next door. The escape hatch is not "drop one hunk" — it is 7 hunks; dropping only the harness leaves an unused use …WorkerPool and fails clippy -D warnings, and dropping the file entirely makes this PR's own tests fail (INV-4 flags opensearch.rs:1516/1517, INV-4b derives it from engine/mod.rs). My earlier non-overlap claim also missed my own import hunk at @@ -16,6, which falls inside #246's @@ -8,12; git merges it because the insertion points differ by two lines — resolved by margin, not by design. git merge-tree is clean for +#251, +#246 and all three together; if a conflict ever appears at that import, keep all three use lines.

Closes #214

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com

[github-actions]

Docker Build Validation

Platforms tested:

• linux/amd64 (built and tested)
• linux/arm64 (build validated)

Tests performed:

• --help command
• --describe datasets command
• Integration test (random-100 dataset + Redis)
• Multi-platform build validation (amd64 + arm64)

[github-actions]

Docker Build Validation

Platforms tested:

• linux/amd64 (built and tested)
• linux/arm64 (build validated)

Tests performed:

• --help command
• --describe datasets command
• Integration test (random-100 dataset + Redis)
• Multi-platform build validation (amd64 + arm64)

[github-actions]

Docker Build Validation

Platforms tested:

• linux/amd64 (built and tested)
• linux/arm64 (build validated)

Tests performed:

• --help command
• --describe datasets command
• Integration test (random-100 dataset + Redis)
• Multi-platform build validation (amd64 + arm64)

[github-actions]

Docker Build Validation

Platforms tested:

• linux/amd64 (built and tested)
• linux/arm64 (build validated)

Tests performed:

• --help command
• --describe datasets command
• Integration test (random-100 dataset + Redis)
• Multi-platform build validation (amd64 + arm64)