fix(db): SIGSEGV when fetch/query race a segment switch; unblock reader concurrency by YongqiYin · Pull Request #715 · alibaba/zvec

YongqiYin · 2026-08-31T12:45:52Z

Closes: #714

What

Both read entry points crash with SIGSEGV on Linux when a writer crosses a
segment switch (details in #714). fetch() faults inside the in-memory table
rebuild; query() faults on the same store, reached through the planner's
fan-out (VectorRecallNode::collect_batch → SegmentImpl::fetch → fetch_normal → convertToTable). Two races and two read-path changes:

Segment list — get_all_segments() takes write_mtx_ shared
(collection.cc), as stats() already does; _unsafe variant added for
callers already holding the lock. Probe on main: 99.7% of doc_ids_
push_backs executed while a reader was inside this function.
Segment teardown — finish_memory_components() /
init_memory_components() take seg_col_mtx_ exclusively, and the two
combined-vector-indexer accessors take it shared; dump() takes seg_mtx_
exclusively. Readers used to reach memory_store_ / persist_stores_ /
the indexer maps while flush() was republishing them under a different
lock, so a reader landing between memory_store_->close() and
persist_stores_.push_back() found neither and dropped that block's rows
with no error and no log. Locking the rebuild covers all three flush()
entry points (buffer-full insert, segment switch, close).
Point-read fast path — MemForwardStore::convertToTable() materializes
only the batch holding the requested rows instead of rebuilding the whole
in-memory store: O(store) → O(1), and it no longer walks the stack the
crash faults in.
Reader locks — seg_mtx_ and cache_mtx_ become shared_mutex;
read-only accessors take them shared (verified member by member).
close() now takes cache_mtx_ exclusively, with flush_locked()
extracted so it does not lock twice.

Also from review: the three convertToTable helpers moved to snake_case, and
tests/db/concurrent_fetch_test.cc was renamed concurrent_read_test.cc now
that it covers both read paths.

Test plan

tests/db/concurrent_read_test.cc, one writer crossing frequent segment
switches throughout:

fetch: 4/8 readers comparing every field of every fetched doc against
the deterministic generator output. 3/3 SIGSEGV on unfixed main, passes
with the fix (2.4M field comparisons on Linux, 1.6M on macOS, 0 errors).
query: 4 queriers running KNN with an output field, asserting no errors,
no empty results, no missing field values, and that each value matches the
pk it came back with. 3/3 SIGSEGV on unfixed main, passes with the fix
(19.5k queries on Linux, 16.5k on macOS, 0 errors).

Two settings are load-bearing: max_doc_count_per_segment = 4,000
(schema-validated ≥1,000) so switches happen inside the run window, and the
writer is capped at 50k docs and asserted to be alive (writer_errors plus
a minimum docs_written) — otherwise a starved writer silently turns the run
into a no-writer one and the test passes without exercising the crash path.

Full suites: 222 tests on macOS arm64, 224 on Linux x86_64.

Measured impact (64-core Linux; same-binary repeat variance <4%)

metric	before	after
point-read latency (500→4,000 docs in the in-memory store)	472→1,982µs, grows with doc count (O(N))	flat ~180µs at any doc count (O(1))
fetch throughput 1→8 threads, no writer	flat (no reader-reader parallelism)	5.8x scaling (5.6k→32k reads/s)
fetch throughput 1→8 threads, with writer	crashes	6.4x scaling (8.7k→56k reads/s)
query throughput 1→8 threads, no writer	1.15x (597→685 q/s) — materialization serialized under the exclusive lock	3.4x (600→2,038 q/s)
writer throughput as readers 0→8	starves (290 docs/s, macOS) / crashes (Linux)	flat (−2.4%)

Known trade-offs

Reads now wait for the in-flight write batch. Before this PR, readers
fetched the segment list with no lock at all — that unlocked read is exactly
the race being fixed. After it, get_all_segments() takes write_mtx_
shared, so a reader waits for the current write_impl batch. This is the
only newly-blocked path at the collection layer: create_index /
add_column already blocked readers for the whole DDL task (they hold the
schema lock exclusively), and per-insert exclusion already existed at the
segment layer.
Reads now wait for a segment teardown or rebuild. Item 2 blocks readers
for the duration of dump() (avg 16ms, max 24ms, 26 occurrences over an
8s window in the regression test) and of a memory-component rebuild.
Previously they were not blocked — they read state that was being torn down.
Measured cost is inside the ±1% run-to-run noise on every workload above.
Writers can be passed by concurrent readers. With std::shared_mutex, a
writer waiting for a segment lock must wait out the readers currently inside
it. The wait stays bounded: new readers cannot arrive (the writer holds the
collection's exclusive write_mtx_) and each in-flight read is now an O(1)
lookup. Writer throughput stays flat as readers go 0→8 (−2.4%).

Fix a real data race: readers calling get_all_segments() (query / fetch / group_by_query / delete_by_filter / stats / prepare_iterate) read the writing_segment_ shared_ptr and doc_ids_ with no lock held in common with writers, which reassign writing_segment_ and push_back into doc_ids_ under exclusive write_mtx_. On Linux glibc this crashes under concurrent read+write load (7/7 runs: reader faults inside arrow::Table::FromRecord- Batches while the writer tears the same segment down via dump()); on macOS the same race silently corrupts results (probe hit rate 98.5%). Fix: take write_mtx_ shared inside get_all_segments() and add get_all_segments_unsafe() for callers that already hold it. Also remove the bottlenecks this exposed on the read path, so the fix does not trade correctness for throughput: - SegmentImpl::seg_mtx_ -> shared_mutex; Fetch()/get_global_doc_id() take it shared (read-only accessors) - MemForwardStore::cache_mtx_ -> shared_mutex; the 4 read-only accessors take it shared - MemForwardStore::convertToTable(): single-source fast path; a point fetch no longer rebuilds the whole in-memory store into an Arrow table (O(N) -> O(1), 500us -> 69us with 4k docs in buffer) Verification: 218 gtests green; 1.4M (macOS) + 2.1M (Linux glibc) field-by-field content-verified concurrent fetches, 0 errors; reader scaling 5.8-6.4x (1->8 readers, 64-core Linux); baseline crashes 7/7 vs fixed 3/3 green under the same load.

Guards the segment-list race fix and the newly-shared reader locks: one writer thread inserts (crossing frequent segment switches) while N reader threads fetch preloaded docs and compare every returned doc field by field against the deterministic expectation. Two load shapes are covered: with a writer the sealed segments serve reads from the persisted (mmap) path; without one every doc stays in MemForwardStore::cache_. max_doc_count_per_segment is deliberately small (4k): on the unfixed baseline reader contention starves the writer to ~2.5k docs/s, so a 40k threshold would produce no segment switch at all inside the run window - and the switch (dump(), which takes no seg_mtx_) is exactly where the baseline crashes. Verified on 64-core Linux glibc: crashes 3/3 on the pre-fix baseline (SIGSEGV while the writer switches segments); passes with the fix (774k field-by-field verified fetches, 0 errors). Also passes on macOS arm64 (489k fetches, 0 errors).

Copilot

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

egolearner · 2026-09-02T07:11:25Z

+  // persist_stores_, the vector indexer maps and the block metadata. Read-only
+  // accessors (Fetch, get_global_doc_id) take it shared. NOT covered: the
+  // segment-switch dump()/flush(), which rewrites the same state without
+  // seg_mtx_ and is serialized only by the collection's write_mtx_.


Collection中get_segments时虽然加了write_mtx_读锁，但是得到segment列表后就释放了锁。后续在调用Segment::fetch时，有可能其他地方调用了SegmentImpl::flush或dump，修改了memory_store_导致有并发问题。

已修，重写 memory_store_ / persist_stores_ 的finish_memory_components()只被 flush() 调用，现在取排他 seg_col_mtx_，所以 flush 的三条触发路径（internal_insert 的 buffer-full、dump、close）都被覆盖；dump() 另外取排他 seg_mtx_

egolearner · 2026-09-02T07:13:16Z


 // Notice: This function just convert the docs to arrow::ArrayBuilder, not clean
 // the cache_.
+arrow::Status MemForwardStore::appendDocToBuilder(


新增代码的函数使用snake_case风格吧，存量代码的风格问题可以后续再另行修复

egolearner · 2026-09-02T07:33:00Z

+  // accessors (Fetch, get_global_doc_id) take it shared. NOT covered: the
+  // segment-switch dump()/flush(), which rewrites the same state without
+  // seg_mtx_ and is serialized only by the collection's write_mtx_.
+  mutable std::shared_mutex seg_mtx_;


seg_mtx_ 和 seg_col_mtx_ 保护的状态存在重叠，包括 memory_store_、persist_stores_ 和 block metadata。使用两把锁容易导致 flush() 和 scan() 在不同锁下访问同一状态。建议合并为一把 shared_mutex：读操作持共享锁，写入、flush、dump 和 DDL 持独占锁。同时需要引入 _unsafe 或内部无锁 helper，避免 Fetch() -> fetch() 这类调用链重复获取不可重入的 mutex。

finish_memory_components() and init_memory_components() rewrite memory_store_, persist_stores_ and the vector indexer maps under the exclusive seg_mtx_, but the query read path reaches the same state under a different lock: fetch_normal()/scan() hold seg_col_mtx_ shared, and get_combined_vector_indexer() held no lock at all. Between memory_store_->close() and persist_stores_.push_back() neither branch in fetch_normal applies, so it skips the block and the query loses those rows -- no crash, no error, no log. With the window widened to 100ms, 12 of 16 queries dropped rows (88 total). Take seg_col_mtx_ exclusively in both rebuild paths and shared in the two combined-indexer accessors. The order stays seg_mtx_ -> seg_col_mtx_: flush() has three call sites (close, internal_insert, dump) and none of them holds seg_col_mtx_. After the fix, 0 rows dropped under the same conditions. From review, also in this commit: - dump() takes the exclusive seg_mtx_, covering Fetch(doc) and get_global_doc_id() against the same teardown. - MemForwardStore::close() takes the exclusive cache_mtx_, with flush_locked() split out so close() does not lock it twice. - The three convertToTable helpers renamed to snake_case. - New QuerySucceedsUnderConcurrentWrites test, asserting field completeness: the usual "no error / non-empty" checks stayed green through all 12 dropped-row events. Throughput stays within the +-1% noise floor (fetch -0.95%/+0.57%, query -0.53%, writer flat). Regression: 222 tests on macOS, 224 on Linux.

…ce-read-scaling

The file now covers both read entry points -- fetch() under seg_mtx_ and query() fanning out under seg_col_mtx_ -- so "fetch" in the name, the fixture and the header comment described only half of it. While renaming, two things in the query case were off: - It only checked that the requested field was present. Every field is derived from the doc id and the pk is "pk_<id>", so the value can be checked against the pk it came back with; that also catches rows stitched together across blocks, not just dropped ones. - Errors matching the known Insert/Query transient were skipped without a trace. They are now counted and printed separately, which shows the transient still fires on macOS (4 in 16k queries) but not on Linux.

YongqiYin added 3 commits August 31, 2026 12:53

docs(db): tighten lock-contract comments

928fdae

Copilot AI lite review requested due to automatic review settings August 31, 2026 12:45

YongqiYin requested a review from zhourrr as a code owner August 31, 2026 12:45

Copilot AI reviewed Aug 31, 2026

github-actions Bot assigned YongqiYin Aug 31, 2026

egolearner reviewed Sep 2, 2026

View reviewed changes

chinaux reviewed Sep 2, 2026

View reviewed changes

Comment thread src/db/index/storage/memory_forward_store.cc

chinaux reviewed Sep 2, 2026

View reviewed changes

Comment thread tests/db/concurrent_fetch_test.cc Outdated

YongqiYin added 3 commits September 4, 2026 14:06

Merge remote-tracking branch 'upstream/main' into fix/segment-list-ra…

a507f96

…ce-read-scaling

YongqiYin changed the title ~~fix(db): SIGSEGV under concurrent fetch + segment switch; unlock concurrent point reads~~ fix(db): SIGSEGV when fetch/query race a segment switch; unblock reader concurrency Sep 7, 2026

Navigation Menu

Uh oh!

fix(db): SIGSEGV when fetch/query race a segment switch; unblock reader concurrency - #715

fix(db): SIGSEGV when fetch/query race a segment switch; unblock reader concurrency#715
YongqiYin wants to merge 6 commits into
alibaba:mainfrom
YongqiYin:fix/segment-list-race-read-scaling

YongqiYin commented Aug 31, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

egolearner Sep 2, 2026

Uh oh!

YongqiYin Sep 7, 2026 •

edited

Loading

Uh oh!

egolearner Sep 2, 2026

Uh oh!

YongqiYin Sep 4, 2026

Uh oh!

egolearner Sep 2, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

YongqiYin commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Test plan

Measured impact (64-core Linux; same-binary repeat variance <4%)

Known trade-offs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

egolearner Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

YongqiYin Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

egolearner Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

YongqiYin Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

egolearner Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

YongqiYin commented Aug 31, 2026 •

edited

Loading

YongqiYin Sep 7, 2026 •

edited

Loading