Skip to content

Commit a5dfec6

Browse files
authored
fix(segment): propagate forward-store writer open failure instead of crashing (#579)
Co-authored-by: mrcs64 <9069178+mrcs64@users.noreply.github.com>
1 parent ec8a78e commit a5dfec6

3 files changed

Lines changed: 35 additions & 0 deletions

File tree

src/db/index/segment/segment.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4117,6 +4117,17 @@ VectorColumnIndexer::Ptr SegmentImpl::create_vector_indexer(
41174117
}
41184118

41194119
Status SegmentImpl::init_memory_components() {
4120+
// Roll back any partially-created components on failure so a failed init
4121+
// leaves memory_store_ null (the caller's `if (!memory_store_)` retry guard
4122+
// depends on it) and never gets flushed on close.
4123+
bool committed = false;
4124+
AILEGO_DEFER([&]() {
4125+
if (!committed) {
4126+
memory_store_.reset();
4127+
memory_vector_indexers_.clear();
4128+
quant_memory_vector_indexers_.clear();
4129+
}
4130+
});
41204131
// init memory block id
41214132
auto &mem_block = segment_meta_->writing_forward_block().value();
41224133

@@ -4187,6 +4198,7 @@ Status SegmentImpl::init_memory_components() {
41874198
}
41884199
}
41894200

4201+
committed = true;
41904202
return Status::OK();
41914203
}
41924204

src/db/index/storage/memory_forward_store.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ Status MemForwardStore::Open() {
5454
physic_schema_ = arrow::schema(fields);
5555
// Initialize file writer
5656
writer_ = ChunkedFileWriter::Open(path_, physic_schema_, format_);
57+
if (!writer_) {
58+
return Status::InternalError("failed to open forward store writer at [",
59+
path_, "]");
60+
}
5761
return Status::OK();
5862
}
5963

@@ -216,6 +220,11 @@ Status MemForwardStore::flush() {
216220
return Status::OK();
217221
}
218222

223+
if (!writer_) {
224+
return Status::InternalError(
225+
"forward store writer not open, cannot flush [", path_, "]");
226+
}
227+
219228
auto result = convertToRecordBatch();
220229
if (!result.ok()) {
221230
return Status::InternalError("failed to convert cache to RecordBatch: ",

tests/db/index/storage/mem_store_test.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,3 +1156,17 @@ TEST_F(MemStoreTest, General) {
11561156
}
11571157

11581158
#endif
1159+
1160+
// Regression: MemForwardStore::Open() used to ignore a failed
1161+
// ChunkedFileWriter::Open (null writer_) and return OK, which then crashed on
1162+
// flush(). Open() must report the failure instead.
1163+
TEST(MemStoreOpenTest, OpenReportsErrorWhenWriterCreationFails) {
1164+
auto schema = GetCollectionSchema();
1165+
// Parent directory does not exist -> arrow FileOutputStream::Open fails ->
1166+
// ChunkedFileWriter::Open returns nullptr.
1167+
auto store = std::make_shared<MemForwardStore>(
1168+
schema, "/nonexistent_zvec_dir_xyz/scalar.block.0", FileFormat::IPC);
1169+
auto status = store->Open();
1170+
EXPECT_FALSE(status.ok());
1171+
EXPECT_EQ(store->writer_, nullptr);
1172+
}

0 commit comments

Comments
 (0)