Skip to content

fix(collection): allow writes and reads to proceed during Optimize - #614

Open
YongqiYin wants to merge 3 commits into
alibaba:mainfrom
YongqiYin:fix/optimize-blocking-writes
Open

fix(collection): allow writes and reads to proceed during Optimize#614
YongqiYin wants to merge 3 commits into
alibaba:mainfrom
YongqiYin:fix/optimize-blocking-writes

Conversation

@YongqiYin

Copy link
Copy Markdown
Collaborator

Optimize() held schema_handle_mtx_ exclusively for its whole duration, so Insert/Query/Fetch/Delete (which take it in shared mode) were blocked until the long-running compact finished.

  • Downgrade Optimize's schema lock to shared mode; writes and reads now proceed during the compact and only wait on the short write_mtx_ critical sections (flush and version commit). Schema operations and close/destroy still take the lock exclusively and remain mutually exclusive with Optimize.
  • Add optimize_mtx_ to keep concurrent Optimize calls serialized, preserving the previous queuing semantics.
  • Make SegmentManager internally thread-safe with a shared_mutex: readers (get_segments) can now run concurrently with the segment replacement in Optimize's commit phase.
  • Add a regression test that asserts inserts and fetches make progress while a background Optimize is running.

Fixes #553

Optimize() held schema_handle_mtx_ exclusively for its whole duration,
so Insert/Query/Fetch/Delete (which take it in shared mode) were
blocked until the long-running compact finished.

- Downgrade Optimize's schema lock to shared mode; writes and reads now
  proceed during the compact and only wait on the short write_mtx_
  critical sections (flush and version commit). Schema operations and
  close/destroy still take the lock exclusively and remain mutually
  exclusive with Optimize.
- Add optimize_mtx_ to keep concurrent Optimize calls serialized,
  preserving the previous queuing semantics.
- Make SegmentManager internally thread-safe with a shared_mutex:
  readers (get_segments) can now run concurrently with the segment
  replacement in Optimize's commit phase.
- Add a regression test that asserts inserts and fetches make progress
  while a background Optimize is running.

Fixes alibaba#553
@YongqiYin
YongqiYin requested a review from zhourrr as a code owner July 23, 2026 09:22
Copilot AI review requested due to automatic review settings July 23, 2026 09:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

egolearner
egolearner previously approved these changes Jul 24, 2026

@egolearner egolearner left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

基本ok,ut中验证optimize和写、fetch、query并发会更加完备

Copilot AI review requested due to automatic review settings July 24, 2026 09:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings July 24, 2026 09:25
@YongqiYin
YongqiYin force-pushed the fix/optimize-blocking-writes branch from a478c5c to 63d26e0 Compare July 24, 2026 09:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Strengthening the UT to run Insert, Fetch and Query concurrently with a
background Optimize (review feedback) exposed a use-after-free: the
commit phase called reload_vector_index() on a live segment, destroying
vector indexers and mutating block metas that concurrent readers were
still using (readers only hold the shared schema lock).

- Commit the new segment set by reopening segments from their new metas
  and swapping instances, never mutating live segments in place;
  in-flight readers keep replaced instances alive via shared_ptr.
- Add SegmentManager::replace_segments() to apply the whole swap under
  one lock (validate first, then mutate), so readers never observe a
  partially committed segment set (no transient duplicate or missing
  segments), and drop the now-unused destroy_segment().
- Extend the regression test: writer, fetcher and querier threads now
  run concurrently with Optimize; worker failures are collected and
  asserted on the main thread. Query errors caused by the pre-existing
  Insert/Query visibility race in the writing segment (reproducible
  without Optimize) are tolerated and documented; a quiescent full-topk
  query is asserted after all threads join.
With Optimize committing via segment reopen-and-swap, the pre-index
vector files of a replaced segment instance (e.g. the flat file that a
newly built HNSW index supersedes) were no longer deleted and lingered
on disk until the segment was compacted away.

Reclaim them with the same mark-and-defer model segments already use:

- VectorColumnIndexer: add MarkDestroyOnRelease(); a marked indexer
  closes and removes its index file in the destructor, i.e. only after
  the last reference is released, so in-flight readers of the replaced
  segment instance keep the file alive until they finish.
- Segment: add mark_vector_index_files_for_removal(). Base and
  quantized indexers are marked independently: when a quantized index
  is built on top of a reused base file (see create_vector_index), only
  the old quantized file is superseded and the base file is still
  referenced by the new segment meta, so it must not be marked.
- Optimize: after the new segment set is committed, mark the replaced
  instances' superseded indexers.
- Add a regression test asserting exactly one index file remains per
  indexed column after Optimize and that a fresh open reads all docs.
Copilot AI review requested due to automatic review settings July 27, 2026 12:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@YongqiYin

Copy link
Copy Markdown
Collaborator Author
  • Commit 2 — 并发完备 UT + commit 阶段修复
    • 按建议把测试扩展为 insert / fetch / query 线程与后台 Optimize 并发运行,暴露出一处真实 use-after-free。
    • 根因:commit 阶段在 live segment 上调用 reload_vector_index(),销毁 indexer 并改写并发读者仍在使用的 block metas。
    • 修复:改为 reopen-and-swap 式提交——依据新 meta 打开新 segment 实例并原子交换(SegmentManager::replace_segments,单锁内 validate-then-mutate)。
    • 效果:读者只看到完整旧集合或完整新集合,不会看到 merge 输入与合并产物的混合态。
    • 在途读者经 shared_ptr 让被替换实例及其文件继续存活。
  • Commit 3 — 回收被取代的索引文件
    • swap 后,被新索引取代的 pre-index flat 文件会滞留磁盘,现改用 segment 既有的 mark-and-defer 模型移除。
    • 被标记的 indexer 在析构函数里删除自身文件,即最后一个引用释放后才删。
    • base 与 quantized indexer 独立标记:基于复用 base 文件构建的 quantized index(见 create_vector_index)只取代旧 quantized 文件,不动 base。
    • 关于 VectorColumnIndexer::Destroy():其 eager close-and-delete 已不在任何并发路径上——Optimize 走 swap + 延迟标记;其余调用方(CreateIndex / DropIndex 触发的 reload_vector_index)在排他 schema 锁下运行,读者全部被排除。

@YongqiYin
YongqiYin requested a review from egolearner July 29, 2026 02:39
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.

[Bug]: optimize会阻塞insert吗?

3 participants