Skip to content

perf(vector): move COLD-segment reload off the shard event loop (audit #18)#251

Merged
pilotspacex-byte merged 1 commit into
mainfrom
feat/vector-offload-reload-pool
Jul 9, 2026
Merged

perf(vector): move COLD-segment reload off the shard event loop (audit #18)#251
pilotspacex-byte merged 1 commit into
mainfrom
feat/vector-offload-reload-pool

Conversation

@pilotspacex-byte

Copy link
Copy Markdown
Contributor

Audit finding #18 — COLD-segment reload off the shard event loop

Stacked on #250 (base = fix/prod-hardening-batch-a), so this PR shows only the #18 delta.

Problem

Under --disk-offload, an idle vector segment is demoted to the COLD tier (UnloadedSegment stub — everything in-memory dropped). The first FT.SEARCH to touch it must reload the whole segment: WarmSearchSegment::from_files (mmap + page-in + optional mlock). That reload ran inline on the single-threaded shard event loop (promote_unloaded at the top of the yielding search capture), so it stalled every other connection on the shard for the full reload — hundreds of ms to seconds on real NVMe/EBS with a cold page cache. A multi-tenancy violation.

Fix — async, await full recall

A new process-global SegmentReloadPool (src/vector/reload_pool.rs) performs the reload I/O on dedicated worker threads. Auto-on; MOON_VEC_RELOAD_WORKERS=0 disables (falls back to the blocking reload).

  • Capture (non-blocking): SegmentHolder::submit_unloaded_reloads submits each COLD stub off-loop and stashes the flume receivers in SearchSnapshot; any already-completed reload is installed into WARM (memory reclaim).
  • Handler: SearchSnapshot::await_pending_reloads awaits the receivers before scanning — recv_async().await parks only the triggering query's task (not the OS thread), then splices the reloaded segments into that query's own snapshot.
  • Net effect: the dense-KNN query that triggered the reload still sees full recall, while sibling connections on the shard keep running.
  • Reloads are single-flight (concurrent first-touches of one segment share a job) and cached until the next capture installs them into WARM.

Design-for-failure

An erroring reload, or a worker panic/death, replies Err/disconnect to every waiter — the query answers with degraded recall (segment stays COLD, retried next touch) rather than hanging or crashing. When the pool is disabled, submit_unloaded_reloads falls back to the blocking promote_unloaded, preserving exact pre-#18 behavior.

Scope / accepted follow-ups

Only the yielding default-field dense-KNN path goes off-loop. Non-yielding sync search paths are unchanged (still block); HYBRID/SPARSE/RANGE/non-default-field queries do not take the off-loop path.

Tests (red/green, both runtimes)

  • reload_pool: off-thread delivery, single-flight dedup, completed cache, default_workers env resolution.
  • holder: submit_unloaded_reloads installs off-loop; await_pending_reloads splices the reloaded segment into the snapshot and drops the cold stub.
  • Full suite: 654 vector + 4 reload_pool + 13 holder green on both monoio and tokio; clippy -D warnings clean on both; fmt clean.

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@TinDang97, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 30 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bb71ac9c-69b3-4b91-b47a-6f7c38dba0f1

📥 Commits

Reviewing files that changed from the base of the PR and between 8b2c0c7 and 1a8820f.

📒 Files selected for processing (10)
  • CHANGELOG.md
  • src/command/vector_search/ft_search/dispatch.rs
  • src/main.rs
  • src/server/conn/handler_monoio/ft.rs
  • src/server/conn/handler_sharded/ft.rs
  • src/vector/mod.rs
  • src/vector/reload_pool.rs
  • src/vector/search_pool.rs
  • src/vector/segment/holder.rs
  • src/vector/segment/holder/promote.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/vector-offload-reload-pool

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pilotspacex-byte
pilotspacex-byte changed the base branch from fix/prod-hardening-batch-a to main July 9, 2026 17:27
@TinDang97
TinDang97 force-pushed the feat/vector-offload-reload-pool branch from 838e79e to 4c318ca Compare July 9, 2026 17:29
Audit finding #18: under --disk-offload an idle vector segment is demoted
to the COLD tier (UnloadedSegment stub). The first FT.SEARCH to touch it
reloaded the whole segment (WarmSearchSegment::from_files — mmap + page-in
+ optional mlock) INLINE on the single-threaded shard event loop via
promote_unloaded, stalling every other connection on that shard for the
full reload (hundreds of ms–seconds on real NVMe/EBS with a cold page
cache). That is a multi-tenancy violation.

Introduce a process-global SegmentReloadPool (src/vector/reload_pool.rs)
that performs the reload I/O on dedicated worker threads:

- Yielding FT.SEARCH capture calls the new non-blocking
  SegmentHolder::submit_unloaded_reloads: it submits each COLD stub to the
  pool off-loop and stashes the flume receivers in SearchSnapshot; already
  completed reloads are installed into WARM (memory reclaim).
- The handler awaits SearchSnapshot::await_pending_reloads before scanning
  — recv_async parks only the triggering query's task (not the OS thread),
  then splices the reloaded segments into that query's own snapshot. The
  dense-KNN query that triggered the reload therefore still sees FULL
  recall while sibling connections keep running.
- Reloads are single-flight (concurrent first-touches of one segment share
  one job) and cached until the next capture installs them into WARM.

Design-for-failure: an erroring reload, or a worker panic/death, replies
Err/disconnect to every waiter — the query answers with degraded recall
(segment stays COLD, retried next touch) rather than hanging or crashing.
When the pool is disabled (MOON_VEC_RELOAD_WORKERS=0), submit_unloaded_
reloads falls back to the blocking promote_unloaded, preserving exact
pre-#18 behavior. Non-yielding sync search paths are unchanged (still
block); HYBRID/SPARSE/RANGE/non-default-field queries do not take the
off-loop path (accepted follow-up).

Tests (red/green, both runtimes):
- reload_pool: off-thread delivery, single-flight dedup, completed cache,
  default_workers env resolution.
- holder: submit_unloaded_reloads installs off-loop; await_pending_reloads
  splices the reloaded segment into the snapshot and drops the cold stub.

author: Tin Dang
@TinDang97
TinDang97 force-pushed the feat/vector-offload-reload-pool branch from 4c318ca to 1a8820f Compare July 9, 2026 18:46
@pilotspacex-byte
pilotspacex-byte merged commit c01b068 into main Jul 9, 2026
11 checks passed
pilotspacex-byte added a commit that referenced this pull request Jul 14, 2026
…sed] + resolve PR #TBD placeholders (#330)

