set ignore_unknown_options when parsing options (#13443)

Summary:
In case the primary host has a new option added which isn't available in the remote worker yet, the remote compaction currently fails. In most cases, these new options are not relevant to the remote compaction and the worker should be able to move on by ignoring it.

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

Test Plan: Verified internally in Meta Infra.

Reviewed By: anand1976

Differential Revision: D70744359

Pulled By: jaykorean

fbshipit-source-id: eb6a388c2358a7f8089f2e35a378b7017b9e03f3

.github/workflows/pr-jobs.yml

@@ -45,7 +45,7 @@ jobs:
     - name: Install argparse
       run: pip install argparse
     - name: Download clang-format-diff.py
-      run: wget https://raw.githubusercontent.com/llvm/llvm-project/release/12.x/clang/tools/clang-format/clang-format-diff.py
+      run: wget https://rocksdb-deps.s3.us-west-2.amazonaws.com/llvm/llvm-project/release/12.x/clang/tools/clang-format/clang-format-diff.py
     - name: Check format
       run: VERBOSE_CHECK=1 make check-format
     - name: Compare buckify output

BUCK

@@ -214,7 +214,7 @@ cpp_library_wrapper(name="rocksdb_lib", srcs=[
         "table/cuckoo/cuckoo_table_builder.cc",
         "table/cuckoo/cuckoo_table_factory.cc",
         "table/cuckoo/cuckoo_table_reader.cc",
-        "table/external_table_reader.cc",
+        "table/external_table.cc",
         "table/format.cc",
         "table/get_context.cc",
         "table/iterator.cc",
@@ -318,6 +318,8 @@ cpp_library_wrapper(name="rocksdb_lib", srcs=[
         "utilities/persistent_cache/block_cache_tier_metadata.cc",
         "utilities/persistent_cache/persistent_cache_tier.cc",
         "utilities/persistent_cache/volatile_tier_impl.cc",
+        "utilities/secondary_index/secondary_index_iterator.cc",
+        "utilities/secondary_index/simple_secondary_index.cc",
         "utilities/simulator_cache/cache_simulator.cc",
         "utilities/simulator_cache/sim_cache.cc",
         "utilities/table_properties_collectors/compact_for_tiering_collector.cc",
@@ -407,6 +409,7 @@ cpp_library_wrapper(name="rocksdb_tools_lib", srcs=[
         "tools/block_cache_analyzer/block_cache_trace_analyzer.cc",
         "tools/db_bench_tool.cc",
         "tools/simulated_hybrid_file_system.cc",
+        "tools/tool_hooks.cc",
         "tools/trace_analyzer_tool.cc",
     ], deps=[":rocksdb_lib"], headers=[], link_whole=False, extra_test_libs=False)
 

CMakeLists.txt

@@ -835,7 +835,7 @@ set(SOURCES
         table/cuckoo/cuckoo_table_builder.cc
         table/cuckoo/cuckoo_table_factory.cc
         table/cuckoo/cuckoo_table_reader.cc
-        table/external_table_reader.cc
+        table/external_table.cc
         table/format.cc
         table/get_context.cc
         table/iterator.cc
@@ -940,6 +940,8 @@ set(SOURCES
         utilities/persistent_cache/block_cache_tier_metadata.cc
         utilities/persistent_cache/persistent_cache_tier.cc
         utilities/persistent_cache/volatile_tier_impl.cc
+        utilities/secondary_index/secondary_index_iterator.cc
+        utilities/secondary_index/simple_secondary_index.cc
         utilities/simulator_cache/cache_simulator.cc
         utilities/simulator_cache/sim_cache.cc
         utilities/table_properties_collectors/compact_for_tiering_collector.cc
@@ -1572,6 +1574,7 @@ if(WITH_BENCHMARK_TOOLS)
   add_executable(db_bench${ARTIFACT_SUFFIX}
     tools/simulated_hybrid_file_system.cc
     tools/db_bench.cc
+    tools/tool_hooks.cc
     tools/db_bench_tool.cc)
   target_link_libraries(db_bench${ARTIFACT_SUFFIX}
     ${ROCKSDB_LIB} ${THIRDPARTY_LIBS})

HISTORY.md

@@ -1,9 +1,34 @@
 # Rocksdb Change Log
 > NOTE: Entries for next release do not go here. Follow instructions in `unreleased_history/README.txt`
 
-## 9.11.1 (02/19/2025)
+## 10.0.1 (03/05/2025)
+### Public API Changes
+* Add an unordered map of name/value pairs, ReadOptions::property_bag, to pass opaque options through to an external table when creating an Iterator.
+* Introduced CompactionServiceJobStatus::kAborted to allow handling aborted scenario in Schedule(), Wait() or OnInstallation() APIs in Remote Compactions.
+* Added a column family option disallow_memtable_writes to safely fail any attempts to write to a non-default column family. This can be used for column families that are ingest only.
+
+## 10.0.0 (02/21/2025)
 ### New Features
+* Introduced new `auto_refresh_iterator_with_snapshot` opt-in knob that (when enabled) will periodically release obsolete memory and storage resources for as long as the iterator is making progress and its supplied `read_options.snapshot` was initialized with non-nullptr value.
 * Added the ability to plug-in a custom table reader implementation. See include/rocksdb/external_table_reader.h for more details.
+* Experimental feature: RocksDB now supports FAISS inverted file based indices via the secondary indexing framework. Applications can use FAISS secondary indices to automatically quantize embeddings and perform K-nearest-neighbors similarity searches. See `FaissIVFIndex` and `SecondaryIndex` for more details. Note: the FAISS integration currently requires using the BUCK build.
+* Add new DB property `num_running_compaction_sorted_runs` that tracks the number of sorted runs being processed by currently running compactions
+* Experimental feature: added support for simple secondary indices that index the specified column as-is. See `SimpleSecondaryIndex` and `SecondaryIndex` for more details.
+* Added new `TransactionDBOptions::txn_commit_bypass_memtable_threshold`, which enables optimized transaction commit (see `TransactionOptions::commit_bypass_memtable`) when the transaction size exceeds a configured threshold.
+
+### Public API Changes
+* Updated the query API of the experimental secondary indexing feature by removing the earlier `SecondaryIndex::NewIterator` virtual and adding a `SecondaryIndexIterator` class that can be utilized by applications to find the primary keys for a given search target.
+* Added back the ability to leverage the primary key when building secondary index entries. This involved changes to the signatures of `SecondaryIndex::GetSecondary{KeyPrefix,Value}` as well as the addition of a new method `SecondaryIndex::FinalizeSecondaryKeyPrefix`. See the API comments for more details.
+* Minimum supported version of ZSTD is now 1.4.0, for code simplification. Obsolete `CompressionType` `kZSTDNotFinalCompression` is also removed.
+
+### Behavior Changes
+* `VerifyBackup` in `verify_with_checksum`=`true` mode will now evaluate checksums in parallel. As a result, unlike in case of original implementation, the API won't bail out on a very first corruption / mismatch and instead will iterate over all the backup files logging success / _degree_of_failure_ for each.
+* Reversed the order of updates to the same key in WriteBatchWithIndex. This means if there are multiple updates to the same key, the most recent update is ordered first. This affects the output of WBWIIterator. When WriteBatchWithIndex is created with `overwrite_key=true`, this affects the output only if Merge is used (#13387).
+* Added support for Merge operations in transactions using option `TransactionOptions::commit_bypass_memtable`.
+
+### Bug Fixes
+* Fixed GetMergeOperands() API in ReadOnlyDB and SecondaryDB
+* Fix a bug in `GetMergeOperands()` that can return incorrect status (MergeInProgress) and incorrect number of merge operands. This can happen when `GetMergeOperandsOptions::continue_cb` is set, both active and immutable memtables have merge operands and the callback stops the look up at the immutable memtable.
 
 ## 9.11.0 (01/17/2025)
 ### New Features

LANGUAGE-BINDINGS.md

@@ -2,7 +2,8 @@ This is the list of all known third-party language bindings for RocksDB. If some
 
 * Java - https://github.com/facebook/rocksdb/tree/main/java
 * Python
-    * http://python-rocksdb.readthedocs.io/en/latest/
+    * https://github.com/rocksdict/RocksDict
+    * http://python-rocksdb.readthedocs.io/en/latest/ (unmaintained) 
     * http://pyrocksdb.readthedocs.org/en/latest/ (unmaintained)
 * Perl - https://metacpan.org/pod/RocksDB
 * Node.js - https://npmjs.org/package/rocksdb

Makefile

@@ -2489,7 +2489,7 @@ checkout_folly:
 	fi
 	@# Pin to a particular version for public CI, so that PR authors don't
 	@# need to worry about folly breaking our integration. Update periodically
-	cd third-party/folly && git reset --hard 62baa6ba07ff0a23ee4f2ea2f5207e4c88464deb
+	cd third-party/folly && git reset --hard f1c1542fb071e9d88181279ec24043400222aef1
 	@# NOTE: this hack is required for clang in some cases
 	perl -pi -e 's/int rv = syscall/int rv = (int)syscall/' third-party/folly/folly/detail/Futex.cpp
 	@# NOTE: this hack is required for gcc in some cases

build_tools/build_detect_platform

@@ -360,9 +360,13 @@ EOF
     fi
 
     if ! test $ROCKSDB_DISABLE_ZSTD; then
-        # Test whether zstd library is installed
+        # Test whether zstd library is installed with minimum version
+        # (Keep in sync with compression.h)
         $CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS -x c++ - -o /dev/null 2>/dev/null  <<EOF
           #include <zstd.h>
+          #if ZSTD_VERSION_NUMBER < 10400
+          #error "ZSTD support requires version >= 1.4.0 (libzstd-devel)"
+          #endif  // ZSTD_VERSION_NUMBER
           int main() {}
 EOF
         if [ "$?" = 0 ]; then

build_tools/fbcode_config_platform010.sh

@@ -59,7 +59,7 @@ fi
 if ! test $ROCKSDB_DISABLE_ZSTD; then
   ZSTD_INCLUDE=" -I $ZSTD_BASE/include/"
   ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd${MAYBE_PIC}.a"
-  CFLAGS+=" -DZSTD"
+  CFLAGS+=" -DZSTD -DZSTD_STATIC_LINKING_ONLY"
 fi
 
 # location of gflags headers and libraries

db/arena_wrapped_db_iter.cc

@@ -64,8 +64,118 @@ void ArenaWrappedDBIter::Init(
   memtable_range_tombstone_iter_ = nullptr;
 }
 
+void ArenaWrappedDBIter::MaybeAutoRefresh(bool is_seek,
+                                          DBIter::Direction direction) {
+  if (cfh_ != nullptr && read_options_.snapshot != nullptr && allow_refresh_ &&
+      read_options_.auto_refresh_iterator_with_snapshot) {
+    // The intent here is to capture the superversion number change
+    // reasonably soon from the time it actually happened. As such,
+    // we're fine with weaker synchronization / ordering guarantees
+    // provided by relaxed atomic (in favor of less CPU / mem overhead).
+    uint64_t cur_sv_number = cfh_->cfd()->GetSuperVersionNumberRelaxed();
+    if ((sv_number_ != cur_sv_number) && status().ok()) {
+      // Changing iterators' direction is pretty heavy-weight operation and
+      // could have unintended consequences when it comes to prefix seek.
+      // Therefore, we need an efficient implementation that does not duplicate
+      // the effort by doing things like double seek(forprev).
+      //
+      // Auto refresh can be triggered on the following groups of operations:
+      //
+      //  1. [Seek]: Seek(), SeekForPrev()
+      //  2. [Non-Seek]: Next(), Prev()
+      //
+      // In case of 'Seek' group, procedure is fairly straightforward as we'll
+      // simply call refresh and then invoke the operation on intended target.
+      //
+      // In case of 'Non-Seek' group, we'll first advance the cursor by invoking
+      // intended user operation (Next() or Prev()), capture the target key T,
+      // refresh the iterator and then reconcile the refreshed iterator by
+      // explicitly calling [Seek(T) or SeekForPrev(T)]. Below is an example
+      // flow for Next(), but same principle applies to Prev():
+      //
+      //
+      //          T0: Before the operation     T1: Execute Next()
+      //                         |                      |
+      //                         |         -------------
+      //                         |        |   * capture the key (T)
+      //           DBIter(SV#A)  |        |
+      //          --------------\ /------\ /---------
+      //  SV #A  |     ... ->  [ X ] -> [ T ] -> ... |
+      //          -----------------------------------
+      //                                / |
+      //                               /  |
+      //                              /  T2: Refresh iterator
+      //                             /
+      //           DBIter(SV#A')    /
+      //          ----------------------------------
+      //  SV #A' |       ... ->  [ T ] -> ...       |
+      //          ----------------/ \---------------
+      //                           |
+      //                            ---- T3: Seek(T)
+      //
+      bool valid = false;
+      std::string key;
+      if (!is_seek && db_iter_->Valid()) {
+        // The key() Slice is valid until the iterator state changes.
+        // Given that refresh is heavy-weight operation it itself,
+        // we should copy the target key upfront to avoid reading bad value.
+        valid = true;
+        key = db_iter_->key().ToString();
+      }
+
+      // It's perfectly fine to unref the corresponding superversion
+      // as we rely on pinning behavior of snapshot for consistency.
+      DoRefresh(read_options_.snapshot, cur_sv_number);
+
+      if (!is_seek && valid) {  // Reconcile new iterator after Next() / Prev()
+        if (direction == DBIter::kForward) {
+          db_iter_->Seek(key);
+        } else {
+          assert(direction == DBIter::kReverse);
+          db_iter_->SeekForPrev(key);
+        }
+      }
+    }
+  }
+}
+
 Status ArenaWrappedDBIter::Refresh() { return Refresh(nullptr); }
 
+void ArenaWrappedDBIter::DoRefresh(const Snapshot* snapshot,
+                                   [[maybe_unused]] uint64_t sv_number) {
+  Env* env = db_iter_->env();
+
+  // NOTE:
+  //
+  // Errors like file deletion (as a part of SV cleanup in ~DBIter) will be
+  // present in the error log, but won't be reflected in the iterator status.
+  // This is by design as we expect compaction to clean up those obsolete files
+  // eventually.
+  db_iter_->~DBIter();
+
+  arena_.~Arena();
+  new (&arena_) Arena();
+
+  auto cfd = cfh_->cfd();
+  auto db_impl = cfh_->db();
+
+  SuperVersion* sv = cfd->GetReferencedSuperVersion(db_impl);
+  assert(sv->version_number >= sv_number);
+  SequenceNumber read_seq = GetSeqNum(db_impl, snapshot);
+  if (read_callback_) {
+    read_callback_->Refresh(read_seq);
+  }
+  Init(env, read_options_, cfd->ioptions(), sv->mutable_cf_options, sv->current,
+       read_seq, sv->mutable_cf_options.max_sequential_skip_in_iterations,
+       sv->version_number, read_callback_, cfh_, expose_blob_index_,
+       allow_refresh_);
+
+  InternalIterator* internal_iter = db_impl->NewInternalIterator(
+      read_options_, cfd, sv, &arena_, read_seq,
+      /* allow_unprepared_value */ true, /* db_iter */ this);
+  SetIterUnderDBIter(internal_iter);
+}
+
 Status ArenaWrappedDBIter::Refresh(const Snapshot* snapshot) {
   if (cfh_ == nullptr || !allow_refresh_) {
     return Status::NotSupported("Creating renew iterator is not allowed.");
@@ -85,32 +195,9 @@ Status ArenaWrappedDBIter::Refresh(const Snapshot* snapshot) {
   TEST_SYNC_POINT("ArenaWrappedDBIter::Refresh:1");
   TEST_SYNC_POINT("ArenaWrappedDBIter::Refresh:2");
 
-  auto reinit_internal_iter = [&]() {
-    Env* env = db_iter_->env();
-    db_iter_->~DBIter();
-    arena_.~Arena();
-    new (&arena_) Arena();
-
-    SuperVersion* sv = cfd->GetReferencedSuperVersion(db_impl);
-    assert(sv->version_number >= cur_sv_number);
-    SequenceNumber read_seq = GetSeqNum(db_impl, snapshot);
-    if (read_callback_) {
-      read_callback_->Refresh(read_seq);
-    }
-    Init(env, read_options_, cfd->ioptions(), sv->mutable_cf_options,
-         sv->current, read_seq,
-         sv->mutable_cf_options.max_sequential_skip_in_iterations,
-         sv->version_number, read_callback_, cfh_, expose_blob_index_,
-         allow_refresh_);
-
-    InternalIterator* internal_iter = db_impl->NewInternalIterator(
-        read_options_, cfd, sv, &arena_, read_seq,
-        /* allow_unprepared_value */ true, /* db_iter */ this);
-    SetIterUnderDBIter(internal_iter);
-  };
   while (true) {
     if (sv_number_ != cur_sv_number) {
-      reinit_internal_iter();
+      DoRefresh(snapshot, cur_sv_number);
       break;
     } else {
       SequenceNumber read_seq = GetSeqNum(db_impl, snapshot);
@@ -138,7 +225,7 @@ Status ArenaWrappedDBIter::Refresh(const Snapshot* snapshot) {
             db_impl->ReturnAndCleanupSuperVersion(cfd, sv);
             // The memtable under DBIter did not have range tombstone before
             // refresh.
-            reinit_internal_iter();
+            DoRefresh(snapshot, cur_sv_number);
             break;
           } else {
             *memtable_range_tombstone_iter_ =

db/arena_wrapped_db_iter.h

@@ -65,12 +65,26 @@ class ArenaWrappedDBIter : public Iterator {
   void SeekToLast() override { db_iter_->SeekToLast(); }
   // 'target' does not contain timestamp, even if user timestamp feature is
   // enabled.
-  void Seek(const Slice& target) override { db_iter_->Seek(target); }
+  void Seek(const Slice& target) override {
+    MaybeAutoRefresh(true /* is_seek */, DBIter::kForward);
+    db_iter_->Seek(target);
+  }
+
   void SeekForPrev(const Slice& target) override {
+    MaybeAutoRefresh(true /* is_seek */, DBIter::kReverse);
     db_iter_->SeekForPrev(target);
   }
-  void Next() override { db_iter_->Next(); }
-  void Prev() override { db_iter_->Prev(); }
+
+  void Next() override {
+    db_iter_->Next();
+    MaybeAutoRefresh(false /* is_seek */, DBIter::kForward);
+  }
+
+  void Prev() override {
+    db_iter_->Prev();
+    MaybeAutoRefresh(false /* is_seek */, DBIter::kReverse);
+  }
+
   Slice key() const override { return db_iter_->key(); }
   Slice value() const override { return db_iter_->value(); }
   const WideColumns& columns() const override { return db_iter_->columns(); }
@@ -103,6 +117,9 @@ class ArenaWrappedDBIter : public Iterator {
   }
 
  private:
+  void DoRefresh(const Snapshot* snapshot, uint64_t sv_number);
+  void MaybeAutoRefresh(bool is_seek, DBIter::Direction direction);
+
   DBIter* db_iter_ = nullptr;
   Arena arena_;
   uint64_t sv_number_;

db/c.cc

@@ -1858,10 +1858,6 @@ extern ROCKSDB_LIBRARY_API void rocksdb_approximate_sizes_cf_with_flags(
   delete[] ranges;
 }
 
-void DEPRECATED_rocksdb_delete_file(rocksdb_t* db, const char* name) {
-  db->rep->DEPRECATED_DeleteFile(name);
-}
-
 const rocksdb_livefiles_t* rocksdb_livefiles(rocksdb_t* db) {
   rocksdb_livefiles_t* result = new rocksdb_livefiles_t;
   db->rep->GetLiveFilesMetaData(&result->rep);

db/column_family.cc

@@ -168,7 +168,8 @@ Status CheckConcurrentWritesSupported(const ColumnFamilyOptions& cf_options) {
   }
   if (!cf_options.memtable_factory->IsInsertConcurrentlySupported()) {
     return Status::InvalidArgument(
-        "Memtable doesn't concurrent writes (allow_concurrent_memtable_write)");
+        "Memtable doesn't allow concurrent writes "
+        "(allow_concurrent_memtable_write)");
   }
   return Status::OK();
 }
@@ -239,6 +240,10 @@ ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options,
 
     result.min_write_buffer_number_to_merge = 1;
   }
+  if (result.disallow_memtable_writes) {
+    // A simple memtable that enforces MarkReadOnly (unlike skip list)
+    result.memtable_factory = std::make_shared<VectorRepFactory>();
+  }
 
   if (result.num_levels < 1) {
     result.num_levels = 1;

db/column_family.h

@@ -210,6 +210,7 @@ struct SuperVersion {
   ReadOnlyMemTable* mem;
   MemTableListVersion* imm;
   Version* current;
+  // TODO: do we really need this in addition to what's in current Version?
   MutableCFOptions mutable_cf_options;
   // Version number of the current SuperVersion
   uint64_t version_number;
@@ -380,9 +381,9 @@ class ColumnFamilyData {
     return mem()->GetFirstSequenceNumber() == 0 && imm()->NumNotFlushed() == 0;
   }
 
-  Version* current() { return current_; }
   Version* dummy_versions() { return dummy_versions_; }
-  void SetCurrent(Version* _current);
+  Version* current() { return current_; }  // REQUIRE: DB mutex held
+  void SetCurrent(Version* _current);      // REQUIRE: DB mutex held
   uint64_t GetNumLiveVersions() const;    // REQUIRE: DB mutex held
   uint64_t GetTotalSstFilesSize() const;  // REQUIRE: DB mutex held
   uint64_t GetLiveSstFilesSize() const;   // REQUIRE: DB mutex held
@@ -391,6 +392,9 @@ class ColumnFamilyData {
   void SetMemtable(MemTable* new_mem) {
     AssignMemtableID(new_mem);
     mem_ = new_mem;
+    if (ioptions_.disallow_memtable_writes) {
+      mem_->MarkImmutable();
+    }
   }
 
   void AssignMemtableID(ReadOnlyMemTable* new_imm) {
@@ -483,6 +487,9 @@ class ColumnFamilyData {
   uint64_t GetSuperVersionNumber() const {
     return super_version_number_.load();
   }
+  uint64_t GetSuperVersionNumberRelaxed() const {
+    return super_version_number_.load(std::memory_order_relaxed);
+  }
   // will return a pointer to SuperVersion* if previous SuperVersion
   // if its reference count is zero and needs deletion or nullptr if not
   // As argument takes a pointer to allocated SuperVersion to enable

db/compaction/compaction_job.cc

@@ -391,6 +391,8 @@ void CompactionJob::Prepare(
   // seqno threshold.)
   penultimate_after_seqno_ = std::max(preclude_last_level_min_seqno,
                                       c->GetKeepInLastLevelThroughSeqno());
+
+  options_file_number_ = versions_->options_file_number();
 }
 
 uint64_t CompactionJob::GetSubcompactionsLimit() {
@@ -1128,8 +1130,7 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
   if (db_options_.compaction_service) {
     CompactionServiceJobStatus comp_status =
         ProcessKeyValueCompactionWithCompactionService(sub_compact);
-    if (comp_status == CompactionServiceJobStatus::kSuccess ||
-        comp_status == CompactionServiceJobStatus::kFailure) {
+    if (comp_status != CompactionServiceJobStatus::kUseLocal) {
       return;
     }
     // fallback to local compaction

db/compaction/compaction_job.h

@@ -366,6 +366,10 @@ class CompactionJob {
   // the last level (output to penultimate level).
   SequenceNumber penultimate_after_seqno_ = kMaxSequenceNumber;
 
+  // Options File Number used for Remote Compaction
+  // Setting this requires DBMutex.
+  uint64_t options_file_number_ = 0;
+
   // Get table file name in where it's outputting to, which should also be in
   // `output_directory_`.
   virtual std::string GetTableFileName(uint64_t file_number);

db/compaction/compaction_service_job.cc

@@ -48,10 +48,7 @@ CompactionJob::ProcessKeyValueCompactionWithCompactionService(
   compaction_input.has_end = sub_compact->end.has_value();
   compaction_input.end =
       compaction_input.has_end ? sub_compact->end->ToString() : "";
-  compaction_input.options_file_number =
-      sub_compact->compaction->input_version()
-          ->version_set()
-          ->options_file_number();
+  compaction_input.options_file_number = options_file_number_;
 
   TEST_SYNC_POINT_CALLBACK(
       "CompactionServiceJob::ProcessKeyValueCompactionWithCompactionService",
@@ -86,6 +83,14 @@ CompactionJob::ProcessKeyValueCompactionWithCompactionService(
   switch (response.status) {
     case CompactionServiceJobStatus::kSuccess:
       break;
+    case CompactionServiceJobStatus::kAborted:
+      sub_compact->status =
+          Status::Aborted("Scheduling a remote compaction job was aborted");
+      ROCKS_LOG_WARN(
+          db_options_.info_log,
+          "[%s] [JOB %d] Remote compaction was aborted at Schedule()",
+          compaction->column_family_data()->GetName().c_str(), job_id_);
+      return response.status;
     case CompactionServiceJobStatus::kFailure:
       sub_compact->status = Status::Incomplete(
           "CompactionService failed to schedule a remote compaction job.");
@@ -121,6 +126,16 @@ CompactionJob::ProcessKeyValueCompactionWithCompactionService(
     return compaction_status;
   }
 
+  if (compaction_status == CompactionServiceJobStatus::kAborted) {
+    sub_compact->status =
+        Status::Aborted("Waiting a remote compaction job was aborted");
+    ROCKS_LOG_INFO(db_options_.info_log,
+                   "[%s] [JOB %d] Remote compaction was aborted during Wait()",
+                   compaction->column_family_data()->GetName().c_str(),
+                   job_id_);
+    return compaction_status;
+  }
+
   CompactionServiceResult compaction_result;
   s = CompactionServiceResult::Read(compaction_result_binary,
                                     &compaction_result);

db/compaction/compaction_service_test.cc

@@ -1471,6 +1471,40 @@ TEST_F(CompactionServiceTest, FallbackLocalManual) {
   VerifyTestData();
 }
 
+TEST_F(CompactionServiceTest, AbortedWhileWait) {
+  Options options = CurrentOptions();
+  options.disable_auto_compactions = true;
+  ReopenWithCompactionService(&options);
+
+  GenerateTestData();
+  VerifyTestData();
+
+  auto my_cs = GetCompactionService();
+  Statistics* compactor_statistics = GetCompactorStatistics();
+  Statistics* primary_statistics = GetPrimaryStatistics();
+
+  my_cs->ResetOverride();
+  std::string start_str = Key(15);
+  std::string end_str = Key(45);
+  Slice start(start_str);
+  Slice end(end_str);
+
+  // Override Wait() result with kAborted
+  my_cs->OverrideWaitStatus(CompactionServiceJobStatus::kAborted);
+  start_str = Key(120);
+  start = start_str;
+
+  Status s = db_->CompactRange(CompactRangeOptions(), &start, nullptr);
+  ASSERT_NOK(s);
+  ASSERT_TRUE(s.IsAborted());
+  // no remote compaction is run
+  ASSERT_EQ(my_cs->GetCompactionNum(), 0);
+  // make sure the compaction statistics is not recorded any side
+  ASSERT_EQ(primary_statistics->getTickerCount(COMPACT_WRITE_BYTES), 0);
+  ASSERT_EQ(primary_statistics->getTickerCount(REMOTE_COMPACT_WRITE_BYTES), 0);
+  ASSERT_EQ(compactor_statistics->getTickerCount(COMPACT_WRITE_BYTES), 0);
+}
+
 TEST_F(CompactionServiceTest, RemoteEventListener) {
   class RemoteEventListenerTest : public EventListener {
    public:

db/db_basic_test.cc

@@ -4994,6 +4994,97 @@ TEST_F(DBBasicTest, VerifyFileChecksumsReadahead) {
             (sst_size + alignment - 1) / (alignment));
 }
 
+TEST_F(DBBasicTest, DisallowMemtableWrite) {
+  // This test is mostly about what you can't do with memtable writes
+  // disallowed. For what you can do, see
+  // ExternalSSTFileBasicTest.FailIfNotBottommostLevelAndDisallowMemtable
+  Options options_allow = GetDefaultOptions();
+  options_allow.create_if_missing = true;
+  Options options_disallow = options_allow;
+  options_disallow.disallow_memtable_writes = true;
+
+  DestroyAndReopen(options_allow);
+  // CFs allowing and disallowing memtable write
+  CreateColumnFamilies({"cf1", "cf2"}, options_allow);
+  CreateColumnFamilies({"cf3"}, options_disallow);
+  // XXX: needed to get consistent handles_ mappings
+  ReopenWithColumnFamilies(
+      {"default", "cf1", "cf2", "cf3"},
+      {options_allow, options_allow, options_allow, options_disallow});
+
+  EXPECT_EQ(Put(0, "a0", "1").code(), Status::Code::kOk);
+  EXPECT_EQ(Put(1, "a1", "1").code(), Status::Code::kOk);
+  EXPECT_EQ(Put(2, "a2", "1").code(), Status::Code::kOk);
+  EXPECT_EQ(Put(3, "a3", "1").code(), Status::Code::kInvalidArgument);
+
+  EXPECT_EQ(Get(0, "a0"), "1");
+  EXPECT_EQ(Get(1, "a1"), "1");
+  EXPECT_EQ(Get(2, "a2"), "1");
+  EXPECT_EQ(Get(3, "a3"), "NOT_FOUND");
+
+  EXPECT_EQ(Delete(0, "z0").code(), Status::Code::kOk);
+  EXPECT_EQ(Delete(1, "z1").code(), Status::Code::kOk);
+  EXPECT_EQ(Delete(2, "z2").code(), Status::Code::kOk);
+  EXPECT_EQ(Delete(3, "z3").code(), Status::Code::kInvalidArgument);
+
+  WriteBatch wb;
+  EXPECT_EQ(wb.Put(handles_[0], "b0", "2").code(), Status::Code::kOk);
+  EXPECT_EQ(wb.Put(handles_[1], "b1", "2").code(), Status::Code::kOk);
+  EXPECT_EQ(wb.Put(handles_[2], "b2", "2").code(), Status::Code::kOk);
+  EXPECT_EQ(wb.Put(handles_[3], "b3", "2").code(),
+            Status::Code::kInvalidArgument);
+  ASSERT_OK(db_->Write({}, &wb));
+  wb.Clear();
+
+  EXPECT_EQ(Get(0, "b0"), "2");
+  EXPECT_EQ(Get(1, "b1"), "2");
+  EXPECT_EQ(Get(2, "b2"), "2");
+  EXPECT_EQ(Get(3, "b3"), "NOT_FOUND");
+
+  // When the DB is re-opened with WAL entries for a CF that is newly setting
+  // disallow_memtable_writes, we detect that and fail the open gracefully.
+  ASSERT_EQ(TryReopenWithColumnFamilies(
+                {"default", "cf1", "cf2", "cf3"},
+                {options_allow, options_allow, options_disallow, options_allow})
+                .code(),
+            Status::Code::kInvalidArgument);
+
+  // Successfully opening with allow creates L0 files from the WAL
+  ReopenWithColumnFamilies({"default", "cf1", "cf2", "cf3"}, options_allow);
+
+  EXPECT_EQ(Get(0, "a0"), "1");
+  EXPECT_EQ(Get(1, "a1"), "1");
+  EXPECT_EQ(Get(2, "a2"), "1");
+  EXPECT_EQ(Get(3, "a3"), "NOT_FOUND");
+
+  // Now able to disallow on CF2 because no relevant WAL entries
+  ReopenWithColumnFamilies(
+      {"default", "cf1", "cf2", "cf3"},
+      {options_allow, options_allow, options_disallow, options_allow});
+
+  EXPECT_EQ(Get(0, "a0"), "1");
+  EXPECT_EQ(Get(1, "a1"), "1");
+  EXPECT_EQ(Get(2, "a2"), "1");
+  EXPECT_EQ(Get(3, "a3"), "NOT_FOUND");
+
+  // Now able to write to CF 3 but not CF 2
+  EXPECT_EQ(Put(0, "c0", "3").code(), Status::Code::kOk);
+  EXPECT_EQ(Put(1, "c1", "3").code(), Status::Code::kOk);
+  EXPECT_EQ(Put(2, "c2", "3").code(), Status::Code::kInvalidArgument);
+  EXPECT_EQ(Put(3, "c3", "3").code(), Status::Code::kOk);
+
+  EXPECT_EQ(Get(0, "c0"), "3");
+  EXPECT_EQ(Get(1, "c1"), "3");
+  EXPECT_EQ(Get(2, "c2"), "NOT_FOUND");
+  EXPECT_EQ(Get(3, "c3"), "3");
+
+  // disallow_memtable_writes not supported on default column family.
+  // (Would be complicated to make a WriteBatch aware of the setting in order
+  // to reject the write before entering the write path.)
+  Destroy(options_allow);
+  EXPECT_EQ(TryReopen(options_disallow).code(), Status::Code::kInvalidArgument);
+}
+
 // TODO: re-enable after we provide finer-grained control for WAL tracking to
 // meet the needs of different use cases, durability levels and recovery modes.
 TEST_F(DBBasicTest, DISABLED_ManualWalSync) {

db/db_block_cache_test.cc

@@ -835,8 +835,6 @@ TEST_F(DBBlockCacheTest, CacheCompressionDict) {
   }
   if (ZSTD_Supported()) {
     compression_types.push_back(kZSTD);
-  } else if (ZSTDNotFinal_Supported()) {
-    compression_types.push_back(kZSTDNotFinalCompression);
   }
   Random rnd(301);
   for (auto compression_type : compression_types) {

db/db_bloom_filter_test.cc

@@ -2244,6 +2244,54 @@ TEST_F(DBBloomFilterTest, MemtableWholeKeyBloomFilterMultiGet) {
 
   db_->ReleaseSnapshot(snapshot);
 }
+
+TEST_F(DBBloomFilterTest, TestMemtableBloomAndWBM) {
+  Options options = CurrentOptions();
+  options.arena_block_size = 4096;
+  options.write_buffer_size = 4000000;
+  std::shared_ptr<Cache> cache = NewLRUCache(LRUCacheOptions(
+      options.write_buffer_size * 3 /* capacity */, 1 /* num_shard_bits */,
+      false /* strict_capacity_limit */, 0.0 /* high_pri_pool_ratio */,
+      nullptr /* memory_allocator */, kDefaultToAdaptiveMutex,
+      kDontChargeCacheMetadata));
+
+  options.write_buffer_manager.reset(
+      new WriteBufferManager(options.write_buffer_size, cache));
+  Reopen(options);
+  ASSERT_OK(Put("foo", "bar"));
+
+  const auto kDummyEntrySize =
+      CacheReservationManagerImpl<CacheEntryRole::kMisc>::GetDummyEntrySize();
+
+  // Just the start of a memtable, no Bloom
+  ASSERT_GE(cache->GetUsage(), options.arena_block_size);
+  ASSERT_LE(cache->GetUsage(), kDummyEntrySize);
+
+  // Now enable memtable bloom filter
+  const double kRatio = 0.25;
+  options.memtable_prefix_bloom_size_ratio = kRatio;
+  options.memtable_whole_key_filtering = true;
+  Reopen(options);
+  ASSERT_OK(Put("foo2", "bar2"));
+
+  // Expecting a memtable Bloom of ratio times write_buffer_size, memory tracked
+  auto bloom_size = static_cast<size_t>(kRatio * options.write_buffer_size);
+  ASSERT_GE(cache->GetUsage(), bloom_size);
+  ASSERT_LE(cache->GetUsage(), bloom_size + 2U * kDummyEntrySize);
+
+  // Now get another memtable and test
+  // Pin this memtable with an iterator
+  std::unique_ptr<Iterator> iter{db_->NewIterator({})};
+  iter->Seek("foo2");
+  ASSERT_TRUE(iter->Valid());
+  ASSERT_OK(Flush());
+  ASSERT_OK(Put("foo2", "bar2"));
+
+  // Expecting twice as much
+  ASSERT_GE(cache->GetUsage(), 2U * bloom_size);
+  ASSERT_LE(cache->GetUsage(), 2U * bloom_size + 2U * kDummyEntrySize);
+}
+
 namespace {
 std::pair<uint64_t, uint64_t> GetBloomStat(const Options& options, bool sst) {
   if (sst) {

db/db_compaction_test.cc

@@ -5856,6 +5856,26 @@ TEST_F(DBCompactionTest, CompactionStatsTest) {
   options.listeners.emplace_back(collector);
   DestroyAndReopen(options);
 
+  // Verify that the internal statistics for num_running_compactions and
+  // num_running_compaction_sorted_runs start and end at valid states
+  uint64_t num_running_compactions = 0;
+  ASSERT_TRUE(db_->GetIntProperty(DB::Properties::kNumRunningCompactions,
+                                  &num_running_compactions));
+  ASSERT_EQ(num_running_compactions, 0);
+  uint64_t num_running_compaction_sorted_runs = 0;
+  ASSERT_TRUE(
+      db_->GetIntProperty(DB::Properties::kNumRunningCompactionSortedRuns,
+                          &num_running_compaction_sorted_runs));
+  ASSERT_EQ(num_running_compaction_sorted_runs, 0);
+  // Check that the stat actually gets changed some time between the start and
+  // end of compaction
+  std::atomic<bool> sorted_runs_count_incremented = false;
+  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
+      "CompactionMergingIterator::UpdateInternalStats",
+      [&](void*) { sorted_runs_count_incremented = true; });
+
+  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
+
   for (int i = 0; i < 32; i++) {
     for (int j = 0; j < 5000; j++) {
       ASSERT_OK(Put(std::to_string(j), std::string(1, 'A')));
@@ -5869,6 +5889,19 @@ TEST_F(DBCompactionTest, CompactionStatsTest) {
   ColumnFamilyData* cfd = cfh->cfd();
 
   VerifyCompactionStats(*cfd, *collector);
+  // There should be no more running compactions, and thus no more sorted runs
+  // to process
+  ASSERT_TRUE(db_->GetIntProperty(DB::Properties::kNumRunningCompactions,
+                                  &num_running_compactions));
+  ASSERT_EQ(num_running_compactions, 0);
+  ASSERT_TRUE(
+      db_->GetIntProperty(DB::Properties::kNumRunningCompactionSortedRuns,
+                          &num_running_compaction_sorted_runs));
+  ASSERT_EQ(num_running_compaction_sorted_runs, 0);
+  ASSERT_TRUE(sorted_runs_count_incremented);
+
+  SyncPoint::GetInstance()->DisableProcessing();
+  SyncPoint::GetInstance()->ClearAllCallBacks();
 }
 
 TEST_F(DBCompactionTest, SubcompactionEvent) {
@@ -6407,10 +6440,27 @@ TEST_F(DBCompactionTest, PersistRoundRobinCompactCursor) {
   }
 }
 
-TEST_P(RoundRobinSubcompactionsAgainstPressureToken, PressureTokenTest) {
+// FIXME: the test is flaky and failing the assertion
+// ASSERT_EQ(num_planned_subcompactions, kNumSubcompactions);
+// It's likely a test set up issue, fix if we are to use RoubdRobin compaction.
+TEST_P(RoundRobinSubcompactionsAgainstPressureToken,
+       DISABLED_PressureTokenTest) {
   const int kKeysPerBuffer = 100;
   const int kNumSubcompactions = 2;
   const int kFilesPerLevel = 50;
+  SyncPoint::GetInstance()->LoadDependency({
+      // SetBackgroundThreads() only starts the bg threads. Background
+      // threads may not be immediately available after SetBackgroundThreads
+      // returns. There are some initialization before they start waiting for
+      // new jobs, see ThreadPoolImpl::Impl::BGThread(). Here is a hacky way
+      // to wait until there are at least two background threads (thread id
+      // starts from 0) start waiting to accept jobs.
+      {"ThreadPoolImpl::BGThread::Start:th1", "WaitForThreadAvailable"},
+  });
+  SyncPoint::GetInstance()->EnableProcessing();
+  env_->SetBackgroundThreads(kNumSubcompactions, Env::LOW);
+  TEST_SYNC_POINT("WaitForThreadAvailable");
+  SyncPoint::GetInstance()->DisableProcessing();
   Options options = CurrentOptions();
   options.num_levels = 3;
   options.max_bytes_for_level_multiplier = 2;
@@ -6431,7 +6481,6 @@ TEST_P(RoundRobinSubcompactionsAgainstPressureToken, PressureTokenTest) {
   options.max_background_compactions = kNumSubcompactions;
   options.max_compaction_bytes = 100000000;
   DestroyAndReopen(options);
-  env_->SetBackgroundThreads(kNumSubcompactions, Env::LOW);
 
   Random rnd(301);
   for (int lvl = 2; lvl > 0; lvl--) {

db/db_impl/db_impl.cc

@@ -1296,18 +1296,42 @@ Status DBImpl::SetOptions(
   {
     auto db_options = GetDBOptions();
     InstrumentedMutexLock l(&mutex_);
-    s = cfd->SetOptions(db_options, options_map);
-    if (s.ok()) {
-      new_options_copy = cfd->GetLatestMutableCFOptions();
-      // Append new version to recompute compaction score.
-      VersionEdit dummy_edit;
-      s = versions_->LogAndApply(cfd, read_options, write_options, &dummy_edit,
-                                 &mutex_, directories_.GetDbDir());
-      if (!versions_->io_status().ok()) {
-        assert(!s.ok());
-        error_handler_.SetBGError(versions_->io_status(),
-                                  BackgroundErrorReason::kManifestWrite);
+    // Manifest writers + Version appenders like flush and compaction use
+    // LogAndApply, which releases DB mutex to wait for other manifest writers
+    // and for the manifest write. We need to append a Version for the options
+    // to take full effect (e.g. compaction scores), but we don't want to
+    // interleave with other callers of LogAndApply, which could at least
+    // temporarily roll back option changes. Thus, we use a special call to
+    // LogAndApply that allows us to
+    //
+    // (a) Apply the options update when we know we are the exclusive version
+    // appender + (fake) manifest writer, and
+    //
+    // (b) Append a new Version without manifest write nor DB mutex release
+    //
+    // Thus aren't releasing the DB mutex again until the end of this block,
+    // after installing the new SuperVersion.
+    auto pre_cb = [&]() -> Status {
+      Status cb_s = cfd->SetOptions(db_options, options_map);
+      if (cb_s.ok()) {
+        new_options_copy = cfd->GetLatestMutableCFOptions();
       }
+      return cb_s;
+    };
+    VersionEdit dummy_edit;
+    dummy_edit.MarkNoManifestWriteDummy();
+    TEST_SYNC_POINT_CALLBACK("DBImpl::SetOptions:dummy_edit", &dummy_edit);
+    s = versions_->LogAndApply(
+        cfd, read_options, write_options, &dummy_edit, &mutex_,
+        directories_.GetDbDir(), false /*new_descriptor_log=*/,
+        nullptr /*new_opts*/, {} /*manifest_wcb*/, pre_cb);
+    if (!versions_->io_status().ok()) {
+      assert(!s.ok());
+      error_handler_.SetBGError(versions_->io_status(),
+                                BackgroundErrorReason::kManifestWrite);
+    }
+
+    if (s.ok()) {
       // Trigger possible flush/compactions. This has to be before we persist
       // options to file, otherwise there will be a deadlock with writer
       // thread.
@@ -1315,6 +1339,14 @@ Status DBImpl::SetOptions(
       persist_options_status =
           WriteOptionsFile(write_options, true /*db_mutex_already_held*/);
       bg_cv_.SignalAll();
+
+#if __cplusplus >= 202002L
+      assert(new_options_copy == cfd->GetLatestMutableCFOptions());
+      assert(cfd->GetLatestMutableCFOptions() ==
+             cfd->GetCurrentMutableCFOptions());
+      assert(cfd->GetCurrentMutableCFOptions() ==
+             cfd->current()->GetMutableCFOptions());
+#endif
     }
   }
   sv_context.Clean();
@@ -2566,6 +2598,8 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, const Slice& key,
         // Return all merge operands for get_impl_options.key
         *get_impl_options.number_of_operands =
             static_cast<int>(merge_context.GetNumOperands());
+        // OK status is returned, some merge operand is found.
+        assert(*get_impl_options.number_of_operands > 0);
         if (*get_impl_options.number_of_operands >
             get_impl_options.get_merge_operands_options
                 ->expected_max_number_of_operands) {
@@ -4883,108 +4917,6 @@ Status DBImpl::GetUpdatesSince(
   return wal_manager_.GetUpdatesSince(seq, iter, read_options, versions_.get());
 }
 
-Status DBImpl::DEPRECATED_DeleteFile(std::string name) {
-  // TODO: plumb Env::IOActivity, Env::IOPriority
-  const ReadOptions read_options;
-  const WriteOptions write_options;
-
-  uint64_t number;
-  FileType type;
-  WalFileType log_type;
-  if (!ParseFileName(name, &number, &type, &log_type) ||
-      (type != kTableFile && type != kWalFile)) {
-    ROCKS_LOG_ERROR(immutable_db_options_.info_log, "DeleteFile %s failed.\n",
-                    name.c_str());
-    return Status::InvalidArgument("Invalid file name");
-  }
-
-  if (type == kWalFile) {
-    // Only allow deleting archived log files
-    if (log_type != kArchivedLogFile) {
-      ROCKS_LOG_ERROR(immutable_db_options_.info_log,
-                      "DeleteFile %s failed - not archived log.\n",
-                      name.c_str());
-      return Status::NotSupported("Delete only supported for archived logs");
-    }
-    Status status = wal_manager_.DeleteFile(name, number);
-    if (!status.ok()) {
-      ROCKS_LOG_ERROR(immutable_db_options_.info_log,
-                      "DeleteFile %s failed -- %s.\n", name.c_str(),
-                      status.ToString().c_str());
-    }
-    return status;
-  }
-
-  Status status;
-  int level;
-  FileMetaData* metadata;
-  ColumnFamilyData* cfd;
-  VersionEdit edit;
-  JobContext job_context(next_job_id_.fetch_add(1), true);
-  {
-    InstrumentedMutexLock l(&mutex_);
-    status = versions_->GetMetadataForFile(number, &level, &metadata, &cfd);
-    if (!status.ok()) {
-      ROCKS_LOG_WARN(immutable_db_options_.info_log,
-                     "DeleteFile %s failed. File not found\n", name.c_str());
-      job_context.Clean();
-      return Status::InvalidArgument("File not found");
-    }
-    assert(level < cfd->NumberLevels());
-
-    // If the file is being compacted no need to delete.
-    if (metadata->being_compacted) {
-      ROCKS_LOG_INFO(immutable_db_options_.info_log,
-                     "DeleteFile %s Skipped. File about to be compacted\n",
-                     name.c_str());
-      job_context.Clean();
-      return Status::OK();
-    }
-
-    // Only the files in the last level can be deleted externally.
-    // This is to make sure that any deletion tombstones are not
-    // lost. Check that the level passed is the last level.
-    auto* vstoreage = cfd->current()->storage_info();
-    for (int i = level + 1; i < cfd->NumberLevels(); i++) {
-      if (vstoreage->NumLevelFiles(i) != 0) {
-        ROCKS_LOG_WARN(immutable_db_options_.info_log,
-                       "DeleteFile %s FAILED. File not in last level\n",
-                       name.c_str());
-        job_context.Clean();
-        return Status::InvalidArgument("File not in last level");
-      }
-    }
-    // if level == 0, it has to be the oldest file
-    if (level == 0 &&
-        vstoreage->LevelFiles(0).back()->fd.GetNumber() != number) {
-      ROCKS_LOG_WARN(immutable_db_options_.info_log,
-                     "DeleteFile %s failed ---"
-                     " target file in level 0 must be the oldest.",
-                     name.c_str());
-      job_context.Clean();
-      return Status::InvalidArgument("File in level 0, but not oldest");
-    }
-    edit.SetColumnFamily(cfd->GetID());
-    edit.DeleteFile(level, number);
-    status = versions_->LogAndApply(cfd, read_options, write_options, &edit,
-                                    &mutex_, directories_.GetDbDir());
-    if (status.ok()) {
-      InstallSuperVersionAndScheduleWork(
-          cfd, job_context.superversion_contexts.data());
-    }
-    FindObsoleteFiles(&job_context, false);
-  }  // lock released here
-
-  LogFlush(immutable_db_options_.info_log);
-  // remove files outside the db-lock
-  if (job_context.HaveSomethingToDelete()) {
-    // Call PurgeObsoleteFiles() without holding mutex.
-    PurgeObsoleteFiles(job_context);
-  }
-  job_context.Clean();
-  return status;
-}
-
 Status DBImpl::DeleteFilesInRanges(ColumnFamilyHandle* column_family,
                                    const RangePtr* ranges, size_t n,
                                    bool include_end) {
@@ -6054,6 +5986,7 @@ Status DBImpl::IngestExternalFiles(
 
     num_running_ingest_file_ += static_cast<int>(num_cfs);
     TEST_SYNC_POINT("DBImpl::IngestExternalFile:AfterIncIngestFileCounter");
+    TEST_SYNC_POINT("DBImpl::IngestExternalFile:AfterIncIngestFileCounter:2");
 
     bool at_least_one_cf_need_flush = false;
     std::vector<bool> need_flush(num_cfs, false);

db/db_impl/db_impl.h

@@ -162,15 +162,21 @@ class Directories {
   std::unique_ptr<FSDirectory> wal_dir_;
 };
 
-struct DBOpenLogReporter : public log::Reader::Reporter {
+struct DBOpenLogRecordReadReporter : public log::Reader::Reporter {
   Env* env;
   Logger* info_log;
   const char* fname;
   Status* status;  // nullptr if immutable_db_options_.paranoid_checks==false
   bool* old_log_record;
-  void Corruption(size_t bytes, const Status& s) override;
+  void Corruption(size_t bytes, const Status& s,
+                  uint64_t log_number = kMaxSequenceNumber) override;
 
   void OldLogRecord(size_t bytes) override;
+
+  uint64_t GetCorruptedLogNumber() const { return corrupted_log_number_; }
+
+ private:
+  uint64_t corrupted_log_number_ = kMaxSequenceNumber;
 };
 
 // While DB is the public interface of RocksDB, and DBImpl is the actual
@@ -535,7 +541,6 @@ class DBImpl : public DB {
       SequenceNumber seq_number, std::unique_ptr<TransactionLogIterator>* iter,
       const TransactionLogIterator::ReadOptions& read_options =
           TransactionLogIterator::ReadOptions()) override;
-  Status DEPRECATED_DeleteFile(std::string name) override;
   Status DeleteFilesInRanges(ColumnFamilyHandle* column_family,
                              const RangePtr* ranges, size_t n,
                              bool include_end = true);
@@ -950,13 +955,6 @@ class DBImpl : public DB {
     return num_running_flushes_;
   }
 
-  // Returns the number of currently running compactions.
-  // REQUIREMENT: mutex_ must be held when calling this function.
-  int num_running_compactions() {
-    mutex_.AssertHeld();
-    return num_running_compactions_;
-  }
-
   const WriteController& write_controller() { return write_controller_; }
 
   // hollow transactions shell used for recovery.
@@ -2075,21 +2073,25 @@ class DBImpl : public DB {
 
   void SetupLogFileProcessing(uint64_t wal_number);
 
-  Status InitializeLogReader(
-      uint64_t wal_number, bool is_retry, std::string& fname,
+  Status InitializeLogReader(uint64_t wal_number, bool is_retry,
+                             std::string& fname,
 
-      bool stop_replay_for_corruption, uint64_t min_wal_number,
-      const PredecessorWALInfo& predecessor_wal_info,
-      bool* const old_log_record, Status* const reporter_status,
-      DBOpenLogReporter* reporter, std::unique_ptr<log::Reader>& reader);
+                             bool stop_replay_for_corruption,
+                             uint64_t min_wal_number,
+                             const PredecessorWALInfo& predecessor_wal_info,
+                             bool* const old_log_record,
+                             Status* const reporter_status,
+                             DBOpenLogRecordReadReporter* reporter,
+                             std::unique_ptr<log::Reader>& reader);
   Status ProcessLogRecord(
       Slice record, const std::unique_ptr<log::Reader>& reader,
       const UnorderedMap<uint32_t, size_t>& running_ts_sz, uint64_t wal_number,
       const std::string& fname, bool read_only, int job_id,
-      std::function<void()> logFileDropped, DBOpenLogReporter* reporter,
-      uint64_t* record_checksum, SequenceNumber* last_seqno_observed,
-      SequenceNumber* next_sequence, bool* stop_replay_for_corruption,
-      Status* status, bool* stop_replay_by_wal_filter,
+      const std::function<void()>& logFileDropped,
+      DBOpenLogRecordReadReporter* reporter, uint64_t* record_checksum,
+      SequenceNumber* last_seqno_observed, SequenceNumber* next_sequence,
+      bool* stop_replay_for_corruption, Status* status,
+      bool* stop_replay_by_wal_filter,
       std::unordered_map<int, VersionEdit>* version_edits, bool* flushed);
 
   Status InitializeWriteBatchForLogRecord(
@@ -2114,9 +2116,9 @@ class DBImpl : public DB {
 
   Status HandleNonOkStatusOrOldLogRecord(
       uint64_t wal_number, SequenceNumber const* const next_sequence,
-      Status log_read_status, bool* old_log_record,
-      bool* stop_replay_for_corruption, uint64_t* corrupted_wal_number,
-      bool* corrupted_wal_found);
+      Status status, const DBOpenLogRecordReadReporter& reporter,
+      bool* old_log_record, bool* stop_replay_for_corruption,
+      uint64_t* corrupted_wal_number, bool* corrupted_wal_found);
 
   Status UpdatePredecessorWALInfo(uint64_t wal_number,
                                   const SequenceNumber last_seqno_observed,

db/db_impl/db_impl_compaction_flush.cc

@@ -3522,6 +3522,7 @@ void DBImpl::BackgroundCallCompaction(PrepickedCompaction* prepicked_compaction,
   }
 }
 
+// Precondition: mutex_ must be held when calling this function.
 Status DBImpl::BackgroundCompaction(bool* made_progress,
                                     JobContext* job_context,
                                     LogBuffer* log_buffer,

db/db_impl/db_impl_open.cc

@@ -224,6 +224,12 @@ Status DBImpl::ValidateOptions(
     if (!s.ok()) {
       return s;
     }
+    if (cfd.name == kDefaultColumnFamilyName) {
+      if (cfd.options.disallow_memtable_writes) {
+        return Status::InvalidArgument(
+            "Default column family cannot use disallow_memtable_writes=true");
+      }
+    }
   }
   s = ValidateOptions(db_options);
   return s;
@@ -1106,16 +1112,18 @@ bool DBImpl::InvokeWalFilterIfNeededOnWalRecord(uint64_t wal_number,
   return true;
 }
 
-void DBOpenLogReporter::Corruption(size_t bytes, const Status& s) {
+void DBOpenLogRecordReadReporter::Corruption(size_t bytes, const Status& s,
+                                             uint64_t log_number) {
   ROCKS_LOG_WARN(info_log, "%s%s: dropping %d bytes; %s",
                  (status == nullptr ? "(ignoring error) " : ""), fname,
                  static_cast<int>(bytes), s.ToString().c_str());
   if (status != nullptr && status->ok()) {
     *status = s;
+    corrupted_log_number_ = log_number;
   }
 }
 
-void DBOpenLogReporter::OldLogRecord(size_t bytes) {
+void DBOpenLogRecordReadReporter::OldLogRecord(size_t bytes) {
   if (old_log_record != nullptr) {
     *old_log_record = true;
   }
@@ -1229,7 +1237,7 @@ Status DBImpl::ProcessLogFile(
   Status status;
   bool old_log_record = false;
 
-  DBOpenLogReporter reporter;
+  DBOpenLogRecordReadReporter reporter;
   std::unique_ptr<log::Reader> reader;
 
   std::string fname =
@@ -1323,7 +1331,7 @@ Status DBImpl::ProcessLogFile(
   }
 
   ROCKS_LOG_INFO(immutable_db_options_.info_log,
-                 "Recovered to log #%" PRIu64 " seq #%" PRIu64, wal_number,
+                 "Recovered to log #%" PRIu64 " next seq #%" PRIu64, wal_number,
                  *next_sequence);
 
   if (status.ok()) {
@@ -1333,7 +1341,7 @@ Status DBImpl::ProcessLogFile(
 
   if (!status.ok() || old_log_record) {
     status = HandleNonOkStatusOrOldLogRecord(
-        wal_number, next_sequence, status, &old_log_record,
+        wal_number, next_sequence, status, reporter, &old_log_record,
         stop_replay_for_corruption, corrupted_wal_number, corrupted_wal_found);
   }
 
@@ -1357,7 +1365,7 @@ Status DBImpl::InitializeLogReader(
     uint64_t wal_number, bool is_retry, std::string& fname,
     bool stop_replay_for_corruption, uint64_t min_wal_number,
     const PredecessorWALInfo& predecessor_wal_info, bool* const old_log_record,
-    Status* const reporter_status, DBOpenLogReporter* reporter,
+    Status* const reporter_status, DBOpenLogRecordReadReporter* reporter,
     std::unique_ptr<log::Reader>& reader) {
   assert(old_log_record);
   assert(reporter_status);
@@ -1408,10 +1416,11 @@ Status DBImpl::ProcessLogRecord(
     Slice record, const std::unique_ptr<log::Reader>& reader,
     const UnorderedMap<uint32_t, size_t>& running_ts_sz, uint64_t wal_number,
     const std::string& fname, bool read_only, int job_id,
-    std::function<void()> logFileDropped, DBOpenLogReporter* reporter,
-    uint64_t* record_checksum, SequenceNumber* last_seqno_observed,
-    SequenceNumber* next_sequence, bool* stop_replay_for_corruption,
-    Status* status, bool* stop_replay_by_wal_filter,
+    const std::function<void()>& logFileDropped,
+    DBOpenLogRecordReadReporter* reporter, uint64_t* record_checksum,
+    SequenceNumber* last_seqno_observed, SequenceNumber* next_sequence,
+    bool* stop_replay_for_corruption, Status* status,
+    bool* stop_replay_by_wal_filter,
     std::unordered_map<int, VersionEdit>* version_edits, bool* flushed) {
   assert(reporter);
   assert(last_seqno_observed);
@@ -1607,7 +1616,8 @@ Status DBImpl::MaybeWriteLevel0TableForRecovery(
 
 Status DBImpl::HandleNonOkStatusOrOldLogRecord(
     uint64_t wal_number, SequenceNumber const* const next_sequence,
-    Status status, bool* old_log_record, bool* stop_replay_for_corruption,
+    Status status, const DBOpenLogRecordReadReporter& reporter,
+    bool* old_log_record, bool* stop_replay_for_corruption,
     uint64_t* corrupted_wal_number, bool* corrupted_wal_found) {
   assert(!status.ok() || *old_log_record);
 
@@ -1641,7 +1651,12 @@ Status DBImpl::HandleNonOkStatusOrOldLogRecord(
     // We should ignore the error but not continue replaying
     *old_log_record = false;
     *stop_replay_for_corruption = true;
-    *corrupted_wal_number = wal_number;
+    // TODO(hx235): have a single source of corrupted WAL number once we
+    // consolidate the statuses
+    uint64_t reporter_corrupted_wal_number = reporter.GetCorruptedLogNumber();
+    *corrupted_wal_number = reporter_corrupted_wal_number != kMaxSequenceNumber
+                                ? reporter_corrupted_wal_number
+                                : wal_number;
     if (corrupted_wal_found != nullptr) {
       *corrupted_wal_found = true;
     }

db/db_impl/db_impl_readonly.cc

@@ -32,7 +32,8 @@ Status DBImplReadOnly::GetImpl(const ReadOptions& read_options,
                                const Slice& key,
                                GetImplOptions& get_impl_options) {
   assert(get_impl_options.value != nullptr ||
-         get_impl_options.columns != nullptr);
+         get_impl_options.columns != nullptr ||
+         get_impl_options.merge_operands != nullptr);
   assert(get_impl_options.column_family);
 
   Status s;
@@ -86,7 +87,11 @@ Status DBImplReadOnly::GetImpl(const ReadOptions& read_options,
       return s;
     }
   }
+  // Prepare to store a list of merge operations if merge occurs.
   MergeContext merge_context;
+  // TODO - Large Result Optimization for Read Only DB
+  // (https://github.com/facebook/rocksdb/pull/10458)
+
   SequenceNumber max_covering_tombstone_seq = 0;
   LookupKey lkey(key, snapshot, read_options.timestamp);
   PERF_TIMER_STOP(get_snapshot_time);
@@ -97,7 +102,8 @@ Status DBImplReadOnly::GetImpl(const ReadOptions& read_options,
           get_impl_options.value ? get_impl_options.value->GetSelf() : nullptr,
           get_impl_options.columns, ts, &s, &merge_context,
           &max_covering_tombstone_seq, read_options,
-          false /* immutable_memtable */, &read_cb)) {
+          false /* immutable_memtable */, &read_cb,
+          /*is_blob_index=*/nullptr, /*do_merge=*/get_impl_options.get_value)) {
     if (get_impl_options.value) {
       get_impl_options.value->PinSelf();
     }
@@ -111,7 +117,7 @@ Status DBImplReadOnly::GetImpl(const ReadOptions& read_options,
         /*value_found*/ nullptr,
         /*key_exists*/ nullptr, /*seq*/ nullptr, &read_cb,
         /*is_blob*/ nullptr,
-        /*do_merge*/ true);
+        /*do_merge=*/get_impl_options.get_value);
     RecordTick(stats_, MEMTABLE_MISS);
   }
   {
@@ -121,6 +127,14 @@ Status DBImplReadOnly::GetImpl(const ReadOptions& read_options,
       size = get_impl_options.value->size();
     } else if (get_impl_options.columns) {
       size = get_impl_options.columns->serialized_size();
+    } else if (get_impl_options.merge_operands) {
+      *get_impl_options.number_of_operands =
+          static_cast<int>(merge_context.GetNumOperands());
+      for (const Slice& sl : merge_context.GetOperands()) {
+        size += sl.size();
+        get_impl_options.merge_operands->PinSelf(sl);
+        get_impl_options.merge_operands++;
+      }
     }
     RecordTick(stats_, BYTES_READ, size);
     RecordInHistogram(stats_, BYTES_PER_READ, size);

db/db_impl/db_impl_secondary.cc

@@ -342,7 +342,8 @@ Status DBImplSecondary::GetImpl(const ReadOptions& read_options,
                                 const Slice& key,
                                 GetImplOptions& get_impl_options) {
   assert(get_impl_options.value != nullptr ||
-         get_impl_options.columns != nullptr);
+         get_impl_options.columns != nullptr ||
+         get_impl_options.merge_operands != nullptr);
   assert(get_impl_options.column_family);
 
   Status s;
@@ -397,37 +398,64 @@ Status DBImplSecondary::GetImpl(const ReadOptions& read_options,
     }
   }
   MergeContext merge_context;
+  // TODO - Large Result Optimization for Secondary DB
+  // (https://github.com/facebook/rocksdb/pull/10458)
+
   SequenceNumber max_covering_tombstone_seq = 0;
   LookupKey lkey(key, snapshot, read_options.timestamp);
   PERF_TIMER_STOP(get_snapshot_time);
   bool done = false;
 
   // Look up starts here
-  if (super_version->mem->Get(
-          lkey,
-          get_impl_options.value ? get_impl_options.value->GetSelf() : nullptr,
-          get_impl_options.columns, ts, &s, &merge_context,
-          &max_covering_tombstone_seq, read_options,
-          false /* immutable_memtable */, &read_cb)) {
-    done = true;
-    if (get_impl_options.value) {
-      get_impl_options.value->PinSelf();
+  if (get_impl_options.get_value) {
+    if (super_version->mem->Get(
+            lkey,
+            get_impl_options.value ? get_impl_options.value->GetSelf()
+                                   : nullptr,
+            get_impl_options.columns, ts, &s, &merge_context,
+            &max_covering_tombstone_seq, read_options,
+            false /* immutable_memtable */, &read_cb,
+            /*is_blob_index=*/nullptr, /*do_merge=*/true)) {
+      done = true;
+      if (get_impl_options.value) {
+        get_impl_options.value->PinSelf();
+      }
+      RecordTick(stats_, MEMTABLE_HIT);
+    } else if ((s.ok() || s.IsMergeInProgress()) &&
+               super_version->imm->Get(
+                   lkey,
+                   get_impl_options.value ? get_impl_options.value->GetSelf()
+                                          : nullptr,
+                   get_impl_options.columns, ts, &s, &merge_context,
+                   &max_covering_tombstone_seq, read_options, &read_cb)) {
+      done = true;
+      if (get_impl_options.value) {
+        get_impl_options.value->PinSelf();
+      }
+      RecordTick(stats_, MEMTABLE_HIT);
     }
-    RecordTick(stats_, MEMTABLE_HIT);
-  } else if ((s.ok() || s.IsMergeInProgress()) &&
-             super_version->imm->Get(
-                 lkey,
-                 get_impl_options.value ? get_impl_options.value->GetSelf()
-                                        : nullptr,
-                 get_impl_options.columns, ts, &s, &merge_context,
-                 &max_covering_tombstone_seq, read_options, &read_cb)) {
-    done = true;
-    if (get_impl_options.value) {
-      get_impl_options.value->PinSelf();
+  } else {
+    // GetMergeOperands
+    if (super_version->mem->Get(
+            lkey,
+            get_impl_options.value ? get_impl_options.value->GetSelf()
+                                   : nullptr,
+            get_impl_options.columns, ts, &s, &merge_context,
+            &max_covering_tombstone_seq, read_options,
+            false /* immutable_memtable */, &read_cb,
+            /*is_blob_index=*/nullptr, /*do_merge=*/false)) {
+      done = true;
+      RecordTick(stats_, MEMTABLE_HIT);
+    } else if ((s.ok() || s.IsMergeInProgress()) &&
+               super_version->imm->GetMergeOperands(lkey, &s, &merge_context,
+                                                    &max_covering_tombstone_seq,
+                                                    read_options)) {
+      done = true;
+      RecordTick(stats_, MEMTABLE_HIT);
     }
-    RecordTick(stats_, MEMTABLE_HIT);
   }
-  if (!done && !s.ok() && !s.IsMergeInProgress()) {
+  if (!s.ok() && !s.IsMergeInProgress() && !s.IsNotFound()) {
+    assert(done);
     ReturnAndCleanupSuperVersion(cfd, super_version);
     return s;
   }
@@ -451,6 +479,14 @@ Status DBImplSecondary::GetImpl(const ReadOptions& read_options,
       size = get_impl_options.value->size();
     } else if (get_impl_options.columns) {
       size = get_impl_options.columns->serialized_size();
+    } else if (get_impl_options.merge_operands) {
+      *get_impl_options.number_of_operands =
+          static_cast<int>(merge_context.GetNumOperands());
+      for (const Slice& sl : merge_context.GetOperands()) {
+        size += sl.size();
+        get_impl_options.merge_operands->PinSelf(sl);
+        get_impl_options.merge_operands++;
+      }
     }
     RecordTick(stats_, BYTES_READ, size);
     RecordTimeToHistogram(stats_, BYTES_PER_READ, size);
@@ -877,6 +913,10 @@ Status DBImplSecondary::CompactWithoutInstallation(
   Status s = cfd->compaction_picker()->GetCompactionInputsFromFileNumbers(
       &input_files, &input_set, vstorage, comp_options);
   if (!s.ok()) {
+    ROCKS_LOG_ERROR(
+        immutable_db_options_.info_log,
+        "GetCompactionInputsFromFileNumbers() failed - %s.\n DebugString: %s",
+        s.ToString().c_str(), version->DebugString().c_str());
     return s;
   }
 
@@ -954,6 +994,7 @@ Status DB::OpenAndCompact(
   DBOptions db_options;
   ConfigOptions config_options;
   config_options.env = override_options.env;
+  config_options.ignore_unknown_options = true;
   std::vector<ColumnFamilyDescriptor> all_column_families;
 
   TEST_SYNC_POINT_CALLBACK(

db/db_impl/db_impl_secondary.h

@@ -52,7 +52,8 @@ class LogReaderContainer {
     Logger* info_log;
     std::string fname;
     Status* status;  // nullptr if immutable_db_options_.paranoid_checks==false
-    void Corruption(size_t bytes, const Status& s) override {
+    void Corruption(size_t bytes, const Status& s,
+                    uint64_t /*log_number*/ = kMaxSequenceNumber) override {
       ROCKS_LOG_WARN(info_log, "%s%s: dropping %d bytes; %s",
                      (this->status == nullptr ? "(ignoring error) " : ""),
                      fname.c_str(), static_cast<int>(bytes),

db/db_impl/db_impl_write.cc

@@ -205,7 +205,7 @@ Status DBImpl::IngestWBWI(std::shared_ptr<WriteBatchWithIndex> wbwi,
   ColumnFamilySet* cf_set = versions_->GetColumnFamilySet();
 
   // Create WBWIMemTables
-  for (const auto [cf_id, stat] : wbwi->GetCFStats()) {
+  for (const auto& [cf_id, stat] : wbwi->GetCFStats()) {
     ColumnFamilyData* cfd = cf_set->GetColumnFamily(cf_id);
     if (!cfd) {
       if (ignore_missing_cf) {
@@ -587,6 +587,7 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options,
 
   IOStatus io_s;
   Status pre_release_cb_status;
+  size_t seq_inc = 0;
   if (status.ok()) {
     // Rules for when we can update the memtable concurrently
     // 1. supported by memtable
@@ -640,7 +641,7 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options,
     // disable_memtable in between; although we do not write this batch to
     // memtable it still consumes a seq. Otherwise, if !seq_per_batch_, we inc
     // the seq per valid written key to mem.
-    size_t seq_inc = seq_per_batch_ ? valid_batches : total_count;
+    seq_inc = seq_per_batch_ ? valid_batches : total_count;
     if (wbwi) {
       // Reserve sequence numbers for the ingested memtable. We need to reserve
       // at lease this amount for recovery. During recovery,
@@ -713,6 +714,7 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options,
     assert(last_sequence != kMaxSequenceNumber);
     const SequenceNumber current_sequence = last_sequence + 1;
     last_sequence += seq_inc;
+    // Seqno assigned to this write are [current_sequence, last_sequence]
 
     if (log_context.need_log_sync) {
       VersionEdit synced_wals;
@@ -836,13 +838,17 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options,
     if (status.ok() && w.status.ok()) {
       // w.batch contains (potentially empty) commit time batch updates,
       // only ingest wbwi if w.batch is applied to memtable successfully
-      assert(wbwi->GetWriteBatch()->Count() > 0);
-
       uint32_t memtable_update_count = w.batch->Count();
-      SequenceNumber lb = versions_->LastSequence() + memtable_update_count + 1;
-      SequenceNumber ub = versions_->LastSequence() + memtable_update_count +
-                          wbwi->GetWriteBatch()->Count();
-      assert(ub == last_sequence);
+      uint32_t wbwi_count = wbwi->GetWriteBatch()->Count();
+      // Seqno assigned to this write are [last_seq + 1 - seq_inc, last_seq].
+      // seq_inc includes w.batch (memtable updates) and wbwi
+      // w.batch gets first `memtable_update_count` sequence numbers.
+      // wbwi gets the rest `wbwi_count` sequence numbers.
+      assert(seq_inc == memtable_update_count + wbwi_count);
+      assert(wbwi_count > 0);
+      assert(last_sequence != kMaxSequenceNumber);
+      SequenceNumber lb = last_sequence + 1 - wbwi_count;
+      SequenceNumber ub = last_sequence;
       if (two_write_queues_) {
         assert(ub <= versions_->LastAllocatedSequence());
       }

db/db_iterator_test.cc

@@ -2544,6 +2544,145 @@ TEST_P(DBIteratorTest, RefreshWithSnapshot) {
   ASSERT_OK(db_->Close());
 }
 
+TEST_P(DBIteratorTest, AutoRefreshIterator) {
+  constexpr int kNumKeys = 1000;
+  Options options = CurrentOptions();
+  options.disable_auto_compactions = true;
+  for (const DBIter::Direction direction :
+       {DBIter::kForward, DBIter::kReverse}) {
+    for (const bool auto_refresh_enabled : {false, true}) {
+      for (const bool explicit_snapshot : {false, true}) {
+        DestroyAndReopen(options);
+        // Multi dimensional iterator:
+        //
+        // L0 (level iterator): [key000000]
+        // L1 (table iterator): [key000001]
+        // Memtable           : [key000000, key000999]
+        for (int i = 0; i < kNumKeys + 2; i++) {
+          ASSERT_OK(Put(Key(i % kNumKeys), "val" + std::to_string(i)));
+          if (i <= 1) {
+            ASSERT_OK(Flush());
+          }
+          if (i == 0) {
+            MoveFilesToLevel(1);
+          }
+        }
+
+        ReadOptions read_options;
+        std::unique_ptr<ManagedSnapshot> snapshot = nullptr;
+        if (explicit_snapshot) {
+          snapshot = std::make_unique<ManagedSnapshot>(db_);
+        }
+        read_options.snapshot =
+            explicit_snapshot ? snapshot->snapshot() : nullptr;
+        read_options.auto_refresh_iterator_with_snapshot = auto_refresh_enabled;
+        std::unique_ptr<Iterator> iter(NewIterator(read_options));
+
+        int trigger_compact_on_it = kNumKeys / 2;
+
+        // This update should NOT be visible from the iterator.
+        ASSERT_OK(Put(Key(trigger_compact_on_it + 1), "new val"));
+
+        ASSERT_EQ(1, NumTableFilesAtLevel(1));
+        ASSERT_EQ(1, NumTableFilesAtLevel(0));
+
+        uint64_t all_memtables_size_before_refresh;
+        uint64_t all_memtables_size_after_refresh;
+
+        std::string prop_value;
+        ASSERT_OK(iter->GetProperty("rocksdb.iterator.super-version-number",
+                                    &prop_value));
+        int superversion_number = std::stoi(prop_value);
+
+        std::vector<LiveFileMetaData> old_files;
+        db_->GetLiveFilesMetaData(&old_files);
+
+        int expected_next_key_int;
+        if (direction == DBIter::kForward) {
+          expected_next_key_int = 0;
+          iter->SeekToFirst();
+        } else {  // DBIter::kReverse
+          expected_next_key_int = kNumKeys - 1;
+          iter->SeekToLast();
+        }
+
+        int it_num = 0;
+        std::unordered_map<std::string, std::string> kvs;
+        while (iter->Valid()) {
+          ASSERT_OK(iter->status());
+          it_num++;
+          if (it_num == trigger_compact_on_it) {
+            // Bump the superversion by manually scheduling flush + compaction.
+            ASSERT_OK(Flush());
+            ASSERT_OK(dbfull()->CompactRange(CompactRangeOptions(), nullptr,
+                                             nullptr));
+            ASSERT_OK(dbfull()->TEST_WaitForBackgroundWork());
+
+            // For accuracy, capture the memtables size right before consecutive
+            // iterator call to Next() will update its' stale superversion ref.
+            dbfull()->GetIntProperty("rocksdb.size-all-mem-tables",
+                                     &all_memtables_size_before_refresh);
+          }
+
+          if (it_num == trigger_compact_on_it + 1) {
+            dbfull()->GetIntProperty("rocksdb.size-all-mem-tables",
+                                     &all_memtables_size_after_refresh);
+            ASSERT_OK(iter->GetProperty("rocksdb.iterator.super-version-number",
+                                        &prop_value));
+            uint64_t new_superversion_number = std::stoi(prop_value);
+            Status expected_status_for_preexisting_files;
+            if (auto_refresh_enabled && explicit_snapshot) {
+              // Iterator is expected to detect its' superversion staleness.
+              ASSERT_LT(superversion_number, new_superversion_number);
+              // ... and since our iterator was the only reference to that very
+              // superversion, we expect most of the active memory to be
+              // returned upon automatical iterator refresh.
+              ASSERT_GT(all_memtables_size_before_refresh,
+                        all_memtables_size_after_refresh);
+              expected_status_for_preexisting_files = Status::NotFound();
+            } else {
+              ASSERT_EQ(superversion_number, new_superversion_number);
+              ASSERT_EQ(all_memtables_size_after_refresh,
+                        all_memtables_size_before_refresh);
+              expected_status_for_preexisting_files = Status::OK();
+            }
+
+            for (const auto& file : old_files) {
+              ASSERT_EQ(env_->FileExists(file.db_path + "/" + file.name),
+                        expected_status_for_preexisting_files);
+            }
+          }
+
+          // Ensure we're visiting the keys in desired order and at most once!
+          ASSERT_EQ(IdFromKey(iter->key().ToString()), expected_next_key_int);
+          kvs[iter->key().ToString()] = iter->value().ToString();
+
+          if (direction == DBIter::kForward) {
+            iter->Next();
+            expected_next_key_int++;
+          } else {
+            iter->Prev();
+            expected_next_key_int--;
+          }
+        }
+        ASSERT_OK(iter->status());
+
+        // Data validation.
+        ASSERT_EQ(kvs.size(), kNumKeys);
+        for (int i = 0; i < kNumKeys; i++) {
+          auto kv = kvs.find(Key(i));
+          ASSERT_TRUE(kv != kvs.end());
+          int val = i;
+          if (i <= 1) {
+            val += kNumKeys;
+          }
+          ASSERT_EQ(kv->second, "val" + std::to_string(val));
+        }
+      }
+    }
+  }
+}
+
 TEST_P(DBIteratorTest, CreationFailure) {
   SyncPoint::GetInstance()->SetCallBack(
       "DBImpl::NewInternalIterator:StatusCallback", [](void* arg) {

db/db_merge_operand_test.cc

@@ -120,8 +120,11 @@ TEST_F(DBMergeOperandTest, FlushedMergeOperandReadAfterFreeBug) {
 
 TEST_F(DBMergeOperandTest, GetMergeOperandsBasic) {
   Options options = CurrentOptions();
+
+  int limit = 2;
   // Use only the latest two merge operands.
-  options.merge_operator = std::make_shared<LimitedStringAppendMergeOp>(2, ',');
+  options.merge_operator =
+      std::make_shared<LimitedStringAppendMergeOp>(limit, ',');
   Reopen(options);
   int num_records = 4;
   int number_of_operands = 0;
@@ -263,6 +266,7 @@ TEST_F(DBMergeOperandTest, GetMergeOperandsBasic) {
   ASSERT_OK(db_->GetMergeOperands(ReadOptions(), db_->DefaultColumnFamily(),
                                   "k3.2", values.data(), &merge_operands_info,
                                   &number_of_operands));
+  ASSERT_EQ(number_of_operands, 2);
   ASSERT_EQ(values[0], "cd");
   ASSERT_EQ(values[1], "de");
 
@@ -280,6 +284,7 @@ TEST_F(DBMergeOperandTest, GetMergeOperandsBasic) {
   ASSERT_OK(db_->GetMergeOperands(ReadOptions(), db_->DefaultColumnFamily(),
                                   "k4", values.data(), &merge_operands_info,
                                   &number_of_operands));
+  ASSERT_EQ(number_of_operands, 4);
   ASSERT_EQ(values[0], "ba");
   ASSERT_EQ(values[1], "cb");
   ASSERT_EQ(values[2], "dc");
@@ -299,9 +304,26 @@ TEST_F(DBMergeOperandTest, GetMergeOperandsBasic) {
   ASSERT_OK(db_->GetMergeOperands(ReadOptions(), db_->DefaultColumnFamily(),
                                   "k5", values.data(), &merge_operands_info,
                                   &number_of_operands));
+  ASSERT_EQ(number_of_operands, 4);
   ASSERT_EQ(values[0], "remember");
   ASSERT_EQ(values[1], "i");
   ASSERT_EQ(values[2], "am");
+  ASSERT_EQ(values[3], "rocks");
+
+  // GetMergeOperands() in ReadOnly DB
+  ASSERT_OK(Merge("k6", "better"));
+  ASSERT_OK(Merge("k6", "call"));
+  ASSERT_OK(Merge("k6", "saul"));
+
+  ASSERT_OK(ReadOnlyReopen(options));
+  std::vector<PinnableSlice> readonly_values(num_records);
+  ASSERT_OK(db_->GetMergeOperands(ReadOptions(), db_->DefaultColumnFamily(),
+                                  "k6", readonly_values.data(),
+                                  &merge_operands_info, &number_of_operands));
+  ASSERT_EQ(number_of_operands, 3);
+  ASSERT_EQ(readonly_values[0], "better");
+  ASSERT_EQ(readonly_values[1], "call");
+  ASSERT_EQ(readonly_values[2], "saul");
 }
 
 TEST_F(DBMergeOperandTest, BlobDBGetMergeOperandsBasic) {
@@ -577,6 +599,42 @@ TEST_F(DBMergeOperandTest, GetMergeOperandsBaseDeletionInImmMem) {
   }
 }
 
+TEST_F(DBMergeOperandTest, GetMergeOperandCallbackStopAtImm) {
+  Options options = CurrentOptions();
+  options.max_write_buffer_number = 10;
+  options.merge_operator = MergeOperators::CreateStringAppendOperator();
+  DestroyAndReopen(options);
+
+  Random rnd(301);
+  ASSERT_OK(db_->PauseBackgroundWork());
+  ASSERT_OK(Merge("key", "v1"));
+  ASSERT_OK(dbfull()->TEST_SwitchMemtable());
+  // Keep this merge in an immutable memtable
+  uint64_t num_imm = 0;
+  ASSERT_TRUE(
+      db_->GetIntProperty(DB::Properties::kNumImmutableMemTable, &num_imm));
+  ASSERT_EQ(num_imm, 1);
+  ASSERT_OK(Merge("key", "v2"));
+
+  std::vector<PinnableSlice> merge_operands(2);
+  GetMergeOperandsOptions merge_operands_info;
+  merge_operands_info.expected_max_number_of_operands = 2;
+  int num_fetched = 0;
+  merge_operands_info.continue_cb = [&num_fetched](Slice /* value */) {
+    num_fetched++;
+    // Stop in the first immutable memtable.
+    return num_fetched < 2;
+  };
+  int num_merge_operands = 0;
+  ASSERT_OK(db_->GetMergeOperands(ReadOptions(), db_->DefaultColumnFamily(),
+                                  "key", merge_operands.data(),
+                                  &merge_operands_info, &num_merge_operands));
+  ASSERT_EQ(2, num_merge_operands);
+  ASSERT_EQ(2, num_fetched);
+
+  ASSERT_EQ("v1", merge_operands[0]);
+  ASSERT_EQ("v2", merge_operands[1]);
+}
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

db/db_merge_operator_test.cc

@@ -120,6 +120,10 @@ TEST_F(DBMergeOperatorTest, LimitMergeOperands) {
   ASSERT_OK(Merge("k3", "de"));
   ASSERT_OK(db_->Get(ReadOptions(), "k3", &value));
   ASSERT_EQ(value, "cd,de");
+  // Tests that merge operands reach exact limit at memtable.
+  ASSERT_OK(Merge("k3", "fg"));
+  ASSERT_OK(db_->Get(ReadOptions(), "k3", &value));
+  ASSERT_EQ(value, "de,fg");
 
   // All K4 values are in different levels
   ASSERT_OK(Merge("k4", "ab"));

db/db_options_test.cc

@@ -1595,6 +1595,69 @@ TEST_F(DBOptionsTest, TempOptionsFailTest) {
   ASSERT_FALSE(found_temp_file);
 }
 
+TEST_F(DBOptionsTest, SetOptionsNoManifestWrite) {
+  ASSERT_OK(Put("x", "x"));
+  ASSERT_OK(Flush());
+
+  // In addition to checking manifest file, we want to ensure that SetOptions
+  // is essentially atomic, without releasing the DB mutex between applying
+  // the options to the cfd and installing new Version and SuperVersion. We
+  // probabilistically verify that by attempting to catch an inconsistency.
+  auto* const cfd =
+      static_cast<ColumnFamilyHandleImpl*>(db_->DefaultColumnFamily())->cfd();
+  SyncPoint::GetInstance()->DisableProcessing();
+  SyncPoint::GetInstance()->ClearAllCallBacks();
+  std::optional<std::thread> t;
+  SyncPoint::GetInstance()->SetCallBack(
+      "VersionSet::LogAndApply:WakeUpAndNotDone", [&](void* arg) {
+        auto* mu = static_cast<InstrumentedMutex*>(arg);
+        // Option not yet modified
+        ASSERT_FALSE(cfd->GetLatestMutableCFOptions().disable_auto_compactions);
+        ASSERT_FALSE(
+            cfd->current()->GetMutableCFOptions().disable_auto_compactions);
+        ASSERT_FALSE(
+            cfd->GetCurrentMutableCFOptions().disable_auto_compactions);
+        t = std::thread([mu, cfd]() {
+          InstrumentedMutexLock l(mu);
+          // Assuming above correctness, we can only acquire the mutex after
+          // options fully installed.
+          ASSERT_TRUE(
+              cfd->GetLatestMutableCFOptions().disable_auto_compactions);
+          ASSERT_TRUE(
+              cfd->current()->GetMutableCFOptions().disable_auto_compactions);
+          ASSERT_TRUE(
+              cfd->GetCurrentMutableCFOptions().disable_auto_compactions);
+        });
+      });
+  SyncPoint::GetInstance()->EnableProcessing();
+
+  // Baseline manifest file info
+  std::vector<std::string> live_files;
+  uint64_t orig_manifest_file_size;
+  ASSERT_OK(dbfull()->GetLiveFiles(live_files, &orig_manifest_file_size));
+  uint64_t orig_manifest_file_num = dbfull()->TEST_Current_Manifest_FileNo();
+
+  // Although this test mostly concerns SetOptions, we also include SetDBOptions
+  // just for the added scope
+  ASSERT_OK(db_->SetDBOptions({{"max_open_files", "100"}}));
+  ASSERT_OK(db_->SetOptions({{"disable_auto_compactions", "true"}}));
+
+  // Verify that our above check was activated and completed
+  ASSERT_TRUE(t.has_value());
+  t->join();
+  SyncPoint::GetInstance()->DisableProcessing();
+  SyncPoint::GetInstance()->ClearAllCallBacks();
+
+  // Verify manifest was not written to
+  uint64_t new_manifest_file_size;
+  ASSERT_OK(dbfull()->GetLiveFiles(live_files, &new_manifest_file_size));
+  uint64_t new_manifest_file_num = dbfull()->TEST_Current_Manifest_FileNo();
+  ASSERT_EQ(orig_manifest_file_num, new_manifest_file_num);
+  ASSERT_EQ(orig_manifest_file_size, new_manifest_file_size);
+
+  ASSERT_EQ(Get("x"), "x");
+}
+
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

db/db_secondary_test.cc

@@ -14,10 +14,9 @@
 #include "rocksdb/utilities/transaction_db.h"
 #include "test_util/sync_point.h"
 #include "test_util/testutil.h"
-#include "utilities/fault_injection_env.h"
+#include "utilities/merge_operators/string_append/stringappend2.h"
 
 namespace ROCKSDB_NAMESPACE {
-
 class DBSecondaryTestBase : public DBBasicTestWithTimestampBase {
  public:
   explicit DBSecondaryTestBase(const std::string& dbname)
@@ -331,6 +330,43 @@ TEST_F(DBSecondaryTest, InternalCompactionMultiLevels) {
   //  cfh, input1, &result));
 }
 
+TEST_F(DBSecondaryTest, GetMergeOperands) {
+  Options options;
+  options.merge_operator = MergeOperators::CreateStringAppendOperator();
+  options.env = env_;
+  Reopen(options);
+
+  ASSERT_OK(Merge("k1", "v1"));
+  ASSERT_OK(Merge("k1", "v2"));
+  ASSERT_OK(Merge("k1", "v3"));
+  ASSERT_OK(Merge("k1", "v4"));
+
+  options.max_open_files = -1;
+  OpenSecondary(options);
+
+  ASSERT_OK(db_secondary_->TryCatchUpWithPrimary());
+
+  int num_records = 4;
+  int number_of_operands = 0;
+  std::vector<PinnableSlice> values(num_records);
+  GetMergeOperandsOptions merge_operands_info;
+  merge_operands_info.expected_max_number_of_operands = num_records;
+
+  auto cfh = db_secondary_->DefaultColumnFamily();
+
+  const Status s = db_secondary_->GetMergeOperands(
+      ReadOptions(), cfh, "k1", values.data(), &merge_operands_info,
+      &number_of_operands);
+  ASSERT_NOK(s);
+  ASSERT_TRUE(s.IsMergeInProgress());
+
+  ASSERT_EQ(number_of_operands, 4);
+  ASSERT_EQ(values[0].ToString(), "v1");
+  ASSERT_EQ(values[1].ToString(), "v2");
+  ASSERT_EQ(values[2].ToString(), "v3");
+  ASSERT_EQ(values[3].ToString(), "v4");
+}
+
 TEST_F(DBSecondaryTest, InternalCompactionCompactedFiles) {
   Options options;
   options.env = env_;

db/db_test.cc

@@ -3409,10 +3409,6 @@ class ModelDB : public DB {
     return Status::NotSupported();
   }
 
-  Status DEPRECATED_DeleteFile(std::string /*name*/) override {
-    return Status::OK();
-  }
-
   Status GetUpdatesSince(
       ROCKSDB_NAMESPACE::SequenceNumber,
       std::unique_ptr<ROCKSDB_NAMESPACE::TransactionLogIterator>*,
@@ -5294,7 +5290,6 @@ TEST_F(DBTest, DynamicLevelCompressionPerLevel) {
   for (int i = 0; i < kNKeys; i++) {
     keys[i] = i;
   }
-  RandomShuffle(std::begin(keys), std::end(keys));
 
   Random rnd(301);
   Options options;
@@ -5319,7 +5314,7 @@ TEST_F(DBTest, DynamicLevelCompressionPerLevel) {
   options.compression_per_level[0] = kNoCompression;
   // No compression for the Ln whre L0 is compacted to
   options.compression_per_level[1] = kNoCompression;
-  // Snpapy compression for Ln+1
+  // Snappy compression for Ln+1
   options.compression_per_level[2] = kSnappyCompression;
 
   OnFileDeletionListener* listener = new OnFileDeletionListener();
@@ -5331,6 +5326,7 @@ TEST_F(DBTest, DynamicLevelCompressionPerLevel) {
   // be compressed, so there shouldn't be any compression.
   for (int i = 0; i < 20; i++) {
     ASSERT_OK(Put(Key(keys[i]), CompressibleString(&rnd, 4000)));
+    ASSERT_OK(dbfull()->TEST_WaitForBackgroundWork());
   }
   ASSERT_OK(Flush());
   ASSERT_OK(dbfull()->TEST_WaitForCompact());
@@ -5349,8 +5345,9 @@ TEST_F(DBTest, DynamicLevelCompressionPerLevel) {
   // above compression settings for each level, there will be some compression.
   ASSERT_OK(options.statistics->Reset());
   ASSERT_EQ(num_block_compressed, 0);
-  for (int i = 21; i < 120; i++) {
+  for (int i = 20; i < 120; i++) {
     ASSERT_OK(Put(Key(keys[i]), CompressibleString(&rnd, 4000)));
+    ASSERT_OK(dbfull()->TEST_WaitForBackgroundWork());
   }
   ASSERT_OK(Flush());
   ASSERT_OK(dbfull()->TEST_WaitForCompact());
@@ -5371,9 +5368,45 @@ TEST_F(DBTest, DynamicLevelCompressionPerLevel) {
   }));
   ColumnFamilyMetaData cf_meta;
   db_->GetColumnFamilyMetaData(&cf_meta);
+
+  // Ensure that L1+ files are non-overlapping and together with L0 encompass
+  // full key range between smallestkey and largestkey from CF file metadata.
+  int largestkey_in_prev_level = -1;
+  int keys_found = 0;
+  for (int level = (int)cf_meta.levels.size() - 1; level >= 0; level--) {
+    int files_in_level = (int)cf_meta.levels[level].files.size();
+    int largestkey_in_prev_file = -1;
+    for (int j = 0; j < files_in_level; j++) {
+      int smallestkey = IdFromKey(cf_meta.levels[level].files[j].smallestkey);
+      int largestkey = IdFromKey(cf_meta.levels[level].files[j].largestkey);
+      int num_entries = (int)cf_meta.levels[level].files[j].num_entries;
+      ASSERT_EQ(num_entries, largestkey - smallestkey + 1);
+      keys_found += num_entries;
+      if (level > 0) {
+        if (j == 0) {
+          ASSERT_GT(smallestkey, largestkey_in_prev_level);
+        }
+        if (j > 0) {
+          ASSERT_GT(smallestkey, largestkey_in_prev_file);
+        }
+        if (j == files_in_level - 1) {
+          largestkey_in_prev_level = largestkey;
+        }
+      }
+      largestkey_in_prev_file = largestkey;
+    }
+  }
+  ASSERT_EQ(keys_found, kNKeys);
+
   for (const auto& file : cf_meta.levels[4].files) {
     listener->SetExpectedFileName(dbname_ + file.name);
-    ASSERT_OK(dbfull()->DEPRECATED_DeleteFile(file.name));
+    Slice start(file.smallestkey), limit(file.largestkey);
+    const RangePtr ranges(&start, &limit);
+    // Given verification from above, we're guaranteed that by deleting all the
+    // files in [<smallestkey>, <largestkey>] range, we're effectively deleting
+    // that very single file and nothing more.
+    EXPECT_OK(dbfull()->DeleteFilesInRanges(dbfull()->DefaultColumnFamily(),
+                                            &ranges, true /* include_end */));
   }
   listener->VerifyMatchedCount(cf_meta.levels[4].files.size());
 

db/db_test2.cc

@@ -1475,8 +1475,7 @@ TEST_P(PresetCompressionDictTest, Flush) {
     // TODO(ajkr): fix the below assertion to work with ZSTD. The expectation on
     // number of bytes needs to be adjusted in case the cached block is in
     // ZSTD's digested dictionary format.
-    if (compression_type_ != kZSTD &&
-        compression_type_ != kZSTDNotFinalCompression) {
+    if (compression_type_ != kZSTD) {
       // Although we limited buffering to `kBlockLen`, there may be up to two
       // blocks of data included in the dictionary since we only check limit
       // after each block is built.
@@ -1553,8 +1552,7 @@ TEST_P(PresetCompressionDictTest, CompactNonBottommost) {
     // TODO(ajkr): fix the below assertion to work with ZSTD. The expectation on
     // number of bytes needs to be adjusted in case the cached block is in
     // ZSTD's digested dictionary format.
-    if (compression_type_ != kZSTD &&
-        compression_type_ != kZSTDNotFinalCompression) {
+    if (compression_type_ != kZSTD) {
       // Although we limited buffering to `kBlockLen`, there may be up to two
       // blocks of data included in the dictionary since we only check limit
       // after each block is built.
@@ -1615,8 +1613,7 @@ TEST_P(PresetCompressionDictTest, CompactBottommost) {
   // TODO(ajkr): fix the below assertion to work with ZSTD. The expectation on
   // number of bytes needs to be adjusted in case the cached block is in ZSTD's
   // digested dictionary format.
-  if (compression_type_ != kZSTD &&
-      compression_type_ != kZSTDNotFinalCompression) {
+  if (compression_type_ != kZSTD) {
     // Although we limited buffering to `kBlockLen`, there may be up to two
     // blocks of data included in the dictionary since we only check limit after
     // each block is built.
@@ -8000,7 +7997,7 @@ TEST_F(DBTest2, GetLatestSeqAndTsForKey) {
   ASSERT_EQ(0, options.statistics->getTickerCount(GET_HIT_L0));
 }
 
-#if defined(ZSTD_ADVANCED)
+#if defined(ZSTD)
 TEST_F(DBTest2, ZSTDChecksum) {
   // Verify that corruption during decompression is caught.
   Options options = CurrentOptions();

db/db_test_util.h

@@ -1106,6 +1106,11 @@ class DBTestBase : public testing::Test {
     return std::string(buf);
   }
 
+  // Expects valid key created by Key().
+  static int IdFromKey(const std::string& key) {
+    return std::stoi(key.substr(3));
+  }
+
   static bool ShouldSkipOptions(int option_config, int skip_mask = kNoSkip);
 
   // Switch to a fresh database with the next option configuration to

db/db_wal_test.cc

@@ -1153,6 +1153,7 @@ TEST_F(DBWALTest, FullPurgePreservesRecycledLog) {
     dbfull()->FindObsoleteFiles(&job_context, true /* force */);
     dbfull()->TEST_UnlockMutex();
     dbfull()->PurgeObsoleteFiles(job_context);
+    job_context.Clean();
 
     if (i == 0) {
       ASSERT_OK(

db/db_with_timestamp_basic_test.cc

@@ -1480,6 +1480,55 @@ TEST_F(DBBasicTestWithTimestamp, ReseekToUserKeyBeforeSavedKey) {
   Close();
 }
 
+TEST_F(DBBasicTestWithTimestamp,
+       FIXME_ReverseIterationWithBlobAndUnpreparedValue) {
+  Options options = CurrentOptions();
+  options.create_if_missing = true;
+  options.env = env_;
+  options.enable_blob_files = true;
+  options.max_sequential_skip_in_iterations = 0;
+
+  const size_t kTimestampSize = Timestamp(0, 0).size();
+  TestComparator test_cmp(kTimestampSize);
+  options.comparator = &test_cmp;
+
+  DestroyAndReopen(options);
+
+  constexpr uint64_t kMaxKey = 1024;
+
+  const std::vector<std::string> write_timestamps = {Timestamp(1, 0),
+                                                     Timestamp(3, 0)};
+  for (uint64_t key = 0; key <= kMaxKey; ++key) {
+    for (size_t i = 0; i < write_timestamps.size(); ++i) {
+      ASSERT_OK(db_->Put(WriteOptions(), Key1(key), write_timestamps[i],
+                         "value" + std::to_string(i)));
+    }
+  }
+
+  ASSERT_OK(Flush());
+
+  {
+    const std::string read_timestamp_str = Timestamp(4, 0);
+    const Slice read_timestamp(read_timestamp_str);
+
+    ReadOptions read_opts;
+    read_opts.timestamp = &read_timestamp;
+    read_opts.allow_unprepared_value = true;
+
+    std::unique_ptr<Iterator> it(db_->NewIterator(read_opts));
+
+    it->SeekForPrev(Key1(kMaxKey));
+    ASSERT_TRUE(it->Valid());
+    ASSERT_OK(it->status());
+
+    // FIXME: PrepareValue() should succeed and status() should remain OK
+    ASSERT_FALSE(it->PrepareValue());
+    ASSERT_TRUE(it->status().IsCorruption());
+  }
+
+  Close();
+}
+
 TEST_F(DBBasicTestWithTimestamp, MultiGetWithFastLocalBloom) {
   Options options = CurrentOptions();
   options.env = env_;

db/deletefile_test.cc

@@ -135,57 +135,6 @@ class DeleteFileTest : public DBTestBase {
   }
 };
 
-TEST_F(DeleteFileTest, AddKeysAndQueryLevels) {
-  Options options = CurrentOptions();
-  SetOptions(&options);
-  Destroy(options);
-  options.create_if_missing = true;
-  Reopen(options);
-
-  CreateTwoLevels();
-  std::vector<LiveFileMetaData> metadata;
-  db_->GetLiveFilesMetaData(&metadata);
-
-  std::string level1file;
-  int level1keycount = 0;
-  std::string level2file;
-  int level2keycount = 0;
-  int level1index = 0;
-  int level2index = 1;
-
-  ASSERT_EQ((int)metadata.size(), 2);
-  if (metadata[0].level == 2) {
-    level1index = 1;
-    level2index = 0;
-  }
-
-  level1file = metadata[level1index].name;
-  int startkey = atoi(metadata[level1index].smallestkey.c_str());
-  int endkey = atoi(metadata[level1index].largestkey.c_str());
-  level1keycount = (endkey - startkey + 1);
-  level2file = metadata[level2index].name;
-  startkey = atoi(metadata[level2index].smallestkey.c_str());
-  endkey = atoi(metadata[level2index].largestkey.c_str());
-  level2keycount = (endkey - startkey + 1);
-
-  // COntrolled setup. Levels 1 and 2 should both have 50K files.
-  // This is a little fragile as it depends on the current
-  // compaction heuristics.
-  ASSERT_EQ(level1keycount, 50000);
-  ASSERT_EQ(level2keycount, 50000);
-
-  Status status = db_->DEPRECATED_DeleteFile("0.sst");
-  ASSERT_TRUE(status.IsInvalidArgument());
-
-  // intermediate level files cannot be deleted.
-  status = db_->DEPRECATED_DeleteFile(level1file);
-  ASSERT_TRUE(status.IsInvalidArgument());
-
-  // Lowest level file deletion should succeed.
-  status = db_->DEPRECATED_DeleteFile(level2file);
-  ASSERT_OK(status);
-}
-
 TEST_F(DeleteFileTest, PurgeObsoleteFilesTest) {
   Options options = CurrentOptions();
   SetOptions(&options);
@@ -496,145 +445,6 @@ TEST_F(DeleteFileTest, BackgroundPurgeTestMultipleJobs) {
   CheckFileTypeCounts(dbname_, 0, 1, 1);
 }
 
-TEST_F(DeleteFileTest, DeleteFileWithIterator) {
-  Options options = CurrentOptions();
-  SetOptions(&options);
-  Destroy(options);
-  options.create_if_missing = true;
-  Reopen(options);
-
-  CreateTwoLevels();
-  ReadOptions read_options;
-  Iterator* it = db_->NewIterator(read_options);
-  ASSERT_OK(it->status());
-  std::vector<LiveFileMetaData> metadata;
-  db_->GetLiveFilesMetaData(&metadata);
-
-  std::string level2file;
-
-  ASSERT_EQ(metadata.size(), static_cast<size_t>(2));
-  if (metadata[0].level == 1) {
-    level2file = metadata[1].name;
-  } else {
-    level2file = metadata[0].name;
-  }
-
-  Status status = db_->DEPRECATED_DeleteFile(level2file);
-  fprintf(stdout, "Deletion status %s: %s\n", level2file.c_str(),
-          status.ToString().c_str());
-  ASSERT_OK(status);
-  it->SeekToFirst();
-  int numKeysIterated = 0;
-  while (it->Valid()) {
-    numKeysIterated++;
-    it->Next();
-  }
-  ASSERT_EQ(numKeysIterated, 50000);
-  delete it;
-}
-
-TEST_F(DeleteFileTest, DeleteLogFiles) {
-  Options options = CurrentOptions();
-  SetOptions(&options);
-  Destroy(options);
-  options.create_if_missing = true;
-  Reopen(options);
-
-  AddKeys(10, 0);
-  VectorLogPtr logfiles;
-  ASSERT_OK(db_->GetSortedWalFiles(logfiles));
-  ASSERT_GT(logfiles.size(), 0UL);
-  // Take the last log file which is expected to be alive and try to delete it
-  // Should not succeed because live logs are not allowed to be deleted
-  std::unique_ptr<LogFile> alive_log = std::move(logfiles.back());
-  ASSERT_EQ(alive_log->Type(), kAliveLogFile);
-  ASSERT_OK(env_->FileExists(wal_dir_ + "/" + alive_log->PathName()));
-  fprintf(stdout, "Deleting alive log file %s\n",
-          alive_log->PathName().c_str());
-  ASSERT_NOK(db_->DEPRECATED_DeleteFile(alive_log->PathName()));
-  ASSERT_OK(env_->FileExists(wal_dir_ + "/" + alive_log->PathName()));
-  logfiles.clear();
-
-  // Call Flush to bring about a new working log file and add more keys
-  // Call Flush again to flush out memtable and move alive log to archived log
-  // and try to delete the archived log file
-  FlushOptions fopts;
-  ASSERT_OK(db_->Flush(fopts));
-  AddKeys(10, 0);
-  ASSERT_OK(db_->Flush(fopts));
-  ASSERT_OK(db_->GetSortedWalFiles(logfiles));
-  ASSERT_GT(logfiles.size(), 0UL);
-  std::unique_ptr<LogFile> archived_log = std::move(logfiles.front());
-  ASSERT_EQ(archived_log->Type(), kArchivedLogFile);
-  ASSERT_OK(env_->FileExists(wal_dir_ + "/" + archived_log->PathName()));
-  fprintf(stdout, "Deleting archived log file %s\n",
-          archived_log->PathName().c_str());
-  ASSERT_OK(db_->DEPRECATED_DeleteFile(archived_log->PathName()));
-  ASSERT_TRUE(
-      env_->FileExists(wal_dir_ + "/" + archived_log->PathName()).IsNotFound());
-}
-
-TEST_F(DeleteFileTest, DeleteNonDefaultColumnFamily) {
-  Options options = CurrentOptions();
-  SetOptions(&options);
-  Destroy(options);
-  options.create_if_missing = true;
-  Reopen(options);
-  CreateAndReopenWithCF({"new_cf"}, options);
-
-  Random rnd(5);
-  for (int i = 0; i < 1000; ++i) {
-    ASSERT_OK(db_->Put(WriteOptions(), handles_[1], test::RandomKey(&rnd, 10),
-                       test::RandomKey(&rnd, 10)));
-  }
-  ASSERT_OK(db_->Flush(FlushOptions(), handles_[1]));
-  for (int i = 0; i < 1000; ++i) {
-    ASSERT_OK(db_->Put(WriteOptions(), handles_[1], test::RandomKey(&rnd, 10),
-                       test::RandomKey(&rnd, 10)));
-  }
-  ASSERT_OK(db_->Flush(FlushOptions(), handles_[1]));
-
-  std::vector<LiveFileMetaData> metadata;
-  db_->GetLiveFilesMetaData(&metadata);
-  ASSERT_EQ(2U, metadata.size());
-  ASSERT_EQ("new_cf", metadata[0].column_family_name);
-  ASSERT_EQ("new_cf", metadata[1].column_family_name);
-  auto old_file = metadata[0].smallest_seqno < metadata[1].smallest_seqno
-                      ? metadata[0].name
-                      : metadata[1].name;
-  auto new_file = metadata[0].smallest_seqno > metadata[1].smallest_seqno
-                      ? metadata[0].name
-                      : metadata[1].name;
-  ASSERT_TRUE(db_->DEPRECATED_DeleteFile(new_file).IsInvalidArgument());
-  ASSERT_OK(db_->DEPRECATED_DeleteFile(old_file));
-
-  {
-    std::unique_ptr<Iterator> itr(db_->NewIterator(ReadOptions(), handles_[1]));
-    ASSERT_OK(itr->status());
-    int count = 0;
-    for (itr->SeekToFirst(); itr->Valid(); itr->Next()) {
-      ASSERT_OK(itr->status());
-      ++count;
-    }
-    ASSERT_OK(itr->status());
-    ASSERT_EQ(count, 1000);
-  }
-
-  Close();
-  ReopenWithColumnFamilies({kDefaultColumnFamilyName, "new_cf"}, options);
-
-  {
-    std::unique_ptr<Iterator> itr(db_->NewIterator(ReadOptions(), handles_[1]));
-    int count = 0;
-    for (itr->SeekToFirst(); itr->Valid(); itr->Next()) {
-      ASSERT_OK(itr->status());
-      ++count;
-    }
-    ASSERT_OK(itr->status());
-    ASSERT_EQ(count, 1000);
-  }
-}
-
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

db/experimental.cc

@@ -74,7 +74,8 @@ Status GetFileChecksumsFromCurrentManifest(FileSystem* fs,
 
   struct LogReporter : public log::Reader::Reporter {
     Status* status_ptr;
-    void Corruption(size_t /*bytes*/, const Status& st) override {
+    void Corruption(size_t /*bytes*/, const Status& st,
+                    uint64_t /*log_number*/ = kMaxSequenceNumber) override {
       if (status_ptr->ok()) {
         *status_ptr = st;
       }

db/external_sst_file_basic_test.cc

@@ -265,8 +265,8 @@ TEST_F(ExternalSSTFileBasicTest, Basic) {
   SyncPoint::GetInstance()->LoadDependency({
       {"DBImpl::IngestExternalFile:AfterIncIngestFileCounter",
        "ExternalSSTFileBasicTest.LiveWriteStart"},
-      {"ExternalSSTFileBasicTest.LiveWriteStart",
-       "DBImpl::IngestExternalFiles:InstallSVForFirstCF:0"},
+      {"WriteThread::JoinBatchGroup:Wait",
+       "DBImpl::IngestExternalFile:AfterIncIngestFileCounter:2"},
   });
   SyncPoint::GetInstance()->EnableProcessing();
   PerfContext* write_thread_perf_context;
@@ -2669,51 +2669,83 @@ TEST_F(ExternalSSTFileBasicTest, IngestWithTemperature) {
   }
 }
 
-TEST_F(ExternalSSTFileBasicTest, FailIfNotBottommostLevel) {
-  Options options = GetDefaultOptions();
+TEST_F(ExternalSSTFileBasicTest, FailIfNotBottommostLevelAndDisallowMemtable) {
+  for (bool disallow_memtable : {false, true}) {
+    Options options = GetDefaultOptions();
 
-  std::string file_path = sst_files_dir_ + std::to_string(1);
-  SstFileWriter sfw(EnvOptions(), options);
+    // First test with universal compaction
+    options.create_if_missing = true;
+    options.compaction_style = CompactionStyle::kCompactionStyleUniversal;
+    DestroyAndReopen(options);
 
-  ASSERT_OK(sfw.Open(file_path));
-  ASSERT_OK(sfw.Put("b", "dontcare"));
-  ASSERT_OK(sfw.Finish());
+    // And a CF potentially disallowing memtable write
+    options.disallow_memtable_writes = disallow_memtable;
+    CreateColumnFamilies({"cf0"}, options);
+    ASSERT_EQ(db_->GetOptions(handles_[0]).disallow_memtable_writes,
+              disallow_memtable);
 
-  // Test universal compaction + ingest with snapshot consistency
-  options.create_if_missing = true;
-  options.compaction_style = CompactionStyle::kCompactionStyleUniversal;
-  DestroyAndReopen(options);
-  {
-    const Snapshot* snapshot = db_->GetSnapshot();
-    ManagedSnapshot snapshot_guard(db_, snapshot);
-    IngestExternalFileOptions ifo;
-    ifo.fail_if_not_bottommost_level = true;
-    ifo.snapshot_consistency = true;
-    const Status s = db_->IngestExternalFile({file_path}, ifo);
-    ASSERT_TRUE(s.ok());
-  }
+    // Ingest with snapshot consistency
+    std::string file_path = sst_files_dir_ + std::to_string(1);
+    SstFileWriter sfw(EnvOptions(), options);
 
-  // Test level compaction
-  options.compaction_style = CompactionStyle::kCompactionStyleLevel;
-  options.num_levels = 2;
-  DestroyAndReopen(options);
-  ASSERT_OK(db_->Put(WriteOptions(), "a", "dontcare"));
-  ASSERT_OK(db_->Put(WriteOptions(), "c", "dontcare"));
-  ASSERT_OK(db_->Flush(FlushOptions()));
+    ASSERT_OK(sfw.Open(file_path));
+    ASSERT_OK(sfw.Put("b", "dontcare"));
+    ASSERT_OK(sfw.Finish());
 
-  ASSERT_OK(db_->Put(WriteOptions(), "b", "dontcare"));
-  ASSERT_OK(db_->Put(WriteOptions(), "d", "dontcare"));
-  ASSERT_OK(db_->Flush(FlushOptions()));
+    {
+      const Snapshot* snapshot = db_->GetSnapshot();
+      ManagedSnapshot snapshot_guard(db_, snapshot);
+      IngestExternalFileOptions ifo;
+      ifo.fail_if_not_bottommost_level = true;
+      ifo.snapshot_consistency = true;
+      ASSERT_OK(db_->IngestExternalFile(handles_[0], {file_path}, ifo));
+    }
 
-  {
-    CompactRangeOptions cro;
-    cro.bottommost_level_compaction = BottommostLevelCompaction::kForce;
-    ASSERT_OK(db_->CompactRange(cro, nullptr, nullptr));
+    // Test level compaction
+    options.compaction_style = CompactionStyle::kCompactionStyleLevel;
+    options.num_levels = 2;
+    CreateColumnFamilies({"cf1"}, options);
+    ASSERT_EQ(db_->GetOptions(handles_[1]).disallow_memtable_writes,
+              disallow_memtable);
 
-    IngestExternalFileOptions ifo;
-    ifo.fail_if_not_bottommost_level = true;
-    const Status s = db_->IngestExternalFile({file_path}, ifo);
-    ASSERT_TRUE(s.IsTryAgain());
+    if (!disallow_memtable) {
+      ASSERT_OK(Put(1, "a", "1"));
+      ASSERT_OK(Put(1, "c", "3"));
+      ASSERT_OK(Flush(1));
+
+      ASSERT_OK(Put(1, "b", "2"));
+      ASSERT_OK(Put(1, "d", "4"));
+      ASSERT_OK(Flush(1));
+    } else {
+      // Memtable write disallowed
+      EXPECT_EQ(Put(1, "a", "1").code(), Status::Code::kInvalidArgument);
+
+      // Use ingestion to get to the same state as above
+      std::string file_path2 = sst_files_dir_ + std::to_string(2);
+
+      ASSERT_OK(sfw.Open(file_path2));
+      ASSERT_OK(sfw.Put("a", "1"));
+      ASSERT_OK(sfw.Put("c", "3"));
+      ASSERT_OK(sfw.Finish());
+      ASSERT_OK(db_->IngestExternalFile(handles_[1], {file_path2}, {}));
+
+      ASSERT_OK(sfw.Open(file_path2));
+      ASSERT_OK(sfw.Put("b", "2"));
+      ASSERT_OK(sfw.Put("d", "4"));
+      ASSERT_OK(sfw.Finish());
+      ASSERT_OK(db_->IngestExternalFile(handles_[1], {file_path2}, {}));
+    }
+
+    {
+      CompactRangeOptions cro;
+      cro.bottommost_level_compaction = BottommostLevelCompaction::kForce;
+      ASSERT_OK(db_->CompactRange(cro, handles_[1], nullptr, nullptr));
+
+      IngestExternalFileOptions ifo;
+      ifo.fail_if_not_bottommost_level = true;
+      const Status s = db_->IngestExternalFile(handles_[1], {file_path}, ifo);
+      ASSERT_TRUE(s.IsTryAgain());
+    }
   }
 }
 

db/internal_stats.cc

@@ -301,6 +301,8 @@ static const std::string aggregated_table_properties =
 static const std::string aggregated_table_properties_at_level =
     aggregated_table_properties + "-at-level";
 static const std::string num_running_compactions = "num-running-compactions";
+static const std::string num_running_compaction_sorted_runs =
+    "num-running-compaction-sorted-runs";
 static const std::string num_running_flushes = "num-running-flushes";
 static const std::string actual_delayed_write_rate =
     "actual-delayed-write-rate";
@@ -351,6 +353,8 @@ const std::string DB::Properties::kCompactionPending =
     rocksdb_prefix + compaction_pending;
 const std::string DB::Properties::kNumRunningCompactions =
     rocksdb_prefix + num_running_compactions;
+const std::string DB::Properties::kNumRunningCompactionSortedRuns =
+    rocksdb_prefix + num_running_compaction_sorted_runs;
 const std::string DB::Properties::kNumRunningFlushes =
     rocksdb_prefix + num_running_flushes;
 const std::string DB::Properties::kBackgroundErrors =
@@ -580,6 +584,9 @@ const UnorderedMap<std::string, DBPropertyInfo>
         {DB::Properties::kNumRunningCompactions,
          {false, nullptr, &InternalStats::HandleNumRunningCompactions, nullptr,
           nullptr}},
+        {DB::Properties::kNumRunningCompactionSortedRuns,
+         {false, nullptr, &InternalStats::HandleNumRunningCompactionSortedRuns,
+          nullptr, nullptr}},
         {DB::Properties::kActualDelayedWriteRate,
          {false, nullptr, &InternalStats::HandleActualDelayedWriteRate, nullptr,
           nullptr}},
@@ -636,6 +643,7 @@ InternalStats::InternalStats(int num_levels, SystemClock* clock,
       file_read_latency_(num_levels),
       has_cf_change_since_dump_(true),
       bg_error_count_(0),
+      num_running_compaction_sorted_runs_(0),
       number_levels_(num_levels),
       clock_(clock),
       cfd_(cfd),
@@ -1265,6 +1273,18 @@ bool InternalStats::HandleNumRunningCompactions(uint64_t* value, DBImpl* db,
   return true;
 }
 
+bool InternalStats::HandleNumRunningCompactionSortedRuns(uint64_t* value,
+                                                         DBImpl* db,
+                                                         Version* /*version*/) {
+  db->mutex()->AssertHeld();
+  uint64_t sorted_runs = 0;
+  for (auto* cfd : *db->versions_->GetColumnFamilySet()) {
+    sorted_runs += cfd->internal_stats()->NumRunningCompactionSortedRuns();
+  }
+  *value = sorted_runs;
+  return true;
+}
+
 bool InternalStats::HandleBackgroundErrors(uint64_t* value, DBImpl* /*db*/,
                                            Version* /*version*/) {
   // Accumulated number of  errors in background flushes or compactions.

db/internal_stats.h

@@ -604,6 +604,20 @@ class InternalStats {
     ++cf_stats_count_[type];
   }
 
+  void IncrNumRunningCompactionSortedRuns(uint64_t value) {
+    num_running_compaction_sorted_runs_.fetch_add(value,
+                                                  std::memory_order_relaxed);
+  }
+
+  void DecrNumRunningCompactionSortedRuns(uint64_t value) {
+    num_running_compaction_sorted_runs_.fetch_sub(value,
+                                                  std::memory_order_relaxed);
+  }
+
+  uint64_t NumRunningCompactionSortedRuns() {
+    return num_running_compaction_sorted_runs_.load(std::memory_order_relaxed);
+  }
+
   void AddDBStats(InternalDBStatsType type, uint64_t value,
                   bool concurrent = false) {
     auto& v = db_stats_[type];
@@ -847,6 +861,8 @@ class InternalStats {
   bool HandleCompactionPending(uint64_t* value, DBImpl* db, Version* version);
   bool HandleNumRunningCompactions(uint64_t* value, DBImpl* db,
                                    Version* version);
+  bool HandleNumRunningCompactionSortedRuns(uint64_t* value, DBImpl* db,
+                                            Version* version);
   bool HandleBackgroundErrors(uint64_t* value, DBImpl* db, Version* version);
   bool HandleCurSizeActiveMemTable(uint64_t* value, DBImpl* db,
                                    Version* version);
@@ -921,6 +937,12 @@ class InternalStats {
   // or compaction will cause the counter to increase too.
   uint64_t bg_error_count_;
 
+  // This is a rolling count of the number of sorted runs being processed by
+  // currently running compactions. Other metrics are only incremented, but this
+  // metric is also decremented. Additionally, we also do not want to reset this
+  // count to zero at a periodic interval.
+  std::atomic<uint64_t> num_running_compaction_sorted_runs_;
+
   const int number_levels_;
   SystemClock* clock_;
   ColumnFamilyData* cfd_;

db/listener_test.cc

@@ -1285,7 +1285,9 @@ class BlobDBJobLevelEventListenerTest : public EventListener {
     ColumnFamilyData* const cfd = versions->GetColumnFamilySet()->GetDefault();
     EXPECT_NE(cfd, nullptr);
 
+    test_->dbfull()->TEST_LockMutex();
     Version* const current = cfd->current();
+    test_->dbfull()->TEST_UnlockMutex();
     EXPECT_NE(current, nullptr);
 
     const VersionStorageInfo* const storage_info = current->storage_info();
@@ -1325,10 +1327,9 @@ class BlobDBJobLevelEventListenerTest : public EventListener {
   }
 
   void OnFlushCompleted(DB* /*db*/, const FlushJobInfo& info) override {
-    call_count_++;
-
     {
       std::lock_guard<std::mutex> lock(mutex_);
+      IncreaseCallCount(/*mutex_locked*/ true);
       flushed_files_.push_back(info.file_path);
     }
 
@@ -1339,7 +1340,7 @@ class BlobDBJobLevelEventListenerTest : public EventListener {
 
   void OnCompactionCompleted(DB* /*db*/,
                              const CompactionJobInfo& info) override {
-    call_count_++;
+    IncreaseCallCount(/*mutex_locked*/ false);
 
     EXPECT_EQ(info.blob_compression_type, kNoCompression);
 
@@ -1355,12 +1356,31 @@ class BlobDBJobLevelEventListenerTest : public EventListener {
     }
   }
 
+  void IncreaseCallCount(bool mutex_locked) {
+    if (!mutex_locked) {
+      std::lock_guard<std::mutex> lock(mutex_);
+      call_count_++;
+    } else {
+      call_count_++;
+    }
+  }
+
+  uint32_t GetCallCount() {
+    std::lock_guard<std::mutex> lock(mutex_);
+    return call_count_;
+  }
+
+  void ResetCallCount() {
+    std::lock_guard<std::mutex> lock(mutex_);
+    call_count_ = 0;
+  }
+
   EventListenerTest* test_;
-  uint32_t call_count_;
 
  private:
-  std::vector<std::string> flushed_files_;
   std::mutex mutex_;
+  std::vector<std::string> flushed_files_;
+  uint32_t call_count_;
 };
 
 // Test OnFlushCompleted EventListener called for blob files
@@ -1389,7 +1409,7 @@ TEST_F(EventListenerTest, BlobDBOnFlushCompleted) {
   ASSERT_EQ(Get("Key2"), "blob_value2");
   ASSERT_EQ(Get("Key3"), "blob_value3");
 
-  ASSERT_GT(blob_event_listener->call_count_, 0U);
+  ASSERT_GT(blob_event_listener->GetCallCount(), 0U);
 }
 
 // Test OnCompactionCompleted EventListener called for blob files
@@ -1423,7 +1443,7 @@ TEST_F(EventListenerTest, BlobDBOnCompactionCompleted) {
   ASSERT_OK(Put("Key6", "blob_value6"));
   ASSERT_OK(Flush());
 
-  blob_event_listener->call_count_ = 0;
+  blob_event_listener->ResetCallCount();
   constexpr Slice* begin = nullptr;
   constexpr Slice* end = nullptr;
 
@@ -1432,7 +1452,7 @@ TEST_F(EventListenerTest, BlobDBOnCompactionCompleted) {
   ASSERT_OK(db_->CompactRange(CompactRangeOptions(), begin, end));
 
   // Make sure, OnCompactionCompleted is called.
-  ASSERT_GT(blob_event_listener->call_count_, 0U);
+  ASSERT_GT(blob_event_listener->GetCallCount(), 0U);
 }
 
 // Test CompactFiles calls OnCompactionCompleted EventListener for blob files

db/log_reader.cc

@@ -374,14 +374,16 @@ void Reader::MaybeVerifyPredecessorWALInfo(
     if (recorded_predecessor_log_number >= min_wal_number_to_keep_) {
       std::string reason = "Missing WAL of log number " +
                            std::to_string(recorded_predecessor_log_number);
-      ReportCorruption(fragment.size(), reason.c_str());
+      ReportCorruption(fragment.size(), reason.c_str(),
+                       recorded_predecessor_log_number);
     }
   } else {
     if (observed_predecessor_wal_info_.GetLogNumber() !=
         recorded_predecessor_log_number) {
       std::string reason = "Missing WAL of log number " +
                            std::to_string(recorded_predecessor_log_number);
-      ReportCorruption(fragment.size(), reason.c_str());
+      ReportCorruption(fragment.size(), reason.c_str(),
+                       recorded_predecessor_log_number);
     } else if (observed_predecessor_wal_info_.GetLastSeqnoRecorded() !=
                recorded_predecessor_wal_info.GetLastSeqnoRecorded()) {
       std::string reason =
@@ -392,7 +394,8 @@ void Reader::MaybeVerifyPredecessorWALInfo(
           std::to_string(
               observed_predecessor_wal_info_.GetLastSeqnoRecorded()) +
           ". (Last sequence number equal to 0 indicates no WAL records)";
-      ReportCorruption(fragment.size(), reason.c_str());
+      ReportCorruption(fragment.size(), reason.c_str(),
+                       recorded_predecessor_log_number);
     } else if (observed_predecessor_wal_info_.GetSizeBytes() !=
                recorded_predecessor_wal_info.GetSizeBytes()) {
       std::string reason =
@@ -402,7 +405,8 @@ void Reader::MaybeVerifyPredecessorWALInfo(
           " bytes. Observed " +
           std::to_string(observed_predecessor_wal_info_.GetSizeBytes()) +
           " bytes.";
-      ReportCorruption(fragment.size(), reason.c_str());
+      ReportCorruption(fragment.size(), reason.c_str(),
+                       recorded_predecessor_log_number);
     }
   }
 }
@@ -483,13 +487,15 @@ void Reader::UnmarkEOFInternal() {
   }
 }
 
-void Reader::ReportCorruption(size_t bytes, const char* reason) {
-  ReportDrop(bytes, Status::Corruption(reason));
+void Reader::ReportCorruption(size_t bytes, const char* reason,
+                              uint64_t log_number) {
+  ReportDrop(bytes, Status::Corruption(reason), log_number);
 }
 
-void Reader::ReportDrop(size_t bytes, const Status& reason) {
+void Reader::ReportDrop(size_t bytes, const Status& reason,
+                        uint64_t log_number) {
   if (reporter_ != nullptr) {
-    reporter_->Corruption(bytes, reason);
+    reporter_->Corruption(bytes, reason, log_number);
   }
 }
 

db/log_reader.h

@@ -45,7 +45,8 @@ class Reader {
 
     // Some corruption was detected.  "size" is the approximate number
     // of bytes dropped due to the corruption.
-    virtual void Corruption(size_t bytes, const Status& status) = 0;
+    virtual void Corruption(size_t bytes, const Status& status,
+                            uint64_t log_number = kMaxSequenceNumber) = 0;
 
     virtual void OldLogRecord(size_t /*bytes*/) {}
   };
@@ -220,8 +221,10 @@ class Reader {
 
   // Reports dropped bytes to the reporter.
   // buffer_ must be updated to remove the dropped bytes prior to invocation.
-  void ReportCorruption(size_t bytes, const char* reason);
-  void ReportDrop(size_t bytes, const Status& reason);
+  void ReportCorruption(size_t bytes, const char* reason,
+                        uint64_t log_number = kMaxSequenceNumber);
+  void ReportDrop(size_t bytes, const Status& reason,
+                  uint64_t log_number = kMaxSequenceNumber);
   void ReportOldLogRecord(size_t bytes);
 
   void InitCompression(const CompressionTypeRecord& compression_record);
@@ -236,17 +239,14 @@ class Reader {
 
 class FragmentBufferedReader : public Reader {
  public:
-  FragmentBufferedReader(
-      std::shared_ptr<Logger> info_log,
-      std::unique_ptr<SequentialFileReader>&& _file, Reporter* reporter,
-      bool checksum, uint64_t log_num, bool verify_and_track_wals = false,
-      bool stop_replay_for_corruption = false,
-      uint64_t min_wal_number_to_keep = std::numeric_limits<uint64_t>::max(),
-      const PredecessorWALInfo& observed_predecessor_wal_info =
-          PredecessorWALInfo())
+  FragmentBufferedReader(std::shared_ptr<Logger> info_log,
+                         std::unique_ptr<SequentialFileReader>&& _file,
+                         Reporter* reporter, bool checksum, uint64_t log_num)
       : Reader(info_log, std::move(_file), reporter, checksum, log_num,
-               verify_and_track_wals, stop_replay_for_corruption,
-               min_wal_number_to_keep, observed_predecessor_wal_info),
+               false /*verify_and_track_wals*/,
+               false /*stop_replay_for_corruption*/,
+               std::numeric_limits<uint64_t>::max() /*min_wal_number_to_keep*/,
+               PredecessorWALInfo() /*observed_predecessor_wal_info*/),
         fragments_(),
         in_fragmented_record_(false) {}
   ~FragmentBufferedReader() override {}

db/log_test.cc

@@ -129,7 +129,8 @@ class LogTest
     std::string message_;
 
     ReportCollector() : dropped_bytes_(0) {}
-    void Corruption(size_t bytes, const Status& status) override {
+    void Corruption(size_t bytes, const Status& status,
+                    uint64_t /*log_number*/ = kMaxSequenceNumber) override {
       dropped_bytes_ += bytes;
       message_.append(status.ToString());
     }
@@ -825,7 +826,8 @@ class RetriableLogTest : public ::testing::TestWithParam<int> {
     std::string message_;
 
     ReportCollector() : dropped_bytes_(0) {}
-    void Corruption(size_t bytes, const Status& status) override {
+    void Corruption(size_t bytes, const Status& status,
+                    uint64_t /*log_number*/ = kMaxSequenceNumber) override {
       dropped_bytes_ += bytes;
       message_.append(status.ToString());
     }

db/memtable.cc

@@ -1325,47 +1325,13 @@ static bool SaveValue(void* arg, const char* entry) {
         return false;
       }
       case kTypeMerge: {
-        if (!merge_operator) {
-          *(s->status) = Status::InvalidArgument(
-              "merge_operator is not properly initialized.");
-          // Normally we continue the loop (return true) when we see a merge
-          // operand.  But in case of an error, we should stop the loop
-          // immediately and pretend we have found the value to stop further
-          // seek.  Otherwise, the later call will override this error status.
-          *(s->found_final_value) = true;
-          return false;
-        }
         Slice v = GetLengthPrefixedSlice(key_ptr + key_length);
         *(s->merge_in_progress) = true;
-        merge_context->PushOperand(
-            v, s->inplace_update_support == false /* operand_pinned */);
-        PERF_COUNTER_ADD(internal_merge_point_lookup_count, 1);
-
-        if (s->do_merge && merge_operator->ShouldMerge(
-                               merge_context->GetOperandsDirectionBackward())) {
-          if (s->value || s->columns) {
-            // `op_failure_scope` (an output parameter) is not provided (set to
-            // nullptr) since a failure must be propagated regardless of its
-            // value.
-            *(s->status) = MergeHelper::TimedFullMerge(
-                merge_operator, s->key->user_key(), MergeHelper::kNoBaseValue,
-                merge_context->GetOperands(), s->logger, s->statistics,
-                s->clock, /* update_num_ops_stats */ true,
-                /* op_failure_scope */ nullptr, s->value, s->columns);
-          }
-
-          *(s->found_final_value) = true;
-          return false;
-        }
-        if (merge_context->get_merge_operands_options != nullptr &&
-            merge_context->get_merge_operands_options->continue_cb != nullptr &&
-            !merge_context->get_merge_operands_options->continue_cb(v)) {
-          // We were told not to continue.
-          *(s->found_final_value) = true;
-          return false;
-        }
-
-        return true;
+        *(s->found_final_value) = ReadOnlyMemTable::HandleTypeMerge(
+            s->key->user_key(), v, s->inplace_update_support == false,
+            s->do_merge, merge_context, s->merge_operator, s->clock,
+            s->statistics, s->logger, s->status, s->value, s->columns);
+        return !*(s->found_final_value);
       }
       default: {
         std::string msg("Corrupted value not expected.");

db/memtable.h

@@ -186,10 +186,13 @@ class ReadOnlyMemTable {
                                               SequenceNumber read_seq,
                                               size_t ts_sz) = 0;
 
-  // Used to Get value associated with key or Get Merge Operands associated
-  // with key.
-  // Keys are considered if they are no larger than the parameter `key` in
+  // Used to get value associated with `key`, or Merge operands associated
+  // with key, or get the latest sequence number of `key` (e.g. transaction
+  // conflict checking).
+  //
+  // Keys are considered if they are no smaller than the parameter `key` in
   // the order defined by comparator and share the save user key with `key`.
+  //
   // If do_merge = true the default behavior which is Get value for key is
   // executed. Expected behavior is described right below.
   // If memtable contains a value for key, store it in *value and return true.
@@ -207,6 +210,7 @@ class ReadOnlyMemTable {
   // returned).  Otherwise, *seq will be set to kMaxSequenceNumber.
   // On success, *s may be set to OK, NotFound, or MergeInProgress.  Any other
   // status returned indicates a corruption or other unexpected error.
+  //
   // If do_merge = false then any Merge Operands encountered for key are simply
   // stored in merge_context.operands_list and never actually merged to get a
   // final value. The raw Merge Operands are eventually returned to the user.
@@ -215,6 +219,9 @@ class ReadOnlyMemTable {
   // @param column If not null and memtable contains a value/WideColumn for key,
   // `column` will be set to the result value/WideColumn.
   // Note: only one of `value` and `column` can be non-nullptr.
+  // To only query for key existence or the latest sequence number of a key,
+  // `value` and `column` can be both nullptr. In this case, returned status can
+  // be OK, NotFound or MergeInProgress if a key is found.
   // @param immutable_memtable Whether this memtable is immutable. Used
   // internally by NewRangeTombstoneIterator(). See comment above
   // NewRangeTombstoneIterator() for more detail.
@@ -394,7 +401,6 @@ class ReadOnlyMemTable {
       // can also be retained.
       merge_context->PushOperand(value, value_pinned);
     } else if (merge_in_progress) {
-      assert(do_merge);
       // `op_failure_scope` (an output parameter) is not provided (set to
       // nullptr) since a failure must be propagated regardless of its
       // value.
@@ -442,6 +448,54 @@ class ReadOnlyMemTable {
     }
   }
 
+  // Returns if a final value is found.
+  static bool HandleTypeMerge(const Slice& lookup_user_key, const Slice& value,
+                              bool value_pinned, bool do_merge,
+                              MergeContext* merge_context,
+                              const MergeOperator* merge_operator,
+                              SystemClock* clock, Statistics* statistics,
+                              Logger* logger, Status* s, std::string* out_value,
+                              PinnableWideColumns* out_columns) {
+    if (!merge_operator) {
+      *s = Status::InvalidArgument(
+          "merge_operator is not properly initialized.");
+      // Normally we continue the loop (return true) when we see a merge
+      // operand.  But in case of an error, we should stop the loop
+      // immediately and pretend we have found the value to stop further
+      // seek.  Otherwise, the later call will override this error status.
+      return true;
+    }
+    merge_context->PushOperand(value, value_pinned /* operand_pinned */);
+    PERF_COUNTER_ADD(internal_merge_point_lookup_count, 1);
+
+    if (do_merge && merge_operator->ShouldMerge(
+                        merge_context->GetOperandsDirectionBackward())) {
+      if (out_value || out_columns) {
+        // `op_failure_scope` (an output parameter) is not provided (set to
+        // nullptr) since a failure must be propagated regardless of its
+        // value.
+        *s = MergeHelper::TimedFullMerge(
+            merge_operator, lookup_user_key, MergeHelper::kNoBaseValue,
+            merge_context->GetOperands(), logger, statistics, clock,
+            /* update_num_ops_stats */ true,
+            /* op_failure_scope */ nullptr, out_value, out_columns);
+      }
+      return true;
+    }
+    if (merge_context->get_merge_operands_options != nullptr &&
+        merge_context->get_merge_operands_options->continue_cb != nullptr &&
+        !merge_context->get_merge_operands_options->continue_cb(value)) {
+      // We were told not to continue. `status` may be MergeInProress(),
+      // overwrite to signal the end of successful get. This status
+      // will be checked at the end of GetImpl().
+      *s = Status::OK();
+      return true;
+    }
+
+    // no final value found yet
+    return false;
+  }
+
  protected:
   friend class MemTableList;
 

db/merge_context.h

@@ -140,10 +140,11 @@ class MergeContext {
     }
   }
 
-  // List of operands
+  // List of operands, the order of operands depends on operands_reversed_.
   mutable std::unique_ptr<std::vector<Slice>> operand_list_;
   // Copy of operands that are not pinned.
   std::unique_ptr<std::vector<std::unique_ptr<std::string>>> copied_operands_;
+  // Reversed means the newest update is ordered first.
   mutable bool operands_reversed_ = true;
 };
 

db/merge_test.cc

@@ -377,6 +377,14 @@ void testCountersWithFlushAndCompaction(Counters& counters, DB* db) {
       {"testCountersWithFlushAndCompaction:AfterGet",
        "testCountersWithFlushAndCompaction::bg_flush_thread:2"},
   });
+  // This test relies on old behavior of SetOptions writing to the
+  // manifest. Here we restore that old behavior for reproducer purposes.
+  // (Brief attempts to use an alternative to SetOptions failed.)
+  SyncPoint::GetInstance()->SetCallBack(
+      "DBImpl::SetOptions:dummy_edit", [&](void* arg) {
+        auto* dummy_edit = static_cast<VersionEdit*>(arg);
+        dummy_edit->Clear();
+      });
   SyncPoint::GetInstance()->EnableProcessing();
 
   port::Thread set_options_thread([&]() {

db/repair.cc

@@ -356,10 +356,12 @@ class Repairer {
       Env* env;
       std::shared_ptr<Logger> info_log;
       uint64_t lognum;
-      void Corruption(size_t bytes, const Status& s) override {
+      void Corruption(size_t bytes, const Status& s,
+                      uint64_t log_number = kMaxSequenceNumber) override {
         // We print error messages for corruption, but continue repairing.
         ROCKS_LOG_ERROR(info_log, "Log #%" PRIu64 ": dropping %d bytes; %s",
-                        lognum, static_cast<int>(bytes), s.ToString().c_str());
+                        log_number == kMaxSequenceNumber ? lognum : log_number,
+                        static_cast<int>(bytes), s.ToString().c_str());
       }
     };
 

db/transaction_log_impl.h

@@ -98,7 +98,8 @@ class TransactionLogIteratorImpl : public TransactionLogIterator {
   struct LogReporter : public log::Reader::Reporter {
     Env* env;
     Logger* info_log;
-    void Corruption(size_t bytes, const Status& s) override {
+    void Corruption(size_t bytes, const Status& s,
+                    uint64_t /*log_number*/ = kMaxSequenceNumber) override {
       ROCKS_LOG_ERROR(info_log, "dropping %" ROCKSDB_PRIszt " bytes; %s", bytes,
                       s.ToString().c_str());
     }

db/version_edit.cc

@@ -59,42 +59,11 @@ Status FileMetaData::UpdateBoundaries(const Slice& key, const Slice& value,
   return Status::OK();
 }
 
-void VersionEdit::Clear() {
-  max_level_ = 0;
-  db_id_.clear();
-  comparator_.clear();
-  log_number_ = 0;
-  prev_log_number_ = 0;
-  next_file_number_ = 0;
-  max_column_family_ = 0;
-  min_log_number_to_keep_ = 0;
-  last_sequence_ = 0;
-  has_db_id_ = false;
-  has_comparator_ = false;
-  has_log_number_ = false;
-  has_prev_log_number_ = false;
-  has_next_file_number_ = false;
-  has_max_column_family_ = false;
-  has_min_log_number_to_keep_ = false;
-  has_last_sequence_ = false;
-  compact_cursors_.clear();
-  deleted_files_.clear();
-  new_files_.clear();
-  blob_file_additions_.clear();
-  blob_file_garbages_.clear();
-  wal_additions_.clear();
-  wal_deletion_.Reset();
-  column_family_ = 0;
-  is_column_family_add_ = false;
-  is_column_family_drop_ = false;
-  column_family_name_.clear();
-  is_in_atomic_group_ = false;
-  remaining_entries_ = 0;
-  full_history_ts_low_.clear();
-}
+void VersionEdit::Clear() { *this = VersionEdit(); }
 
 bool VersionEdit::EncodeTo(std::string* dst,
                            std::optional<size_t> ts_sz) const {
+  assert(!IsNoManifestWriteDummy());
   if (has_db_id_) {
     PutVarint32(dst, kDbId);
     PutLengthPrefixedSlice(dst, db_id_);

db/version_edit.h

@@ -685,6 +685,9 @@ class VersionEdit {
 
   bool IsColumnFamilyDrop() const { return is_column_family_drop_; }
 
+  void MarkNoManifestWriteDummy() { is_no_manifest_write_dummy_ = true; }
+  bool IsNoManifestWriteDummy() const { return is_no_manifest_write_dummy_; }
+
   void MarkAtomicGroup(uint32_t remaining_entries) {
     is_in_atomic_group_ = true;
     remaining_entries_ = remaining_entries;
@@ -779,8 +782,9 @@ class VersionEdit {
   bool is_column_family_add_ = false;
   std::string column_family_name_;
 
-  bool is_in_atomic_group_ = false;
   uint32_t remaining_entries_ = 0;
+  bool is_in_atomic_group_ = false;
+  bool is_no_manifest_write_dummy_ = false;
 
   std::string full_history_ts_low_;
   bool persist_user_defined_timestamps_ = true;

db/version_set.cc

@@ -5305,6 +5305,9 @@ Status VersionSet::ProcessManifestWrites(
   // succeeds.
   SequenceNumber max_last_sequence = descriptor_last_sequence_;
 
+  bool skip_manifest_write =
+      first_writer.edit_list.front()->IsNoManifestWriteDummy();
+
   if (first_writer.edit_list.front()->IsColumnFamilyManipulation()) {
     // No group commits for column family add or drop
     LogAndApplyCFHelper(first_writer.edit_list.front(), &max_last_sequence);
@@ -5313,12 +5316,9 @@ Status VersionSet::ProcessManifestWrites(
   } else {
     auto it = manifest_writers_.cbegin();
     size_t group_start = std::numeric_limits<size_t>::max();
-    while (it != manifest_writers_.cend()) {
-      if ((*it)->edit_list.front()->IsColumnFamilyManipulation()) {
-        // no group commits for column family add or drop
-        break;
-      }
-      last_writer = *(it++);
+    for (;;) {
+      assert(!(*it)->edit_list.front()->IsColumnFamilyManipulation());
+      last_writer = *it;
       assert(last_writer != nullptr);
       assert(last_writer->cfd != nullptr);
       if (last_writer->cfd->IsDropped()) {
@@ -5353,66 +5353,83 @@ Status VersionSet::ProcessManifestWrites(
             }
           }
         }
-        continue;
-      }
-      // We do a linear search on versions because versions is small.
-      // TODO(yanqin) maybe consider unordered_map
-      Version* version = nullptr;
-      VersionBuilder* builder = nullptr;
-      for (int i = 0; i != static_cast<int>(versions.size()); ++i) {
-        uint32_t cf_id = last_writer->cfd->GetID();
-        if (versions[i]->cfd()->GetID() == cf_id) {
-          version = versions[i];
-          assert(!builder_guards.empty() &&
-                 builder_guards.size() == versions.size());
-          builder = builder_guards[i]->version_builder();
+      } else {
+        // We do a linear search on versions because versions is small.
+        // TODO(yanqin) maybe consider unordered_map
+        Version* version = nullptr;
+        VersionBuilder* builder = nullptr;
+        for (int i = 0; i != static_cast<int>(versions.size()); ++i) {
+          uint32_t cf_id = last_writer->cfd->GetID();
+          if (versions[i]->cfd()->GetID() == cf_id) {
+            version = versions[i];
+            assert(!builder_guards.empty() &&
+                   builder_guards.size() == versions.size());
+            builder = builder_guards[i]->version_builder();
+            TEST_SYNC_POINT_CALLBACK(
+                "VersionSet::ProcessManifestWrites:SameColumnFamily", &cf_id);
+            break;
+          }
+        }
+        if (version == nullptr) {
+          // WAL manipulations do not need to be applied to versions.
+          if (!last_writer->IsAllWalEdits()) {
+            version = new Version(
+                last_writer->cfd, this, file_options_,
+                last_writer->cfd ? last_writer->cfd->GetLatestMutableCFOptions()
+                                 : MutableCFOptions(*new_cf_options),
+                io_tracer_, current_version_number_++);
+            versions.push_back(version);
+            builder_guards.emplace_back(
+                new BaseReferencedVersionBuilder(last_writer->cfd));
+            builder = builder_guards.back()->version_builder();
+          }
+          assert(last_writer->IsAllWalEdits() || builder);
+          assert(last_writer->IsAllWalEdits() || version);
           TEST_SYNC_POINT_CALLBACK(
-              "VersionSet::ProcessManifestWrites:SameColumnFamily", &cf_id);
-          break;
+              "VersionSet::ProcessManifestWrites:NewVersion", version);
+        }
+        const Comparator* ucmp = last_writer->cfd->user_comparator();
+        assert(ucmp);
+        std::optional<size_t> edit_ts_sz = ucmp->timestamp_size();
+        for (const auto& e : last_writer->edit_list) {
+          if (e->IsInAtomicGroup()) {
+            if (batch_edits.empty() || !batch_edits.back()->IsInAtomicGroup() ||
+                (batch_edits.back()->IsInAtomicGroup() &&
+                 batch_edits.back()->GetRemainingEntries() == 0)) {
+              group_start = batch_edits.size();
+            }
+          } else if (group_start != std::numeric_limits<size_t>::max()) {
+            group_start = std::numeric_limits<size_t>::max();
+          }
+          Status s = LogAndApplyHelper(last_writer->cfd, builder, e,
+                                       &max_last_sequence, mu);
+          if (!s.ok()) {
+            // free up the allocated memory
+            for (auto v : versions) {
+              delete v;
+            }
+            // FIXME? manifest_writers_ still has requested updates
+            return s;
+          }
+          batch_edits.push_back(e);
+          batch_edits_ts_sz.push_back(edit_ts_sz);
         }
       }
-      if (version == nullptr) {
-        // WAL manipulations do not need to be applied to versions.
-        if (!last_writer->IsAllWalEdits()) {
-          version = new Version(
-              last_writer->cfd, this, file_options_,
-              last_writer->cfd ? last_writer->cfd->GetLatestMutableCFOptions()
-                               : MutableCFOptions(*new_cf_options),
-              io_tracer_, current_version_number_++);
-          versions.push_back(version);
-          builder_guards.emplace_back(
-              new BaseReferencedVersionBuilder(last_writer->cfd));
-          builder = builder_guards.back()->version_builder();
-        }
-        assert(last_writer->IsAllWalEdits() || builder);
-        assert(last_writer->IsAllWalEdits() || version);
-        TEST_SYNC_POINT_CALLBACK("VersionSet::ProcessManifestWrites:NewVersion",
-                                 version);
+      // Loop increment/conditions
+      ++it;
+      if (it == manifest_writers_.cend()) {
+        break;
       }
-      const Comparator* ucmp = last_writer->cfd->user_comparator();
-      assert(ucmp);
-      std::optional<size_t> edit_ts_sz = ucmp->timestamp_size();
-      for (const auto& e : last_writer->edit_list) {
-        if (e->IsInAtomicGroup()) {
-          if (batch_edits.empty() || !batch_edits.back()->IsInAtomicGroup() ||
-              (batch_edits.back()->IsInAtomicGroup() &&
-               batch_edits.back()->GetRemainingEntries() == 0)) {
-            group_start = batch_edits.size();
-          }
-        } else if (group_start != std::numeric_limits<size_t>::max()) {
-          group_start = std::numeric_limits<size_t>::max();
-        }
-        Status s = LogAndApplyHelper(last_writer->cfd, builder, e,
-                                     &max_last_sequence, mu);
-        if (!s.ok()) {
-          // free up the allocated memory
-          for (auto v : versions) {
-            delete v;
-          }
-          return s;
-        }
-        batch_edits.push_back(e);
-        batch_edits_ts_sz.push_back(edit_ts_sz);
+      if (skip_manifest_write) {
+        // no grouping when skipping manifest write
+        break;
+      }
+      const auto* next = (*it)->edit_list.front();
+      if (next->IsColumnFamilyManipulation() ||
+          next->IsNoManifestWriteDummy()) {
+        // no group commits for column family add or drop
+        // nor for dummy skipping manifest write
+        break;
       }
     }
     for (int i = 0; i < static_cast<int>(versions.size()); ++i) {
@@ -5425,6 +5442,7 @@ Status VersionSet::ProcessManifestWrites(
         for (auto v : versions) {
           delete v;
         }
+        // FIXME? manifest_writers_ still has requested updates
         return s;
       }
     }
@@ -5464,11 +5482,16 @@ Status VersionSet::ProcessManifestWrites(
         "VersionSet::ProcessManifestWrites:CheckOneAtomicGroup", &tmp);
     k = i;
   }
+  if (skip_manifest_write) {
+    // no grouping when skipping manifest write
+    assert(last_writer == &first_writer);
+  }
 #endif  // NDEBUG
 
   assert(pending_manifest_file_number_ == 0);
-  if (!descriptor_log_ ||
-      manifest_file_size_ > db_options_->max_manifest_file_size) {
+  if (!skip_manifest_write &&
+      (!descriptor_log_ ||
+       manifest_file_size_ > db_options_->max_manifest_file_size)) {
     TEST_SYNC_POINT("VersionSet::ProcessManifestWrites:BeforeNewManifest");
     new_descriptor_log = true;
   } else {
@@ -5506,8 +5529,18 @@ Status VersionSet::ProcessManifestWrites(
   Status s;
   IOStatus io_s;
   IOStatus manifest_io_status;
+  manifest_io_status.PermitUncheckedError();
   std::unique_ptr<log::Writer> new_desc_log_ptr;
-  {
+  if (skip_manifest_write) {
+    if (s.ok()) {
+      constexpr bool update_stats = true;
+      for (int i = 0; i < static_cast<int>(versions.size()); ++i) {
+        // NOTE: normally called with DB mutex released, but we don't
+        // want to release the DB mutex in this mode of LogAndApply
+        versions[i]->PrepareAppend(read_options, update_stats);
+      }
+    }
+  } else {
     FileOptions opt_file_opts = fs_->OptimizeForManifestWrite(file_options_);
     // DB option (in file_options_) takes precedence when not kUnknown
     if (file_options_.temperature != Temperature::kUnknown) {
@@ -5750,11 +5783,13 @@ Status VersionSet::ProcessManifestWrites(
         AppendVersion(cfd, versions[i]);
       }
     }
-    assert(max_last_sequence >= descriptor_last_sequence_);
-    descriptor_last_sequence_ = max_last_sequence;
-    manifest_file_number_ = pending_manifest_file_number_;
-    manifest_file_size_ = new_manifest_file_size;
-    prev_log_number_ = first_writer.edit_list.front()->GetPrevLogNumber();
+    if (!skip_manifest_write) {
+      assert(max_last_sequence >= descriptor_last_sequence_);
+      descriptor_last_sequence_ = max_last_sequence;
+      manifest_file_number_ = pending_manifest_file_number_;
+      manifest_file_size_ = new_manifest_file_size;
+      prev_log_number_ = first_writer.edit_list.front()->GetPrevLogNumber();
+    }
   } else {
     std::string version_edits;
     for (auto& e : batch_edits) {
@@ -5877,7 +5912,8 @@ Status VersionSet::LogAndApply(
     const autovector<autovector<VersionEdit*>>& edit_lists,
     InstrumentedMutex* mu, FSDirectory* dir_contains_current_file,
     bool new_descriptor_log, const ColumnFamilyOptions* new_cf_options,
-    const std::vector<std::function<void(const Status&)>>& manifest_wcbs) {
+    const std::vector<std::function<void(const Status&)>>& manifest_wcbs,
+    const std::function<Status()>& pre_cb) {
   mu->AssertHeld();
   int num_edits = 0;
   for (const auto& elist : edit_lists) {
@@ -5890,6 +5926,7 @@ Status VersionSet::LogAndApply(
     for (const auto& edit_list : edit_lists) {
       for (const auto& edit : edit_list) {
         assert(!edit->IsColumnFamilyManipulation());
+        assert(!edit->IsNoManifestWriteDummy());
       }
     }
 #endif /* ! NDEBUG */
@@ -5932,6 +5969,7 @@ Status VersionSet::LogAndApply(
     // should only SetBGError once.
     return first_writer.status;
   }
+  TEST_SYNC_POINT_CALLBACK("VersionSet::LogAndApply:WakeUpAndNotDone", mu);
 
   int num_undropped_cfds = 0;
   for (auto cfd : column_family_datas) {
@@ -5940,7 +5978,17 @@ Status VersionSet::LogAndApply(
       ++num_undropped_cfds;
     }
   }
+  Status s;
   if (0 == num_undropped_cfds) {
+    s = Status::ColumnFamilyDropped();
+  }
+  // Call pre_cb once we know we have work to do and are scheduled as the
+  // exclusive manifest writer (and new Version appender)
+  if (s.ok() && pre_cb) {
+    s = pre_cb();
+  }
+  if (!s.ok()) {
+    // Revert manifest_writers_
     for (int i = 0; i != num_cfds; ++i) {
       manifest_writers_.pop_front();
     }
@@ -5948,11 +5996,12 @@ Status VersionSet::LogAndApply(
     if (!manifest_writers_.empty()) {
       manifest_writers_.front()->cv.Signal();
     }
-    return Status::ColumnFamilyDropped();
+    return s;
+  } else {
+    return ProcessManifestWrites(writers, mu, dir_contains_current_file,
+                                 new_descriptor_log, new_cf_options,
+                                 read_options, write_options);
   }
-  return ProcessManifestWrites(writers, mu, dir_contains_current_file,
-                               new_descriptor_log, new_cf_options, read_options,
-                               write_options);
 }
 
 void VersionSet::LogAndApplyCFHelper(VersionEdit* edit,
@@ -7110,7 +7159,8 @@ InternalIterator* VersionSet::MakeInputIterator(
   assert(num <= space);
   InternalIterator* result = NewCompactionMergingIterator(
       &c->column_family_data()->internal_comparator(), list,
-      static_cast<int>(num), range_tombstones);
+      static_cast<int>(num), range_tombstones, /*arena=*/nullptr,
+      c->column_family_data()->internal_stats());
   delete[] list;
   return result;
 }

db/version_set.h

@@ -1216,7 +1216,8 @@ class VersionSet {
       InstrumentedMutex* mu, FSDirectory* dir_contains_current_file,
       bool new_descriptor_log = false,
       const ColumnFamilyOptions* column_family_options = nullptr,
-      const std::function<void(const Status&)>& manifest_wcb = {}) {
+      const std::function<void(const Status&)>& manifest_wcb = {},
+      const std::function<Status()>& pre_cb = {}) {
     autovector<ColumnFamilyData*> cfds;
     cfds.emplace_back(column_family_data);
     autovector<autovector<VersionEdit*>> edit_lists;
@@ -1225,7 +1226,7 @@ class VersionSet {
     edit_lists.emplace_back(edit_list);
     return LogAndApply(cfds, read_options, write_options, edit_lists, mu,
                        dir_contains_current_file, new_descriptor_log,
-                       column_family_options, {manifest_wcb});
+                       column_family_options, {manifest_wcb}, pre_cb);
   }
   // The batch version. If edit_list.size() > 1, caller must ensure that
   // no edit in the list column family add or drop
@@ -1235,14 +1236,15 @@ class VersionSet {
       const autovector<VersionEdit*>& edit_list, InstrumentedMutex* mu,
       FSDirectory* dir_contains_current_file, bool new_descriptor_log = false,
       const ColumnFamilyOptions* column_family_options = nullptr,
-      const std::function<void(const Status&)>& manifest_wcb = {}) {
+      const std::function<void(const Status&)>& manifest_wcb = {},
+      const std::function<Status()>& pre_cb = {}) {
     autovector<ColumnFamilyData*> cfds;
     cfds.emplace_back(column_family_data);
     autovector<autovector<VersionEdit*>> edit_lists;
     edit_lists.emplace_back(edit_list);
     return LogAndApply(cfds, read_options, write_options, edit_lists, mu,
                        dir_contains_current_file, new_descriptor_log,
-                       column_family_options, {manifest_wcb});
+                       column_family_options, {manifest_wcb}, pre_cb);
   }
 
   // The across-multi-cf batch version. If edit_lists contain more than
@@ -1255,8 +1257,8 @@ class VersionSet {
       InstrumentedMutex* mu, FSDirectory* dir_contains_current_file,
       bool new_descriptor_log = false,
       const ColumnFamilyOptions* new_cf_options = nullptr,
-      const std::vector<std::function<void(const Status&)>>& manifest_wcbs =
-          {});
+      const std::vector<std::function<void(const Status&)>>& manifest_wcbs = {},
+      const std::function<Status()>& pre_cb = {});
 
   void WakeUpWaitingManifestWriters();
 
@@ -1583,7 +1585,8 @@ class VersionSet {
 
   struct LogReporter : public log::Reader::Reporter {
     Status* status;
-    void Corruption(size_t /*bytes*/, const Status& s) override {
+    void Corruption(size_t /*bytes*/, const Status& s,
+                    uint64_t /*log_number*/ = kMaxSequenceNumber) override {
       if (status->ok()) {
         *status = s;
       }
@@ -1783,8 +1786,8 @@ class ReactiveVersionSet : public VersionSet {
       const autovector<autovector<VersionEdit*>>& /*edit_lists*/,
       InstrumentedMutex* /*mu*/, FSDirectory* /*dir_contains_current_file*/,
       bool /*new_descriptor_log*/, const ColumnFamilyOptions* /*new_cf_option*/,
-      const std::vector<std::function<void(const Status&)>>& /*manifest_wcbs*/)
-      override {
+      const std::vector<std::function<void(const Status&)>>& /*manifest_wcbs*/,
+      const std::function<Status()>& /*pre_cb*/) override {
     return Status::NotSupported("not supported in reactive mode");
   }
 

db/wal_manager.cc

@@ -468,7 +468,8 @@ Status WalManager::ReadFirstLine(const std::string& fname,
 
     Status* status;
     bool ignore_error;  // true if db_options_.paranoid_checks==false
-    void Corruption(size_t bytes, const Status& s) override {
+    void Corruption(size_t bytes, const Status& s,
+                    uint64_t /*log_number*/ = kMaxSequenceNumber) override {
       ROCKS_LOG_WARN(info_log, "[WalManager] %s%s: dropping %d bytes; %s",
                      (this->ignore_error ? "(ignoring error) " : ""), fname,
                      static_cast<int>(bytes), s.ToString().c_str());

db/write_batch.cc

@@ -815,6 +815,12 @@ WriteBatchInternal::GetColumnFamilyIdAndTimestampSize(
         s = Status::InvalidArgument("Default cf timestamp size mismatch");
       }
     }
+    auto* cfd =
+        static_cast_with_check<ColumnFamilyHandleImpl>(column_family)->cfd();
+    if (cfd && cfd->ioptions().disallow_memtable_writes) {
+      s = Status::InvalidArgument(
+          "This column family has disallow_memtable_writes=true");
+    }
   } else if (b->default_cf_ts_sz_ > 0) {
     ts_sz = b->default_cf_ts_sz_;
   }
@@ -836,6 +842,12 @@ Status CheckColumnFamilyTimestampSize(ColumnFamilyHandle* column_family,
   if (cf_ts_sz != ts.size()) {
     return Status::InvalidArgument("timestamp size mismatch");
   }
+  auto* cfd =
+      static_cast_with_check<ColumnFamilyHandleImpl>(column_family)->cfd();
+  if (cfd && cfd->ioptions().disallow_memtable_writes) {
+    return Status::InvalidArgument(
+        "This column family has disallow_memtable_writes=true");
+  }
   return Status::OK();
 }
 }  // anonymous namespace
@@ -2185,6 +2197,13 @@ class MemTableInserter : public WriteBatch::Handler {
       }
       return false;
     }
+    auto* current = cf_mems_->current();
+    if (current && current->ioptions().disallow_memtable_writes) {
+      *s = Status::InvalidArgument(
+          "This column family has disallow_memtable_writes=true");
+      return false;
+    }
+
     if (recovering_log_number_ != 0 &&
         recovering_log_number_ < cf_mems_->GetLogNumber()) {
       // This is true only in recovery environment (recovering_log_number_ is

db/write_batch_test.cc

@@ -856,15 +856,16 @@ TEST_F(WriteBatchTest, ColumnFamiliesBatchWithIndexTest) {
   iter->Seek("eightfoo");
   ASSERT_OK(iter->status());
   ASSERT_TRUE(iter->Valid());
-  ASSERT_EQ(WriteType::kPutRecord, iter->Entry().type);
+  // For the same key, most recent update is ordered first.
+  ASSERT_EQ(WriteType::kDeleteRecord, iter->Entry().type);
   ASSERT_EQ("eightfoo", iter->Entry().key.ToString());
-  ASSERT_EQ("bar8", iter->Entry().value.ToString());
 
   iter->Next();
   ASSERT_OK(iter->status());
   ASSERT_TRUE(iter->Valid());
-  ASSERT_EQ(WriteType::kDeleteRecord, iter->Entry().type);
+  ASSERT_EQ(WriteType::kPutRecord, iter->Entry().type);
   ASSERT_EQ("eightfoo", iter->Entry().key.ToString());
+  ASSERT_EQ("bar8", iter->Entry().value.ToString());
 
   iter->Next();
   ASSERT_OK(iter->status());
@@ -874,15 +875,15 @@ TEST_F(WriteBatchTest, ColumnFamiliesBatchWithIndexTest) {
   iter->Seek("twofoo");
   ASSERT_OK(iter->status());
   ASSERT_TRUE(iter->Valid());
-  ASSERT_EQ(WriteType::kPutRecord, iter->Entry().type);
+  ASSERT_EQ(WriteType::kSingleDeleteRecord, iter->Entry().type);
   ASSERT_EQ("twofoo", iter->Entry().key.ToString());
-  ASSERT_EQ("bar2", iter->Entry().value.ToString());
 
   iter->Next();
   ASSERT_OK(iter->status());
   ASSERT_TRUE(iter->Valid());
-  ASSERT_EQ(WriteType::kSingleDeleteRecord, iter->Entry().type);
+  ASSERT_EQ(WriteType::kPutRecord, iter->Entry().type);
   ASSERT_EQ("twofoo", iter->Entry().key.ToString());
+  ASSERT_EQ("bar2", iter->Entry().value.ToString());
 
   iter->Next();
   ASSERT_OK(iter->status());

db_stress_tool/cf_consistency_stress.cc

@@ -776,6 +776,11 @@ class CfConsistencyStressTest : public StressTest {
     Slice ub_slice;
 
     ReadOptions ro_copy = readoptions;
+    std::unique_ptr<ManagedSnapshot> snapshot = nullptr;
+    if (ro_copy.auto_refresh_iterator_with_snapshot) {
+      snapshot = std::make_unique<ManagedSnapshot>(db_);
+      ro_copy.snapshot = snapshot->snapshot();
+    }
 
     // Get the next prefix first and then see if we want to set upper bound.
     // We'll use the next prefix in an assertion later on
@@ -858,6 +863,8 @@ class CfConsistencyStressTest : public StressTest {
 
     ManagedSnapshot snapshot_guard(db_);
     options.snapshot = snapshot_guard.snapshot();
+    options.auto_refresh_iterator_with_snapshot =
+        FLAGS_auto_refresh_iterator_with_snapshot;
 
     const size_t num = column_families_.size();
 
@@ -1040,8 +1047,8 @@ class CfConsistencyStressTest : public StressTest {
     assert(thread);
     Status status;
 
-    DB* db_ptr = cmp_db_ ? cmp_db_ : db_;
-    const auto& cfhs = cmp_db_ ? cmp_cfhs_ : column_families_;
+    DB* db_ptr = secondary_db_ ? secondary_db_ : db_;
+    const auto& cfhs = secondary_db_ ? secondary_cfhs_ : column_families_;
 
     // Take a snapshot to preserve the state of primary db.
     ManagedSnapshot snapshot_guard(db_);
@@ -1049,8 +1056,8 @@ class CfConsistencyStressTest : public StressTest {
     SharedState* shared = thread->shared;
     assert(shared);
 
-    if (cmp_db_) {
-      status = cmp_db_->TryCatchUpWithPrimary();
+    if (secondary_db_) {
+      status = secondary_db_->TryCatchUpWithPrimary();
       if (!status.ok()) {
         fprintf(stderr, "TryCatchUpWithPrimary: %s\n",
                 status.ToString().c_str());
@@ -1083,8 +1090,9 @@ class CfConsistencyStressTest : public StressTest {
     // `FLAGS_rate_limit_user_ops` to avoid slowing any validation.
     ReadOptions ropts(FLAGS_verify_checksum, true);
     ropts.total_order_seek = true;
-    if (nullptr == cmp_db_) {
+    if (nullptr == secondary_db_ || FLAGS_auto_refresh_iterator_with_snapshot) {
       ropts.snapshot = snapshot_guard.snapshot();
+      ropts.auto_refresh_iterator_with_snapshot = true;
     }
     uint32_t crc = 0;
     {

db_stress_tool/db_stress_common.h

@@ -422,6 +422,8 @@ DECLARE_bool(allow_unprepared_value);
 DECLARE_string(file_temperature_age_thresholds);
 DECLARE_uint32(commit_bypass_memtable_one_in);
 DECLARE_bool(track_and_verify_wals);
+DECLARE_bool(enable_remote_compaction);
+DECLARE_bool(auto_refresh_iterator_with_snapshot);
 
 constexpr long KB = 1024;
 constexpr int kRandomValueMaxFactor = 3;

db_stress_tool/db_stress_compaction_service.h

@@ -0,0 +1,39 @@
+//  Copyright (c) 2011-present, Facebook, Inc.  All rights reserved.
+//  This source code is licensed under both the GPLv2 (found in the
+//  COPYING file in the root directory) and Apache 2.0 License
+//  (found in the LICENSE.Apache file in the root directory).
+
+#pragma once
+
+#include "rocksdb/options.h"
+
+namespace ROCKSDB_NAMESPACE {
+
+// Service to simulate Remote Compaction in Stress Test
+class DbStressCompactionService : public CompactionService {
+ public:
+  explicit DbStressCompactionService() {}
+
+  static const char* kClassName() { return "DbStressCompactionService"; }
+
+  const char* Name() const override { return kClassName(); }
+
+  CompactionServiceScheduleResponse Schedule(
+      const CompactionServiceJobInfo& /*info*/,
+      const std::string& /*compaction_service_input*/) override {
+    CompactionServiceScheduleResponse response(
+        "Implement Me", CompactionServiceJobStatus::kUseLocal);
+    return response;
+  }
+
+  CompactionServiceJobStatus Wait(const std::string& /*scheduled_job_id*/,
+                                  std::string* /*result*/) override {
+    // TODO - Implement
+    return CompactionServiceJobStatus::kUseLocal;
+  }
+
+  // TODO - Implement
+  void CancelAwaitingJobs() override {}
+};
+
+}  // namespace ROCKSDB_NAMESPACE

db_stress_tool/db_stress_gflags.cc

@@ -831,13 +831,15 @@ DEFINE_uint64(snapshot_hold_ops, 0,
 DEFINE_bool(long_running_snapshots, false,
             "If set, hold on some some snapshots for much longer time.");
 
+// The following three options affect both regular read operations during the
+// test and initial/final database verification through VerifyDB.
 DEFINE_bool(use_multiget, false,
-            "If set, use the batched MultiGet API for reads");
+            "If set, use the batched MultiGet API for reads.");
 
-DEFINE_bool(use_get_entity, false, "If set, use the GetEntity API for reads");
+DEFINE_bool(use_get_entity, false, "If set, use the GetEntity API for reads.");
 
 DEFINE_bool(use_multi_get_entity, false,
-            "If set, use the MultiGetEntity API for reads");
+            "If set, use the MultiGetEntity API for reads.");
 
 DEFINE_int32(test_ingest_standalone_range_deletion_one_in, 0,
              "If non-zero, file ingestion flow will test standalone range "
@@ -851,6 +853,9 @@ DEFINE_bool(track_and_verify_wals,
             ROCKSDB_NAMESPACE::Options().track_and_verify_wals,
             "See Options::track_and_verify_wals");
 
+DEFINE_bool(enable_remote_compaction, false,
+            "Enable (simulated) Remote Compaction");
+
 static bool ValidateInt32Percent(const char* flagname, int32_t value) {
   if (value < 0 || value > 100) {
     fprintf(stderr, "Invalid value for --%s: %d, 0<= pct <=100 \n", flagname,
@@ -1476,4 +1481,10 @@ DEFINE_bool(paranoid_memory_checks,
 DEFINE_uint32(commit_bypass_memtable_one_in, 0,
               "If greater than zero, transaction option will set "
               "commit_bypass_memtable to per every N transactions on average.");
+
+DEFINE_bool(
+    auto_refresh_iterator_with_snapshot,
+    ROCKSDB_NAMESPACE::ReadOptions().auto_refresh_iterator_with_snapshot,
+    "ReadOptions.auto_refresh_iterator_with_snapshot");
+
 #endif  // GFLAGS

db_stress_tool/db_stress_test_base.cc

@@ -19,6 +19,7 @@
 #ifdef GFLAGS
 #include "db_stress_tool/db_stress_common.h"
 #include "db_stress_tool/db_stress_compaction_filter.h"
+#include "db_stress_tool/db_stress_compaction_service.h"
 #include "db_stress_tool/db_stress_driver.h"
 #include "db_stress_tool/db_stress_filters.h"
 #include "db_stress_tool/db_stress_table_properties_collector.h"
@@ -69,7 +70,7 @@ StressTest::StressTest()
       new_column_family_name_(1),
       num_times_reopened_(0),
       db_preload_finished_(false),
-      cmp_db_(nullptr),
+      secondary_db_(nullptr),
       is_db_stopped_(false) {
   if (FLAGS_destroy_db_initially) {
     std::vector<std::string> files;
@@ -104,21 +105,26 @@ StressTest::StressTest()
 }
 
 void StressTest::CleanUp() {
-  for (auto cf : column_families_) {
-    delete cf;
-  }
-  column_families_.clear();
+  CleanUpColumnFamilies();
   if (db_) {
     db_->Close();
   }
   delete db_;
   db_ = nullptr;
 
-  for (auto* cf : cmp_cfhs_) {
+  delete secondary_db_;
+  secondary_db_ = nullptr;
+}
+
+void StressTest::CleanUpColumnFamilies() {
+  for (auto cf : column_families_) {
     delete cf;
   }
-  cmp_cfhs_.clear();
-  delete cmp_db_;
+  column_families_.clear();
+  for (auto* cf : secondary_cfhs_) {
+    delete cf;
+  }
+  secondary_cfhs_.clear();
 }
 
 std::shared_ptr<Cache> StressTest::NewCache(size_t capacity,
@@ -514,6 +520,8 @@ Status StressTest::AssertSame(DB* db, ColumnFamilyHandle* cf,
   // `FLAGS_rate_limit_user_ops` to avoid slowing any validation.
   ReadOptions ropt;
   ropt.snapshot = snap_state.snapshot;
+  ropt.auto_refresh_iterator_with_snapshot =
+      FLAGS_auto_refresh_iterator_with_snapshot;
   Slice ts;
   if (!snap_state.timestamp.empty()) {
     ts = snap_state.timestamp;
@@ -734,14 +742,13 @@ void StressTest::PreloadDbAndReopenAsReadOnly(int64_t number_of_keys,
     s = db_->Flush(FlushOptions(), column_families_);
   }
   if (s.ok()) {
-    for (auto cf : column_families_) {
-      delete cf;
-    }
-    column_families_.clear();
+    CleanUpColumnFamilies();
     delete db_;
     db_ = nullptr;
     txn_db_ = nullptr;
     optimistic_txn_db_ = nullptr;
+    delete secondary_db_;
+    secondary_db_ = nullptr;
 
     db_preload_finished_.store(true);
     auto now = clock_->NowMicros();
@@ -949,6 +956,8 @@ void StressTest::OperateDb(ThreadState* thread) {
   read_opts.fill_cache = FLAGS_fill_cache;
   read_opts.optimize_multiget_for_io = FLAGS_optimize_multiget_for_io;
   read_opts.allow_unprepared_value = FLAGS_allow_unprepared_value;
+  read_opts.auto_refresh_iterator_with_snapshot =
+      FLAGS_auto_refresh_iterator_with_snapshot;
 
   WriteOptions write_opts;
   if (FLAGS_rate_limit_auto_wal_flush) {
@@ -1729,6 +1738,8 @@ Status StressTest::TestIterateImpl(ThreadState* thread,
     cmp_ro.timestamp = ro.timestamp;
     cmp_ro.iter_start_ts = ro.iter_start_ts;
     cmp_ro.snapshot = snapshot_guard.snapshot();
+    cmp_ro.auto_refresh_iterator_with_snapshot =
+        ro.auto_refresh_iterator_with_snapshot;
     cmp_ro.total_order_seek = true;
 
     ColumnFamilyHandle* const cmp_cfh =
@@ -1957,7 +1968,15 @@ void StressTest::VerifyIterator(
                << (ro.iterate_lower_bound
                        ? ro.iterate_lower_bound->ToString(true).c_str()
                        : "")
-               << ", allow_unprepared_value: " << ro.allow_unprepared_value;
+               << ", allow_unprepared_value: " << ro.allow_unprepared_value
+               << ", auto_refresh_iterator_with_snapshot: "
+               << ro.auto_refresh_iterator_with_snapshot
+               << ", snapshot: " << (ro.snapshot ? "non-nullptr" : "nullptr")
+               << ", timestamp: "
+               << (ro.timestamp ? ro.timestamp->ToString(true).c_str() : "")
+               << ", iter_start_ts: "
+               << (ro.iter_start_ts ? ro.iter_start_ts->ToString(true).c_str()
+                                    : "");
 
   if (iter->Valid() && !cmp_iter->Valid()) {
     if (pe != nullptr) {
@@ -2913,6 +2932,8 @@ void StressTest::TestAcquireSnapshot(ThreadState* thread,
       ww_snapshot ? db_impl->GetSnapshotForWriteConflictBoundary()
                   : db_->GetSnapshot();
   ropt.snapshot = snapshot;
+  ropt.auto_refresh_iterator_with_snapshot =
+      FLAGS_auto_refresh_iterator_with_snapshot;
 
   // Ideally, we want snapshot taking and timestamp generation to be atomic
   // here, so that the snapshot corresponds to the timestamp. However, it is
@@ -3122,6 +3143,8 @@ uint32_t StressTest::GetRangeHash(ThreadState* thread, const Snapshot* snapshot,
   ReadOptions ro;
   ro.snapshot = snapshot;
   ro.total_order_seek = true;
+  ro.auto_refresh_iterator_with_snapshot =
+      FLAGS_auto_refresh_iterator_with_snapshot;
   std::string ts_str;
   Slice ts;
   if (FLAGS_user_timestamp_size > 0) {
@@ -3578,12 +3601,11 @@ void StressTest::Open(SharedState* shared, bool reopen) {
             // clean state before executing queries.
             s = db_->GetRootDB()->WaitForCompact(WaitForCompactOptions());
             if (!s.ok()) {
-              for (auto cf : column_families_) {
-                delete cf;
-              }
-              column_families_.clear();
+              CleanUpColumnFamilies();
               delete db_;
               db_ = nullptr;
+              delete secondary_db_;
+              secondary_db_ = nullptr;
             }
           }
           if (!s.ok()) {
@@ -3696,9 +3718,10 @@ void StressTest::Open(SharedState* shared, bool reopen) {
       tmp_opts.env = db_stress_env;
       const std::string& secondary_path = FLAGS_secondaries_base;
       s = DB::OpenAsSecondary(tmp_opts, FLAGS_db, secondary_path,
-                              cf_descriptors, &cmp_cfhs_, &cmp_db_);
+                              cf_descriptors, &secondary_cfhs_, &secondary_db_);
       assert(s.ok());
-      assert(cmp_cfhs_.size() == static_cast<size_t>(FLAGS_column_families));
+      assert(secondary_cfhs_.size() ==
+             static_cast<size_t>(FLAGS_column_families));
     }
   } else {
     DBWithTTL* db_with_ttl;
@@ -3749,15 +3772,12 @@ void StressTest::Reopen(ThreadState* thread) {
   }
   assert(!write_prepared || bg_canceled);
 
-  for (auto cf : column_families_) {
-    delete cf;
-  }
-  column_families_.clear();
+  CleanUpColumnFamilies();
 
   // Currently reopen does not restore expected state
   // with potential data loss in mind like the first open before
-  // crash-recovery verification does. Therefore it always expects no data loss
-  // and we should ensure no data loss in testing.
+  // crash-recovery verification does. Therefore it always expects no data
+  // loss and we should ensure no data loss in testing.
   // TODO(hx235): eliminate the FlushWAL(true /* sync */)/SyncWAL() below
   if (!FLAGS_disable_wal) {
     Status s;
@@ -3789,6 +3809,8 @@ void StressTest::Reopen(ThreadState* thread) {
   db_ = nullptr;
   txn_db_ = nullptr;
   optimistic_txn_db_ = nullptr;
+  delete secondary_db_;
+  secondary_db_ = nullptr;
 
   num_times_reopened_++;
   auto now = clock_->NowMicros();
@@ -4294,6 +4316,11 @@ void InitializeOptionsFromFlags(
       static_cast<CacheTier>(FLAGS_lowest_used_cache_tier);
   options.inplace_update_support = FLAGS_inplace_update_support;
   options.uncache_aggressiveness = FLAGS_uncache_aggressiveness;
+
+  // Remote Compaction
+  if (FLAGS_enable_remote_compaction) {
+    options.compaction_service = std::make_shared<DbStressCompactionService>();
+  }
 }
 
 void InitializeOptionsGeneral(

db_stress_tool/db_stress_test_base.h

@@ -391,6 +391,8 @@ class StressTest {
                                           std::string& ts_str, Slice& ts_slice,
                                           ReadOptions& read_opts);
 
+  void CleanUpColumnFamilies();
+
   std::shared_ptr<Cache> cache_;
   std::shared_ptr<Cache> compressed_cache_;
   std::shared_ptr<const FilterPolicy> filter_policy_;
@@ -412,9 +414,8 @@ class StressTest {
   std::atomic<bool> db_preload_finished_;
   std::shared_ptr<SstQueryFilterConfigsManager::Factory> sqfc_factory_;
 
-  // Fields used for continuous verification from another thread
-  DB* cmp_db_;
-  std::vector<ColumnFamilyHandle*> cmp_cfhs_;
+  DB* secondary_db_;
+  std::vector<ColumnFamilyHandle*> secondary_cfhs_;
   bool is_db_stopped_;
 };
 

db_stress_tool/multi_ops_txns_stress.cc

@@ -348,7 +348,14 @@ void MultiOpsTxnsStressTest::ReopenAndPreloadDbIfNeeded(SharedState* shared) {
   (void)shared;
   bool db_empty = false;
   {
-    std::unique_ptr<Iterator> iter(db_->NewIterator(ReadOptions()));
+    ReadOptions ropt;
+    std::unique_ptr<ManagedSnapshot> snapshot = nullptr;
+    if (FLAGS_auto_refresh_iterator_with_snapshot) {
+      snapshot = std::make_unique<ManagedSnapshot>(db_);
+      ropt.snapshot = snapshot->snapshot();
+      ropt.auto_refresh_iterator_with_snapshot = true;
+    }
+    std::unique_ptr<Iterator> iter(db_->NewIterator(ropt));
     iter->SeekToFirst();
     if (!iter->Valid()) {
       db_empty = true;
@@ -767,6 +774,8 @@ Status MultiOpsTxnsStressTest::SecondaryKeyUpdateTxn(ThreadState* thread,
   Slice iter_ub = iter_ub_str;
   ReadOptions ropts;
   ropts.snapshot = txn->GetSnapshot();
+  ropts.auto_refresh_iterator_with_snapshot =
+      FLAGS_auto_refresh_iterator_with_snapshot;
   ropts.total_order_seek = true;
   ropts.iterate_upper_bound = &iter_ub;
   ropts.rate_limiter_priority =
@@ -1136,6 +1145,8 @@ void MultiOpsTxnsStressTest::VerifyDb(ThreadState* thread) const {
     // `FLAGS_rate_limit_user_ops` to avoid slowing any validation.
     ReadOptions ropts;
     ropts.snapshot = snapshot;
+    ropts.auto_refresh_iterator_with_snapshot =
+        FLAGS_auto_refresh_iterator_with_snapshot;
     ropts.total_order_seek = true;
     ropts.iterate_upper_bound = &iter_ub;
     if (FLAGS_use_sqfc_for_range_queries) {
@@ -1191,6 +1202,8 @@ void MultiOpsTxnsStressTest::VerifyDb(ThreadState* thread) const {
     // `FLAGS_rate_limit_user_ops` to avoid slowing any validation.
     ReadOptions ropts;
     ropts.snapshot = snapshot;
+    ropts.auto_refresh_iterator_with_snapshot =
+        FLAGS_auto_refresh_iterator_with_snapshot;
     ropts.total_order_seek = true;
 
     std::unique_ptr<Iterator> it(db_->NewIterator(ropts));
@@ -1289,6 +1302,8 @@ void MultiOpsTxnsStressTest::VerifyPkSkFast(const ReadOptions& read_options,
   // `FLAGS_rate_limit_user_ops` to avoid slowing any validation.
   ReadOptions ropts;
   ropts.snapshot = snapshot;
+  ropts.auto_refresh_iterator_with_snapshot =
+      FLAGS_auto_refresh_iterator_with_snapshot;
   ropts.total_order_seek = true;
   ropts.io_activity = read_options.io_activity;
 
@@ -1643,6 +1658,13 @@ void MultiOpsTxnsStressTest::ScanExistingDb(SharedState* shared, int threads) {
 
   assert(db_);
   ReadOptions ropts;
+  std::unique_ptr<ManagedSnapshot> snapshot = nullptr;
+  if (FLAGS_auto_refresh_iterator_with_snapshot) {
+    snapshot = std::make_unique<ManagedSnapshot>(db_);
+    ropts.snapshot = snapshot->snapshot();
+    ropts.auto_refresh_iterator_with_snapshot = true;
+  }
+
   std::vector<KeySet> existing_a_uniqs(threads);
   std::vector<KeySet> non_existing_a_uniqs(threads);
   std::vector<KeySet> existing_c_uniqs(threads);

db_stress_tool/no_batched_ops_stress.cc

@@ -49,6 +49,10 @@ class NonBatchedOpsStressTest : public StressTest {
       end = max_key;
     }
 
+    if (FLAGS_auto_refresh_iterator_with_snapshot) {
+      options.auto_refresh_iterator_with_snapshot = true;
+    }
+
     for (size_t cf = 0; cf < column_families_.size(); ++cf) {
       if (thread->shared->HasVerificationFailedYet()) {
         break;
@@ -68,11 +72,28 @@ class NonBatchedOpsStressTest : public StressTest {
       constexpr int num_methods =
           static_cast<int>(VerificationMethod::kNumberOfMethods);
 
-      const VerificationMethod method =
+      VerificationMethod method =
           static_cast<VerificationMethod>(thread->rand.Uniform(
               (FLAGS_user_timestamp_size > 0) ? num_methods - 1 : num_methods));
 
+      if (method == VerificationMethod::kGetEntity && !FLAGS_use_get_entity) {
+        method = VerificationMethod::kGet;
+      }
+      if (method == VerificationMethod::kMultiGetEntity &&
+          !FLAGS_use_multi_get_entity) {
+        method = VerificationMethod::kMultiGet;
+      }
+      if (method == VerificationMethod::kMultiGet && !FLAGS_use_multiget) {
+        method = VerificationMethod::kGet;
+      }
+
       if (method == VerificationMethod::kIterator) {
+        std::unique_ptr<ManagedSnapshot> snapshot = nullptr;
+        if (options.auto_refresh_iterator_with_snapshot) {
+          snapshot = std::make_unique<ManagedSnapshot>(db_);
+          options.snapshot = snapshot->snapshot();
+        }
+
         std::unique_ptr<Iterator> iter(
             db_->NewIterator(options, column_families_[cf]));
 
@@ -134,6 +155,10 @@ class NonBatchedOpsStressTest : public StressTest {
                           from_db.data(), from_db.size());
           }
         }
+
+        if (options.auto_refresh_iterator_with_snapshot) {
+          options.snapshot = nullptr;
+        }
       } else if (method == VerificationMethod::kGet) {
         for (int64_t i = start; i < end; ++i) {
           if (thread->shared->HasVerificationFailedYet()) {
@@ -153,6 +178,100 @@ class NonBatchedOpsStressTest : public StressTest {
                           from_db.data(), from_db.size());
           }
         }
+
+        if (secondary_db_) {
+          assert(secondary_cfhs_.size() == column_families_.size());
+          // We are going to read in the expected values before catching the
+          // secondary up to the primary. This sets the lower bound of the
+          // acceptable values that can be returned from the secondary. After
+          // each Get() to the secondary, we are going to read in the expected
+          // value again to determine the upper bound. As long as the returned
+          // value from Get() is within these bounds, we consider that okay. The
+          // lower bound will always be moving forwards anyways as
+          // TryCatchUpWithPrimary() gets called.
+          std::vector<ExpectedValue> pre_read_expected_values;
+          for (int64_t i = start; i < end; ++i) {
+            pre_read_expected_values.push_back(
+                shared->Get(static_cast<int>(cf), i));
+          }
+
+          if (FLAGS_disable_wal) {
+            // The secondary relies on the WAL to be able to catch up with the
+            // primary's memtable changes. If there is no WAL, before
+            // verification we should make sure the changes are reflected in the
+            // SST files
+            Status memtable_flush_status =
+                db_->Flush(FlushOptions(), column_families_[cf]);
+            if (!memtable_flush_status.ok()) {
+              if (IsErrorInjectedAndRetryable(memtable_flush_status)) {
+                fprintf(stdout,
+                        "Skipping secondary verification because error was "
+                        "injected into memtable flush\n");
+                continue;
+              }
+              VerificationAbort(shared,
+                                "Failed to flush primary's memtables before "
+                                "secondary verification");
+            }
+          } else if (FLAGS_manual_wal_flush_one_in > 0) {
+            // RocksDB maintains internal buffers of WAL data when
+            // manual_wal_flush is used. The secondary can read the WAL to catch
+            // up with the primary's memtable changes, but these changes need to
+            // be flushed first.
+            Status flush_wal_status = db_->FlushWAL(/*sync=*/true);
+            if (!flush_wal_status.ok()) {
+              if (IsErrorInjectedAndRetryable(flush_wal_status)) {
+                fprintf(stdout,
+                        "Skipping secondary verification because error was "
+                        "injected into WAL flush\n");
+                continue;
+              }
+              VerificationAbort(shared,
+                                "Failed to flush primary's WAL before "
+                                "secondary verification");
+            }
+          }
+
+          Status s = secondary_db_->TryCatchUpWithPrimary();
+          if (!s.ok()) {
+            VerificationAbort(shared,
+                              "Secondary failed to catch up to the primary");
+          }
+
+          for (int64_t i = start; i < end; ++i) {
+            if (thread->shared->HasVerificationFailedYet()) {
+              break;
+            }
+
+            const std::string key = Key(i);
+            std::string from_db;
+
+            // Temporarily disable error injection to verify the secondary
+            if (fault_fs_guard) {
+              fault_fs_guard->DisableThreadLocalErrorInjection(
+                  FaultInjectionIOType::kRead);
+              fault_fs_guard->DisableThreadLocalErrorInjection(
+                  FaultInjectionIOType::kMetadataRead);
+            }
+
+            s = secondary_db_->Get(options, secondary_cfhs_[cf], key, &from_db);
+
+            // Re-enable error injection after verifying the secondary
+            if (fault_fs_guard) {
+              fault_fs_guard->EnableThreadLocalErrorInjection(
+                  FaultInjectionIOType::kRead);
+              fault_fs_guard->EnableThreadLocalErrorInjection(
+                  FaultInjectionIOType::kMetadataRead);
+            }
+
+            assert(!pre_read_expected_values.empty() &&
+                   static_cast<size_t>(i - start) <
+                       pre_read_expected_values.size());
+            VerifyValueRange(static_cast<int>(cf), i, options, shared, from_db,
+                             /* msg_prefix */ "Secondary get verification", s,
+                             pre_read_expected_values[i - start]);
+          }
+        }
       } else if (method == VerificationMethod::kGetEntity) {
         for (int64_t i = start; i < end; ++i) {
           if (thread->shared->HasVerificationFailedYet()) {
@@ -333,12 +452,19 @@ class NonBatchedOpsStressTest : public StressTest {
   }
 
   void ContinuouslyVerifyDb(ThreadState* thread) const override {
-    if (!cmp_db_) {
+    // For automated crash tests, we only want to run this continous
+    // verification when continuous_verification_interval > 0 and there is
+    // a secondary db. This continous verification currently fails when there is
+    // a secondary db during the iterator scan. The stack trace mentions
+    // BlobReader/BlobSource but it may not necessarily be related to BlobDB.
+    // Regardless, we only want to run this function if we are experimenting and
+    // explicitly setting continuous_verification_interval.
+    if (!secondary_db_ || !FLAGS_continuous_verification_interval) {
       return;
     }
-    assert(cmp_db_);
-    assert(!cmp_cfhs_.empty());
-    Status s = cmp_db_->TryCatchUpWithPrimary();
+    assert(secondary_db_);
+    assert(!secondary_cfhs_.empty());
+    Status s = secondary_db_->TryCatchUpWithPrimary();
     if (!s.ok()) {
       assert(false);
       exit(1);
@@ -368,11 +494,18 @@ class NonBatchedOpsStressTest : public StressTest {
       read_opts.timestamp = &ts;
     }
 
+    std::unique_ptr<ManagedSnapshot> snapshot = nullptr;
+    if (FLAGS_auto_refresh_iterator_with_snapshot) {
+      snapshot = std::make_unique<ManagedSnapshot>(db_);
+      read_opts.snapshot = snapshot->snapshot();
+      read_opts.auto_refresh_iterator_with_snapshot = true;
+    }
+
     static Random64 rand64(shared->GetSeed());
 
     {
       uint32_t crc = 0;
-      std::unique_ptr<Iterator> it(cmp_db_->NewIterator(read_opts));
+      std::unique_ptr<Iterator> it(secondary_db_->NewIterator(read_opts));
       s = checksum_column_family(it.get(), &crc);
       if (!s.ok()) {
         fprintf(stderr, "Computing checksum of default cf: %s\n",
@@ -381,19 +514,25 @@ class NonBatchedOpsStressTest : public StressTest {
       }
     }
 
-    for (auto* handle : cmp_cfhs_) {
+    for (auto* handle : secondary_cfhs_) {
       if (thread->rand.OneInOpt(3)) {
         // Use Get()
         uint64_t key = rand64.Uniform(static_cast<uint64_t>(max_key));
         std::string key_str = Key(key);
         std::string value;
         std::string key_ts;
-        s = cmp_db_->Get(read_opts, handle, key_str, &value,
-                         FLAGS_user_timestamp_size > 0 ? &key_ts : nullptr);
+        s = secondary_db_->Get(
+            read_opts, handle, key_str, &value,
+            FLAGS_user_timestamp_size > 0 ? &key_ts : nullptr);
         s.PermitUncheckedError();
       } else {
         // Use range scan
-        std::unique_ptr<Iterator> iter(cmp_db_->NewIterator(read_opts, handle));
+        if (read_opts.auto_refresh_iterator_with_snapshot) {
+          snapshot = std::make_unique<ManagedSnapshot>(db_);
+          read_opts.snapshot = snapshot->snapshot();
+        }
+        std::unique_ptr<Iterator> iter(
+            secondary_db_->NewIterator(read_opts, handle));
         uint32_t rnd = (thread->rand.Next()) % 4;
         if (0 == rnd) {
           // SeekToFirst() + Next()*5
@@ -422,6 +561,9 @@ class NonBatchedOpsStressTest : public StressTest {
           for (int i = 0; i < 5 && iter->Valid(); ++i, iter->Prev()) {
           }
         }
+        if (read_opts.auto_refresh_iterator_with_snapshot) {
+          read_opts.snapshot = nullptr;
+        }
       }
     }
   }
@@ -1458,6 +1600,12 @@ class NonBatchedOpsStressTest : public StressTest {
     Slice ub_slice;
     ReadOptions ro_copy = read_opts;
 
+    // There is a narrow window in iterator auto refresh run where injected read
+    // errors are simply untraceable, ex. failure to delete file as a part of
+    // superversion cleanup callback invoked by the DBIter destructor.
+    bool ignore_injected_read_error_in_iter =
+        ro_copy.auto_refresh_iterator_with_snapshot;
+
     // Randomly test with `iterate_upper_bound` and `prefix_same_as_start`
     //
     // Get the next prefix first and then see if we want to set it to be the
@@ -1487,6 +1635,12 @@ class NonBatchedOpsStressTest : public StressTest {
       SharedState::ignore_read_error = false;
     }
 
+    std::unique_ptr<ManagedSnapshot> snapshot = nullptr;
+    if (ro_copy.snapshot == nullptr &&
+        ro_copy.auto_refresh_iterator_with_snapshot) {
+      snapshot = std::make_unique<ManagedSnapshot>(db_);
+      ro_copy.snapshot = snapshot->snapshot();
+    }
     std::unique_ptr<Iterator> iter(db_->NewIterator(ro_copy, cfh));
 
     uint64_t count = 0;
@@ -1544,7 +1698,8 @@ class NonBatchedOpsStressTest : public StressTest {
               FaultInjectionIOType::kRead),
           fault_fs_guard->GetAndResetInjectedThreadLocalErrorCount(
               FaultInjectionIOType::kMetadataRead));
-      if (!SharedState::ignore_read_error && injected_error_count > 0 &&
+      if (!ignore_injected_read_error_in_iter &&
+          !SharedState::ignore_read_error && injected_error_count > 0 &&
           s.ok()) {
         // Grab mutex so multiple thread don't try to print the
         // stack trace at the same time
@@ -2237,6 +2392,7 @@ class NonBatchedOpsStressTest : public StressTest {
     }
 
     ReadOptions ro(read_opts);
+
     if (FLAGS_prefix_size > 0) {
       ro.total_order_seek = true;
     }
@@ -2278,6 +2434,16 @@ class NonBatchedOpsStressTest : public StressTest {
       pre_read_expected_values.push_back(
           shared->Get(rand_column_family, i + lb));
     }
+
+    // Snapshot initialization timing plays a crucial role here.
+    // We want the iterator to reflect the state of the DB between
+    // reading `pre_read_expected_values` and `post_read_expected_values`.
+    std::unique_ptr<ManagedSnapshot> snapshot = nullptr;
+    if (ro.auto_refresh_iterator_with_snapshot) {
+      snapshot = std::make_unique<ManagedSnapshot>(db_);
+      ro.snapshot = snapshot->snapshot();
+    }
+
     std::unique_ptr<Iterator> iter;
     if (FLAGS_use_multi_cf_iterator) {
       std::vector<ColumnFamilyHandle*> cfhs;
@@ -2520,7 +2686,11 @@ class NonBatchedOpsStressTest : public StressTest {
         pre_read_expected_values.push_back(
             shared->Get(rand_column_family, i + lb));
       }
-      Status rs = iter->Refresh();
+      if (ro.auto_refresh_iterator_with_snapshot) {
+        snapshot = std::make_unique<ManagedSnapshot>(db_);
+        ro.snapshot = snapshot->snapshot();
+      }
+      Status rs = iter->Refresh(ro.snapshot);
       if (!rs.ok() && IsErrorInjectedAndRetryable(rs)) {
         return rs;
       }
@@ -2810,6 +2980,84 @@ class NonBatchedOpsStressTest : public StressTest {
     return true;
   }
 
+  // Compared to VerifyOrSyncValue, VerifyValueRange takes in a
+  // pre_read_expected_value to determine the lower bound of acceptable values.
+  // Anything from the pre_read_expected_value to the post_read_expected_value
+  // is considered acceptable. VerifyValueRange does not perform the initial
+  // "sync" step and does not compare the exact data/lengths for the values.
+  // This verification is suitable for verifying secondary or follower databases
+  bool VerifyValueRange(int cf, int64_t key, const ReadOptions& opts,
+                        SharedState* shared, const std::string& value_from_db,
+                        const std::string& msg_prefix, const Status& s,
+                        const ExpectedValue& pre_read_expected_value) const {
+    if (shared->HasVerificationFailedYet()) {
+      return false;
+    }
+    const ExpectedValue post_read_expected_value = shared->Get(cf, key);
+    char expected_value_data[kValueMaxLen];
+    size_t expected_value_data_size =
+        GenerateValue(post_read_expected_value.GetValueBase(),
+                      expected_value_data, sizeof(expected_value_data));
+
+    std::ostringstream read_u64ts;
+    if (opts.timestamp) {
+      read_u64ts << " while read with timestamp: ";
+      uint64_t read_ts;
+      if (DecodeU64Ts(*opts.timestamp, &read_ts).ok()) {
+        read_u64ts << std::to_string(read_ts) << ", ";
+      } else {
+        read_u64ts << s.ToString()
+                   << " Encoded read timestamp: " << opts.timestamp->ToString()
+                   << ", ";
+      }
+    }
+
+    // Compare value_from_db with the range of possible values from
+    // pre_read_expected_value to post_read_expected_value
+    if (s.ok()) {
+      const Slice slice(value_from_db);
+      const uint32_t value_base_from_db = GetValueBase(slice);
+      if (ExpectedValueHelper::MustHaveNotExisted(pre_read_expected_value,
+                                                  post_read_expected_value)) {
+        VerificationAbort(shared,
+                          msg_prefix +
+                              ": Unexpected value found that should not exist" +
+                              read_u64ts.str(),
+                          cf, key, value_from_db, "");
+        return false;
+      }
+      if (!ExpectedValueHelper::InExpectedValueBaseRange(
+              value_base_from_db, pre_read_expected_value,
+              post_read_expected_value)) {
+        VerificationAbort(
+            shared,
+            msg_prefix +
+                ": Unexpected value found outside of the value base range" +
+                read_u64ts.str(),
+            cf, key, value_from_db,
+            Slice(expected_value_data, expected_value_data_size));
+        return false;
+      }
+    } else if (s.IsNotFound()) {
+      if (ExpectedValueHelper::MustHaveExisted(pre_read_expected_value,
+                                               post_read_expected_value)) {
+        VerificationAbort(shared,
+                          msg_prefix + ": Value not found which should exist" +
+                              read_u64ts.str() + s.ToString(),
+                          cf, key, "",
+                          Slice(expected_value_data, expected_value_data_size));
+        return false;
+      }
+    } else {
+      VerificationAbort(
+          shared,
+          msg_prefix + ": Non-OK status" + read_u64ts.str() + s.ToString(), cf,
+          key, "", Slice(expected_value_data, expected_value_data_size));
+      return false;
+    }
+    return true;
+  }
+
   void PrepareTxnDbOptions(SharedState* shared,
                            TransactionDBOptions& txn_db_opts) override {
     txn_db_opts.rollback_deletion_type_callback =

docs/_data/authors.yml

@@ -79,3 +79,7 @@ cbi42:
 zjay:
   full_name: Jay Zhuang
   fbid: 100032386042884
+
+hx235:
+  full_name: Hui Xiao
+  fbid: 100037058588280

docs/_posts/2025-02-07-mitigated-bug-update.markdown

@@ -0,0 +1,36 @@
+---
+title: Addressing a Mitigated Misconfig Bug in the RocksDB OSS Repository
+layout: post
+author: hx235
+category: blog
+---
+
+Dear RocksDB Community,
+
+We want to share an update about the bug that allowed our bug bounty researcher to update the release note title in August 2024 involving the RocksDB open-source repository on GitHub. This issue was found and responsibly disclosed to us by an external bug bounty researcher through our [Meta Bug Bounty program](https://www.facebook.com/whitehat) and quickly mitigated by our teams. We have not seen any evidence of malicious exploitation. Please note that no action is required from our community, as we have taken all necessary steps to remediate the issue.
+
+## Background
+
+RocksDB is a high-performance storage engine library widely used in various large-scale applications. On August 21, 2024, a bug was reported to us by one of our  bug bounty researchers. They were able to demonstrate the ability to obtain the GITHUB_TOKEN used in GitHub Actions workflows. This token provides write access to the metadata of the repository, and the researcher used it to change the title of the release note 9.5.2 as proof of concept. The researcher also unsuccessfully attempted to merge a change to the main branch of the repository; however, we had access controls set up to prevent it from going through.
+
+## Key Details
+
+- **Incident Discovery**: After the bug bounty researcher changed the open source release note title to demonstrate the vulnerability, external users noticed this change and [notified](https://github.com/facebook/rocksdb/issues/12962) RocksDB. RocksDB then reached out to the Bug Bounty program to confirm this was the result of security research.
+- **No Malicious Abuse**: The investigation confirmed that no code or data was compromised. The change was public and visible on GitHub.
+- **Tag Reversion Clarification**: On August 21, a tag named "v9.5.2" was initially published pointing to an incorrect commit. This was unrelated to the bug described here and was promptly corrected by pointing the tag to the correct commit. The release binary remains safe to use, and this correction does not impact the security or integrity of the release.
+
+We’ve taken the following steps to mitigate and remediate the issue:
+- The release note title was corrected.
+- The workflow running on the self-hosted runner was disabled immediately.
+- It was confirmed that the GITHUB_TOKEN expired and is no longer in use.
+- The binary tagged for public release was examined to confirm that it was not compromised.
+- Action logs were cross-checked to ensure no other actions were taken with the compromised token, other than the release note title change and the failed attempts to merge self-approved pull requests to the main branch.
+- We have [scoped down](https://github.com/facebook/rocksdb/pull/12973) the access level of tokens generated for workflows to prevent similar issues. Additionally, we are developing better guidelines for bug bounty researchers to minimize disruptions during their research.
+
+
+Thank you for your continued support and trust in RocksDB.
+
+
+Sincerely,
+
+The RocksDB Team

file/file_prefetch_buffer.cc

@@ -865,10 +865,12 @@ bool FilePrefetchBuffer::TryReadFromCacheUntracked(
   if (copy_to_overlap_buffer) {
     buf = overlap_buf_;
   }
-  assert(buf->offset_ <= offset);
-  assert(buf->IsDataBlockInBuffer(offset, n));
+  assert(buf->IsOffsetInBuffer(offset));
   uint64_t offset_in_buffer = offset - buf->offset_;
-  *result = Slice(buf->buffer_.BufferStart() + offset_in_buffer, n);
+  assert(offset_in_buffer < buf->CurrentSize());
+  *result = Slice(
+      buf->buffer_.BufferStart() + offset_in_buffer,
+      std::min(n, buf->CurrentSize() - static_cast<size_t>(offset_in_buffer)));
   if (prefetched) {
     readahead_size_ = std::min(max_readahead_size_, readahead_size_ * 2);
   }

file/prefetch_test.cc

@@ -3317,25 +3317,39 @@ TEST_F(FilePrefetchBufferTest, ForCompaction) {
   ASSERT_TRUE(fpb.TryReadFromCache(IOOptions(), r.get(), 0 /* offset */,
                                    3000 /* n */, &result, &s, true));
   ASSERT_EQ(s, Status::OK());
+  ASSERT_EQ(result.size(), 3000);
   ASSERT_EQ(strncmp(result.data(), content.substr(0, 3000).c_str(), 3000), 0);
 
   ASSERT_TRUE(fpb.TryReadFromCache(IOOptions(), r.get(), 3000 /* offset */,
                                    10000 /* n */, &result, &s, true));
   ASSERT_EQ(s, Status::OK());
+  ASSERT_EQ(result.size(), 10000);
   ASSERT_EQ(strncmp(result.data(), content.substr(3000, 10000).c_str(), 10000),
             0);
 
   ASSERT_TRUE(fpb.TryReadFromCache(IOOptions(), r.get(), 15000 /* offset */,
                                    4096 /* n */, &result, &s, true));
   ASSERT_EQ(s, Status::OK());
+  ASSERT_EQ(result.size(), 4096);
   ASSERT_EQ(strncmp(result.data(), content.substr(15000, 4096).c_str(), 4096),
             0);
 
   ASSERT_TRUE(fpb.TryReadFromCache(IOOptions(), r.get(), 40000 /* offset */,
                                    20000 /* n */, &result, &s, true));
   ASSERT_EQ(s, Status::OK());
+  ASSERT_EQ(result.size(), 20000);
   ASSERT_EQ(strncmp(result.data(), content.substr(40000, 20000).c_str(), 20000),
             0);
+
+  // Try reading past end of file
+  ASSERT_TRUE(fpb.TryReadFromCache(IOOptions(), r.get(), 60000 /* offset */,
+                                   10000 /* n */, &result, &s, true));
+  ASSERT_EQ(s, Status::OK());
+  ASSERT_EQ(result.size(), 64 * 1024 - 60000);
+  ASSERT_EQ(
+      strncmp(result.data(), content.substr(60000, 64 * 1024 - 60000).c_str(),
+              64 * 1024 - 60000),
+      0);
 }
 
 class FSBufferPrefetchTest
@@ -3858,6 +3872,8 @@ TEST_P(FSBufferPrefetchTest, FSBufferPrefetchRandomized) {
     }
     ASSERT_TRUE(could_read_from_cache);
     ASSERT_EQ(s, Status::OK());
+    ASSERT_EQ(result.size(),
+              std::min(len, content.size() - static_cast<size_t>(offset)));
     ASSERT_EQ(strncmp(result.data(),
                       content.substr(offset, offset + len).c_str(), len),
               0);

include/rocksdb/advanced_options.h

@@ -64,6 +64,9 @@ enum CompactionPri : char {
 struct FileTemperatureAge {
   Temperature temperature = Temperature::kUnknown;
   uint64_t age = 0;
+#if __cplusplus >= 202002L
+  bool operator==(const FileTemperatureAge& rhs) const = default;
+#endif
 };
 
 struct CompactionOptionsFIFO {
@@ -116,6 +119,10 @@ struct CompactionOptionsFIFO {
   CompactionOptionsFIFO(uint64_t _max_table_files_size, bool _allow_compaction)
       : max_table_files_size(_max_table_files_size),
         allow_compaction(_allow_compaction) {}
+
+#if __cplusplus >= 202002L
+  bool operator==(const CompactionOptionsFIFO& rhs) const = default;
+#endif
 };
 
 // The control option of how the cache tiers will be used. Currently rocksdb
@@ -712,6 +719,17 @@ struct AdvancedColumnFamilyOptions {
   // Dynamically changeable through SetOptions() API
   bool report_bg_io_stats = false;
 
+  // Setting this option to true disallows ordinary writes to the column family
+  // and it can only be populated through import and ingestion. It is intended
+  // to protect "ingestion only" column families. This option is not currently
+  // supported on the default column family because of error handling challenges
+  // analogous to https://github.com/facebook/rocksdb/issues/13429
+  //
+  // This option is not mutable with SetOptions(). It can be changed between
+  // DB::Open() calls, but open will fail if recovering WAL writes to a CF with
+  // this option set.
+  bool disallow_memtable_writes = false;
+
   // This option has different meanings for different compaction styles:
   //
   // Leveled: Non-bottom-level files with all keys older than TTL will go

include/rocksdb/c.h

@@ -692,9 +692,6 @@ extern ROCKSDB_LIBRARY_API void rocksdb_compact_range_cf_opt(
     rocksdb_compactoptions_t* opt, const char* start_key, size_t start_key_len,
     const char* limit_key, size_t limit_key_len);
 
-extern ROCKSDB_LIBRARY_API void DEPRECATED_rocksdb_delete_file(
-    rocksdb_t* db, const char* name);
-
 extern ROCKSDB_LIBRARY_API const rocksdb_livefiles_t* rocksdb_livefiles(
     rocksdb_t* db);
 

include/rocksdb/compression_type.h

@@ -27,13 +27,6 @@ enum CompressionType : unsigned char {
   kXpressCompression = 0x6,
   kZSTD = 0x7,
 
-  // Only use kZSTDNotFinalCompression if you have to use ZSTD lib older than
-  // 0.8.0 or consider a possibility of downgrading the service or copying
-  // the database files to another service running with an older version of
-  // RocksDB that doesn't have kZSTD. Otherwise, you should use kZSTD. We will
-  // eventually remove the option from the public API.
-  kZSTDNotFinalCompression = 0x40,
-
   // kDisableCompressionOption is used to disable some compression options.
   kDisableCompressionOption = 0xff,
 };
@@ -181,6 +174,10 @@ struct CompressionOptions {
   void SetMinRatio(double min_ratio) {
     max_compressed_bytes_per_kb = static_cast<int>(1024.0 / min_ratio + 0.5);
   }
+
+#if __cplusplus >= 202002L
+  bool operator==(const CompressionOptions& rhs) const = default;
+#endif
 };
 
 }  // namespace ROCKSDB_NAMESPACE

include/rocksdb/db.h

@@ -294,6 +294,13 @@ class DB {
   // delete it after use.
   //
   // Return OK on success, non-OK on failures.
+  //
+  // WARNING: Secondary databases cannot read shared SST files that have been
+  // truncated in the primary database. To avoid compatibility issues, users
+  // should refrain from using features in the primary database that can cause
+  // truncation, such as setting `rate_bytes_per_sec > 0` and
+  // `bytes_max_delete_chunk > 0` when invoking RocksDB's implementation of
+  // `NewSstFileManager()`.
   static Status OpenAsSecondary(const Options& options, const std::string& name,
                                 const std::string& secondary_path,
                                 std::unique_ptr<DB>* dbptr);
@@ -764,8 +771,9 @@ class DB {
   // Populates the `merge_operands` array with all the merge operands in the DB
   // for `key`, or a customizable suffix of merge operands when
   // `GetMergeOperandsOptions::continue_cb` is set. The `merge_operands` array
-  // will be populated in the order of insertion. The number of entries
-  // populated in `merge_operands` will be assigned to `*number_of_operands`.
+  // will be populated in the order of insertion (older insertions first). The
+  // number of entries populated in `merge_operands` will be assigned to
+  // `*number_of_operands`.
   //
   // If the number of merge operands to return for `key` is greater than
   // `merge_operands_options.expected_max_number_of_operands`,
@@ -780,6 +788,9 @@ class DB {
   // The caller should delete or `Reset()` the `merge_operands` entries when
   // they are no longer needed. All `merge_operands` entries must be destroyed
   // or `Reset()` before this DB is closed or destroyed.
+  // OK status is returned if any merge operand is found.
+  // NotFound status is returned if no merge operand is found.
+  // Error status is returned if there is an error.
   virtual Status GetMergeOperands(
       const ReadOptions& options, ColumnFamilyHandle* column_family,
       const Slice& key, PinnableSlice* merge_operands,
@@ -1210,6 +1221,10 @@ class DB {
     //      running compactions.
     static const std::string kNumRunningCompactions;
 
+    //  "rocksdb.num-running-compaction-sorted-runs" - returns the number of
+    //  sorted runs being processed by currently running compactions.
+    static const std::string kNumRunningCompactionSortedRuns;
+
     //  "rocksdb.background-errors" - returns accumulated number of background
     //      errors.
     static const std::string kBackgroundErrors;
@@ -1836,19 +1851,6 @@ class DB {
       const TransactionLogIterator::ReadOptions& read_options =
           TransactionLogIterator::ReadOptions()) = 0;
 
-  // WARNING: This API is planned for removal in RocksDB 7.0 since it does not
-  // operate at the proper level of abstraction for a key-value store, and its
-  // contract/restrictions are poorly documented. For example, it returns non-OK
-  // `Status` for non-bottommost files and files undergoing compaction. Since we
-  // do not plan to maintain it, the contract will likely remain underspecified
-  // until its removal. Any user is encouraged to read the implementation
-  // carefully and migrate away from it when possible.
-  //
-  // Delete the file name from the db directory and update the internal state to
-  // reflect that. Supports deletion of sst and log files only. 'name' must be
-  // path relative to the db directory. eg. 000001.sst, /archive/000003.log
-  virtual Status DEPRECATED_DeleteFile(std::string name) = 0;
-
   // Obtains a list of all live table (SST) files and how they fit into the
   // LSM-trees, such as column family, level, key range, etc.
   // This builds a de-normalized form of GetAllColumnFamilyMetaData().

include/rocksdb/db_bench_tool.h

@@ -5,7 +5,8 @@
 #pragma once
 
 #include "rocksdb/rocksdb_namespace.h"
+#include "rocksdb/tool_hooks.h"
 
 namespace ROCKSDB_NAMESPACE {
-int db_bench_tool(int argc, char** argv);
+int db_bench_tool(int argc, char** argv, ToolHooks& hooks = defaultHooks);
 }  // namespace ROCKSDB_NAMESPACE

include/rocksdb/env.h

@@ -627,7 +627,7 @@ class Env : public Customizable {
       const EnvOptions& env_options,
       const ImmutableDBOptions& immutable_ops) const;
 
-  // OptimizeForCompactionTableWrite will create a new EnvOptions object that
+  // OptimizeForCompactionTableRead will create a new EnvOptions object that
   // is a copy of the EnvOptions in the parameters, but is optimized for reading
   // table files.
   virtual EnvOptions OptimizeForCompactionTableRead(

include/rocksdb/external_table.h

@@ -6,6 +6,7 @@
 #pragma once
 
 #include "rocksdb/customizable.h"
+#include "rocksdb/file_checksum.h"
 #include "rocksdb/iterator.h"
 #include "rocksdb/options.h"
 #include "rocksdb/status.h"
@@ -18,22 +19,21 @@ class ExternalTableFactory;
 // The interface defined in this file is subject to change at any time without
 // warning!!
 
-// This file defines an interface for plugging in an external table reader
+// This file defines an interface for plugging in an external table
 // into RocksDB. The external table reader will be used instead of the
-// BlockBasedTable to load and query sst files. As of now, creating the
-// external table files using RocksDB is not supported, but will be added in
-// the near future. The external table files can be created outside and
-// RocksDB and ingested into a RocksDB instance using the IngestExternalFIle()
-// API.
+// BlockBasedTable to load and query sst files.
+// The external table files can be created using an SstFileWriter. Eventually
+// external tables will be allowed to be ingested into a RocksDB instance
+// using the IngestExternalFIle() API.
 //
-// Initial support is for loading and querying the files using an
-// SstFileReader. We will add support for ingestion of an external table
-// into a limited RocksDB instance that only supports ingestion and not live
-// writes in the near future. It'll be followed by support for replacing the
-// column family by ingesting a new set of files. In all cases, the external
-// table files will only be allowed in the bottommost level.
+// Initial support is for writing and querying the files using an
+// SstFileWriter and SstFileReader. We will add support for ingestion of an
+// external table into a limited RocksDB instance that only supports ingestion
+// and not live writes in the near future. It'll be followed by support for
+// replacing the column family by ingesting a new set of files. In all cases,
+// the external table files will only be allowed in the bottommost level.
 //
-// The external table reader can support one or both of the following layouts -
+// The external table can support one or both of the following layouts -
 // 1. Total order seek - All the keys in the files are in sorted order, and a
 //    user can seek to the first, last, or any key in between and iterate
 //    forwards or backwards till the end of the range. To support this mode,
@@ -54,8 +54,8 @@ class ExternalTableFactory;
 //    true to seek to the first non-empty prefix (as determined by the key
 //    order) if the seek prefix is empty.
 //
-// Many of the options in ReadOptions may not be relevant to the external
-// table implementation.
+// Many of the options in ReadOptions and WriteOptions may not be relevant to
+// the external table implementation.
 // TODO: Specify which options are relevant
 
 class ExternalTableReader {
@@ -94,6 +94,62 @@ class ExternalTableReader {
   }
 };
 
+// A table builder interface that can be used by SstFileWriter to allow
+// RocksDB users to write external table files. The sequence of operations
+// to write an external table is as follows -
+// 1. Add() is called one or more times to write all key-values to the table.
+//    Its called in increasing key order, as determined by the comparator.
+//    The input key is a user key, i.e sequence number and value type are
+//    stripped out.
+// 2. After every Add() operation, status() is called to check the current
+//    status.
+// 3. After the last key is added, Finish() is called to do whatever is
+//    necessary to ensure the data is persisted in the table file.
+// 4. If there is a failure midway for some reason, Abandon() is called
+//    instead of Finish().
+// 5. At the end, FileSize(), GetTableProperties(), and status() are called to
+//    get the final size of the file, the table properties, and the final
+//    status. GetFileChecksum() and GetFileChecksumFuncName() may also be
+//    called to get checksum information about the whole file, but their
+//    implementation is optional.
+class ExternalTableBuilder {
+ public:
+  virtual ~ExternalTableBuilder() {}
+
+  // Write a single KV to the table file. This is guaranteed to be called
+  // in key order, and the write may be buffered and flushed at a later time.
+  virtual void Add(const Slice& key, const Slice& value) = 0;
+
+  // Return the current Status. This could return non-ok, for example, if
+  // Add() fails for some reason.
+  virtual Status status() const = 0;
+
+  // Flush and close the table file
+  virtual Status Finish() = 0;
+
+  // Delete the partial file and release any allocated resources. Either this
+  // or Finish() will be called, but not both.
+  virtual void Abandon() = 0;
+
+  // Return the size of the table file. Will be called at the end, after
+  // Finish().
+  virtual uint64_t FileSize() const = 0;
+
+  //  As mentioned in earlier comments, the following table properties must be
+  //  returned at a minimum -
+  //  comparator_name
+  //  num_entries
+  //  raw_key_size
+  //  raw_value_size
+  virtual TableProperties GetTableProperties() const = 0;
+
+  virtual std::string GetFileChecksum() const { return kUnknownFileChecksum; }
+
+  virtual const char* GetFileChecksumFuncName() const {
+    return kUnknownFileChecksumFuncName;
+  }
+};
+
 struct ExternalTableOptions {
   const std::shared_ptr<const SliceTransform>& prefix_extractor;
   const Comparator* comparator;
@@ -104,6 +160,29 @@ struct ExternalTableOptions {
       : prefix_extractor(_prefix_extractor), comparator(_comparator) {}
 };
 
+struct ExternalTableBuilderOptions {
+  const ReadOptions& read_options;
+  const WriteOptions& write_options;
+  const std::shared_ptr<const SliceTransform>& prefix_extractor;
+  const Comparator* comparator;
+  const std::string& column_family_name;
+  const std::string db_id;
+  const std::string db_session_id;
+  const TableFileCreationReason reason;
+
+  ExternalTableBuilderOptions(
+      const ReadOptions& _read_options, const WriteOptions& _write_options,
+      const std::shared_ptr<const SliceTransform>& _prefix_extractor,
+      const Comparator* _comparator, const std::string& _column_family_name,
+      const TableFileCreationReason _reason)
+      : read_options(_read_options),
+        write_options(_write_options),
+        prefix_extractor(_prefix_extractor),
+        comparator(_comparator),
+        column_family_name(_column_family_name),
+        reason(_reason) {}
+};
+
 class ExternalTableFactory : public Customizable {
  public:
   ~ExternalTableFactory() override {}
@@ -113,7 +192,11 @@ class ExternalTableFactory : public Customizable {
   virtual Status NewTableReader(
       const ReadOptions& read_options, const std::string& file_path,
       const ExternalTableOptions& table_options,
-      std::unique_ptr<ExternalTableReader>* table_reader) = 0;
+      std::unique_ptr<ExternalTableReader>* table_reader) const = 0;
+
+  virtual ExternalTableBuilder* NewTableBuilder(
+      const ExternalTableBuilderOptions& builder_options,
+      const std::string& file_path) const = 0;
 };
 
 // Allocate a TableFactory that wraps around an ExternalTableFactory. Use this

include/rocksdb/options.h

@@ -454,6 +454,7 @@ extern const char* kHostnameForDbHostId;
 enum class CompactionServiceJobStatus : char {
   kSuccess,
   kFailure,
+  kAborted,
   kUseLocal,
 };
 
@@ -1946,6 +1947,28 @@ struct ReadOptions {
   // Default: false
   bool allow_unprepared_value = false;
 
+  // EXPERIMENTAL
+  //
+  // Long-running iterators are holding onto memory and storage resources long
+  // after they are obsolete. This setting (when enabled) will fix that problem
+  // for as long as iterator periodically makes some progress and its supplied
+  // `read_options` was configured with non-nullptr `snapshot` value.
+  // The feature is engineered so that the performance impact should be
+  // negligible. We expect the default value to be true some time in the future.
+  //
+  // NOTE 1: Does not have effect on TransactionDB with WRITE_PREPARED or
+  //         WRITE_UNPREPARED policies (currently incompatible).
+  //
+  // NOTE 2: True is not recommended if using user-defined timestamp with
+  //         persist_user_defined_timestamps=false and non-nullptr
+  //         ReadOptions::timestamp or ReadOptions::iter_start_ts, because
+  //         auto-refreshing iterator will not prevent user timestamp
+  //         information from being dropped during iteration. Auto-refresh might
+  //         be disabled for this combination in the future.
+  //
+  // Default: false
+  bool auto_refresh_iterator_with_snapshot = false;
+
   // *** END options only relevant to iterators or scans ***
 
   // *** BEGIN options for RocksDB internal use only ***
@@ -1963,6 +1986,12 @@ struct ReadOptions {
   // reader implementation.
   uint64_t weight = 0;
 
+  // A map of name,value pairs that can be passed by the user to an
+  // external table reader. This is completely opaque to RocksDB and is
+  // ignored by the natively supported table readers like block based and plain
+  // table. This is only useful for Iterator.
+  std::optional<std::unordered_map<std::string, std::string>> property_bag;
+
   // *** END options for RocksDB internal use only ***
 
   ReadOptions() {}

include/rocksdb/tool_hooks.h

@@ -0,0 +1,124 @@
+//  Copyright (c) Meta Platforms, Inc. and affiliates.  All rights reserved.
+//  This source code is licensed under both the GPLv2 (found in the
+//  COPYING file in the root directory) and Apache 2.0 License
+//  (found in the LICENSE.Apache file in the root directory).
+
+#pragma once
+
+#include "rocksdb/db.h"
+
+namespace ROCKSDB_NAMESPACE {
+
+struct TransactionDBOptions;
+class TransactionDB;
+class OptimisticTransactionDB;
+
+namespace blob_db {
+struct BlobDBOptions;
+class BlobDB;
+}  // namespace blob_db
+
+/*
+ * ToolHooks is currently a WORK IN PROGRESS, API is subject to change.
+ * ToolHooks is a class that allows users to override the default behavior of
+ * the various OpenDB calls used by db_bench_tool.  This allows users to easily
+ * extend the functionality of db_bench_tool to support their own open
+ * implementations.
+ */
+class ToolHooks {
+ public:
+  ToolHooks() = default;
+  virtual ~ToolHooks() = default;
+  virtual Status Open(const Options& db_options, const std::string& name,
+                      DB** dbptr) = 0;
+  virtual Status Open(
+      const DBOptions& db_options, const std::string& name,
+      const std::vector<ColumnFamilyDescriptor>& column_families,
+      std::vector<ColumnFamilyHandle*>* handles, DB** dbptr) = 0;
+  virtual Status OpenForReadOnly(const Options& options,
+                                 const std::string& name, DB** dbptr,
+                                 bool error_if_wal_file_exists) = 0;
+  virtual Status OpenForReadOnly(
+      const Options& options, const std::string& name,
+      const std::vector<ColumnFamilyDescriptor>& column_families,
+      std::vector<ColumnFamilyHandle*>* handles, DB** dbptr) = 0;
+  virtual Status OpenTransactionDB(const Options& db_options,
+                                   const TransactionDBOptions& txn_db_options,
+                                   const std::string& dbname,
+                                   TransactionDB** dbptr) = 0;
+  virtual Status OpenTransactionDB(
+      const DBOptions& db_options, const TransactionDBOptions& txn_db_options,
+      const std::string& dbname,
+      const std::vector<ColumnFamilyDescriptor>& column_families,
+      std::vector<ColumnFamilyHandle*>* handles, TransactionDB** dbptr) = 0;
+  virtual Status OpenOptimisticTransactionDB(
+      const Options& options, const std::string& dbname,
+      OptimisticTransactionDB** dbptr) = 0;
+  virtual Status OpenOptimisticTransactionDB(
+      const DBOptions& db_options, const std::string& dbname,
+      const std::vector<ColumnFamilyDescriptor>& column_families,
+      std::vector<ColumnFamilyHandle*>* handles,
+      OptimisticTransactionDB** dbptr) = 0;
+  virtual Status OpenAsSecondary(const Options& options,
+                                 const std::string& name,
+                                 const std::string& secondary_path,
+                                 DB** dbptr) = 0;
+  virtual Status OpenAsFollower(const Options& options, const std::string& name,
+                                const std::string& leader_path,
+                                std::unique_ptr<DB>* dbptr) = 0;
+  virtual Status Open(const Options& options,
+                      const blob_db::BlobDBOptions& bdb_options,
+                      const std::string& dbname, blob_db::BlobDB** blob_db) = 0;
+};
+
+class DefaultHooks : public ToolHooks {
+ public:
+  DefaultHooks() = default;
+  ~DefaultHooks() override = default;
+  virtual Status Open(const Options& db_options, const std::string& name,
+                      DB** dbptr) override;
+  virtual Status Open(
+      const DBOptions& db_options, const std::string& name,
+      const std::vector<ColumnFamilyDescriptor>& column_families,
+      std::vector<ColumnFamilyHandle*>* handles, DB** dbptr) override;
+  virtual Status OpenForReadOnly(const Options& options,
+                                 const std::string& name, DB** dbptr,
+                                 bool error_if_wal_file_exists) override;
+  virtual Status OpenForReadOnly(
+      const Options& options, const std::string& name,
+      const std::vector<ColumnFamilyDescriptor>& column_families,
+      std::vector<ColumnFamilyHandle*>* handles, DB** dbptr) override;
+  virtual Status OpenTransactionDB(const Options& db_options,
+                                   const TransactionDBOptions& txn_db_options,
+                                   const std::string& dbname,
+                                   TransactionDB** dbptr) override;
+  virtual Status OpenTransactionDB(
+      const DBOptions& db_options, const TransactionDBOptions& txn_db_options,
+      const std::string& dbname,
+      const std::vector<ColumnFamilyDescriptor>& column_families,
+      std::vector<ColumnFamilyHandle*>* handles,
+      TransactionDB** dbptr) override;
+  virtual Status OpenOptimisticTransactionDB(
+      const Options& options, const std::string& dbname,
+      OptimisticTransactionDB** dbptr) override;
+  virtual Status OpenOptimisticTransactionDB(
+      const DBOptions& db_options, const std::string& dbname,
+      const std::vector<ColumnFamilyDescriptor>& column_families,
+      std::vector<ColumnFamilyHandle*>* handles,
+      OptimisticTransactionDB** dbptr) override;
+  virtual Status OpenAsSecondary(const Options& options,
+                                 const std::string& name,
+                                 const std::string& secondary_path,
+                                 DB** dbptr) override;
+  virtual Status OpenAsFollower(const Options& options, const std::string& name,
+                                const std::string& leader_path,
+                                std::unique_ptr<DB>* dbptr) override;
+  virtual Status Open(const Options& options,
+                      const blob_db::BlobDBOptions& bdb_options,
+                      const std::string& dbname,
+                      blob_db::BlobDB** blob_db) override;
+};
+
+extern DefaultHooks defaultHooks;
+
+}  // namespace ROCKSDB_NAMESPACE

include/rocksdb/universal_compaction.h

@@ -122,6 +122,10 @@ class CompactionOptionsUniversal {
         stop_style(kCompactionStopStyleTotalSize),
         allow_trivial_move(false),
         incremental(false) {}
+
+#if __cplusplus >= 202002L
+  bool operator==(const CompactionOptionsUniversal& rhs) const = default;
+#endif
 };
 
 }  // namespace ROCKSDB_NAMESPACE

include/rocksdb/utilities/backup_engine.h

@@ -126,8 +126,8 @@ struct BackupEngineOptions {
   // Default: true
   bool share_files_with_checksum;
 
-  // Up to this many background threads will copy files for CreateNewBackup()
-  // and RestoreDBFromBackup()
+  // Up to this many background threads will be used to copy files & compute
+  // checksums for CreateNewBackup() and RestoreDBFromBackup().
   // Default: 1
   int max_background_operations;
 
@@ -349,6 +349,39 @@ struct CreateBackupOptions {
 };
 
 struct RestoreOptions {
+  // Enum reflecting tiered approach to restores.
+  //
+  // Options `kKeepLatestDbSessionIdFiles`, `kVerifyChecksum` introduce
+  // incremental restore capability and are intended to be used separately.
+  enum Mode : uint32_t {
+    // Most efficient way to restore a healthy / non-corrupted DB from
+    // the backup(s). This mode can almost always successfully recover from
+    // incomplete / missing files, as in an incomplete copy of a DB.
+    // This mode is also integrated with `exclude_files_callback` feature
+    // and will opportunistically try to find excluded files in existing db
+    // filesystem if missing in all supplied backup directories.
+    //
+    // Effective on data files following modern share files naming schemes.
+    kKeepLatestDbSessionIdFiles = 1U,
+
+    // Recommended when db is suspected to be unhealthy, ex. we want to retain
+    // most of the files (therefore saving on write I/O) with an exception of
+    // a few corrupted ones.
+    //
+    // When opted-in, restore engine will scan the db file, compute the
+    // checksum and compare it against the checksum hardened in the backup file
+    // metadata. If checksums match, existing file will be retained as-is.
+    // Otherwise, it will be deleted and replaced it with its' restored backup
+    // counterpart. If backup file doesn't have a checksum hardened in the
+    // metadata, we'll schedule an async task to compute it.
+    kVerifyChecksum = 2U,
+
+    // Zero trust. Least efficient.
+    //
+    // Purge all the destination files and restores all files from the backup.
+    kPurgeAllFiles = 0xffffU,
+  };
+
   // If true, restore won't overwrite the existing log files in wal_dir. It will
   // also move all log files from archive directory to wal_dir. Use this option
   // in combination with BackupEngineOptions::backup_log_files = false for
@@ -361,8 +394,13 @@ struct RestoreOptions {
   // directories known to contain the required files.
   std::forward_list<BackupEngineReadOnlyBase*> alternate_dirs;
 
-  explicit RestoreOptions(bool _keep_log_files = false)
-      : keep_log_files(_keep_log_files) {}
+  // Specifies the level of incremental restore. 'kPurgeAllFiles' by default.
+  Mode mode;
+
+  // FIXME(https://github.com/facebook/rocksdb/issues/13293)
+  explicit RestoreOptions(bool _keep_log_files = false,
+                          Mode _mode = Mode::kPurgeAllFiles)
+      : keep_log_files(_keep_log_files), mode(_mode) {}
 };
 
 using BackupID = uint32_t;

include/rocksdb/utilities/secondary_index.h

@@ -15,7 +15,6 @@
 #include "rocksdb/rocksdb_namespace.h"
 #include "rocksdb/slice.h"
 #include "rocksdb/status.h"
-#include "rocksdb/utilities/secondary_index_options.h"
 
 namespace ROCKSDB_NAMESPACE {
 
@@ -45,8 +44,7 @@ class ColumnFamilyHandle;
 // explicit or implicit one), RocksDB will invoke any applicable SecondaryIndex
 // objects based on primary column family and column name, and it will
 // automatically add or remove any secondary index entries as needed (using
-// the same transaction). Secondary indices can also be queried using an
-// iterator API, which has implementation-specific semantics.
+// the same transaction).
 //
 // Note: the methods of SecondaryIndex implementations are expected to be
 // thread-safe with the exception of Set{Primary,Secondary}ColumnFamily (which
@@ -80,12 +78,22 @@ class SecondaryIndex {
   // called by the transaction layer when adding or removing secondary index
   // entries (which have the form <secondary_key_prefix><primary_key> ->
   // <secondary_value>) and should be deterministic. The output parameter
-  // secondary_key_prefix is expected to be based on primary_column_value,
-  // potentially with some additional metadata to prevent ambiguities (e.g.
-  // index id or length indicator). Returning a non-OK status rolls back all
-  // operations in the transaction related to this primary key-value.
+  // secondary_key_prefix is expected to be based on primary_key and/or
+  // primary_column_value. Returning a non-OK status rolls back all operations
+  // in the transaction related to this primary key-value.
   virtual Status GetSecondaryKeyPrefix(
-      const Slice& primary_column_value,
+      const Slice& primary_key, const Slice& primary_column_value,
+      std::variant<Slice, std::string>* secondary_key_prefix) const = 0;
+
+  // Finalize the secondary key prefix, for instance by adding some metadata to
+  // prevent ambiguities (e.g. index id or length indicator). This method is
+  // called by the transaction layer when adding or removing secondary index
+  // entries (which have the form <secondary_key_prefix><primary_key> ->
+  // <secondary_value>) and also when querying the index (in which case it is
+  // called with the search target). The method should be deterministic.
+  // Returning a non-OK status rolls back all operations in the transaction
+  // related to this primary key-value.
+  virtual Status FinalizeSecondaryKeyPrefix(
       std::variant<Slice, std::string>* secondary_key_prefix) const = 0;
 
   // Get the optional secondary value for a given primary key-value. This method
@@ -96,35 +104,87 @@ class SecondaryIndex {
   // Returning a non-OK status rolls back all operations in the transaction
   // related to this primary key-value.
   virtual Status GetSecondaryValue(
-      const Slice& primary_column_value, const Slice& previous_column_value,
+      const Slice& primary_key, const Slice& primary_column_value,
+      const Slice& previous_column_value,
       std::optional<std::variant<Slice, std::string>>* secondary_value)
       const = 0;
+};
 
-  // Create an iterator that can be used by applications to query the index.
-  // This method takes a SecondaryIndexReadOptions structure, which can be used
-  // by applications to provide (implementation-specific) query parameters to
-  // the index as well as an underlying iterator over the index's secondary
-  // column family, which the returned iterator is expected to take ownership of
-  // and use to read the actual secondary index entries. (Providing the
-  // underlying iterator this way enables querying the index as of a specific
-  // point in time for example.)
-  //
-  // Querying the index can be performed by calling the returned iterator's
-  // Seek API with a search target, and then using Next (and potentially
-  // Prev) to iterate through the matching index entries. SeekToFirst,
-  // SeekToLast, and SeekForPrev are not expected to be supported by the
-  // iterator. The iterator should expose primary keys, that is, the secondary
-  // key prefix should be stripped from the index entries.
-  //
-  // The exact semantics of the returned iterator depend on the index and are
-  // implementation-specific. For simple indices, the search target might be a
-  // primary column value, and the iterator might return all primary keys that
-  // have the given column value; however, other semantics are also possible.
-  // For vector indices, the search target might be a vector, and the iterator
-  // might return similar vectors from the index.
-  virtual std::unique_ptr<Iterator> NewIterator(
-      const SecondaryIndexReadOptions& read_options,
-      std::unique_ptr<Iterator>&& underlying_it) const = 0;
+// SecondaryIndexIterator can be used to find the primary keys for a given
+// search target. It can be used as-is or as a building block. Its interface
+// mirrors most of the Iterator API, with the exception of SeekToFirst,
+// SeekToLast, and SeekForPrev, which are not applicable to secondary indices
+// and thus not present. Querying the index can be performed by calling the
+// returned iterator's Seek API with a search target, and then using Next (and
+// potentially Prev) to iterate through the matching index entries. The iterator
+// exposes primary keys, that is, the secondary key prefix is stripped from the
+// index entries.
+
+class SecondaryIndexIterator {
+ public:
+  // Constructs a SecondaryIndexIterator. The SecondaryIndexIterator takes
+  // ownership of the underlying iterator.
+  // PRE: index is not nullptr
+  // PRE: underlying_it is not nullptr and points to an iterator over the
+  // index's secondary column family
+  SecondaryIndexIterator(const SecondaryIndex* index,
+                         std::unique_ptr<Iterator>&& underlying_it);
+
+  // Returns whether the iterator is valid, i.e. whether it is positioned on a
+  // secondary index entry matching the search target.
+  bool Valid() const;
+
+  // Returns the status of the iterator, which is guaranteed to be OK if the
+  // iterator is valid. Otherwise, it might be non-OK, which indicates an error,
+  // or OK, which means that the iterator has reached the end of the applicable
+  // secondary index entries.
+  Status status() const;
+
+  // Query the index with the given search target.
+  void Seek(const Slice& target);
+
+  // Move the iterator to the next entry.
+  // PRE: Valid()
+  void Next();
+
+  // Move the iterator back to the previous entry.
+  // PRE: Valid()
+  void Prev();
+
+  // Prepare the value of the current entry. Should be called before calling
+  // value() or columns() if the underlying iterator was constructed with the
+  // read option allow_unprepared_value set to true. Returns true upon success.
+  // Returns false and sets the status to non-OK upon failure.
+  // PRE: Valid()
+  bool PrepareValue();
+
+  // Returns the primary key from the current secondary index entry.
+  // PRE: Valid()
+  Slice key() const;
+
+  // Returns the value of the current secondary index entry.
+  // PRE: Valid()
+  Slice value() const;
+
+  // Returns the value of the current secondary index entry as a wide-column
+  // structure.
+  // PRE: Valid()
+  const WideColumns& columns() const;
+
+  // Returns the timestamp of the current secondary index entry.
+  // PRE: Valid()
+  Slice timestamp() const;
+
+  // Queries the given property of the underlying iterator. Returns OK on
+  // success, non-OK on failure.
+  // PRE: Valid()
+  Status GetProperty(std::string prop_name, std::string* prop) const;
+
+ private:
+  const SecondaryIndex* index_;
+  std::unique_ptr<Iterator> underlying_it_;
+  Status status_;
+  std::string prefix_;
 };
 
 }  // namespace ROCKSDB_NAMESPACE

include/rocksdb/utilities/secondary_index_faiss.h

@@ -8,8 +8,11 @@
 
 #include <memory>
 #include <string>
+#include <utility>
+#include <vector>
 
 #include "rocksdb/rocksdb_namespace.h"
+#include "rocksdb/slice.h"
 #include "rocksdb/utilities/secondary_index.h"
 
 namespace faiss {
@@ -20,13 +23,95 @@ namespace ROCKSDB_NAMESPACE {
 
 // EXPERIMENTAL
 //
-// Creates a new FAISS inverted file based secondary index that indexes the
-// embedding in the specified primary column using the given pre-trained
-// faiss::IndexIVF object (which the secondary index takes ownership of).
-// The secondary index iterator returned by the index can be used to perform
-// K-nearest-neighbors queries (see also SecondaryIndex::NewIterator and
-// SecondaryIndexReadOptions).
-std::unique_ptr<SecondaryIndex> NewFaissIVFIndex(
-    std::unique_ptr<faiss::IndexIVF>&& index, std::string primary_column_name);
+// A SecondaryIndex implementation that wraps a FAISS inverted file based index.
+// Indexes the embedding in the specified primary column using the given
+// pre-trained faiss::IndexIVF object. Can be used to perform
+// K-nearest-neighbors queries.
+
+class FaissIVFIndex : public SecondaryIndex {
+ public:
+  // Constructs a FaissIVFIndex object. Takes ownership of the given
+  // faiss::IndexIVF instance.
+  // PRE: index is not nullptr
+  FaissIVFIndex(std::unique_ptr<faiss::IndexIVF>&& index,
+                std::string primary_column_name);
+  ~FaissIVFIndex() override;
+
+  void SetPrimaryColumnFamily(ColumnFamilyHandle* column_family) override;
+  void SetSecondaryColumnFamily(ColumnFamilyHandle* column_family) override;
+
+  ColumnFamilyHandle* GetPrimaryColumnFamily() const override;
+  ColumnFamilyHandle* GetSecondaryColumnFamily() const override;
+
+  Slice GetPrimaryColumnName() const override;
+
+  Status UpdatePrimaryColumnValue(
+      const Slice& primary_key, const Slice& primary_column_value,
+      std::optional<std::variant<Slice, std::string>>* updated_column_value)
+      const override;
+
+  Status GetSecondaryKeyPrefix(
+      const Slice& primary_key, const Slice& primary_column_value,
+      std::variant<Slice, std::string>* secondary_key_prefix) const override;
+
+  Status FinalizeSecondaryKeyPrefix(
+      std::variant<Slice, std::string>* secondary_key_prefix) const override;
+
+  Status GetSecondaryValue(const Slice& primary_key,
+                           const Slice& primary_column_value,
+                           const Slice& original_column_value,
+                           std::optional<std::variant<Slice, std::string>>*
+                               secondary_value) const override;
+
+  // Performs a K-nearest-neighbors vector similarity search for the target
+  // using the given secondary index iterator, where K is given by the parameter
+  // neighbors and the number of inverted lists to search is given by the
+  // parameter probes. The resulting primary keys and distances are returned in
+  // the result output parameter. Note that the search may return less than the
+  // requested number of results if the inverted lists probed are exhausted
+  // before finding K items.
+  //
+  // The parameter it should be non-nullptr and point to a secondary index
+  // iterator corresponding to this index. The search target should be of the
+  // correct dimension (i.e. target.size() == dim * sizeof(float), where dim is
+  // the dimensionality of the index), neighbors and probes should be positive,
+  // and result should be non-nullptr.
+  //
+  // Returns OK on success, InvalidArgument if the preconditions above are not
+  // met, or some other non-OK status if there is an error during the search.
+  Status FindKNearestNeighbors(
+      SecondaryIndexIterator* it, const Slice& target, size_t neighbors,
+      size_t probes, std::vector<std::pair<std::string, float>>* result) const;
+
+ private:
+  struct KNNContext;
+  class Adapter;
+
+  std::unique_ptr<Adapter> adapter_;
+  std::unique_ptr<faiss::IndexIVF> index_;
+  std::string primary_column_name_;
+  ColumnFamilyHandle* primary_column_family_{};
+  ColumnFamilyHandle* secondary_column_family_{};
+};
+
+// Helper methods to convert embeddings from a span of floats to Slice or vice
+// versa
+
+// Convert the given span of floats of size dim to a Slice.
+// PRE: embedding points to a contiguous span of floats of size dim
+inline Slice ConvertFloatsToSlice(const float* embedding, size_t dim) {
+  return Slice(reinterpret_cast<const char*>(embedding), dim * sizeof(float));
+}
+
+// Convert the given Slice to a span of floats of size dim.
+// PRE: embedding.size() == dim * sizeof(float)
+// Returns nullptr if the precondition is violated.
+inline const float* ConvertSliceToFloats(const Slice& embedding, size_t dim) {
+  if (embedding.size() != dim * sizeof(float)) {
+    return nullptr;
+  }
+
+  return reinterpret_cast<const float*>(embedding.data());
+}
 
 }  // namespace ROCKSDB_NAMESPACE

include/rocksdb/utilities/secondary_index_options.h

@@ -1,40 +0,0 @@
-//  Copyright (c) Meta Platforms, Inc. and affiliates.
-//
-//  This source code is licensed under both the GPLv2 (found in the
-//  COPYING file in the root directory) and Apache 2.0 License
-//  (found in the LICENSE.Apache file in the root directory).
-
-#pragma once
-
-#include <optional>
-
-#include "rocksdb/rocksdb_namespace.h"
-
-namespace ROCKSDB_NAMESPACE {
-
-// EXPERIMENTAL
-//
-// Read options for secondary index iterators (see also
-// SecondaryIndex::NewIterator)
-
-struct SecondaryIndexReadOptions {
-  // The maximum number of neighbors K to return when performing a
-  // K-nearest-neighbors vector similarity search. The number of neighbors
-  // returned can be smaller if there are not enough vectors in the inverted
-  // lists probed. Only applicable to FAISS IVF secondary indices, where it must
-  // be specified and positive. See also SecondaryIndex::NewIterator and
-  // similarity_search_probes below.
-  //
-  // Default: none
-  std::optional<size_t> similarity_search_neighbors;
-
-  // The number of inverted lists to probe when performing a K-nearest-neighbors
-  // vector similarity search. Only applicable to FAISS IVF secondary indices,
-  // where it must be specified and positive. See also
-  // SecondaryIndex::NewIterator and similarity_search_neighbors above.
-  //
-  // Default: none
-  std::optional<size_t> similarity_search_probes;
-};
-
-}  // namespace ROCKSDB_NAMESPACE

include/rocksdb/utilities/secondary_index_simple.h

@@ -6,20 +6,21 @@
 
 #pragma once
 
-#include <memory>
 #include <string>
 
-#include "faiss/IndexIVF.h"
-#include "rocksdb/utilities/secondary_index_faiss.h"
+#include "rocksdb/rocksdb_namespace.h"
+#include "rocksdb/utilities/secondary_index.h"
 
 namespace ROCKSDB_NAMESPACE {
 
-// A SecondaryIndex implementation that wraps a FAISS inverted file index.
-class FaissIVFIndex : public SecondaryIndex {
+// EXPERIMENTAL
+//
+// A simple secondary index implementation that indexes the specified column
+// as-is.
+
+class SimpleSecondaryIndex : public SecondaryIndex {
  public:
-  explicit FaissIVFIndex(std::unique_ptr<faiss::IndexIVF>&& index,
-                         std::string primary_column_name);
-  ~FaissIVFIndex() override;
+  explicit SimpleSecondaryIndex(std::string primary_column_name);
 
   void SetPrimaryColumnFamily(ColumnFamilyHandle* column_family) override;
   void SetSecondaryColumnFamily(ColumnFamilyHandle* column_family) override;
@@ -35,27 +36,19 @@ class FaissIVFIndex : public SecondaryIndex {
       const override;
 
   Status GetSecondaryKeyPrefix(
-      const Slice& primary_column_value,
+      const Slice& primary_key, const Slice& primary_column_value,
       std::variant<Slice, std::string>* secondary_key_prefix) const override;
 
-  Status GetSecondaryValue(const Slice& primary_column_value,
+  Status FinalizeSecondaryKeyPrefix(
+      std::variant<Slice, std::string>* secondary_key_prefix) const override;
+
+  Status GetSecondaryValue(const Slice& primary_key,
+                           const Slice& primary_column_value,
                            const Slice& original_column_value,
                            std::optional<std::variant<Slice, std::string>>*
                                secondary_value) const override;
 
-  std::unique_ptr<Iterator> NewIterator(
-      const SecondaryIndexReadOptions& read_options,
-      std::unique_ptr<Iterator>&& underlying_it) const override;
-
  private:
-  class KNNIterator;
-  class Adapter;
-
-  static std::string SerializeLabel(faiss::idx_t label);
-  static faiss::idx_t DeserializeLabel(Slice label_slice);
-
-  std::unique_ptr<Adapter> adapter_;
-  std::unique_ptr<faiss::IndexIVF> index_;
   std::string primary_column_name_;
   ColumnFamilyHandle* primary_column_family_{};
   ColumnFamilyHandle* secondary_column_family_{};

include/rocksdb/utilities/stackable_db.h

@@ -518,17 +518,6 @@ class StackableDB : public DB {
     return db_->GetCreationTimeOfOldestFile(creation_time);
   }
 
-  // WARNING: This API is planned for removal in RocksDB 7.0 since it does not
-  // operate at the proper level of abstraction for a key-value store, and its
-  // contract/restrictions are poorly documented. For example, it returns non-OK
-  // `Status` for non-bottommost files and files undergoing compaction. Since we
-  // do not plan to maintain it, the contract will likely remain underspecified
-  // until its removal. Any user is encouraged to read the implementation
-  // carefully and migrate away from it when possible.
-  Status DEPRECATED_DeleteFile(std::string name) override {
-    return db_->DEPRECATED_DeleteFile(name);
-  }
-
   Status GetDbIdentity(std::string& identity) const override {
     return db_->GetDbIdentity(identity);
   }

include/rocksdb/utilities/transaction.h

@@ -750,8 +750,6 @@ class Transaction {
 
   virtual TxnTimestamp GetCommitTimestamp() const { return kMaxTxnTimestamp; }
 
-  virtual bool GetCommitBypassMemTable() const { return false; }
-
  protected:
   explicit Transaction(const TransactionDB* /*db*/) {}
   Transaction() : log_number_(0), txn_state_(STARTED) {}

include/rocksdb/utilities/transaction_db.h

@@ -247,6 +247,16 @@ struct TransactionDBOptions {
   // for more details.
   std::vector<std::shared_ptr<SecondaryIndex>> secondary_indices;
 
+  // EXPERIMENTAL, SUBJECT TO CHANGE
+  // This option is only valid for write committed. If the number of updates in
+  // a transaction exceeds this threshold, then the transaction commit will skip
+  // insertions into memtable as an optimization to reduce commit latency.
+  // See comment for TransactionOptions::commit_bypass_memtable for more detail.
+  // Setting TransactionOptions::commit_bypass_memtable to true takes precedence
+  // over this option.
+  uint32_t txn_commit_bypass_memtable_threshold =
+      std::numeric_limits<uint32_t>::max();
+
  private:
   // 128 entries
   // Should the default value change, please also update wp_snapshot_cache_bits
@@ -347,7 +357,7 @@ struct TransactionOptions {
   // DeleteRange, SingleDelete.
   bool write_batch_track_timestamp_size = false;
 
-  // EXPERIMENTAL
+  // EXPERIMENTAL, SUBJECT TO CHANGE
   // Only supports write-committed policy. If set to true, the transaction will
   // skip memtable write and ingest into the DB directly during Commit(). This
   // makes Commit() much faster for transactions with many operations.
@@ -362,6 +372,11 @@ struct TransactionOptions {
   // without indexing (e.g. added directly to the transaction underlying
   // write batch through Transaction::GetWriteBatch()->GetWriteBatch())
   // are not supported. They will not be applied to the DB.
+  //
+  // NOTE: since WBWI keep track of the most recent update per key, a Put
+  // followed by a SingleDelete will be written to DB as a SingleDelete. This
+  // can cause flush/compaction to report `num_single_del_mismatch` due to
+  // consecutive SingleDeletes.
   bool commit_bypass_memtable = false;
 };
 

include/rocksdb/utilities/write_batch_with_index.h

@@ -62,23 +62,35 @@ class WBWIIterator {
 
   virtual void SeekToLast() = 0;
 
-  virtual void Seek(const Slice& key) = 0;
+  // Move to the first entry with key >= target.
+  // If there are multiple updates to the same key, the most recent update is
+  // ordered first. If `overwrite_key` is true for this WBWI, this should only
+  // affect iterator output if the write batch contains Merge.
+  virtual void Seek(const Slice& target) = 0;
 
-  virtual void SeekForPrev(const Slice& key) = 0;
+  // Move to the last entry with key <= target.
+  // If there are multiple updates to the same key, this will move iterator
+  // to the last entry, which is the oldest update.
+  virtual void SeekForPrev(const Slice& target) = 0;
 
   virtual void Next() = 0;
 
   virtual void Prev() = 0;
 
-  // the return WriteEntry is only valid until the next mutation of
-  // WriteBatchWithIndex
+  virtual Status status() const = 0;
+
+  // The returned WriteEntry is only valid until the next mutation of
+  // WriteBatchWithIndex.
   virtual WriteEntry Entry() const = 0;
 
   // For this user key, there is a single delete in this write batch,
   // and it was overwritten by another update.
   virtual bool HasOverWrittenSingleDel() const { return false; }
 
-  virtual Status status() const = 0;
+  // Returns n where the current entry is the n-th update to the current key.
+  // The update count starts from 1.
+  // Only valid if WBWI is created with overwrite_key = true.
+  virtual uint32_t GetUpdateCount() const { return 0; }
 };
 
 // A WriteBatchWithIndex with a binary searchable index built for all the keys
@@ -87,6 +99,8 @@ class WBWIIterator {
 // time, indexes will be built. By calling GetWriteBatch(), a user will get the
 // WriteBatch for the data they inserted, which can be used for DB::Write(). A
 // user can call NewIterator() to create an iterator.
+// If there are multiple updates to the same key, the most recent update is
+// ordered first (i.e. the iterator will return the most recent update first).
 class WriteBatchWithIndex : public WriteBatchBase {
  public:
   // backup_index_comparator: the backup comparator used to compare keys
@@ -98,6 +112,8 @@ class WriteBatchWithIndex : public WriteBatchBase {
   // overwrite_key: if true, overwrite the key in the index when inserting
   //                the same key as previously, so iterator will never
   //                show two entries with the same key.
+  //                Note that for Merge, it's added as a new update instead
+  //                of overwriting the existing one.
   explicit WriteBatchWithIndex(
       const Comparator* backup_index_comparator = BytewiseComparator(),
       size_t reserved_bytes = 0, bool overwrite_key = false,
@@ -195,8 +211,8 @@ class WriteBatchWithIndex : public WriteBatchBase {
   // if overwrite_key=false, then each update will be returned as a separate
   // entry, in the order of update time.
   // if overwrite_key=true, then one entry per key will be returned. Merge
-  // updates on the same key will be returned as separate entries.
-  //
+  // updates on the same key will be returned as separate entries, with most
+  // recent update ordered first.
   // The returned iterator should be deleted by the caller.
   WBWIIterator* NewIterator(ColumnFamilyHandle* column_family);
   // Create an iterator of the default column family.
@@ -229,7 +245,7 @@ class WriteBatchWithIndex : public WriteBatchBase {
 
   // Similar to previous function but does not require a column_family.
   // Note:  An InvalidArgument status will be returned if there are any Merge
-  // operators for this key.  Use previous method instead.
+  // operators for this key. Use previous method instead.
   Status GetFromBatch(const DBOptions& options, const Slice& key,
                       std::string* value) {
     return GetFromBatch(nullptr, options, key, value);

include/rocksdb/version.h

@@ -11,9 +11,9 @@
 
 // NOTE: in 'main' development branch, this should be the *next*
 // minor or major version number planned for release.
-#define ROCKSDB_MAJOR 9
-#define ROCKSDB_MINOR 11
-#define ROCKSDB_PATCH 2
+#define ROCKSDB_MAJOR 10
+#define ROCKSDB_MINOR 0
+#define ROCKSDB_PATCH 1
 
 // Do not use these. We made the mistake of declaring macros starting with
 // double underscore. Now we have to live with our choice. We'll deprecate these

java/rocksjni/rocksjni.cc

@@ -3240,25 +3240,6 @@ jlong Java_org_rocksdb_RocksDB_getUpdatesSince(JNIEnv* env, jclass,
   return 0;
 }
 
-/*
- * Class:     org_rocksdb_RocksDB
- * Method:    deprecated_deleteFile
- * Signature: (JLjava/lang/String;)V
- */
-void Java_org_rocksdb_RocksDB_deprecated_1deleteFile(JNIEnv* env, jclass,
-                                                     jlong jdb_handle,
-                                                     jstring jname) {
-  auto* db = reinterpret_cast<ROCKSDB_NAMESPACE::DB*>(jdb_handle);
-  jboolean has_exception = JNI_FALSE;
-  std::string name =
-      ROCKSDB_NAMESPACE::JniUtil::copyStdString(env, jname, &has_exception);
-  if (has_exception == JNI_TRUE) {
-    // exception occurred
-    return;
-  }
-  db->DEPRECATED_DeleteFile(name);
-}
-
 /*
  * Class:     org_rocksdb_RocksDB
  * Method:    getLiveFilesMetaData

java/src/main/java/org/rocksdb/RocksDB.java

@@ -4419,20 +4419,6 @@ public class RocksDB extends RocksObject {
         getUpdatesSince(nativeHandle_, sequenceNumber));
   }
 
-  /**
-   * Delete the file name from the db directory and update the internal state to
-   * reflect that. Supports deletion of sst and log files only. 'name' must be
-   * path relative to the db directory. eg. 000001.sst, /archive/000003.log
-   *
-   * @param name the file name
-   *
-   * @throws RocksDBException if an error occurs whilst deleting the file
-   */
-  @Deprecated
-  public void deprecated_deleteFile(final String name) throws RocksDBException {
-    deprecated_deleteFile(nativeHandle_, name);
-  }
-
   /**
    * Gets a list of all table files metadata.
    *
@@ -5055,9 +5041,6 @@ public class RocksDB extends RocksObject {
   private static native LogFile[] getSortedWalFiles(final long handle) throws RocksDBException;
   private static native long getUpdatesSince(final long handle, final long sequenceNumber)
       throws RocksDBException;
-  @Deprecated
-  private static native void deprecated_deleteFile(final long handle, final String name)
-      throws RocksDBException;
   private static native LiveFileMetaData[] getLiveFilesMetaData(final long handle);
   private static native ColumnFamilyMetaData getColumnFamilyMetaData(
       final long handle, final long columnFamilyHandle);

java/src/test/java/org/rocksdb/EventListenerTest.java

@@ -75,20 +75,49 @@ public class EventListenerTest {
 
   void deleteTableFile(final AbstractEventListener el, final AtomicBoolean wasCbCalled)
       throws RocksDBException {
-    try (final Options opt =
-             new Options().setCreateIfMissing(true).setListeners(Collections.singletonList(el));
+    final int KEY_SIZE = 20;
+    final int VALUE_SIZE = 1000;
+    final int FILE_SIZE = 64000;
+    final int NUM_FILES = 2;
+
+    final int KEY_INTERVAL = 10000;
+    /*
+     * Intention of these options is to end up reliably with NUM_FILES files.
+     * we will be deleting using deleteFilesInRange.
+     * It is writing roughly number of keys that will fit in NUM_FILES files (target size).
+     * It is writing interleaved so that files from memory on L0 will overlap.
+     * Then compaction cleans everything, and we should end up with NUM_FILES files.
+     */
+    try (final Options opt = new Options()
+                                 .setCreateIfMissing(true)
+                                 .setListeners(Collections.singletonList(el))
+                                 .setCompressionType(CompressionType.NO_COMPRESSION)
+                                 .setTargetFileSizeBase(FILE_SIZE)
+                                 .setWriteBufferSize(FILE_SIZE / 2)
+                                 .setDisableAutoCompactions(true)
+                                 .setLevelCompactionDynamicLevelBytes(false);
          final RocksDB db = RocksDB.open(opt, dbFolder.getRoot().getAbsolutePath())) {
-      assertThat(db).isNotNull();
-      final byte[] value = new byte[24];
-      rand.nextBytes(value);
-      db.put("testKey".getBytes(), value);
-      final RocksDB.LiveFiles liveFiles = db.getLiveFiles();
-      assertThat(liveFiles).isNotNull();
-      assertThat(liveFiles.files).isNotNull();
-      assertThat(liveFiles.files.isEmpty()).isFalse();
-      db.deprecated_deleteFile(liveFiles.files.get(0));
-      assertThat(wasCbCalled.get()).isTrue();
+      final int records = FILE_SIZE / (KEY_SIZE + VALUE_SIZE);
+
+      // fill database with key/value pairs
+      final byte[] value = new byte[VALUE_SIZE];
+      int key_init = 0;
+      for (int o = 0; o < NUM_FILES; ++o) {
+        int int_key = key_init++;
+        for (int i = 0; i < records; ++i) {
+          int_key += KEY_INTERVAL;
+          rand.nextBytes(value);
+
+          db.put(String.format("%020d", int_key).getBytes(), value);
+        }
+      }
+      try (final FlushOptions flushOptions = new FlushOptions().setWaitForFlush(true)) {
+        db.flush(flushOptions);
+      }
+      db.compactRange();
+      db.deleteFilesInRanges(null, Arrays.asList(null, null), false /* includeEnd */);
     }
+    assertThat(wasCbCalled.get()).isTrue();
   }
 
   @Test

java/src/test/java/org/rocksdb/RocksDBTest.java

@@ -1665,16 +1665,6 @@ public class RocksDBTest {
     }
   }
 
-  @Test
-  public void deprecated_deleteFile() throws RocksDBException {
-    try (final Options options = new Options().setCreateIfMissing(true)) {
-      final String dbPath = dbFolder.getRoot().getAbsolutePath();
-      try (final RocksDB db = RocksDB.open(options, dbPath)) {
-        db.deprecated_deleteFile("unknown");
-      }
-    }
-  }
-
   @Test
   public void getLiveFilesMetaData() throws RocksDBException {
     try (final Options options = new Options().setCreateIfMissing(true)) {

memtable/wbwi_memtable.cc

@@ -49,22 +49,28 @@ bool WBWIMemTable::Get(const LookupKey& key, std::string* value,
                        SequenceNumber* out_seq, const ReadOptions&,
                        bool immutable_memtable, ReadCallback* callback,
                        bool* is_blob_index, bool do_merge) {
+  assert(s->ok() || s->IsMergeInProgress());
   (void)immutable_memtable;
   (void)timestamp;
   (void)columns;
   assert(immutable_memtable);
   assert(!timestamp);  // TODO: support UDT
-  assert(!columns);    // TODO: support WideColumn
   assert(assigned_seqno_.upper_bound != kMaxSequenceNumber);
   assert(assigned_seqno_.lower_bound != kMaxSequenceNumber);
   // WBWI does not support DeleteRange yet.
   assert(!wbwi_->GetWriteBatch()->HasDeleteRange());
+  assert(merge_context);
 
   [[maybe_unused]] SequenceNumber read_seq =
       GetInternalKeySeqno(key.internal_key());
+  // This is memtable is a single write batch, no snapshot can be taken within
+  // assigned seqnos for this memtable.
+  assert(read_seq >= assigned_seqno_.upper_bound ||
+         read_seq < assigned_seqno_.lower_bound);
   std::unique_ptr<InternalIterator> iter{NewIterator()};
   iter->Seek(key.internal_key());
   const Slice lookup_user_key = key.user_key();
+  bool merge_in_progress = s->IsMergeInProgress();
 
   while (iter->Valid() && comparator_->EqualWithoutTimestamp(
                               ExtractUserKey(iter->key()), lookup_user_key)) {
@@ -77,7 +83,6 @@ bool WBWIMemTable::Get(const LookupKey& key, std::string* value,
     assert(type != kTypeWideColumnEntity);
     assert(type != kTypeValuePreferredSeqno);
     assert(type != kTypeDeletionWithTimestamp);
-    assert(type != kTypeMerge);
     if (!callback || callback->IsVisible(seq)) {
       if (*out_seq == kMaxSequenceNumber) {
         *out_seq = std::max(seq, *max_covering_tombstone_seq);
@@ -88,7 +93,7 @@ bool WBWIMemTable::Get(const LookupKey& key, std::string* value,
       switch (type) {
         case kTypeValue: {
           HandleTypeValue(lookup_user_key, iter->value(), iter->IsValuePinned(),
-                          do_merge, s->IsMergeInProgress(), merge_context,
+                          do_merge, merge_in_progress, merge_context,
                           moptions_.merge_operator, clock_,
                           moptions_.statistics, moptions_.info_log, s, value,
                           columns, is_blob_index);
@@ -98,16 +103,29 @@ bool WBWIMemTable::Get(const LookupKey& key, std::string* value,
         case kTypeDeletion:
         case kTypeSingleDeletion:
         case kTypeRangeDeletion: {
-          HandleTypeDeletion(lookup_user_key, s->IsMergeInProgress(),
-                             merge_context, moptions_.merge_operator, clock_,
+          HandleTypeDeletion(lookup_user_key, merge_in_progress, merge_context,
+                             moptions_.merge_operator, clock_,
                              moptions_.statistics, moptions_.info_log, s, value,
                              columns);
           assert(seq <= read_seq);
           return /*found_final_value=*/true;
         }
+        case kTypeMerge: {
+          merge_in_progress = true;
+          if (ReadOnlyMemTable::HandleTypeMerge(
+                  lookup_user_key, iter->value(), iter->IsValuePinned(),
+                  do_merge, merge_context, moptions_.merge_operator, clock_,
+                  moptions_.statistics, moptions_.info_log, s, value,
+                  columns)) {
+            return true;
+          }
+          break;
+        }
         default: {
-          std::string msg("Unrecognized or unsupported value type: " +
-                          std::to_string(static_cast<int>(type)) + ". ");
+          std::string msg(
+              "Unrecognized or unsupported value type for "
+              "WBWI-based memtable: " +
+              std::to_string(static_cast<int>(type)) + ". ");
           msg.append("User key: " +
                      ExtractUserKey(iter->key()).ToString(/*hex=*/true) + ". ");
           msg.append("seq: " + std::to_string(seq) + ".");
@@ -116,8 +134,8 @@ bool WBWIMemTable::Get(const LookupKey& key, std::string* value,
         }
       }
     }
-    // Current key not visible or we read a merge key
-    assert(s->IsMergeInProgress() || (callback && !callback->IsVisible(seq)));
+    // Current key is a merge key or not visible
+    assert(merge_in_progress || (callback && !callback->IsVisible(seq)));
     iter->Next();
   }
   if (!iter->status().ok() &&
@@ -126,6 +144,10 @@ bool WBWIMemTable::Get(const LookupKey& key, std::string* value,
     // stop further look up
     return true;
   }
+  if (merge_in_progress) {
+    assert(s->ok() || s->IsMergeInProgress());
+    *s = Status::MergeInProgress();
+  }
   return /*found_final_value=*/false;
 }
 

memtable/wbwi_memtable.h

@@ -12,13 +12,25 @@ namespace ROCKSDB_NAMESPACE {
 // of the given write batch with index (WBWI) object. This can be used to ingest
 // a transaction (which is based on WBWI) into the DB as an immutable memtable.
 //
-// REQUIRE overwrite_key to be true for the WBWI
-// Since the keys in WBWI do not have sequence number, the memtable needs to be
+// REQUIRES: overwrite_key to be true for the WBWI
+// Since the keys in WBWI do not have sequence number, this class is responsible
+// for assigning sequence numbers to the keys. This memtable needs to be
 // assigned a range of sequence numbers through AssignSequenceNumbers(seqno)
 // before being available for reads.
-// With overwrite_key = true, WBWI keeps track of the most recent update for
-// each key, and each such key will be assigned seqno.upper_bound during reads.
-// One exception is during flush, consider the following scenario:
+//
+// The sequence number assignment uses the update count for each key
+// tracked in WBWI (see WBWIIterator::GetUpdateCount()). For each key, the
+// sequence number assigned is seqno.lower_bound + update_count - 1. So more
+// recent updates will have higher sequence number.
+//
+// Since WBWI with overwrite mode keeps track of the most recent update for
+// each key, this memtable contains one update per key usually. However, there
+// are two exceptions:
+// 1. Merge operations: Each Merge operation do not overwrite existing entries,
+// if a user uses Merge, multiple entries may be kept.
+// 2. Overwriten SingleDelete: this memtable needs to emit an extra
+// SingleDelete even when the SD is overwritten by another update.
+// Consider the following scenario:
 // - WBWI has SD(k) then PUT(k, v1)
 // - DB has PUT(k, v2) in L1
 // - flush WBWI adds PUT(k, v1) into L0
@@ -26,8 +38,7 @@ namespace ROCKSDB_NAMESPACE {
 // - flush live memtable and compact it with L0 will drop SD(k) and PUT(k, v1)
 // - the PUT(k, v2) in L1 incorrectly becomes visible
 // So during flush, iterator from this memtable will need emit overwritten
-// single deletion. These single deletion entries will be
-// assigned seqno.upper_bound - 1.
+// single deletion. This SD will be assigned seqno.lower_bound.
 class WBWIMemTable final : public ReadOnlyMemTable {
  public:
   struct SeqnoRange {
@@ -253,59 +264,51 @@ class WBWIMemTableIterator final : public InternalIterator {
   }
 
   void SeekToLast() override {
+    assert(!emit_overwritten_single_del_);
     it_->SeekToLast();
     UpdateKey();
   }
 
   void Seek(const Slice& target) override {
+    // `emit_overwritten_single_del_` is only used for flush, which does
+    // sequential forward scan from the beginning.
+    assert(!emit_overwritten_single_del_);
     Slice target_user_key = ExtractUserKey(target);
+    // Moves to first update >= target_user_key
     it_->Seek(target_user_key);
-    if (it_->Valid()) {
-      // compare seqno
-      SequenceNumber seqno = GetInternalKeySeqno(target);
-      assert(!emit_overwritten_single_del_);
-      // For now all keys are assigned seqno_ub_, this may change after merge
-      // is supported.
-      assert(seqno <= assigned_seqno_.lower_bound ||
-             seqno >= assigned_seqno_.upper_bound);
-      if (seqno < assigned_seqno_.upper_bound &&
-          comparator_->Compare(it_->Entry().key, target_user_key) == 0) {
-        it_->Next();
-        // TODO: cannot assume distinct keys once Merge is supported
-        if (it_->Valid()) {
-          assert(comparator_->Compare(it_->Entry().key, target_user_key) > 0);
-        }
-      }
+    SequenceNumber target_seqno = GetInternalKeySeqno(target);
+    // Move to the first entry with seqno <= target_seqno for the same
+    // user key or a different user key.
+    while (it_->Valid() &&
+           comparator_->Compare(it_->Entry().key, target_user_key) == 0 &&
+           target_seqno < CurrentKeySeqno()) {
+      it_->Next();
     }
     UpdateKey();
   }
 
   void SeekForPrev(const Slice& target) override {
+    assert(!emit_overwritten_single_del_);
     Slice target_user_key = ExtractUserKey(target);
+    // Moves to last update <= target_user_key
     it_->SeekForPrev(target_user_key);
-    if (it_->Valid()) {
-      SequenceNumber seqno = GetInternalKeySeqno(target);
-      assert(seqno <= assigned_seqno_.lower_bound ||
-             seqno >= assigned_seqno_.upper_bound);
-      if (seqno > assigned_seqno_.upper_bound &&
-          comparator_->Compare(it_->Entry().key, target_user_key) == 0) {
-        it_->Prev();
-        if (it_->Valid()) {
-          // TODO: cannot assume distinct keys once Merge is supported
-          assert(comparator_->Compare(it_->Entry().key, target_user_key) < 0);
-        }
-      }
+    SequenceNumber target_seqno = GetInternalKeySeqno(target);
+    // Move to the first entry with seqno >= target_seqno for the same
+    // user key or a different user key.
+    while (it_->Valid() &&
+           comparator_->Compare(it_->Entry().key, target_user_key) == 0 &&
+           CurrentKeySeqno() < target_seqno) {
+      it_->Prev();
     }
     UpdateKey();
   }
 
   void Next() override {
-    // Only need to emit single deletion during flush. Since Flush does
-    // sequential forward scan, we only need to emit single deletion in Next(),
-    // and do not need to consider iterator direction change.
     assert(Valid());
     if (emit_overwritten_single_del_) {
       if (it_->HasOverWrittenSingleDel() && !at_overwritten_single_del_) {
+        // Merge and SingleDelete on the same key is undefined behavior.
+        assert(it_->Entry().type != kMergeRecord);
         UpdateSingleDeleteKey();
         return;
       }
@@ -329,6 +332,7 @@ class WBWIMemTableIterator final : public InternalIterator {
   }
 
   void Prev() override {
+    assert(!emit_overwritten_single_del_);
     assert(Valid());
     it_->Prev();
     UpdateKey();
@@ -341,7 +345,6 @@ class WBWIMemTableIterator final : public InternalIterator {
 
   Slice value() const override {
     assert(Valid());
-    // TODO: it_->Entry() is not trivial, cache it
     return it_->Entry().value;
   }
 
@@ -355,6 +358,16 @@ class WBWIMemTableIterator final : public InternalIterator {
  private:
   static const std::unordered_map<WriteType, ValueType> WriteTypeToValueTypeMap;
 
+  SequenceNumber CurrentKeySeqno() {
+    assert(it_->Valid());
+    assert(it_->GetUpdateCount() >= 1);
+    auto seq = assigned_seqno_.lower_bound + it_->GetUpdateCount() - 1;
+    assert(seq <= assigned_seqno_.upper_bound);
+    return seq;
+  }
+
+  // If it_ is valid, udate key_ to an internal key containing it_ current
+  // key, CurrentKeySeqno() and a type corresponding to it_ current entry type.
   void UpdateKey() {
     valid_ = it_->Valid();
     if (!Valid()) {
@@ -370,16 +383,16 @@ class WBWIMemTableIterator final : public InternalIterator {
                               std::to_string(it_->Entry().type));
       return;
     }
-    key_buf_.SetInternalKey(it_->Entry().key, assigned_seqno_.upper_bound,
-                            t->second);
+    key_buf_.SetInternalKey(it_->Entry().key, CurrentKeySeqno(), t->second);
     key_ = key_buf_.GetInternalKey();
   }
 
   void UpdateSingleDeleteKey() {
     assert(it_->Valid());
     assert(Valid());
-    assert(assigned_seqno_.lower_bound < assigned_seqno_.upper_bound);
-    key_buf_.SetInternalKey(it_->Entry().key, assigned_seqno_.upper_bound - 1,
+    // The key that overwrites this SingleDelete will be assigned at least
+    // seqno lower_bound + 1 (see CurrentKeySeqno()).
+    key_buf_.SetInternalKey(it_->Entry().key, assigned_seqno_.lower_bound,
                             kTypeSingleDeletion);
     key_ = key_buf_.GetInternalKey();
     at_overwritten_single_del_ = true;

options/cf_options.cc

@@ -736,6 +736,10 @@ static std::unordered_map<std::string, OptionTypeInfo>
          {offsetof(struct ImmutableCFOptions, force_consistency_checks),
           OptionType::kBoolean, OptionVerificationType::kNormal,
           OptionTypeFlags::kNone}},
+        {"disallow_memtable_writes",
+         {offsetof(struct ImmutableCFOptions, disallow_memtable_writes),
+          OptionType::kBoolean, OptionVerificationType::kNormal,
+          OptionTypeFlags::kNone}},
         {"default_temperature",
          {offsetof(struct ImmutableCFOptions, default_temperature),
           OptionType::kTemperature, OptionVerificationType::kNormal,
@@ -998,6 +1002,7 @@ ImmutableCFOptions::ImmutableCFOptions(const ColumnFamilyOptions& cf_options)
       num_levels(cf_options.num_levels),
       optimize_filters_for_hits(cf_options.optimize_filters_for_hits),
       force_consistency_checks(cf_options.force_consistency_checks),
+      disallow_memtable_writes(cf_options.disallow_memtable_writes),
       default_temperature(cf_options.default_temperature),
       memtable_insert_with_hint_prefix_extractor(
           cf_options.memtable_insert_with_hint_prefix_extractor),