Skip to content

[Bug]: File descriptor exhaustion during segment rollover duplicates documents and leaves the collection unwritable #811

Open

Description

@jak-pan

Description

When creating the next writing segment fails, the old segment can remain both in the segment manager and in writing_segment_. stats() and query() then count its documents twice. Later inserts fail, and the broken state can survive a close and reopen. Some documents whose insert returned an error become visible through a forward scan after reopening.

switch_to_new_segment_for_writing() dumps the old segment and adds it to the segment manager before Segment::CreateAndOpen() creates its replacement. A replacement-creation failure returns without undoing either step. get_all_segments_unsafe() lists both owners.

zvec/src/db/collection.cc

Lines 1782 to 1836 in 8944774

Status CollectionImpl::switch_to_new_segment_for_writing(
const CollectionSchema::Ptr &schema) {
if (writing_segment_->doc_count() == 0) {
return writing_segment_->flush();
}
// Allocate first, so that running out of ids changes nothing.
SegmentID new_segment_id = 0;
auto s = allocate_segment_id(&new_segment_id);
CHECK_RETURN_STATUS(s);
s = writing_segment_->dump();
CHECK_RETURN_STATUS(s);
s = segment_manager_->add_segment(writing_segment_);
CHECK_RETURN_STATUS(s);
// when create new segment, segment meta should create a first new block
// meta
auto new_segment = Segment::CreateAndOpen(
path_, schema == nullptr ? *schema_ : *schema, new_segment_id,
writing_segment_->meta()->max_doc_id() + 1, id_map_, delete_store_,
version_manager_,
SegmentOptions{false, options_.enable_mmap_, options_.max_buffer_size_});
if (!new_segment) {
return new_segment.error();
}
Version version = version_manager_->get_current_version();
auto writing_segment_meta = writing_segment_->meta();
writing_segment_->remove_writing_forward_block();
s = version.add_persisted_segment_meta(writing_segment_meta);
CHECK_RETURN_STATUS(s);
writing_segment_ = new_segment.value();
version.reset_writing_segment_meta(writing_segment_->meta());
version.set_next_segment_id(segment_id_allocator_.load());
s = version_manager_->apply(version);
CHECK_RETURN_STATUS(s);
s = version_manager_->flush();
CHECK_RETURN_STATUS(s);
return Status::OK();
}
Result<WriteResults> CollectionImpl::delete_(
const std::vector<std::string> &pks) {
CHECK_READONLY_RETURN_STATUS_EXPECTED();
std::shared_lock lock(schema_handle_mtx_);
CHECK_DESTROY_RETURN_STATUS_EXPECTED(destroyed_, false);
CHECK_CLOSED_RETURN_STATUS_EXPECTED(closed_, false);

Steps to Reproduce

Reproduced on unmodified 98aa126, macOS arm64, through the C++ API with default collection options:

  1. Create a collection with an inverted-indexed string field cls and a plain integer field n. Insert 1,000 documents with cls = 'a' and n = 1, then close it.
  2. Open it and lower the process's soft RLIMIT_NOFILE to the number of open descriptors plus a small allowance. The verifier swept allowances from 4 to 30 in fresh processes.
  3. Call optimize(). Opening the new segment's scalar index fails with EMFILE ("Too many open files"). Restore the descriptor limit.
  4. Compare stats().doc_count with the row count and distinct primary keys from query(cls = 'a'). Try another insert and another optimize(), then close and reopen.
  5. Query both cls = 'a' and the plain field n. Try more inserts and query their plain-field values.

Logs / Stack Trace

At allowance 4 on 98aa126:

optimize: Failed to open scalar indexer
stats().doc_count: 2000
query(cls = 'a'): 2000 rows, 1000 distinct primary keys
insert(x0): failed, empty error message
second optimize: Segment has been dumped.
close: OK
reopen: doc_count=1001, query(cls = 'a') returns 1000 rows
query(n = 7): returns x0, whose insert failed
insert(y0), insert(y1), insert(y2): failed, empty error messages
query(n = 101): returns y1, whose insert failed

Expected: a failed replacement creation leaves each acknowledged document visible once, does not publish a rejected insert, and permits writes and a rollover retry after resources are available. The same consistency must hold after reopening.

Operating System

macOS 27.0.1, arm64. Linux and Windows were not run.

Build & Runtime Environment

C++ API, Apple clang, CMake Release. The independent reproduction used unmodified 98aa126. The fix tests used the cached 98aa126 source with all six #790 files and the rollover addition checked byte-for-byte against the PR branch. Current main was not built because its unrelated RaBitQ 0.3.8 dependency was unavailable locally.

Additional Context

The fix creates the replacement before dumping the old segment, and adds the old segment to the segment manager only when the replacement is ready. It retires failed replacement creations through the existing segment lifecycle, leaves pre-existing conflicting directories untouched, and adds no on-disk format, public API or persistent structure.

A test-only RLIMIT_NOFILE regression covers rollover from a full-segment insert, optimize(), and create_index(). It checks two successive failed creations, cleanup of their unused directories, indexed queries, an unconditional forward scan after index creation, unique primary keys, retry, writes, and reopening. A seventh case checks the results of a batch whose rollover fails after a successful prefix. The updated regression fails all seven cases on 8944774 and passes all seven with the finished fix. The final targeted run passed 293 cases, including the seven regressions; one exhaustive manifest-truncation case timed out at 60 seconds, as it also did on unchanged 8944774. No timeout is counted as a pass. Linux and Windows were not run.

The fix is part of #790.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

  • Status
    In progress

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions