Skip to content

[MOD-16610] Lazy spawn + logical shrink for shared SVS pool: unblock CONFIG SET WORKERS#989

Draft
dor-forer wants to merge 2 commits into
mainfrom
dor-mod16610-lazy-svs-thread-spawn
Draft

[MOD-16610] Lazy spawn + logical shrink for shared SVS pool: unblock CONFIG SET WORKERS#989
dor-forer wants to merge 2 commits into
mainfrom
dor-mod16610-lazy-svs-thread-spawn

Conversation

@dor-forer

@dor-forer dor-forer commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Describe the changes in the pull request

FT.CONFIG SET WORKERS N resizes the shared SVS thread pool synchronously on the Redis main thread. Each slot eagerly constructed an svs::threads::Thread — a serialized pthread_create plus a boot handshake that blocks until the worker reaches its Spinning state — and freshly booted threads burn a 1M-pause idle spin with no work, oversubscribing the CPU so the handshakes stretch superlinearly: 52.5 s at N=2000 (vs ~60 µs on 8.6, which only toggled the write mode). On Redis Enterprise this stalls the shard past CNM's 3 s socket timeout (PUT /v2/bdbs flakes). Shrinking a warmed pool had the symmetric cost: a shutdown handshake plus join per spawned thread on the main thread (~3.9 s at N=2000).

This PR makes both resize directions O(slots) on the main thread — never O(threads):

  • Lazy spawn (grow): ThreadSlot holds std::optional<svs::threads::Thread>, empty at creation. Resize only allocates slots; the OS thread is spawned by the renter that wins the occupied CAS, interleaved with work assignment and outside pool_mutex_, so the fresh thread's first spin window immediately catches its partition.
  • Logical shrink: slots are never destroyed. logical_size_ (the rent limit) is tracked separately from physical capacity (a high-water mark). Shrink is a size-field write through the existing deferred-resize protocol; parked threads sleep (one final spin window, no periodic wakeup) and are reused warm on regrow.
  • Reclaim: reclaimExcessThreads() (noexcept, detach-under-lock / join-outside) joins unoccupied slots above the logical limit. Triggered only from the worker-executed completion path of a scheduled SVSMultiThreadJob — never from endScheduledJob, which also fires from FT.DROPINDEX teardown on the main thread (counter-only there).
  • Degraded execution: a spawn failure (std::system_error), assign failure, or rent shortfall stops dispatch and runs the remaining partitions on the calling thread — partitions are never dropped (this also fixes a pre-existing silent partition-drop on rent shortfall in release builds). Crashed workers are retired to the unspawned state and lazily respawn.
  • RAII completion guard in ExecuteMultiThreadJobImpl: a throwing task can no longer wedge reserve jobs or the pending_jobs_ counter (which deferred shrink and reclaim depend on). Typed task wrappers log failures with index context before rethrowing.

Measurements (16-core Linux, N=2000 workers):

Operation (main thread) Before After
Grow 1 → 2000 52.5 s 0.19 ms
Shrink 2000 → 1 (warmed) ~3.9 s ~1 µs
Reclaim of 2000 parked threads ~3.1 s (background worker)
First rent after grow (spawn cost) ~5.2 s (background worker, one-time)

Semantics note: VecSim_GetSharedMemory() now reports slot capacity at its high-water mark (~64 B/slot) — it no longer drops on shrink, since slots are retained for warm reuse. Known accepted gaps (documented in code): no reclaim trigger in write-in-place mode (WORKERS 0) or after the last SVS index is dropped, until async SVS activity resumes.

Tests: lazy-spawn / park / warm-reuse / reclaim OS-thread-count tests in an isolated fixture; real-trigger integration tests for the reclaim path (executed scheduled job reclaims, unexecuted-job teardown does not); completion-guard exception test; crashed-worker retire/respawn; degraded-execution release-mode semantics; shared-memory high-water contract.

Which issues this PR fixes

  1. MOD-16610

Main objects this PR modified

  1. ThreadSlot / VecSimSVSThreadPoolImpl (svs_utils.h) — lazy spawn, logical size vs. physical capacity, reclaimExcessThreads(), degraded-execution dispatch
  2. SVSMultiThreadJob::ExecuteMultiThreadJobImpl (svs_tiered.h) — RAII completion guard, reclaim trigger on the worker-executed completion path

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

🤖 Generated with Claude Code


Note

Medium Risk
Changes core tiered SVS threading, deferred resize, and async job lifecycle; mis-ordering reclaim vs main-thread teardown could wedge workers or leak threads, though the PR explicitly gates joins to background completion paths.

Overview
CONFIG SET WORKERS no longer blocks the Redis main thread on mass pthread_create/join during shared SVS pool resize (MOD-16610).

VecSimSVSThreadPoolImpl now separates logical rent limit (logical_size_) from physical slot capacity (high-water). Resize only allocates slot structs; OS threads spawn on first rent via ThreadSlot::ensureThread(). Shrink lowers the logical limit and parks excess threads; reclaimExcessThreads() joins parked threads off the main thread. rent() only hands out slots below the logical limit. parallel_for uses degraded execution: spawn/assign/rent failures run remaining partitions on the caller instead of dropping work (release builds previously could run only partition 0). Crashed workers are retired with resetThreadAt for lazy respawn. VecSim_GetSharedMemory() reflects slot high-water—it no longer drops on shrink.

