Skip to content

[Bug]: After a crash during create_index on a scalar field, every retry fails with "Directory exists" #801

Open

Description

@jak-pan

Description

Building a scalar (inverted) index takes a new block id and copies the segment's scalar index RocksDB into scalar.index.<block id>.rocksdb with a RocksDB checkpoint:

auto block_id = allocate_block_id();
std::string new_invert_index_path =
FileHelper::MakeInvertIndexPath(path_, id(), block_id);
Status s;
InvertedIndexer::Ptr new_scalar_indexer{nullptr};
if (invert_indexers_) {
s = invert_indexers_->create_snapshot(new_invert_index_path);
CHECK_RETURN_STATUS(s);

The new block becomes visible only when the collection manifest that names it is published. If the process dies after the checkpoint directory exists and before that publish, the directory stays behind. On the next open, the block id allocator restarts from the blocks in the manifest:

// init doc_id_allocator and block_id_allocator
doc_id_allocator_ = writing_block.value().min_doc_id();
BlockID max_block_id{writing_block.value().id()};
for (auto &block : segment_meta_->persisted_blocks()) {
max_block_id = std::max(max_block_id, block.id());
}
block_id_allocator_ = max_block_id + 1;
// recover writing block
s = recover();
CHECK_RETURN_STATUS(s);
} else {
// Update block_id_allocator_
BlockID max_block_id{0};
auto &persist_blocks = segment_meta_->persisted_blocks();
for (const auto &block : persist_blocks) {
max_block_id = std::max(max_block_id, block.id());
}
block_id_allocator_.store(max_block_id + 1);

So the retried create_index takes the same block id again, and RocksDB refuses to write a checkpoint into the existing directory:

[ERROR rocksdb_context.cc] Failed to create a checkpoint of Rocksdb[<collection>/0/scalar.index.1.rocksdb], code[4], reason[Invalid argument: Directory exists]

create_index fails at every retry. The collection still opens and serves reads, but the index can never be built, and the directory is never removed. The vector index path already handles this case for its index file ("possible crash residue; cleaning and overwriting"):

std::string index_file_path = FileHelper::MakeVectorIndexPath(
path_, column, segment_meta_->id(), block_id);
if (FileHelper::FileExists(index_file_path)) {
LOG_WARN(
"Index file[%s] already exists (possible crash residue); cleaning "
"and overwriting.",
index_file_path.c_str());
FileHelper::RemoveFile(index_file_path);
}

Steps to Reproduce

On 98aa126 (no source change; main at 7f54d94 does not change the files involved), macOS arm64:

  1. Create a collection with max_doc_count_per_segment = 1000, three inverted-indexed fields and one plain INT32 field plain. Insert 1,001 documents (one sealed segment and a writing segment with one document) and close it.
  2. In a child process, open it and call create_index("plain", std::make_shared<InvertIndexParams>()). Kill the child with SIGKILL before its n-th file-system mutation below the collection directory, for every n.
  3. After each kill, open the collection for writing; if plain is not indexed, call create_index again.

We kill the child with a small preloaded library (DYLD_INSERT_LIBRARIES / LD_PRELOAD) that counts open-for-write, write, ftruncate, rename, unlink, mkdir, rmdir and fsync calls; it needs no hook in zvec. Of 664 kill points, the retry fails with "Directory exists" at 411: every point from 251 to 661. Point 250 is the RocksDB checkpoint's rename of 0/scalar.index.2.rocksdb.tmp to 0/scalar.index.2.rocksdb.

The same sweep also finds 54 points where the collection cannot be opened at all, from a separate seal defect (reported separately in #798). With that seal fix and a column-family creation change (#799) applied together, the index build makes 445 mutations, and the retry still fails with "Directory exists" at 309 of them (points 134 to 442).

Logs / Stack Trace

See above. create_index returns a non-OK Status with an empty message.

Operating System

macOS 27 (arm64). The code path is platform independent.

Build & Runtime Environment

Apple clang, CMake Release build of 98aa126.

Additional Context

Two possible fixes, either enough for this case:

  1. On a writable open, after the segments are opened, remove scalar.index.<id>.rocksdb and fts.<id>.rocksdb directories (and their .tmp checkpoint names) that the manifest does not reference. This extends the orphan-directory cleanup of fix(db): remove orphaned segment directories during crash recovery #674 from segment directories to index directories, under the same exclusive lock.
  2. Before the checkpoint, remove an existing directory at the new block's path, as the vector index path does for its file.

Earlier leftovers of the same kind were treated as bugs: #246 removes stale vector index files left by a crashed optimize, #674 removes orphaned segment directories that blocked id reuse, and #485 (for #427) removes the old scalar index directory on drop_index so create_index can be repeated. Only directories that the published manifest does not name would be removed, and zvec opens scalar and FTS indexes only from the blocks the manifest names, so no index zvec reads is affected.

I can open a PR with the first approach and the crash test if it is welcome.

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

  • Status
    In progress

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions