From 13c4c8e0c7a5801871b95ebf16ff926006009a05 Mon Sep 17 00:00:00 2001 From: Qinren Zhou Date: Tue, 14 Jul 2026 17:58:23 +0800 Subject: [PATCH 1/4] fix: backport RocksDB filter alias cleanup --- thirdparty/rocksdb/CMakeLists.txt | 10 ++ .../rocksdb.filter_alias_cleanup.patch | 117 ++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 thirdparty/rocksdb/rocksdb.filter_alias_cleanup.patch diff --git a/thirdparty/rocksdb/CMakeLists.txt b/thirdparty/rocksdb/CMakeLists.txt index 8d95e882f..0e684a926 100644 --- a/thirdparty/rocksdb/CMakeLists.txt +++ b/thirdparty/rocksdb/CMakeLists.txt @@ -1,4 +1,14 @@ set(ROCKSDB_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR}/rocksdb-8.1.1) + +# Remove RocksDB's obsolete Bloom filter alias lookup. Besides no longer being +# needed for zvec-generated SSTs, its function-local hash table is the observed +# crash site during highly concurrent collection opens. This backports +# upstream RocksDB commit 44b741e9ccc2a99334806c08e8b09c3ad8010f63. +set(ROCKSDB_FILTER_ALIAS_PATCH + ${CMAKE_CURRENT_SOURCE_DIR}/rocksdb.filter_alias_cleanup.patch) +apply_patch_once("rocksdb_filter_alias_cleanup" "${ROCKSDB_SRC_DIR}" + "${ROCKSDB_FILTER_ALIAS_PATCH}") + if (ANDROID) set(ROCKSDB_ANDROID_PATCH ${CMAKE_CURRENT_SOURCE_DIR}/rocksdb.android.patch) apply_patch_once("rocksdb_android_fix" "${ROCKSDB_SRC_DIR}" "${ROCKSDB_ANDROID_PATCH}") diff --git a/thirdparty/rocksdb/rocksdb.filter_alias_cleanup.patch b/thirdparty/rocksdb/rocksdb.filter_alias_cleanup.patch new file mode 100644 index 000000000..54fa4e5cb --- /dev/null +++ b/thirdparty/rocksdb/rocksdb.filter_alias_cleanup.patch @@ -0,0 +1,117 @@ +diff --git a/table/block_based/block_based_table_reader.cc b/table/block_based/block_based_table_reader.cc +index 0592b1a..fcfdb4a 100644 +--- a/table/block_based/block_based_table_reader.cc ++++ b/table/block_based/block_based_table_reader.cc +@@ -15,6 +15,5 @@ + #include + #include + #include +-#include + #include + #include +@@ -980,71 +979,24 @@ Status BlockBasedTable::PrefetchIndexAndFilterBlocks( + // Find filter handle and filter type + if (rep_->filter_policy) { + auto name = rep_->filter_policy->CompatibilityName(); +- bool builtin_compatible = +- strcmp(name, BuiltinFilterPolicy::kCompatibilityName()) == 0; +- + for (const auto& [filter_type, prefix] : + {std::make_pair(Rep::FilterType::kFullFilter, kFullFilterBlockPrefix), + std::make_pair(Rep::FilterType::kPartitionedFilter, + kPartitionedFilterBlockPrefix), + std::make_pair(Rep::FilterType::kNoFilter, + kObsoleteFilterBlockPrefix)}) { +- if (builtin_compatible) { +- // This code is only here to deal with a hiccup in early 7.0.x where +- // there was an unintentional name change in the SST files metadata. +- // It should be OK to remove this in the future (late 2022) and just +- // have the 'else' code. +- // NOTE: the test:: names below are likely not needed but included +- // out of caution +- static const std::unordered_set kBuiltinNameAndAliases = { +- BuiltinFilterPolicy::kCompatibilityName(), +- test::LegacyBloomFilterPolicy::kClassName(), +- test::FastLocalBloomFilterPolicy::kClassName(), +- test::Standard128RibbonFilterPolicy::kClassName(), +- "rocksdb.internal.DeprecatedBlockBasedBloomFilter", +- BloomFilterPolicy::kClassName(), +- RibbonFilterPolicy::kClassName(), +- }; +- +- // For efficiency, do a prefix seek and see if the first match is +- // good. +- meta_iter->Seek(prefix); +- if (meta_iter->status().ok() && meta_iter->Valid()) { +- Slice key = meta_iter->key(); +- if (key.starts_with(prefix)) { +- key.remove_prefix(prefix.size()); +- if (kBuiltinNameAndAliases.find(key.ToString()) != +- kBuiltinNameAndAliases.end()) { +- Slice v = meta_iter->value(); +- Status s = rep_->filter_handle.DecodeFrom(&v); +- if (s.ok()) { +- rep_->filter_type = filter_type; +- if (filter_type == Rep::FilterType::kNoFilter) { +- ROCKS_LOG_WARN(rep_->ioptions.logger, +- "Detected obsolete filter type in %s. Read " +- "performance might suffer until DB is fully " +- "re-compacted.", +- rep_->file->file_name().c_str()); +- } +- break; +- } +- } +- } +- } +- } else { +- std::string filter_block_key = prefix + name; +- if (FindMetaBlock(meta_iter, filter_block_key, &rep_->filter_handle) +- .ok()) { +- rep_->filter_type = filter_type; +- if (filter_type == Rep::FilterType::kNoFilter) { +- ROCKS_LOG_WARN( +- rep_->ioptions.logger, +- "Detected obsolete filter type in %s. Read performance might " +- "suffer until DB is fully re-compacted.", +- rep_->file->file_name().c_str()); +- } +- break; ++ std::string filter_block_key = prefix + name; ++ if (FindMetaBlock(meta_iter, filter_block_key, &rep_->filter_handle) ++ .ok()) { ++ rep_->filter_type = filter_type; ++ if (filter_type == Rep::FilterType::kNoFilter) { ++ ROCKS_LOG_WARN( ++ rep_->ioptions.logger, ++ "Detected obsolete filter type in %s. Read performance might " ++ "suffer until DB is fully re-compacted.", ++ rep_->file->file_name().c_str()); + } ++ break; + } + } + } +diff --git a/table/block_based/filter_policy.cc b/table/block_based/filter_policy.cc +index 44b003b..e65259f 100644 +--- a/table/block_based/filter_policy.cc ++++ b/table/block_based/filter_policy.cc +@@ -1317,7 +1317,3 @@ + static const char* kBuiltinFilterMetadataName = "rocksdb.BuiltinBloomFilter"; +- +-const char* BuiltinFilterPolicy::kCompatibilityName() { +- return kBuiltinFilterMetadataName; +-} +- ++ + const char* BuiltinFilterPolicy::CompatibilityName() const { +diff --git a/table/block_based/filter_policy_internal.h b/table/block_based/filter_policy_internal.h +index 85ec39c..f8fcc56 100644 +--- a/table/block_based/filter_policy_internal.h ++++ b/table/block_based/filter_policy_internal.h +@@ -139,4 +139,3 @@ class BuiltinFilterPolicy : public FilterPolicy { + const char* CompatibilityName() const override; +- static const char* kCompatibilityName(); +- ++ + public: // new From 3f2dc9d6934d501bbbc511c0492e740db5c940fb Mon Sep 17 00:00:00 2001 From: Qinren Zhou Date: Wed, 15 Jul 2026 10:03:09 +0800 Subject: [PATCH 2/4] fix: always close RocksDB after flush failure --- src/db/common/rocksdb_context.cc | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/db/common/rocksdb_context.cc b/src/db/common/rocksdb_context.cc index 24176e876..78e036b9f 100644 --- a/src/db/common/rocksdb_context.cc +++ b/src/db/common/rocksdb_context.cc @@ -310,15 +310,16 @@ Status RocksdbContext::close() { std::lock_guard lock(mutex_); if (db_ == nullptr) { - LOG_ERROR("RocksDB[%s] is not opened", db_path_.c_str()); - return Status::InternalError(); + return Status::OK(); } + Status result = Status::OK(); if (!read_only_) { if (auto s = flush_unlocked(); !s.ok()) { - LOG_ERROR("Failed to close RocksDB[%s] due to flush failure", + LOG_ERROR("Failed to flush RocksDB[%s] while closing; continuing with " + "resource cleanup", db_path_.c_str()); - return s; + result = s; } } @@ -326,13 +327,19 @@ Status RocksdbContext::close() { if (auto s = db_->Close(); s.ok()) { LOG_DEBUG("Closed RocksDB[%s]", db_path_.c_str()); - db_.reset(); - return Status::OK(); } else { LOG_ERROR("Failed to close RocksDB[%s], code[%d], reason[%s]", db_path_.c_str(), s.code(), s.ToString().c_str()); - return Status::InternalError(); + if (result.ok()) { + result = Status::InternalError(); + } } + + // Release the DB even when Flush() or Close() reports an error. DBImpl's + // destructor provides the final synchronous shutdown fallback, preventing + // background work from surviving until process-wide static destruction. + db_.reset(); + return result; } @@ -577,4 +584,4 @@ size_t RocksdbContext::count() { return 0; } } -} // namespace zvec \ No newline at end of file +} // namespace zvec From 6adf2b11698bc3bf0ab11cbef9c0eebab0ac0e56 Mon Sep 17 00:00:00 2001 From: Qinren Zhou Date: Wed, 15 Jul 2026 10:14:07 +0800 Subject: [PATCH 3/4] style: format RocksDB close logging --- src/db/common/rocksdb_context.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/db/common/rocksdb_context.cc b/src/db/common/rocksdb_context.cc index 78e036b9f..22148e407 100644 --- a/src/db/common/rocksdb_context.cc +++ b/src/db/common/rocksdb_context.cc @@ -316,9 +316,10 @@ Status RocksdbContext::close() { Status result = Status::OK(); if (!read_only_) { if (auto s = flush_unlocked(); !s.ok()) { - LOG_ERROR("Failed to flush RocksDB[%s] while closing; continuing with " - "resource cleanup", - db_path_.c_str()); + LOG_ERROR( + "Failed to flush RocksDB[%s] while closing; continuing with " + "resource cleanup", + db_path_.c_str()); result = s; } } From 47b4d0258f2957e7b22e452d82caa6ab6139fee7 Mon Sep 17 00:00:00 2001 From: Qinren Zhou Date: Wed, 15 Jul 2026 10:28:12 +0800 Subject: [PATCH 4/4] fix: mark IDMap closed after shutdown --- src/db/index/common/id_map.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/db/index/common/id_map.cc b/src/db/index/common/id_map.cc index e1e03cf3e..797a5c335 100644 --- a/src/db/index/common/id_map.cc +++ b/src/db/index/common/id_map.cc @@ -70,6 +70,7 @@ Status IDMap::close() { } Status status = rocksdb_context_.close(); + opened_ = false; if (status.ok()) { LOG_INFO("Closed IDMap[%s]", working_dir_.c_str()); } else {