The v0.6.0 tag (355f68d, 2026-07-10) absorbed several PRs that merged after
the v0.6.0 release PR itself but before the tag was cut. Their changelog
entries were left under [Unreleased], understating what shipped in the
tagged v0.6.0 binary.

Audit (task #65, v0.7.0 roll-up prep) identified 23 sections whose PR merge
commits are verified ancestors of v0.6.0 via `git merge-base --is-ancestor`:
PR #248 (FastScan SIMD, SQ8 default, TQ ADC L2 fix, EF_RUNTIME FT.CONFIG,
RERANK_MULT+EXACT_BEAM, CLI/moon.conf tuning defaults), PR #250 (13
production-hardening sections), PR #251 (COLD-segment reload off event
loop), PR #254 (roadmap docs suite), PR #255 (CI macOS 30m), PR #256 (v0.6.0
ledger closure). Moved verbatim to a new subsection at the top of [0.6.0],
with a note explaining the absorption. Sections from PR #252, #253, and
#257-#263 were verified NOT ancestors of v0.6.0 and correctly remain in
[Unreleased].

Also resolves the "PR #TBD" placeholders left by those sections (now #248,
#257, #261) now that the real PR numbers are known.

Section-count invariant holds (306 before, 306 after); content unchanged
except for the PR-number substitutions.

author: Tin Dang

Co-authored-by: Tin Dang <tindang.ht97@gmail.com>
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.

2 participants