SVSMultiThreadJob::ExecuteMultiThreadJobImpl adds an RAII completion guard so task exceptions still release reserve jobs, delete the job, and decrement scheduled-job state; exceptions are swallowed at the job boundary while updateSVSIndexWrapper / SVSIndexGCWrapper log with index context then rethrow. reclaimExcessThreads() runs only after scheduled jobs complete on a worker—not from endScheduledJob() (main-thread teardown).

Tests cover lazy spawn, logical shrink/park/warm reuse, reclaim triggers, completion guard on failure, shared-memory contract, and degraded parallel_for semantics.

Reviewed by Cursor Bugbot for commit 7b401a9. Bugbot is set up for automated code reviews on this repo. Configure here.

…CONFIG SET WORKERS

FT.CONFIG SET WORKERS N resizes the shared SVS thread pool synchronously on
the Redis main thread. Each slot eagerly constructed an svs::threads::Thread —
a serialized pthread_create plus a boot handshake that blocks until the worker
reaches its spinning state — and freshly booted threads burn a 1M-pause idle
spin with no work, oversubscribing the CPU so the handshakes stretch
superlinearly: 52.5 s at N=2000 (vs ~60 us on 8.6, which only toggled the
write mode). On Redis Enterprise this stalls the shard past CNM's 3 s socket
timeout. Shrinking a warmed pool had the symmetric cost: a shutdown handshake
plus join per spawned thread on the main thread (~3.9 s at N=2000).

Make both resize directions O(slots) on the main thread — never O(threads):

* Lazy spawn (grow): ThreadSlot holds std::optional<svs::threads::Thread>,
  empty at creation. Resize only allocates slots; the OS thread is spawned by
  the renter that wins the occupancy CAS (ensureThread), interleaved with
  assign so the new thread's first spin window immediately catches its
  partition. Spawn cost moves to the first job that uses the slot, on a
  background thread.
* Logical shrink: physical slots are never destroyed. Shrink lowers
  logical_size_ (tracked separately from capacity; grows/shrinks are
  classified against it) through the existing deferred-resize protocol; rent()
  only hands out slots below the limit. Excess threads park asleep (one final
  idle-spin window, no periodic wakeup) and are reused warm if the pool grows
  again. This mirrors the logical-only resize the per-index pools used before
  the shared pool existed, minus its defect: capacity can still grow above the
  high-water mark. Slots are heap-allocated and immortal, so renters' raw
  ThreadSlot pointers are structurally safe.
* Thread reclaim ("GC"): reclaimExcessThreads() — noexcept, detach-under-lock /
  join-outside-lock — joins parked threads above the logical limit. Triggered
  only from the worker-executed completion path of a scheduled job (gated on
  isScheduled saved before the job self-deletes), so the periodic SVS GC and
  update jobs drive reclamation. Deliberately NOT in endScheduledJob(): that
  also fires from JobsRegistry teardown of unexecuted jobs on the Redis main
  thread, which must stay counter-only. Accepted gap: in write-in-place mode
  (WORKERS 0) or after the last SVS index is dropped, parked threads persist
  until async SVS activity resumes.
* parallel_for gains a unified degraded-execution path: a spawn failure
  (std::system_error — thread exhaustion now happens at first use, mid-job),
  an assign failure (crashed worker), or a rent shortfall all fall back to
  running the missing partitions on the calling thread — partitions are never
  dropped (previously a shortfall silently ran only partition 0 in release
  builds). Crashed workers are retired to the unspawned state and lazily
  respawn on the next rent.
* ExecuteMultiThreadJobImpl gets an RAII completion guard: a throwing task can
  no longer leave reserve jobs blocked forever or wedge the pending-jobs
  reservation that deferred shrink and reclaim depend on. Task failures are
  logged with index context by the typed wrappers.
* VecSim_GetSharedMemory() contract change: slot capacity is a high-water mark
  (~64 bytes/slot), so the reported allocation no longer drops on shrink;
  sharedMemoryTracksThreadPoolResize is updated accordingly.

Measured on 14 cores (pool-level bench): grow to N=2000 drops from 52.5 s to
0.19 ms on the main thread; warmed shrink from ~3.9 s to ~1 us (a size-field
update). The joins (~3.1 s at N=2000 worst case) run on the job-completion
path off the main thread; the first big rent pays the spawn cost off the main
thread; reuse and warm regrow pay nothing.
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.41935% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.88%. Comparing base (45daaf3) to head (7b401a9).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/VecSim/algorithms/svs/svs_utils.h 81.15% 13 Missing ⚠️
src/VecSim/algorithms/svs/svs_tiered.h 66.66% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #989      +/-   ##
==========================================
- Coverage   97.12%   96.88%   -0.25%     
==========================================
  Files         141      141              
  Lines        8164     8239      +75     
==========================================
+ Hits         7929     7982      +53     
- Misses        235      257      +22     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pthread_join returning does not guarantee the joined thread's
/proc/self/task entry is gone — the kernel wakes the joiner before it
releases the task entry, so a just-joined thread can linger in the count
briefly (caught by the sanitizer job's slowdown). Post-reclaim OS-thread
counts now poll with a deadline, and baselines are captured only after
the count holds steady; spawn-side assertions stay exact (thread creation
is synchronous).
@dor-forer dor-forer force-pushed the dor-mod16610-lazy-svs-thread-spawn branch 2 times, most recently from c87ab5b to 7b401a9 Compare July 8, 2026 08:09
@dor-forer dor-forer marked this pull request as draft July 8, 2026 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant