fix: backport RocksDB filter alias cleanup

src/db/common/rocksdb_context.cc

@@ -310,29 +310,37 @@ Status RocksdbContext::close() {
   std::lock_guard<std::mutex> 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",
-                db_path_.c_str());
-      return s;
+      LOG_ERROR(
+          "Failed to flush RocksDB[%s] while closing; continuing with "
+          "resource cleanup",
+          db_path_.c_str());
+      result = s;
     }
   }
 
   delete_cf_handles();
 
   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 +585,4 @@ size_t RocksdbContext::count() {
     return 0;
   }
 }
-}  // namespace zvec
+}  // namespace zvec

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 {

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}")

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 <limits>
+ #include <memory>
+ #include <string>
+-#include <unordered_set>
+ #include <utility>
+ #include <vector>
+@@ -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<std::string> 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