Reserve key_context capacity in MultiGetCommon to avoid autovector reallocs (#14909)

Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/14909

Reviewed By: kunalspathak

Differential Revision: D110380492

fbshipit-source-id: d1d3214c52a65326de237adcbfe6ea5d23d94960

HISTORY.md

@@ -1,6 +1,18 @@
 # Rocksdb Change Log
 > NOTE: Entries for next release do not go here. Follow instructions in `unreleased_history/README.txt`
 
+## 11.6.0 (07/02/2026)
+### New Features
+* Added EXPERIMENTAL embedded blob SST support through `SstFileWriter::OpenWithEmbeddedBlobs()`, storing eligible large values as same-file blob records in block-based SST files and resolving them transparently for reads. This niche feature currently supports uncompressed embedded blobs only; compression options are placeholders and compression support is deferred to follow-up work.
+
+### Public API Changes
+* Expanded the C API (`include/rocksdb/c.h`) with a large set of new `rocksdb_*` functions, mostly option getters/setters plus table-properties, job/event-listener, and metadata accessors, a WAL filter, a ReadOptions table filter, and a backup exclude-files callback. Many are now produced by a new semi-automated generator (`tools/c_api_gen/`) from the C++ headers; `include/rocksdb/c.h` remains a single self-contained header and the signatures of pre-existing functions are unchanged.
+
+### Bug Fixes
+* Reverted PR14831 that made range_lock_manager aware of reverse-order CF
+* Fixed a bug in `RandomAccessFileReader::ReadAsync` where an already-aligned direct-IO read request with a null `scratch` and a caller-provided `aligned_buf` would take the "already aligned" fast path and submit the null buffer to the underlying async read (e.g. a null iovec base to io_uring, failing with EFAULT). This could surface as spurious iterator failures during async prefetch (MultiScan with async IO) on direct-IO databases. The async path now allocates a backing buffer in this case, matching the synchronous `Read` path.
+* Fixed a bug where closing a read-only DB instance could delete live SST files created by a concurrent read-write DB sharing the same directory.
+
 ## 11.5.0 (06/16/2026)
 ### New Features
 * External table readers that open files through `ExternalTableOptions::fs` now update RocksDB SST/file-read statistics and file IO listener callbacks, making external file IO activity visible in existing read metrics.

db/db_impl/db_impl.cc

@@ -3761,6 +3761,7 @@ void DBImpl::MultiGetCommon(const ReadOptions& read_options,
   }
 
   autovector<KeyContext, MultiGetContext::MAX_BATCH_SIZE> key_context;
+  key_context.reserve(num_keys);
   autovector<KeyContext*, MultiGetContext::MAX_BATCH_SIZE> sorted_keys;
   sorted_keys.resize(num_keys);
   for (size_t i = 0; i < num_keys; ++i) {
@@ -3951,6 +3952,7 @@ void DBImpl::MultiGetCommon(const ReadOptions& read_options,
     }
   }
   autovector<KeyContext, MultiGetContext::MAX_BATCH_SIZE> key_context;
+  key_context.reserve(num_keys);
   autovector<KeyContext*, MultiGetContext::MAX_BATCH_SIZE> sorted_keys;
   sorted_keys.resize(num_keys);
   for (size_t i = 0; i < num_keys; ++i) {

db/version_builder.cc

@@ -439,6 +439,15 @@ class VersionBuilder::Rep {
         f->fd.pinned_reader.Release(table_cache_->get_cache().get());
       }
 
+      // Evict from table cache to prevent leaked entries for files whose
+      // last reference is held by the VersionBuilder (e.g., files from a
+      // failed LogAndApply that were never installed in any Version).
+      // Release() above only releases the pinned handle's ref but does not
+      // remove the entry from the cache hash table.
+      if (table_cache_ != nullptr) {
+        TableCache::Evict(table_cache_->get_cache().get(), f->fd.GetNumber());
+      }
+
       if (file_metadata_cache_res_mgr_) {
         Status s = file_metadata_cache_res_mgr_->UpdateCacheReservation(
             f->ApproximateMemoryUsage(), false /* increase */);

db/version_builder_test.cc

@@ -3,19 +3,25 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
+#include "db/version_builder.h"
+
 #include <cstring>
 #include <iomanip>
 #include <memory>
 #include <sstream>
 #include <string>
 
+#include "db/db_test_util.h"
 #include "db/version_edit.h"
 #include "db/version_set.h"
 #include "rocksdb/advanced_options.h"
+#include "rocksdb/sst_file_writer.h"
 #include "table/unique_id_impl.h"
 #include "test_util/testharness.h"
 #include "test_util/testutil.h"
+#include "util/defer.h"
 #include "util/string_util.h"
+#include "utilities/fault_injection_fs.h"
 
 namespace ROCKSDB_NAMESPACE {
 
@@ -153,6 +159,123 @@ class VersionBuilderTest : public testing::Test {
   void UpdateVersionStorageInfo() { UpdateVersionStorageInfo(&vstorage_); }
 };
 
+class VersionBuilderDBTest : public DBTestBase {
+ public:
+  VersionBuilderDBTest()
+      : DBTestBase("version_builder_db_test", /*env_do_fsync=*/false) {}
+};
+
+// Regression test: when LogAndApply fails after VersionBuilder pinned a
+// table reader for a newly-added file (via LoadTableHandlers), the table
+// cache entry must be evicted by VersionBuilder::UnrefFile when the
+// FileMetaData is destroyed.
+//
+// This test faithfully reproduces the crash-test failure mode
+// ("File N is not live nor quarantined" from TEST_VerifyNoObsoleteFilesCached
+// inside Close on ASAN):
+//
+//   1. IngestExternalFile submits an AddFile edit through LogAndApply.
+//   2. LoadTableHandlers inside ProcessManifestWrites pins a table reader
+//      for the new file, creating a table cache entry.
+//   3. SyncManifest fails (fault injected). The error handler adds the
+//      ingested file to its quarantine list.
+//   4. A subsequent successful LogAndApply (via manual Resume() and then
+//      Flush) clears the quarantine (version_set.cc:
+//      `ClearFilesToQuarantine`).
+//   5. Metadata read fault injection prevents FindObsoleteFiles from
+//      scanning the directory, so the on-disk orphan is not discovered.
+//   6. On DB Close, TEST_VerifyNoObsoleteFilesCached iterates the table
+//      cache and finds an entry for a file that is neither live nor
+//      quarantined -> assertion fires (without the fix).
+//
+// The IngestExternalFile path issues a few no-op manifest writes before
+// the actual AddFile edit; we arm the injection only after
+// LoadTableHandlers has actually pinned a reader for the ingested file.
+TEST_F(VersionBuilderDBTest,
+       IngestExternalFileEvictsTableCacheOnLogAndApplyFailure) {
+  auto fault_fs = std::make_shared<FaultInjectionTestFS>(env_->GetFileSystem());
+  std::unique_ptr<Env> fault_env(NewCompositeEnv(fault_fs));
+
+  Options options = CurrentOptions();
+  options.env = fault_env.get();
+  options.disable_auto_compactions = true;
+  // Disable auto-recovery so recovery runs synchronously via our explicit
+  // Resume() call -- no background thread to race with test teardown.
+  options.max_bgerror_resume_count = 0;
+  options.paranoid_file_checks = true;
+  DestroyAndReopen(options);
+  // RAII: ensure the DB is closed before fault_env goes out of scope,
+  // even if an ASSERT_* below early-returns.
+  Defer closer([this]() { Close(); });
+
+  // Seed the memtable with data so a subsequent Flush actually produces a
+  // successful LogAndApply (and thus clears the quarantine).
+  ASSERT_OK(Put("k1", "v1"));
+
+  // Build an external SST through the public SstFileWriter API.
+  const std::string ext_file = dbname_ + "/ingest_me.sst";
+  SstFileWriter writer(EnvOptions(), options);
+  ASSERT_OK(writer.Open(ext_file));
+  ASSERT_OK(writer.Put("k2", "v2"));
+  ExternalSstFileInfo file_info;
+  ASSERT_OK(writer.Finish(&file_info));
+
+  // Arm the injection only once we have observed VersionBuilder actually
+  // pin a table reader for the ingested file (i.e. after FindTable runs).
+  std::atomic<bool> injection_armed{false};
+  std::atomic<bool> injected_once{false};
+  SyncPoint::GetInstance()->SetCallBack("TableCache::FindTable:0",
+                                        [&](void*) { injection_armed = true; });
+  SyncPoint::GetInstance()->SetCallBack(
+      "VersionSet::ProcessManifestWrites:AfterSyncManifest", [&](void* arg) {
+        if (!injection_armed.load()) {
+          return;
+        }
+        if (injected_once.exchange(true)) {
+          return;
+        }
+        auto* ptr = static_cast<IOStatus*>(arg);
+        assert(ptr);
+        *ptr = IOStatus::IOError("injected");
+        // Retryable IOError -> soft error, recoverable via Resume().
+        ptr->SetRetryable(true);
+      });
+  SyncPoint::GetInstance()->EnableProcessing();
+
+  IngestExternalFileOptions ifo;
+  Status s = db_->IngestExternalFile({ext_file}, ifo);
+  ASSERT_NOK(s);
+
+  SyncPoint::GetInstance()->DisableProcessing();
+  SyncPoint::GetInstance()->ClearAllCallBacks();
+
+  // Manual synchronous recovery: clear the soft bg error, then do a
+  // successful LogAndApply (via Flush) which triggers
+  // ClearFilesToQuarantine (version_set.cc line ~6591). This puts the
+  // ingested file into the "not live nor quarantined" state.
+  ASSERT_OK(dbfull()->Resume());
+  ASSERT_OK(Flush());
+
+  // Prevent the FindObsoleteFiles full-directory scan from finding the
+  // orphan on disk. Mirrors the crash test's open_metadata_read_fault_one_in.
+  fault_fs->SetThreadLocalErrorContext(
+      FaultInjectionIOType::kMetadataRead, /*seed=*/0, /*one_in=*/1,
+      /*retryable=*/false, /*has_data_loss=*/false);
+  fault_fs->EnableThreadLocalErrorInjection(
+      FaultInjectionIOType::kMetadataRead);
+  Defer disable_fault([&] {
+    fault_fs->DisableThreadLocalErrorInjection(
+        FaultInjectionIOType::kMetadataRead);
+  });
+
+  // The RAII Defer above will run Close() on scope exit. Close's
+  // TEST_VerifyNoObsoleteFilesCached (active on ASAN) iterates the table
+  // cache. Without the D99759696 fix, an entry for the ingested file
+  // remains -- a file that is neither live nor quarantined -- and the
+  // assertion fires. With the fix, VersionBuilder::UnrefFile evicts the
+  // entry when the discarded FileMetaData is destroyed.
+}
+
 void UnrefFilesInVersion(VersionStorageInfo* new_vstorage) {
   for (int i = 0; i < new_vstorage->num_levels(); i++) {
     for (auto* f : new_vstorage->LevelFiles(i)) {

include/rocksdb/version.h

@@ -12,7 +12,7 @@
 // NOTE: in 'main' development branch, this should be the *next*
 // minor or major version number planned for release.
 #define ROCKSDB_MAJOR 11
-#define ROCKSDB_MINOR 6
+#define ROCKSDB_MINOR 7
 #define ROCKSDB_PATCH 0
 
 // Make it easy to do conditional compilation based on version checks, i.e.

tools/check_format_compatible.sh

@@ -161,7 +161,7 @@ EOF
 
 # To check for DB forward compatibility with loading options (old version
 # reading data from new), as well as backward compatibility
-declare -a db_forward_with_options_refs=("10.4.fb" "10.5.fb" "10.6.fb" "10.7.fb" "10.8.fb" "10.9.fb" "10.10.fb" "10.11.fb" "11.0.fb" "11.1.fb" "11.2.fb" "11.3.fb" "11.4.fb" "11.5.fb")
+declare -a db_forward_with_options_refs=("10.4.fb" "10.5.fb" "10.6.fb" "10.7.fb" "10.8.fb" "10.9.fb" "10.10.fb" "10.11.fb" "11.0.fb" "11.1.fb" "11.2.fb" "11.3.fb" "11.4.fb" "11.5.fb" "11.6.fb")
 # To check for DB forward compatibility without loading options (in addition
 # to the "with loading options" set), as well as backward compatibility
 declare -a db_forward_no_options_refs=() # N/A at the moment

unreleased_history/bug_fixes/range_lock_manager_fix_revert.md

@@ -1 +0,0 @@
-Reverted PR14831 that made range_lock_manager aware of reverse-order CF

unreleased_history/bug_fixes/read_async_direct_io_null_scratch.md

@@ -1 +0,0 @@
-Fixed a bug in `RandomAccessFileReader::ReadAsync` where an already-aligned direct-IO read request with a null `scratch` and a caller-provided `aligned_buf` would take the "already aligned" fast path and submit the null buffer to the underlying async read (e.g. a null iovec base to io_uring, failing with EFAULT). This could surface as spurious iterator failures during async prefetch (MultiScan with async IO) on direct-IO databases. The async path now allocates a backing buffer in this case, matching the synchronous `Read` path.

unreleased_history/bug_fixes/read_only_close_no_delete.md

@@ -1 +0,0 @@
-Fixed a bug where closing a read-only DB instance could delete live SST files created by a concurrent read-write DB sharing the same directory.

unreleased_history/new_features/embedded_blob_sst_writer.md

@@ -1 +0,0 @@
-Added EXPERIMENTAL embedded blob SST support through `SstFileWriter::OpenWithEmbeddedBlobs()`, storing eligible large values as same-file blob records in block-based SST files and resolving them transparently for reads. This niche feature currently supports uncompressed embedded blobs only; compression options are placeholders and compression support is deferred to follow-up work.

unreleased_history/public_api_changes/c_api_codegen_bindings.md

@@ -1 +0,0 @@
-Expanded the C API (`include/rocksdb/c.h`) with a large set of new `rocksdb_*` functions, mostly option getters/setters plus table-properties, job/event-listener, and metadata accessors, a WAL filter, a ReadOptions table filter, and a backup exclude-files callback. Many are now produced by a new semi-automated generator (`tools/c_api_gen/`) from the C++ headers; `include/rocksdb/c.h` remains a single self-contained header and the signatures of pre-existing functions are unchanged.