Update HISTORY.md for 11.6.0 release

.github/workflows/ai-review-analysis.yml

@@ -714,6 +714,8 @@ jobs:
           CODEX_HOME: /tmp/codex-home
           CODEX_MODEL: ${{ inputs.default_model }}
           CODEX_REASONING_EFFORT: ${{ steps.budget_codex.outputs.effort }}
+          BASE_SHA: ${{ steps.pr_info.outputs.base_sha }}
+          PR_TITLE: ${{ steps.pr_info.outputs.pr_title }}
         run: |
           if [ -z "${OPENAI_API_KEY}" ]; then
             echo "OPENAI_API_KEY not configured, skipping Codex review"
@@ -721,7 +723,9 @@ jobs:
           fi
           mkdir -p "${CODEX_HOME}"
           set +e
-          codex exec \
+          codex exec review \
+            --base "${BASE_SHA}" \
+            --title "${PR_TITLE}" \
             -m "${CODEX_MODEL}" \
             -c model_reasoning_effort="${CODEX_REASONING_EFFORT}" \
             --dangerously-bypass-approvals-and-sandbox \
@@ -1336,6 +1340,8 @@ jobs:
           CODEX_HOME: /tmp/codex-home
           CODEX_MODEL: ${{ inputs.selected_model != '' && inputs.selected_model || inputs.default_model }}
           CODEX_REASONING_EFFORT: ${{ steps.manual_budget_codex.outputs.effort }}
+          BASE_SHA: ${{ steps.pr_info.outputs.base_sha }}
+          PR_TITLE: ${{ steps.pr_info.outputs.pr_title }}
           COMMAND_TYPE: ${{ steps.request.outputs.type }}
         run: |
           mkdir -p "${CODEX_HOME}"
@@ -1348,7 +1354,9 @@ jobs:
               --output-last-message codex-review-output.md \
               - < /tmp/prompt.txt > codex-review-execution.log 2>&1
           else
-            codex exec \
+            codex exec review \
+              --base "${BASE_SHA}" \
+              --title "${PR_TITLE}" \
               -m "${CODEX_MODEL}" \
               -c model_reasoning_effort="${CODEX_REASONING_EFFORT}" \
               --dangerously-bypass-approvals-and-sandbox \

.github/workflows/ai-review-comment.yml

@@ -40,7 +40,6 @@ jobs:
           name: ${{ inputs.result_artifact_name }}
           run-id: ${{ github.event.workflow_run.id }}
           github-token: ${{ secrets.GITHUB_TOKEN }}
-          path: _artifact
         continue-on-error: true
 
       - name: Post or update PR comment
@@ -53,18 +52,8 @@ jobs:
         with:
           script: |
             const fs = require('fs');
-            const path = require('path');
-            const artifactDir = '_artifact';
-            const commentFile = path.join(
-              artifactDir,
-              process.env.COMMENT_FILE
-            );
-            const prNumberFile = path.join(artifactDir, 'pr_number.txt');
-            const triggerTypeFile = path.join(artifactDir, 'trigger_type.txt');
-            const headShaFile = path.join(artifactDir, 'head_sha.txt');
-
-            if (!fs.existsSync(commentFile) ||
-                !fs.existsSync(prNumberFile)) {
+            if (!fs.existsSync(process.env.COMMENT_FILE) ||
+                !fs.existsSync('pr_number.txt')) {
               core.info('No review results found; skipping.');
               return;
             }
@@ -72,16 +61,16 @@ jobs:
             const post = require('./.github/scripts/post-pr-comment.js');
             const provider = process.env.PROVIDER;
             const providerTitle = provider === 'codex' ? 'Codex' : 'Claude';
-            const body = fs.readFileSync(commentFile, 'utf8');
+            const body = fs.readFileSync(process.env.COMMENT_FILE, 'utf8');
             const prNumber = parseInt(
-              fs.readFileSync(prNumberFile, 'utf8').trim(),
+              fs.readFileSync('pr_number.txt', 'utf8').trim(),
               10
             );
-            const trigger = fs.existsSync(triggerTypeFile) ?
-              fs.readFileSync(triggerTypeFile, 'utf8').trim() :
+            const trigger = fs.existsSync('trigger_type.txt') ?
+              fs.readFileSync('trigger_type.txt', 'utf8').trim() :
               'auto';
-            const headSha = fs.existsSync(headShaFile) ?
-              fs.readFileSync(headShaFile, 'utf8').trim() :
+            const headSha = fs.existsSync('head_sha.txt') ?
+              fs.readFileSync('head_sha.txt', 'utf8').trim() :
               '';
             const shortSha = headSha ? headSha.substring(0, 7) : '';
             const runId = process.env.RUN_ID;
@@ -130,16 +119,12 @@ jobs:
         with:
           script: |
             const fs = require('fs');
-            const path = require('path');
-            const artifactDir = '_artifact';
-            const triggerTypeFile = path.join(artifactDir, 'trigger_type.txt');
-            const commentIdFile = path.join(artifactDir, 'comment_id.txt');
-            if (!fs.existsSync(triggerTypeFile)) return;
-            const trigger = fs.readFileSync(triggerTypeFile, 'utf8').trim();
+            if (!fs.existsSync('trigger_type.txt')) return;
+            const trigger = fs.readFileSync('trigger_type.txt', 'utf8').trim();
             if (trigger !== 'manual') return;
-            if (!fs.existsSync(commentIdFile)) return;
+            if (!fs.existsSync('comment_id.txt')) return;
             const commentId = parseInt(
-              fs.readFileSync(commentIdFile, 'utf8').trim(),
+              fs.readFileSync('comment_id.txt', 'utf8').trim(),
               10
             );
             if (!commentId) return;
@@ -162,7 +147,6 @@ jobs:
           name: ${{ inputs.result_artifact_name }}
           run-id: ${{ github.event.workflow_run.id }}
           github-token: ${{ secrets.GITHUB_TOKEN }}
-          path: _artifact
         continue-on-error: true
 
       - name: React with failure
@@ -171,11 +155,9 @@ jobs:
         with:
           script: |
             const fs = require('fs');
-            const path = require('path');
-            const commentIdFile = path.join('_artifact', 'comment_id.txt');
-            if (!fs.existsSync(commentIdFile)) return;
+            if (!fs.existsSync('comment_id.txt')) return;
             const commentId = parseInt(
-              fs.readFileSync(commentIdFile, 'utf8').trim(),
+              fs.readFileSync('comment_id.txt', 'utf8').trim(),
               10
             );
             if (!commentId) return;

.github/workflows/clang-tidy-comment.yml

@@ -25,7 +25,6 @@ jobs:
         name: clang-tidy-result
         run-id: ${{ github.event.workflow_run.id }}
         github-token: ${{ secrets.GITHUB_TOKEN }}
-        path: _artifact
       continue-on-error: true
 
     - name: Post or update PR comment
@@ -34,19 +33,12 @@ jobs:
       with:
         script: |
           const fs = require('fs');
-          if (!fs.existsSync('_artifact/clang-tidy-comment.md') ||
-              !fs.existsSync('_artifact/pr_number.txt')) {
+          if (!fs.existsSync('clang-tidy-comment.md') || !fs.existsSync('pr_number.txt')) {
             core.info('No clang-tidy results found; skipping.');
             return;
           }
-          const body = fs.readFileSync(
-            '_artifact/clang-tidy-comment.md',
-            'utf8'
-          );
-          const prNumber = parseInt(
-            fs.readFileSync('_artifact/pr_number.txt', 'utf8').trim(),
-            10
-          );
+          const body = fs.readFileSync('clang-tidy-comment.md', 'utf8');
+          const prNumber = parseInt(fs.readFileSync('pr_number.txt', 'utf8').trim());
           const post = require('./.github/scripts/post-pr-comment.js');
 
           await post({

.github/workflows/claude-review-comment.yml

@@ -22,3 +22,4 @@ jobs:
       provider: claude
       result_artifact_name: claude-review-result
       comment_file: claude-review-comment.md
+    secrets: inherit

.github/workflows/codex-review-comment.yml

@@ -22,3 +22,4 @@ jobs:
       provider: codex
       result_artifact_name: codex-review-result
       comment_file: codex-review-comment.md
+    secrets: inherit

.github/workflows/codex-review.yml

@@ -7,9 +7,14 @@
 name: Codex Code Review
 
 on:
-  # Automatic PR reviews are temporarily disabled while Codex authentication is
-  # repaired. Manual review requests remain available through issue comments
-  # and workflow_dispatch.
+  workflow_run:
+    workflows: ["facebook/rocksdb/pr-jobs"]
+    types: [completed]
+
+  # The early pull_request_target path is limited to same-repo PRs by the job
+  # condition below. Fork PRs skip this path and rely on workflow_run instead.
+  pull_request_target:
+    types: [opened, reopened, synchronize]
 
   issue_comment:
     types: [created]

CLAUDE.md

@@ -243,11 +243,6 @@ from an implementation detail instead of an explicit option.
 * Don't manually edit BUCK file, after updating src.mk, run
     /usr/local/bin/python3 buckifier/buckify_rocksdb.py to update it
 * For -j in make command, use the number of CPU cores to decide it.
-* When searching for references to something (a symbol, library, etc.), do not
-  restrict or truncate your search based on presumed relevance or scope. It is
-  important and time-saving to keep the repo reasonably consistent across
-  different build systems, programming languages, and even between
-  documentation and implementation.
 
 ### Avoiding mixed build modes with Make (use `AUTO_CLEAN=1`)
 
@@ -298,7 +293,6 @@ build (and where noted, run tests) across:
 | Axis | Must support |
 |------|--------------|
 | OS | Linux (x86_64 + ARM), macOS, Windows |
-| Architecture | Little-endian and big-endian targets, including Linux on s390x |
 | Compiler | GCC, Clang (libstdc++ **and** libc++), AppleClang, **MSVC (VS2022)**, MinGW (Linux cross-compile, build-only, no gflags) |
 | Build system | Make, CMake, and BUCK (internal) -- keep all in sync (see "Build system" above) |
 | Config | release (`-fno-rtti`), `ASSERT_STATUS_CHECKED`, ASAN/UBSAN/TSAN, folly, unity build, JNI/Java |
@@ -313,12 +307,6 @@ abstractions; otherwise guard with `#ifdef OS_WIN` (POSIX `<unistd.h>` ->
 Windows `<process.h>`). Because libc++ is also tested, include what you use
 rather than relying on libstdc++ transitive includes.
 
-Do not use `static_assert` to reject a supported architecture for an optional
-feature. Check an existing `port::` capability such as `port::kLittleEndian`
-and disable only the unsupported feature so the rest of RocksDB still builds.
-Use a portable representation when the feature itself must work across
-architectures.
-
 ### Unit Test
 * After all of the unit tests are added, review them and try to extract common
     reusable utility functions to reduce code duplication due to copy past between

CMakeLists.txt

@@ -698,19 +698,10 @@ if(USE_FOLLY)
 
     include(${FOLLY_INST_PATH}/lib/cmake/folly/folly-config.cmake)
 
-    # Fix gflags library name for debug builds. The getdeps gflags shared
-    # library is versioned (e.g. libgflags_debug.so.2.3), so resolve the
-    # actual file by glob rather than hard-coding a version.
+    # Fix gflags library name for debug builds
     if(CMAKE_BUILD_TYPE STREQUAL "Debug")
-      file(GLOB GFLAGS_DEBUG_SHARED_LIBS
-        "${GFLAGS_INST_PATH}/lib/libgflags_debug.so.*")
-      if(NOT GFLAGS_DEBUG_SHARED_LIBS)
-        message(FATAL_ERROR
-          "Could not find getdeps libgflags_debug.so under ${GFLAGS_INST_PATH}/lib")
-      endif()
-      list(GET GFLAGS_DEBUG_SHARED_LIBS 0 GFLAGS_DEBUG_SHARED_LIB)
       set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-rpath=${GFLAGS_INST_PATH}/lib")
-      set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${GFLAGS_DEBUG_SHARED_LIB}")
+      set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${GFLAGS_INST_PATH}/lib/libgflags_debug.so.2.2")
     endif()
   endif()
 

HISTORY.md

@@ -1,17 +1,6 @@
 # Rocksdb Change Log
 > NOTE: Entries for next release do not go here. Follow instructions in `unreleased_history/README.txt`
 
-## 11.7.0 (07/16/2026)
-### Public API Changes
-* Added `RangeLockManagerHandle::SetIsKilledCallback()`, a `std::function<bool()>` predicate polled during range-lock waits. When it returns true, a blocked `GetRangeLock()` wait returns promptly with `Status::Aborted()` instead of waiting for the lock timeout. Range-lock users that install no callback keep the original wait-until-grant-or-timeout behavior.
-* Added `MultiScanArgs::reverse` to support reverse MultiScan reads over bounded scan ranges.
-
-### Behavior Changes
-* When using `LockWAL()`, live-file capture APIs such as checkpoint and backup now flush WAL-disabled unpersisted data; they can block until `UnlockWAL()` or return `Aborted` when called from the same thread that holds `LockWAL()` to avoid deadlock. `UnlockWAL()` must now also be called from the same thread that called `LockWAL()`.
-
-### Performance Improvements
-* Fixed slow DB open with `best_efforts_recovery`, and slow catch-up of secondary and follower (read-only) instances, in which time and memory grew quadratically with the number of files in the DB (so the slowdown was worst for large DBs with long history).
-
 ## 11.6.0 (07/02/2026)
 ### New Features
 * Added EXPERIMENTAL embedded blob SST support through `SstFileWriter::OpenWithEmbeddedBlobs()`, storing eligible large values as same-file blob records in block-based SST files and resolving them transparently for reads. This niche feature currently supports uncompressed embedded blobs only; compression options are placeholders and compression support is deferred to follow-up work.

db/compaction/compaction_service_job.cc

@@ -777,40 +777,6 @@ static std::unordered_map<std::string, OptionTypeInfo>
           OptionTypeFlags::kNone}},
 };
 
-namespace {
-
-// Like OptionTypeInfo::Array<T, kSize> but deserialization tolerates
-// size mismatches: extra elements beyond kSize are silently ignored,
-// missing elements are zero-filled. Serialization and comparison are
-// inherited from Array<T, kSize> unchanged; only the parse
-// (deserialize) callback is overridden.
-template <typename T, size_t kSize>
-OptionTypeInfo SizeTolerantDeserializeArray(int offset,
-                                            OptionVerificationType verification,
-                                            OptionTypeFlags flags,
-                                            const OptionTypeInfo& elem_info,
-                                            char separator = ':') {
-  return OptionTypeInfo::Array<T, kSize>(offset, verification, flags, elem_info,
-                                         separator)
-      .SetParseFunc([elem_info, separator](
-                        const ConfigOptions& opts, const std::string& name,
-                        const std::string& value, void* addr) {
-        std::vector<T> parsed;
-        Status s =
-            ParseVector<T>(opts, elem_info, separator, name, value, &parsed);
-        if (!s.ok()) {
-          return Status(std::move(s));
-        }
-        auto* arr = static_cast<std::array<T, kSize>*>(addr);
-        for (size_t i = 0; i < kSize; ++i) {
-          (*arr)[i] = (i < parsed.size()) ? parsed[i] : T{};
-        }
-        return Status::OK();
-      });
-}
-
-}  // namespace
-
 static std::unordered_map<std::string, OptionTypeInfo>
     compaction_stats_type_info = {
         {"micros",
@@ -902,20 +868,12 @@ static std::unordered_map<std::string, OptionTypeInfo>
          {offsetof(struct InternalStats::CompactionStats, count),
           OptionType::kUInt64T, OptionVerificationType::kNormal,
           OptionTypeFlags::kNone}},
-        // The remote worker may serialize a different number of
-        // CompactionReason counts than this primary expects (RocksDB versions
-        // can disagree on kNumOfReasons). Deserialization tolerates the
-        // mismatch: extra trailing elements are dropped, missing ones are
-        // zero-filled.
-        //
-        // The serialized width is intentionally kept at kNumOfReasons - 1 (the
-        // reduced width) so this change does NOT alter what is written -- only
-        // the read path becomes tolerant. Widening the serialized width to the
-        // full kNumOfReasons is a separate, later change that must not land
-        // until this tolerant read has been deployed to every primary,
-        // including rollback targets.
         {"counts",
-         SizeTolerantDeserializeArray<
+         OptionTypeInfo::Array<
+             /* In release 11.2, a new compaction reason was added. This broken
+              * the reader and writer. To unblock release 11.1, we temporarily
+              * reduce the count array size to the old one. TODO add a proper
+              * serialization and deserialization method. */
              int, static_cast<int>(CompactionReason::kNumOfReasons) - 1>(
              offsetof(struct InternalStats::CompactionStats, counts),
              OptionVerificationType::kNormal, OptionTypeFlags::kNone,

db/compaction/compaction_service_test.cc

@@ -3,8 +3,6 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
-#include <memory>
-
 #include "db/db_test_util.h"
 #include "file/file_util.h"
 #include "port/stack_trace.h"
@@ -14,71 +12,6 @@
 
 namespace ROCKSDB_NAMESPACE {
 
-namespace {
-
-// Append ":extra_value" to every "counts=...;" field in a serialized
-// CompactionServiceResult string to simulate a worker with more
-// CompactionReason enums than the reader. Loops because the serialized
-// result contains multiple CompactionStats structs (output_level_stats,
-// proximal_level_stats), each with its own "counts=" field.
-void AppendToCountsFields(std::string* s, const std::string& extra_value) {
-  const std::string prefix = "counts=";
-  size_t pos = s->find(prefix);
-  while (pos != std::string::npos) {
-    // Find the semicolon that terminates this field's value.
-    size_t semi = s->find(';', pos + prefix.size());
-    assert(semi != std::string::npos);
-    // Insert ":extra_value" just before the semicolon.
-    s->insert(semi, ":" + extra_value);
-    // Advance past the inserted text to find the next occurrence.
-    pos = s->find(prefix, semi + extra_value.size() + 2);
-  }
-}
-
-// Remove the last colon-separated element from every "counts=...;" field
-// to simulate a worker with fewer CompactionReason enums than the reader.
-// Loops for the same reason as AppendToCountsFields (multiple counts fields).
-void RemoveLastFromCountsFields(std::string* s) {
-  const std::string prefix = "counts=";
-  size_t pos = s->find(prefix);
-  while (pos != std::string::npos) {
-    size_t val_begin = pos + prefix.size();
-    size_t semi = s->find(';', val_begin);
-    assert(semi != std::string::npos);
-    // Find the last ':' separator within the value.
-    size_t last_sep = s->rfind(':', semi - 1);
-    assert(last_sep != std::string::npos && last_sep >= val_begin);
-    // Erase from the last ':' up to (not including) the ';'.
-    // e.g. "counts=0:3:5;" -> erase ":5" -> "counts=0:3;"
-    s->erase(last_sep, semi - last_sep);
-    pos = s->find(prefix, last_sep);
-  }
-}
-
-// Count the colon-separated elements in the first "counts=...;" field of a
-// serialized CompactionServiceResult. Used to assert the serialized width
-// (how many CompactionReason counts this binary writes).
-size_t CountFirstCountsFieldElements(const std::string& s) {
-  const std::string prefix = "counts=";
-  size_t pos = s.find(prefix);
-  assert(pos != std::string::npos);
-  size_t val_begin = pos + prefix.size();
-  size_t semi = s.find(';', val_begin);
-  assert(semi != std::string::npos);
-  if (semi == val_begin) {
-    return 0;
-  }
-  size_t count = 1;
-  for (size_t i = val_begin; i < semi; ++i) {
-    if (s[i] == ':') {
-      ++count;
-    }
-  }
-  return count;
-}
-
-}  // namespace
-
 class MyTestCompactionService : public CompactionService {
  public:
   MyTestCompactionService(
@@ -1676,68 +1609,6 @@ TEST_F(CompactionServiceTest, InvalidResultFallsBackToLocal) {
             my_cs->GetFinalCompactionServiceJobStatus());
 }
 
-TEST_F(CompactionServiceTest, CompatCheckCountsWidthTolerance) {
-  // Number of per-CompactionReason counts this binary serializes and expects.
-  constexpr size_t kExpected =
-      static_cast<size_t>(CompactionReason::kNumOfReasons) - 1;
-  constexpr size_t kLastIndex = kExpected - 1;
-  constexpr int kFirstCount = 3;
-  constexpr int kLastCount = 5;
-
-  CompactionServiceResult result;
-  result.status = Status::OK();
-  result.output_level = 2;
-  result.output_path = "/tmp/output";
-  result.internal_stats.output_level_stats.counts[0] = kFirstCount;
-  result.internal_stats.output_level_stats.counts[kLastIndex] = kLastCount;
-  std::string serialized;
-  ASSERT_OK(result.Write(&serialized));
-
-  // Guard the produced format: this binary writes exactly kExpected counts.
-  // That element count is what a remote worker hands to the primary to read,
-  // so pinning it catches an accidental change to what is written.
-  ASSERT_EQ(CountFirstCountsFieldElements(serialized), kExpected);
-
-  // Peer agrees on kNumOfReasons: every written count is read back unchanged.
-  {
-    CompactionServiceResult parsed;
-    ASSERT_OK(CompactionServiceResult::Read(serialized, &parsed));
-    ASSERT_OK(parsed.status);
-    const auto& counts = parsed.internal_stats.output_level_stats.counts;
-    ASSERT_EQ(counts[0], kFirstCount);
-    ASSERT_EQ(counts[kLastIndex], kLastCount);
-  }
-
-  // Peer serialized one more count than this binary knows (a worker with an
-  // extra CompactionReason): the surplus trailing count is ignored, and the
-  // counts this binary understands are read unchanged.
-  {
-    std::string from_wider_peer = serialized;
-    AppendToCountsFields(&from_wider_peer, "7");
-    ASSERT_EQ(CountFirstCountsFieldElements(from_wider_peer), kExpected + 1);
-    CompactionServiceResult parsed;
-    ASSERT_OK(CompactionServiceResult::Read(from_wider_peer, &parsed));
-    ASSERT_OK(parsed.status);
-    const auto& counts = parsed.internal_stats.output_level_stats.counts;
-    ASSERT_EQ(counts[0], kFirstCount);
-    ASSERT_EQ(counts[kLastIndex], kLastCount);
-  }
-
-  // Peer serialized one fewer count than this binary knows (a worker missing
-  // the newest CompactionReason): the absent trailing count reads as zero.
-  {
-    std::string from_narrower_peer = serialized;
-    RemoveLastFromCountsFields(&from_narrower_peer);
-    ASSERT_EQ(CountFirstCountsFieldElements(from_narrower_peer), kExpected - 1);
-    CompactionServiceResult parsed;
-    ASSERT_OK(CompactionServiceResult::Read(from_narrower_peer, &parsed));
-    ASSERT_OK(parsed.status);
-    const auto& counts = parsed.internal_stats.output_level_stats.counts;
-    ASSERT_EQ(counts[0], kFirstCount);
-    ASSERT_EQ(counts[kLastIndex], 0);
-  }
-}
-
 TEST_F(CompactionServiceTest, SubCompaction) {
   Options options = CurrentOptions();
   options.max_subcompactions = 10;

db/db_basic_test.cc

@@ -207,9 +207,8 @@ TEST_F(DBBasicTest, OptimizeManifestForRecoveryEmitsPerCFWhenLogAdvances) {
   ASSERT_EQ("v", Get("k"));
 }
 
-// With track_and_verify_wals_in_manifest=true and a clean close, the
-// close-time code already advances WalSet markers. Recovery's
-// DeleteWalsBefore is redundant and should be skipped.
+// With track_and_verify_wals_in_manifest=true, the wal_deletion edit
+// must STILL be emitted (the DeleteWalsBefore record is required).
 TEST_F(DBBasicTest, OptimizeManifestForRecoveryPreservesWalTracking) {
   Options options = CurrentOptions();
   options.create_if_missing = true;
@@ -225,7 +224,7 @@ TEST_F(DBBasicTest, OptimizeManifestForRecoveryPreservesWalTracking) {
   Reopen(options);
   counters.Uninstall();
 
-  ASSERT_EQ(1, counters.wal_deletion.load());
+  ASSERT_EQ(0, counters.wal_deletion.load());
 }
 
 // best_efforts_recovery requires a fresh MANIFEST + CURRENT to be
@@ -498,60 +497,11 @@ TEST_F(DBBasicTest, OptimizeManifestForRecoveryEmitsWalDeletionWhenTracking) {
   counters.Uninstall();
 
   ASSERT_GT(counters.per_cf.load(), 0);
-  ASSERT_EQ(0, records.load());
+  ASSERT_EQ(1, records.load());
   ASSERT_GT(dbfull()->GetVersionSet()->GetWalSet().GetMinWalNumberToKeep(), 0u);
   ASSERT_EQ("v", Get("k"));
 }
 
-// Zero MANIFEST growth on clean reopen with optimize_manifest_for_recovery=1,
-// reuse_manifest_on_open=1, and track_and_verify_wals_in_manifest=1.
-// Regression test for the stress test failure where a redundant
-// DeleteWalsBefore edit was written during recovery.
-TEST_F(DBBasicTest, OptimizeManifestForRecoveryZeroGrowthOnCleanReopen) {
-  Options options = CurrentOptions();
-  options.create_if_missing = true;
-  options.optimize_manifest_for_recovery = true;
-  options.reuse_manifest_on_open = true;
-  options.track_and_verify_wals_in_manifest = true;
-  options.avoid_flush_during_recovery = true;
-  options.write_dbid_to_manifest = false;
-  DestroyAndReopen(options);
-  ASSERT_OK(Put("k", "v"));
-  ASSERT_OK(Flush());
-  Close();
-
-  // Get MANIFEST size after close
-  std::vector<std::string> files;
-  ASSERT_OK(env_->GetChildren(dbname_, &files));
-  uint64_t manifest_size_before = 0;
-  for (const auto& f : files) {
-    if (f.find("MANIFEST-") == 0) {
-      ASSERT_OK(env_->GetFileSize(dbname_ + "/" + f, &manifest_size_before));
-      break;
-    }
-  }
-  ASSERT_GT(manifest_size_before, 0u);
-
-  // Reopen - should not grow MANIFEST at all
-  Reopen(options);
-
-  // Get MANIFEST size after reopen
-  files.clear();
-  ASSERT_OK(env_->GetChildren(dbname_, &files));
-  uint64_t manifest_size_after = 0;
-  for (const auto& f : files) {
-    if (f.find("MANIFEST-") == 0) {
-      ASSERT_OK(env_->GetFileSize(dbname_ + "/" + f, &manifest_size_after));
-      break;
-    }
-  }
-
-  ASSERT_EQ(manifest_size_before, manifest_size_after)
-      << "MANIFEST grew by " << (manifest_size_after - manifest_size_before)
-      << " bytes on clean reopen; expected zero growth";
-  ASSERT_EQ("v", Get("k"));
-}
-
 // Regression for the close-time marker path: if a new WAL is created while an
 // otherwise-empty user CF stays empty, Close() must reserve the next file
 // number before persisting SetLogNumber(cur_wal + 1) for that CF.

db/db_filesnapshot.cc

@@ -252,7 +252,7 @@ Status DBImpl::GetLiveFilesStorageInfo(
         "capturing live files. Retry with flush enabled.");
   }
   if (flush_memtable) {
-    wal_locked = !lock_wal_owner_thread_id_counts_.empty();
+    wal_locked = lock_wal_count_ > 0;
     if (wal_locked) {
       if (needs_blob_direct_write_flush) {
         mutex_.Unlock();
@@ -260,12 +260,8 @@ Status DBImpl::GetLiveFilesStorageInfo(
             "Blob direct write requires flushing active blob files before "
             "capturing live files. Retry with WAL unlocked.");
       }
-    }
-    if (wal_locked && !has_unpersisted_data_.load(std::memory_order_relaxed)) {
-      ROCKS_LOG_INFO(
-          immutable_db_options_.info_log,
-          "Skipping FlushForGetLiveFiles while WAL is locked and all data is "
-          "already persisted");
+      ROCKS_LOG_INFO(immutable_db_options_.info_log,
+                     "Can't FlushForGetLiveFiles while WAL is locked");
     } else {
       Status status = FlushForGetLiveFiles(opts.atomic_flush);
       if (!status.ok()) {

db/db_impl/db_impl.cc

@@ -2169,12 +2169,11 @@ Status DBImpl::ApplyWALToManifest(const ReadOptions& read_options,
 }
 
 Status DBImpl::LockWAL() {
-  const uint64_t thread_id = env_->GetThreadID();
   {
     InstrumentedMutexLock lock(&mutex_);
-    if (!lock_wal_owner_thread_id_counts_.empty()) {
+    if (lock_wal_count_ > 0) {
       assert(lock_wal_write_token_);
-      ++lock_wal_owner_thread_id_counts_[thread_id];
+      ++lock_wal_count_;
     } else {
       // NOTE: this will "unnecessarily" wait for other non-LockWAL() write
       // stalls to clear before LockWAL returns, however fixing that would
@@ -2192,13 +2191,12 @@ Status DBImpl::LockWAL() {
       }
 
       // NOTE: releasing mutex in EnterUnbatched might mean we are actually
-      // now locked by another thread.
-      if (lock_wal_owner_thread_id_counts_.empty()) {
+      // now lock_wal_count > 0
+      if (lock_wal_count_ == 0) {
         assert(!lock_wal_write_token_);
         lock_wal_write_token_ = write_controller_.GetStopToken();
       }
-      assert(lock_wal_write_token_);
-      ++lock_wal_owner_thread_id_counts_[thread_id];
+      ++lock_wal_count_;
 
       if (two_write_queues_) {
         nonmem_write_thread_.ExitUnbatched(&nonmem_w);
@@ -2209,27 +2207,23 @@ Status DBImpl::LockWAL() {
   // NOTE: avoid I/O holding DB mutex
   Status s = FlushWAL(/*sync=*/false);
   if (!s.ok()) {
-    // Undo this LockWAL().
+    // Non-OK return should not be in locked state
     UnlockWAL().PermitUncheckedError();
   }
   return s;
 }
 
 Status DBImpl::UnlockWAL() {
-  const uint64_t thread_id = env_->GetThreadID();
   bool signal = false;
   uint64_t maybe_stall_begun_count = 0;
   uint64_t nonmem_maybe_stall_begun_count = 0;
   {
     InstrumentedMutexLock lock(&mutex_);
-    const auto owner = lock_wal_owner_thread_id_counts_.find(thread_id);
-    if (owner == lock_wal_owner_thread_id_counts_.end()) {
-      return Status::Aborted("No LockWAL() held by current thread");
+    if (lock_wal_count_ == 0) {
+      return Status::Aborted("No LockWAL() in effect");
     }
-    if (--owner->second == 0) {
-      lock_wal_owner_thread_id_counts_.erase(owner);
-    }
-    if (lock_wal_owner_thread_id_counts_.empty()) {
+    --lock_wal_count_;
+    if (lock_wal_count_ == 0) {
       lock_wal_write_token_.reset();
       signal = true;
       // For the last UnlockWAL, we don't want to return from UnlockWAL()
@@ -3767,7 +3761,6 @@ void DBImpl::MultiGetCommon(const ReadOptions& read_options,
   }
 
   autovector<KeyContext, MultiGetContext::MAX_BATCH_SIZE> key_context;
-  key_context.reserve(num_keys);
   autovector<KeyContext*, MultiGetContext::MAX_BATCH_SIZE> sorted_keys;
   sorted_keys.resize(num_keys);
   for (size_t i = 0; i < num_keys; ++i) {
@@ -3958,7 +3951,6 @@ void DBImpl::MultiGetCommon(const ReadOptions& read_options,
     }
   }
   autovector<KeyContext, MultiGetContext::MAX_BATCH_SIZE> key_context;
-  key_context.reserve(num_keys);
   autovector<KeyContext*, MultiGetContext::MAX_BATCH_SIZE> sorted_keys;
   sorted_keys.resize(num_keys);
   for (size_t i = 0; i < num_keys; ++i) {

db/db_impl/db_impl.h

@@ -2541,7 +2541,7 @@ class DBImpl : public DB {
     }
 
     // Wait for any LockWAL to clear
-    while (!lock_wal_owner_thread_id_counts_.empty()) {
+    while (lock_wal_count_ > 0) {
       bg_cv_.Wait();
     }
   }
@@ -3600,13 +3600,12 @@ class DBImpl : public DB {
 
   // Stop write token that is acquired when first LockWAL() is called.
   // Destroyed when last UnlockWAL() is called. Controlled by DB mutex.
-  // See lock_wal_owner_thread_id_counts_
+  // See lock_wal_count_
   std::unique_ptr<WriteControllerToken> lock_wal_write_token_;
 
-  // Thread IDs of valid LockWAL() owners and their recursive lock counts. Each
-  // owning thread must call UnlockWAL() the same number of times before writes
-  // can resume.
-  std::unordered_map<uint64_t, uint32_t> lock_wal_owner_thread_id_counts_;
+  // The number of LockWAL called without matching UnlockWAL call.
+  // See also lock_wal_write_token_
+  uint32_t lock_wal_count_ = 0;
 };
 
 class GetWithTimestampReadCallback : public ReadCallback {

db/db_impl/db_impl_compaction_flush.cc

@@ -2649,13 +2649,6 @@ Status DBImpl::FlushMemTable(ColumnFamilyData* cfd,
            "Please try again later after writes are resumed";
     return Status::TryAgain(oss.str());
   }
-  if (flush_options.wait) {
-    InstrumentedMutexLock lock(&mutex_);
-    if (lock_wal_owner_thread_id_counts_.count(env_->GetThreadID()) > 0) {
-      return Status::Aborted(
-          "Likely deadlock as the same thread called LockWAL()");
-    }
-  }
   Status s;
   if (!flush_options.allow_write_stall) {
     bool flush_needed = true;
@@ -2814,13 +2807,6 @@ Status DBImpl::AtomicFlushMemTables(
            "Please try again later after writes are resumed";
     return Status::TryAgain(oss.str());
   }
-  if (flush_options.wait) {
-    InstrumentedMutexLock lock(&mutex_);
-    if (lock_wal_owner_thread_id_counts_.count(env_->GetThreadID()) > 0) {
-      return Status::Aborted(
-          "Likely deadlock as the same thread called LockWAL()");
-    }
-  }
   Status s;
   autovector<ColumnFamilyData*> candidate_cfds;
   if (provided_candidate_cfds.empty()) {

db/db_impl/db_impl_open.cc

@@ -1941,8 +1941,10 @@ Status DBImpl::MaybeFlushFinalMemtableOrRestoreActiveLogFiles(
         const uint64_t new_min_log = max_wal_number + 1;
         VersionEdit wal_deletion;
         bool emit_wal_deletion = false;
-        if (immutable_db_options_.track_and_verify_wals_in_manifest &&
-            new_min_log > versions_->GetWalSet().GetMinWalNumberToKeep()) {
+        if (immutable_db_options_.track_and_verify_wals_in_manifest) {
+          // Determining whether DeleteWalsBefore actually shrinks WalSet
+          // membership requires WalSet state outside this site, so emit
+          // unconditionally (pre-existing behavior).
           wal_deletion.DeleteWalsBefore(new_min_log);
           emit_wal_deletion = true;
         }

db/db_impl/db_impl_write.cc

@@ -3119,7 +3119,7 @@ Status DBImpl::SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context,
                               ReadOnlyMemTable* new_imm,
                               SequenceNumber last_seqno) {
   mutex_.AssertHeld();
-  assert(lock_wal_owner_thread_id_counts_.empty());
+  assert(lock_wal_count_ == 0);
 
   // TODO: plumb Env::IOActivity, Env::IOPriority
   const WriteOptions write_options;

db/db_iter.cc

@@ -1921,9 +1921,6 @@ Status DBIter::ValidateScanOptions(const MultiScanArgs& multiscan_opts) const {
 
   const std::vector<ScanOptions>& scan_opts = multiscan_opts.GetScanRanges();
   const bool has_limit = scan_opts.front().range.limit.has_value();
-  if (multiscan_opts.reverse && !has_limit) {
-    return Status::InvalidArgument("Reverse MultiScan requires limit");
-  }
   if (!has_limit && scan_opts.size() > 1) {
     return Status::InvalidArgument("Scan has no upper bound");
   }
@@ -2007,12 +2004,6 @@ void DBIter::Seek(const Slice& target) {
   StopWatch sw(clock_, statistics_, DB_SEEK);
 
   if (scan_opts_.has_value()) {
-    if (scan_opts_->reverse) {
-      status_ = Status::InvalidArgument("Seek called on reverse MultiScan");
-      valid_ = false;
-      return;
-    }
-
     // Validate the seek target is as expected in the previously prepared range
     auto const& scan_ranges = scan_opts_.value().GetScanRanges();
     if (scan_index_ >= scan_ranges.size()) {
@@ -2119,61 +2110,6 @@ void DBIter::SeekForPrev(const Slice& target) {
   PERF_CPU_TIMER_GUARD(iter_seek_cpu_nanos, clock_);
   StopWatch sw(clock_, statistics_, DB_SEEK);
 
-  if (scan_opts_.has_value()) {
-    if (!scan_opts_->reverse) {
-      status_ =
-          Status::InvalidArgument("SeekForPrev called on forward MultiScan");
-      valid_ = false;
-      return;
-    }
-
-    auto const& scan_ranges = scan_opts_.value().GetScanRanges();
-    if (scan_index_ >= scan_ranges.size()) {
-      status_ = Status::InvalidArgument(
-          "SeekForPrev called after exhausting all of the scan ranges");
-      valid_ = false;
-      return;
-    }
-
-    auto const& range = scan_ranges[scan_index_];
-    auto const& limit = range.range.limit;
-    assert(limit.has_value());
-    if (!limit.has_value() ||
-        user_comparator_.CompareWithoutTimestamp(target, *limit) != 0) {
-      status_ = Status::InvalidArgument(
-          "SeekForPrev target does not match the limit of the next prepared "
-          "range at index " +
-          std::to_string(scan_index_));
-      valid_ = false;
-      return;
-    }
-
-    auto const& start = range.range.start;
-    assert(start.has_value());
-    if (iterate_lower_bound_ == nullptr ||
-        user_comparator_.CompareWithoutTimestamp(start.value(),
-                                                 *iterate_lower_bound_) != 0) {
-      status_ = Status::InvalidArgument(
-          "Lower bound is not set to the same start value of the next "
-          "prepared range at index " +
-          std::to_string(scan_index_));
-      valid_ = false;
-      return;
-    }
-    if (iterate_upper_bound_ == nullptr ||
-        user_comparator_.CompareWithoutTimestamp(limit.value(),
-                                                 *iterate_upper_bound_) != 0) {
-      status_ = Status::InvalidArgument(
-          "Upper bound is not set to the same limit value of the next "
-          "prepared range at index " +
-          std::to_string(scan_index_));
-      valid_ = false;
-      return;
-    }
-
-    scan_index_++;
-  }
-
   if (has_trace_state_) {
     // TODO: What do we do if this returns an error?
     Slice lower_bound, upper_bound;

db/db_iterator_test.cc

@@ -5418,157 +5418,6 @@ TEST_P(DBMultiScanIteratorTest, AsyncPrefetchAcrossMultipleFiles) {
   iter.reset();
 }
 
-TEST_P(DBMultiScanIteratorTest, ReversePrefetchAcrossMultipleRanges) {
-  auto options = CurrentOptions();
-  options.target_file_size_base = 1 << 15;  // 32KiB
-  options.compaction_style = kCompactionStyleUniversal;
-  options.num_levels = 50;
-  options.compression = kNoCompression;
-  DestroyAndReopen(options);
-
-  Random rnd(303);
-  for (int i = 0; i < 1000; ++i) {
-    ASSERT_OK(Put(Key(i), rnd.RandomString(1 << 10)));
-  }
-  ASSERT_OK(Flush());
-  ASSERT_OK(db_->CompactRange({}, nullptr, nullptr));
-  ASSERT_GT(NumTableFilesAtLevel(49), 3);
-
-  ReadOptions ro;
-  ro.fill_cache = GetParam();
-  ColumnFamilyHandle* cfh = dbfull()->DefaultColumnFamily();
-  auto tracking_dispatcher = std::make_shared<TrackingIODispatcher>();
-
-  MultiScanArgs scan_options(BytewiseComparator());
-  scan_options.use_async_io = false;
-  scan_options.reverse = true;
-  scan_options.io_dispatcher = tracking_dispatcher;
-  std::vector<std::string> key_ranges(
-      {Key(100), Key(200), Key(500), Key(600), Key(800), Key(900)});
-  scan_options.insert(key_ranges[0], key_ranges[1]);
-  scan_options.insert(key_ranges[2], key_ranges[3]);
-  scan_options.insert(key_ranges[4], key_ranges[5]);
-
-  std::unique_ptr<MultiScan> iter =
-      dbfull()->NewMultiScan(ro, cfh, scan_options);
-  ASSERT_NE(iter, nullptr);
-
-  std::vector<std::string> actual_keys;
-  try {
-    for (auto range : *iter) {
-      for (auto it : range) {
-        actual_keys.push_back(it.first.ToString());
-      }
-    }
-  } catch (MultiScanException& ex) {
-    FAIL() << "Iterator returned status " << ex.what();
-  } catch (std::logic_error& ex) {
-    FAIL() << "Iterator returned logic error " << ex.what();
-  }
-
-  std::vector<std::string> expected_keys;
-  for (int i = 199; i >= 100; --i) {
-    expected_keys.push_back(Key(i));
-  }
-  for (int i = 599; i >= 500; --i) {
-    expected_keys.push_back(Key(i));
-  }
-  for (int i = 899; i >= 800; --i) {
-    expected_keys.push_back(Key(i));
-  }
-  ASSERT_EQ(actual_keys, expected_keys);
-  ASSERT_GT(tracking_dispatcher->GetReadSets().size(), 0);
-}
-
-TEST_P(DBMultiScanIteratorTest, ReverseUnreleasedPrefetchBlocksCountAsWasted) {
-  auto options = CurrentOptions();
-  options.compression = kNoCompression;
-  options.disable_auto_compactions = true;
-  options.statistics = CreateDBStatistics();
-
-  BlockBasedTableOptions table_options;
-  table_options.flush_block_policy_factory =
-      std::make_shared<FlushBlockEveryKeyPolicyFactory>();
-  table_options.block_cache = NewLRUCache(10 * 1024 * 1024);
-  options.table_factory.reset(NewBlockBasedTableFactory(table_options));
-  DestroyAndReopen(options);
-
-  constexpr int kNumKeys = 40;
-  const std::string value(100, 'v');
-  for (int i = 0; i < kNumKeys; ++i) {
-    ASSERT_OK(Put(Key(i), value));
-  }
-  ASSERT_OK(Flush());
-  ASSERT_OK(db_->CompactRange({}, nullptr, nullptr));
-
-  ReadOptions ro;
-  ro.fill_cache = GetParam();
-  ColumnFamilyHandle* cfh = dbfull()->DefaultColumnFamily();
-  auto tracking_dispatcher = std::make_shared<TrackingIODispatcher>();
-
-  MultiScanArgs scan_options(BytewiseComparator());
-  scan_options.use_async_io = false;
-  scan_options.reverse = true;
-  scan_options.io_dispatcher = tracking_dispatcher;
-  const std::string start_key = Key(0);
-  const std::string limit_key = Key(kNumKeys);
-  scan_options.insert(start_key, limit_key);
-
-  {
-    std::unique_ptr<MultiScan> iter =
-        dbfull()->NewMultiScan(ro, cfh, scan_options);
-    ASSERT_NE(iter, nullptr);
-
-    try {
-      auto range_it = iter->begin();
-      ASSERT_NE(range_it, iter->end());
-      auto range = *range_it;
-      auto row_it = range.begin();
-      ASSERT_NE(row_it, range.end());
-      ASSERT_EQ((*row_it).first.ToString(), Key(kNumKeys - 1));
-    } catch (MultiScanException& ex) {
-      FAIL() << "Iterator returned status " << ex.what();
-    } catch (std::logic_error& ex) {
-      FAIL() << "Iterator returned logic error " << ex.what();
-    }
-
-    ASSERT_GT(tracking_dispatcher->GetReadSets().size(), 0);
-  }
-
-  ASSERT_GT(
-      options.statistics->getTickerCount(MULTISCAN_PREFETCH_BLOCKS_WASTED), 0);
-}
-
-TEST_P(DBMultiScanIteratorTest, ReverseRequiresLimits) {
-  auto options = CurrentOptions();
-  options.compression = kNoCompression;
-  DestroyAndReopen(options);
-
-  ASSERT_OK(Put("a", "va"));
-  ASSERT_OK(Put("b", "vb"));
-  ASSERT_OK(Flush());
-
-  ReadOptions ro;
-  ro.fill_cache = GetParam();
-  ColumnFamilyHandle* cfh = dbfull()->DefaultColumnFamily();
-
-  MultiScanArgs scan_options(BytewiseComparator());
-  scan_options.reverse = true;
-  scan_options.insert("a");
-  scan_options.insert("b", "c");
-
-  std::unique_ptr<MultiScan> iter =
-      dbfull()->NewMultiScan(ro, cfh, scan_options);
-  ASSERT_NE(iter, nullptr);
-  try {
-    (void)iter->begin();
-    FAIL() << "Expected reverse MultiScan without an upper bound to fail";
-  } catch (MultiScanException& ex) {
-    ASSERT_NOK(ex.status());
-    ASSERT_TRUE(ex.status().IsInvalidArgument());
-  }
-}
-
 // Wrapper filesystem that does not support async IO.
 // Used to verify that MultiScan gracefully falls back to sync IO.
 class NoAsyncIOFS : public FileSystemWrapper {
@@ -6265,7 +6114,13 @@ TEST_P(DBMultiScanIteratorTest, WastedBlocksTracking) {
   uint64_t wasted =
       options.statistics->getTickerCount(MULTISCAN_PREFETCH_BLOCKS_WASTED);
 
-  ASSERT_GT(wasted, 0);
+  // We expect some wasted blocks due to the gap between ranges
+  // The exact number depends on prefetch behavior, but should be > 0
+  // if blocks between k020-k050 were prefetched
+  std::cout << "Wasted blocks: " << wasted << std::endl;
+
+  // Note: The test verifies the tracking mechanism works.
+  // The actual count depends on prefetch heuristics which may vary.
 }
 
 class ReadPathRangeTombstoneTest : public DBIteratorBaseTest,

db/db_write_test.cc

@@ -782,30 +782,19 @@ TEST_P(DBWriteTest, LockWALConcurrentRecursive) {
     ASSERT_OK(db_->GetLiveFilesStorageInfo({lf_opts}, &files));
   }
 
-  SyncPoint::GetInstance()->LoadDependency(
-      {{"DBWriteTest::LockWALConcurrentRecursive:ParallelLockAcquired",
-        "DBWriteTest::LockWALConcurrentRecursive:MainUnlocks"},
-       {"DBWriteTest::LockWALConcurrentRecursive:MainAllowsParallelUnlock",
-        "DBWriteTest::LockWALConcurrentRecursive:ParallelUnlocks"}});
-  SyncPoint::GetInstance()->EnableProcessing();
-
   port::Thread parallel_lock_wal{[&]() {
-    ASSERT_OK(db_->LockWAL());  // 2 -> 3
-    TEST_SYNC_POINT(
-        "DBWriteTest::LockWALConcurrentRecursive:ParallelLockAcquired");
-    TEST_SYNC_POINT("DBWriteTest::LockWALConcurrentRecursive:ParallelUnlocks");
-    ASSERT_OK(db_->UnlockWAL());
+    ASSERT_OK(db_->LockWAL());  // 2 -> 3 or 1 -> 2
   }};
-  TEST_SYNC_POINT("DBWriteTest::LockWALConcurrentRecursive:MainUnlocks");
 
-  ASSERT_OK(db_->UnlockWAL());  // 3 -> 2
+  ASSERT_OK(db_->UnlockWAL());  // 2 -> 1 or 3 -> 2
   // Give parallel_put an extra chance to jump in case of bug
   std::this_thread::yield();
+  parallel_lock_wal.join();
   ASSERT_FALSE(parallel_put_completed.Load());
   ASSERT_FALSE(parallel_ingest_completed.Load());
   ASSERT_FALSE(flush_completed.Load());
 
-  // Should still have 2 outstanding LockWAL().
+  // Should now have 2 outstanding LockWAL
   ASSERT_EQ(Get("k1"), "k1_orig");
 
   ASSERT_OK(db_->UnlockWAL());  // 2 -> 1
@@ -818,11 +807,26 @@ TEST_P(DBWriteTest, LockWALConcurrentRecursive) {
   ASSERT_EQ(Get("k2"), "NOT_FOUND");
   ASSERT_EQ(frozen_seqno, db_->GetLatestSequenceNumber());
 
-  TEST_SYNC_POINT(
-      "DBWriteTest::LockWALConcurrentRecursive:MainAllowsParallelUnlock");
-  parallel_lock_wal.join();
-  SyncPoint::GetInstance()->DisableProcessing();
-  SyncPoint::GetInstance()->LoadDependency({});
+  // Ensure final Unlock is concurrency safe and extra Unlock is safe but
+  // non-OK
+  std::atomic<int> unlock_ok{0};
+  port::Thread parallel_stuff{[&]() {
+    if (db_->UnlockWAL().ok()) {
+      unlock_ok++;
+    }
+    ASSERT_OK(db_->LockWAL());
+    if (db_->UnlockWAL().ok()) {
+      unlock_ok++;
+    }
+  }};
+
+  if (db_->UnlockWAL().ok()) {
+    unlock_ok++;
+  }
+  parallel_stuff.join();
+
+  // There was one extra unlock, so just one non-ok
+  ASSERT_EQ(unlock_ok.load(), 2);
 
   // Write can proceed
   parallel_put.join();

db/multi_scan.cc

@@ -3,56 +3,14 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
-#include <string>
-#include <vector>
-
 #include "file/file_util.h"
 #include "rocksdb/db.h"
 #include "rocksdb/file_system.h"
-#include "rocksdb/iterator.h"
 
 namespace ROCKSDB_NAMESPACE {
 
 using MultiScanIterator = MultiScan::MultiScanIterator;
 
-namespace {
-
-Status ValidateScanOptionsForMultiScan(const MultiScanArgs& multiscan_opts) {
-  if (multiscan_opts.empty()) {
-    return Status::InvalidArgument("Empty MultiScanArgs");
-  }
-
-  const Comparator* comparator = multiscan_opts.GetComparator();
-  if (comparator == nullptr) {
-    return Status::InvalidArgument("MultiScanArgs requires comparator");
-  }
-
-  const std::vector<ScanOptions>& scan_opts = multiscan_opts.GetScanRanges();
-  for (size_t i = 0; i < scan_opts.size(); ++i) {
-    const auto& scan_range = scan_opts[i].range;
-    if (!scan_range.start.has_value()) {
-      return Status::InvalidArgument("Scan has no start key at index " +
-                                     std::to_string(i));
-    }
-    if (multiscan_opts.reverse && !scan_range.limit.has_value()) {
-      return Status::InvalidArgument(
-          "Reverse MultiScan requires limit at index " + std::to_string(i));
-    }
-    if (scan_range.limit.has_value() &&
-        comparator->CompareWithoutTimestamp(
-            scan_range.start.value(), /*a_has_ts=*/false,
-            scan_range.limit.value(), /*b_has_ts=*/false) >= 0) {
-      return Status::InvalidArgument(
-          "Scan start key is large or equal than limit at index " +
-          std::to_string(i));
-    }
-  }
-
-  return Status::OK();
-}
-
-}  // namespace
-
 MultiScan::MultiScan(const ReadOptions& read_options,
                      const MultiScanArgs& scan_opts, DB* db,
                      ColumnFamilyHandle* cfh)
@@ -70,28 +28,14 @@ MultiScan::MultiScan(const ReadOptions& read_options,
       }()),
       db_(db),
       cfh_(cfh) {
-  Status validate_status = ValidateScanOptionsForMultiScan(scan_opts_);
-  if (!validate_status.ok()) {
-    db_iter_.reset(NewErrorIterator(validate_status));
-    return;
-  }
-
   bool slow_path = false;
-  // Setup read_options bounds based on the first scan. Subsequent scans will
-  // update the bounds and allocate a new DB iterator as necessary.
-  {
-    const ScanOptions& first_scan = scan_opts_.GetScanRanges()[0];
-    if (scan_opts_.reverse) {
-      lower_bound_ = *first_scan.range.start;
-      read_options_.iterate_lower_bound = &lower_bound_;
-      upper_bound_ = *first_scan.range.limit;
-      read_options_.iterate_upper_bound = &upper_bound_;
-    } else if (first_scan.range.limit) {
-      upper_bound_ = *first_scan.range.limit;
-      read_options_.iterate_upper_bound = &upper_bound_;
-    } else {
-      read_options_.iterate_upper_bound = nullptr;
-    }
+  // Setup read_options with iterate_uuper_bound based on the first scan.
+  // Subsequent scans will update and allocate a new DB iterator as necessary
+  if (scan_opts_.GetScanRanges()[0].range.limit) {
+    upper_bound_ = *scan_opts_.GetScanRanges()[0].range.limit;
+    read_options_.iterate_upper_bound = &upper_bound_;
+  } else {
+    read_options_.iterate_upper_bound = nullptr;
   }
   for (const auto& opts : scan_opts_.GetScanRanges()) {
     // Check that all the ScanOptions either specify an upper bound or not. If
@@ -110,55 +54,36 @@ MultiScan::MultiScan(const ReadOptions& read_options,
   }
 }
 
-void MultiScanIterator::SeekCurrentRange() {
-  const ScanOptions& scan_opt = scan_opts_[idx_];
-  if (scan_opt.range.limit) {
-    *upper_bound_ = *scan_opt.range.limit;
-    read_options_.iterate_upper_bound = upper_bound_;
-  } else {
-    read_options_.iterate_upper_bound = nullptr;
-  }
-
-  if (reverse_) {
-    *lower_bound_ = *scan_opt.range.start;
-    read_options_.iterate_lower_bound = lower_bound_;
-    assert(scan_opt.range.limit.has_value());
-    db_iter_->SeekForPrev(*scan_opt.range.limit);
-  } else {
-    db_iter_->Seek(*scan_opt.range.start);
-  }
-}
-
 MultiScanIterator& MultiScanIterator::operator++() {
   status_ = db_iter_->status();
   if (!status_.ok()) {
     throw MultiScanException(status_);
   }
 
-  if (!valid_) {
-    throw std::logic_error("Incrementing exhausted MultiScan iterator");
-  }
-  ++idx_;
   if (idx_ >= scan_opts_.size()) {
-    valid_ = false;
-    return *this;
+    throw std::logic_error("Index out of range");
   }
-
-  // Check if we need to update read_options_
-  if (scan_opts_[idx_].range.limit.has_value() !=
-      (read_options_.iterate_upper_bound != nullptr)) {
-    if (scan_opts_[idx_].range.limit) {
-      read_options_.iterate_upper_bound = upper_bound_;
-    } else {
-      read_options_.iterate_upper_bound = nullptr;
+  idx_++;
+  if (idx_ < scan_opts_.size()) {
+    // Check if we need to update read_options_
+    if (scan_opts_[idx_].range.limit.has_value() !=
+        (read_options_.iterate_upper_bound != nullptr)) {
+      if (scan_opts_[idx_].range.limit) {
+        *upper_bound_ = *scan_opts_[idx_].range.limit;
+        read_options_.iterate_upper_bound = upper_bound_;
+      } else {
+        read_options_.iterate_upper_bound = nullptr;
+      }
+      db_iter_.reset(db_->NewIterator(read_options_, cfh_));
+      scan_.Reset(db_iter_.get());
+    } else if (scan_opts_[idx_].range.limit) {
+      *upper_bound_ = *scan_opts_[idx_].range.limit;
+    }
+    db_iter_->Seek(*scan_opts_[idx_].range.start);
+    status_ = db_iter_->status();
+    if (!status_.ok()) {
+      throw MultiScanException(status_);
     }
-    db_iter_.reset(db_->NewIterator(read_options_, cfh_));
-    scan_.Reset(db_iter_.get(), reverse_);
-  }
-  SeekCurrentRange();
-  status_ = db_iter_->status();
-  if (!status_.ok()) {
-    throw MultiScanException(status_);
   }
   return *this;
 }

db/version_builder.cc

@@ -15,7 +15,6 @@
 #include <functional>
 #include <map>
 #include <memory>
-#include <optional>
 #include <set>
 #include <sstream>
 #include <thread>
@@ -37,46 +36,11 @@
 #include "port/port.h"
 #include "table/table_reader.h"
 #include "test_util/sync_point.h"
-#include "util/cast_util.h"
-#include "util/hash_containers.h"
 #include "util/string_util.h"
 
 namespace ROCKSDB_NAMESPACE {
 
-// Scalar/flag state that a VersionEdit can mutate while being applied. Factored
-// into a small struct so the single-edit undo (VersionBuilder::Rep::EditUndo)
-// can snapshot and restore all of it with a single copy/swap, and so the field
-// list is defined once rather than duplicated between the live state and its
-// undo record. VersionBuilder::Rep inherits this (as does EditUndo) so the
-// scalar state can be snapshotted/restored via up_cast<MutableScalars> with a
-// single copy/swap; Rep is a file-local implementation class, so exposing the
-// base is inconsequential.
-struct MutableScalars {
-  // Whether there are invalid new files or invalid deletions on levels larger
-  // than num_levels_.
-  bool has_invalid_levels_ = false;
-
-  // The fields below are only meaningful when track_found_and_missing_files_ is
-  // enabled.
-
-  // The highest file number among all missing blob files, or
-  // kInvalidBlobFileNumber if none are missing. Useful to check whether a
-  // complete Version is available.
-  uint64_t missing_blob_files_high_ = kInvalidBlobFileNumber;
-  // Cached result of the last validity check: true if all the files making up
-  // the Version can be found. Or, when allow_incomplete_valid_version_ is true
-  // and the version was never edited in an atomic group, true if only a suffix
-  // of L0 SST files and their associated blob files are missing.
-  bool valid_version_available_ = false;
-  // True if the version was ever edited in an atomic group.
-  bool edited_in_atomic_group_ = false;
-  // True if the Version was updated since the last validity check, so the
-  // cached `valid_version_available_` must be recomputed on the next check
-  // rather than reused.
-  bool version_updated_since_last_check_ = false;
-};
-
-class VersionBuilder::Rep : public MutableScalars {
+class VersionBuilder::Rep {
   class NewestFirstBySeqNo {
    public:
     bool operator()(const FileMetaData* lhs, const FileMetaData* rhs) const {
@@ -301,13 +265,17 @@ class VersionBuilder::Rep : public MutableScalars {
   // on invalid levels. The version is not consistent if in the end the files
   // on invalid levels don't cancel out.
   std::unordered_map<int, size_t> invalid_level_sizes_;
+  // Whether there are invalid new files or invalid deletion on levels larger
+  // than num_levels_.
+  bool has_invalid_levels_;
   // Current levels of table files affected by additions/deletions.
   std::unordered_map<uint64_t, int> table_file_levels_;
   // Current compact cursors that should be changed after the last compaction
   std::unordered_map<int, InternalKey> updated_compact_cursors_;
-  const NewestFirstByEpochNumber level_zero_cmp_by_epochno_;
-  const NewestFirstBySeqNo level_zero_cmp_by_seqno_;
-  const BySmallestKey level_nonzero_cmp_;
+  const std::shared_ptr<const NewestFirstByEpochNumber>
+      level_zero_cmp_by_epochno_;
+  const std::shared_ptr<const NewestFirstBySeqNo> level_zero_cmp_by_seqno_;
+  const std::shared_ptr<const BySmallestKey> level_nonzero_cmp_;
 
   // Mutable metadata objects for all blob files affected by the series of
   // version edits.
@@ -334,88 +302,30 @@ class VersionBuilder::Rep : public MutableScalars {
   std::unordered_set<uint64_t> non_l0_missing_files_;
   // Intermediate SST files (blob files not included yet)
   std::vector<std::string> intermediate_files_;
+  // The highest file number for all the missing blob files, useful to check
+  // if a complete Version is available.
+  uint64_t missing_blob_files_high_ = kInvalidBlobFileNumber;
   // Missing blob files, useful to check if only the missing L0 files'
   // associated blob files are missing.
   std::unordered_set<uint64_t> missing_blob_files_;
+  // True if all files consisting the Version can be found. Or if
+  // `allow_incomplete_valid_version_` is true and the version history is not
+  // ever edited in an atomic group, this will be true if only a
+  // suffix of L0 SST files and their associated blob files are missing.
+  bool valid_version_available_;
+  // True if version is ever edited in an atomic group.
+  bool edited_in_atomic_group_;
 
-  // Note: additional scalar/flag state mutated while applying edits
-  // (has_invalid_levels_, missing_blob_files_high_, valid_version_available_,
-  // edited_in_atomic_group_, version_updated_since_last_check_) lives in the
-  // MutableScalars base so the single-edit undo can snapshot/restore it in one
-  // copy/swap.
+  // Flag to indicate if the Version is updated since last validity check. If no
+  // `Apply` call is made between a `Rep`'s construction and a
+  // `ValidVersionAvailable` check or between two `ValidVersionAvailable` calls.
+  // This flag will be true to indicate the cached validity value can be
+  // directly used without a recheck.
+  bool version_updated_since_last_check_;
 
   // End of fields that are only tracked when `track_found_and_missing_files_`
   // is enabled.
 
-  // Records how to undo (and redo) the most recent `Apply()` so that
-  // VersionEditHandlerPointInTime can build a `Version` reflecting the state
-  // *before* an edit (on a valid->invalid negative edge) without copying the
-  // whole `Rep` on every edit. Only populated when
-  // `track_found_and_missing_files_` is true.
-  //
-  // Each recorded map/set entry stores the "other side" value (the value not
-  // currently live). Applying `ToggleUndo()` swaps every recorded slot with the
-  // live state, so it is self-inverse: one call rolls back to the pre-edit
-  // state, a second call redoes to the post-edit state. Objects momentarily
-  // removed from the live state (e.g. blob metadata) are held inside these
-  // records via value/shared_ptr, so nothing referenced by the pre-edit state
-  // is destroyed during the tentative window. `FileMetaData` removed from
-  // `added_files` by a deletion is instead kept alive via `deferred_unref` and
-  // only released by `CommitLastApply()`.
-  struct EditUndo : MutableScalars {
-    // True while the live state has been rolled back to the pre-edit state
-    // (i.e. `ToggleUndo()` has run an odd number of times).
-    bool rolled_back = false;
-
-    // Each map below records, for the keys the edit touches, the value on the
-    // "other side" (the value not currently live): before the edit while in the
-    // forward/post-edit state, and after the edit once rolled back. `nullopt`
-    // means the key is absent on that side. `ToggleUndo()` swaps each recorded
-    // entry with the corresponding live container, so it is self-inverse.
-
-    // levels_[level].added_files entries, keyed by (level, file number). Holds
-    // the FileMetaData* on the non-live side (the map only owns the pointer,
-    // not the object; see `deferred_unref` for lifetime of deleted files).
-    std::map<std::pair<int, uint64_t>, std::optional<FileMetaData*>>
-        added_files;
-    // levels_[level].deleted_files membership, keyed by (level, file number);
-    // value is whether the file number is present on the non-live side.
-    std::map<std::pair<int, uint64_t>, bool> deleted_files;
-    // table_file_levels_ entries, keyed by file number (level, or absent).
-    UnorderedMap<uint64_t, std::optional<int>> table_file_levels;
-    // invalid_level_sizes_ entries, keyed by level (count, or absent).
-    UnorderedMap<int, std::optional<size_t>> invalid_level_sizes;
-    // updated_compact_cursors_ entries, keyed by level (cursor key, or absent).
-    UnorderedMap<int, std::optional<InternalKey>> compact_cursors;
-    // mutable_blob_file_metas_ entries, keyed by blob file number. Holds the
-    // MutableBlobFileMetaData by value on the non-live side; because it owns a
-    // shared_ptr to the SharedBlobFileMetaData, a rolled-back blob addition is
-    // kept alive here rather than destroyed (which would spuriously mark the
-    // blob obsolete).
-    UnorderedMap<uint64_t, std::optional<MutableBlobFileMetaData>> blob_metas;
-    // Membership on the non-live side for each tracking set, keyed by file
-    // number (blob file number for `missing_blob_files`).
-    UnorderedMap<uint64_t, bool> found_files;
-    UnorderedMap<uint64_t, bool> l0_missing_files;
-    UnorderedMap<uint64_t, bool> non_l0_missing_files;
-    UnorderedMap<uint64_t, bool> missing_blob_files;
-
-    // `intermediate_files_` is append-only within an `Apply()`. Restore by
-    // truncating to `intermediate_files_size` (rollback) / re-appending
-    // `intermediate_tail` (redo).
-    size_t intermediate_files_size = 0;
-    std::vector<std::string> intermediate_tail;
-
-    // (Scalar/flag state on the non-live side is held in the MutableScalars
-    // base subobject.)
-
-    // `FileMetaData` removed from `added_files` by a deletion in this edit. The
-    // ref is held here (not dropped) until `CommitLastApply()`, so a rollback
-    // can reinstall the file.
-    std::vector<FileMetaData*> deferred_unref;
-  };
-  std::unique_ptr<EditUndo> undo_;
-
  public:
   Rep(const FileOptions& file_options, const ImmutableCFOptions* ioptions,
       TableCache* table_cache, VersionStorageInfo* base_vstorage,
@@ -429,9 +339,12 @@ class VersionBuilder::Rep : public MutableScalars {
         base_vstorage_(base_vstorage),
         version_set_(version_set),
         num_levels_(base_vstorage->num_levels()),
-        level_zero_cmp_by_epochno_(),
-        level_zero_cmp_by_seqno_(),
-        level_nonzero_cmp_(base_vstorage_->InternalComparator()),
+        has_invalid_levels_(false),
+        level_zero_cmp_by_epochno_(
+            std::make_shared<NewestFirstByEpochNumber>()),
+        level_zero_cmp_by_seqno_(std::make_shared<NewestFirstBySeqNo>()),
+        level_nonzero_cmp_(std::make_shared<BySmallestKey>(
+            base_vstorage_->InternalComparator())),
         file_metadata_cache_res_mgr_(file_metadata_cache_res_mgr),
         cfd_(cfd),
         version_edit_handler_(version_edit_handler),
@@ -454,17 +367,52 @@ class VersionBuilder::Rep : public MutableScalars {
     }
   }
 
-  // Rep is only ever held via std::unique_ptr and is not copied. (The previous
-  // save-point mechanism deep-copied Rep on every edit, which was quadratic;
-  // it has been replaced by single-edit undo, see EditUndo above.)
-  Rep(const Rep&) = delete;
-  Rep& operator=(const Rep&) = delete;
+  Rep(const Rep& other)
+      : file_options_(other.file_options_),
+        ioptions_(other.ioptions_),
+        table_cache_(other.table_cache_),
+        base_vstorage_(other.base_vstorage_),
+        version_set_(other.version_set_),
+        num_levels_(other.num_levels_),
+        invalid_level_sizes_(other.invalid_level_sizes_),
+        has_invalid_levels_(other.has_invalid_levels_),
+        table_file_levels_(other.table_file_levels_),
+        updated_compact_cursors_(other.updated_compact_cursors_),
+        level_zero_cmp_by_epochno_(other.level_zero_cmp_by_epochno_),
+        level_zero_cmp_by_seqno_(other.level_zero_cmp_by_seqno_),
+        level_nonzero_cmp_(other.level_nonzero_cmp_),
+        mutable_blob_file_metas_(other.mutable_blob_file_metas_),
+        file_metadata_cache_res_mgr_(other.file_metadata_cache_res_mgr_),
+        cfd_(other.cfd_),
+        version_edit_handler_(other.version_edit_handler_),
+        track_found_and_missing_files_(other.track_found_and_missing_files_),
+        allow_incomplete_valid_version_(other.allow_incomplete_valid_version_),
+        found_files_(other.found_files_),
+        l0_missing_files_(other.l0_missing_files_),
+        non_l0_missing_files_(other.non_l0_missing_files_),
+        intermediate_files_(other.intermediate_files_),
+        missing_blob_files_high_(other.missing_blob_files_high_),
+        missing_blob_files_(other.missing_blob_files_),
+        valid_version_available_(other.valid_version_available_),
+        edited_in_atomic_group_(other.edited_in_atomic_group_),
+        version_updated_since_last_check_(
+            other.version_updated_since_last_check_) {
+    assert(ioptions_);
+    levels_ = new LevelState[num_levels_];
+    for (int level = 0; level < num_levels_; level++) {
+      levels_[level] = other.levels_[level];
+      const auto& added = levels_[level].added_files;
+      for (auto& pair : added) {
+        RefFile(pair.second);
+      }
+    }
+    if (track_found_and_missing_files_) {
+      assert(cfd_);
+      assert(version_edit_handler_);
+    }
+  }
 
   ~Rep() {
-    // Release any FileMetaData whose deletion was deferred by an uncommitted
-    // Apply(), and ensure the live state is in the forward (post-edit) position
-    // so the loop below unrefs exactly the files still owned by added_files.
-    CommitLastApply();
     for (int level = 0; level < num_levels_; level++) {
       const auto& added = levels_[level].added_files;
       for (auto& pair : added) {
@@ -475,6 +423,12 @@ class VersionBuilder::Rep : public MutableScalars {
     delete[] levels_;
   }
 
+  void RefFile(FileMetaData* f) {
+    assert(f);
+    assert(f->refs > 0);
+    f->refs++;
+  }
+
   void UnrefFile(FileMetaData* f) {
     f->refs--;
     if (f->refs <= 0) {
@@ -485,15 +439,6 @@ class VersionBuilder::Rep : public MutableScalars {
         f->fd.pinned_reader.Release(table_cache_->get_cache().get());
       }
 
-      // Evict from table cache to prevent leaked entries for files whose
-      // last reference is held by the VersionBuilder (e.g., files from a
-      // failed LogAndApply that were never installed in any Version).
-      // Release() above only releases the pinned handle's ref but does not
-      // remove the entry from the cache hash table.
-      if (table_cache_ != nullptr) {
-        TableCache::Evict(table_cache_->get_cache().get(), f->fd.GetNumber());
-      }
-
       if (file_metadata_cache_res_mgr_) {
         Status s = file_metadata_cache_res_mgr_->UpdateCacheReservation(
             f->ApproximateMemoryUsage(), false /* increase */);
@@ -503,252 +448,6 @@ class VersionBuilder::Rep : public MutableScalars {
     }
   }
 
-  //======== Single-edit undo support (track_found_and_missing_files_) =======//
-  // See the EditUndo declaration above for the overall model.
-
-  // Records the pre-edit value of `map[key]` into `saved` (first-touch only, so
-  // repeated touches of the same key within one edit keep the true pre-edit
-  // value). The stored value is the "other side" for ToggleMap().
-  template <typename Map, typename Saved, typename K>
-  static void RecordMapEntry(const Map& map, Saved& saved, const K& key) {
-    if (saved.find(key) != saved.end()) {
-      return;
-    }
-    auto it = map.find(key);
-    saved.emplace(key,
-                  it != map.end()
-                      ? std::optional<typename Map::mapped_type>(it->second)
-                      : std::nullopt);
-  }
-
-  // Swaps every recorded entry between `map` and `saved` (self-inverse).
-  template <typename Map, typename Saved>
-  static void ToggleMapEntries(Map& map, Saved& saved) {
-    for (auto& elem : saved) {
-      const auto& key = elem.first;
-      auto& other = elem.second;
-      auto it = map.find(key);
-      std::optional<typename Map::mapped_type> live =
-          it != map.end() ? std::optional<typename Map::mapped_type>(it->second)
-                          : std::nullopt;
-      if (other.has_value()) {
-        // insert_or_assign avoids requiring a default-constructible mapped_type
-        // (e.g. MutableBlobFileMetaData has no default constructor).
-        map.insert_or_assign(key, std::move(*other));
-      } else if (it != map.end()) {
-        map.erase(it);
-      }
-      other = std::move(live);
-    }
-  }
-
-  template <typename Set, typename Saved, typename K>
-  static void RecordSetMembership(const Set& set, Saved& saved, const K& key) {
-    if (saved.find(key) != saved.end()) {
-      return;
-    }
-    saved.emplace(key, set.find(key) != set.end());
-  }
-
-  template <typename Set, typename Saved>
-  static void ToggleSetMembership(Set& set, Saved& saved) {
-    for (auto& elem : saved) {
-      const auto& key = elem.first;
-      bool& other_present = elem.second;
-      bool live_present = set.find(key) != set.end();
-      if (other_present) {
-        set.insert(key);
-      } else {
-        set.erase(key);
-      }
-      other_present = live_present;
-    }
-  }
-
-  void RecordAddedFileEntry(int level, uint64_t fn) {
-    assert(undo_);
-    assert(level < num_levels_);
-    auto key = std::make_pair(level, fn);
-    if (undo_->added_files.find(key) != undo_->added_files.end()) {
-      return;
-    }
-    const auto& map = levels_[level].added_files;
-    auto it = map.find(fn);
-    undo_->added_files.emplace(
-        key, it != map.end() ? std::optional<FileMetaData*>(it->second)
-                             : std::nullopt);
-  }
-
-  void RecordDeletedFileEntry(int level, uint64_t fn) {
-    assert(undo_);
-    assert(level < num_levels_);
-    auto key = std::make_pair(level, fn);
-    if (undo_->deleted_files.find(key) != undo_->deleted_files.end()) {
-      return;
-    }
-    const auto& set = levels_[level].deleted_files;
-    undo_->deleted_files.emplace(key, set.find(fn) != set.end());
-  }
-
-  // Snapshots the pre-edit value of every piece of state that `edit` will
-  // mutate, so the edit can later be rolled back / redone. Must be called
-  // before the edit is applied and only in tracking mode.
-  void CaptureUndo(const VersionEdit* edit) {
-    assert(track_found_and_missing_files_);
-    assert(!undo_);
-    undo_ = std::make_unique<EditUndo>();
-    undo_->intermediate_files_size = intermediate_files_.size();
-    // Snapshot the pre-edit scalars (the MutableScalars base subobject).
-    up_cast<MutableScalars>(*undo_) = up_cast<MutableScalars>(*this);
-
-    auto touch_table_file = [&](int level, uint64_t fn) {
-      RecordMapEntry(table_file_levels_, undo_->table_file_levels, fn);
-      RecordSetMembership(found_files_, undo_->found_files, fn);
-      RecordSetMembership(l0_missing_files_, undo_->l0_missing_files, fn);
-      RecordSetMembership(non_l0_missing_files_, undo_->non_l0_missing_files,
-                          fn);
-      if (level >= 0 && level < num_levels_) {
-        RecordAddedFileEntry(level, fn);
-        RecordDeletedFileEntry(level, fn);
-      } else if (level >= num_levels_) {
-        RecordMapEntry(invalid_level_sizes_, undo_->invalid_level_sizes, level);
-      }
-    };
-    auto touch_blob_file = [&](uint64_t blob_file_number) {
-      if (blob_file_number == kInvalidBlobFileNumber) {
-        return;
-      }
-      RecordMapEntry(mutable_blob_file_metas_, undo_->blob_metas,
-                     blob_file_number);
-      RecordSetMembership(missing_blob_files_, undo_->missing_blob_files,
-                          blob_file_number);
-    };
-
-    for (const auto& ba : edit->GetBlobFileAdditions()) {
-      touch_blob_file(ba.GetBlobFileNumber());
-    }
-    for (const auto& bg : edit->GetBlobFileGarbages()) {
-      touch_blob_file(bg.GetBlobFileNumber());
-    }
-    for (const auto& deleted_file : edit->GetDeletedFiles()) {
-      const int level = deleted_file.first;
-      const uint64_t fn = deleted_file.second;
-      touch_table_file(level, fn);
-      // Only reachable blob metadata is touched (mirrors ApplyFileDeletion).
-      if (level >= 0 && level < num_levels_ &&
-          GetCurrentLevelForTableFile(fn) == level) {
-        touch_blob_file(GetOldestBlobFileNumberForTableFile(level, fn));
-      }
-    }
-    for (const auto& new_file : edit->GetNewFiles()) {
-      const int level = new_file.first;
-      const FileMetaData& meta = new_file.second;
-      touch_table_file(level, meta.fd.GetNumber());
-      touch_blob_file(meta.oldest_blob_file_number);
-    }
-    for (const auto& cursor : edit->GetCompactCursors()) {
-      const int level = cursor.first;
-      if (level >= 0 && level < num_levels_) {
-        RecordMapEntry(updated_compact_cursors_, undo_->compact_cursors, level);
-      }
-    }
-  }
-
-  // Swaps the live state with the recorded "other side", moving between the
-  // post-edit and pre-edit states. Self-inverse.
-  void ToggleUndo() {
-    assert(undo_);
-    for (auto& elem : undo_->added_files) {
-      const int level = elem.first.first;
-      const uint64_t fn = elem.first.second;
-      auto& other = elem.second;
-      auto& map = levels_[level].added_files;
-      auto it = map.find(fn);
-      std::optional<FileMetaData*> live =
-          it != map.end() ? std::optional<FileMetaData*>(it->second)
-                          : std::nullopt;
-      if (other.has_value()) {
-        map[fn] = *other;
-      } else if (it != map.end()) {
-        // Note: erasing the map entry does not delete the FileMetaData; its
-        // lifetime is managed via `deferred_unref` / the built Version's ref.
-        map.erase(it);
-      }
-      other = live;
-    }
-    for (auto& elem : undo_->deleted_files) {
-      const int level = elem.first.first;
-      const uint64_t fn = elem.first.second;
-      bool& other_present = elem.second;
-      auto& set = levels_[level].deleted_files;
-      bool live_present = set.find(fn) != set.end();
-      if (other_present) {
-        set.insert(fn);
-      } else {
-        set.erase(fn);
-      }
-      other_present = live_present;
-    }
-    ToggleMapEntries(table_file_levels_, undo_->table_file_levels);
-    ToggleMapEntries(invalid_level_sizes_, undo_->invalid_level_sizes);
-    ToggleMapEntries(updated_compact_cursors_, undo_->compact_cursors);
-    ToggleMapEntries(mutable_blob_file_metas_, undo_->blob_metas);
-    ToggleSetMembership(found_files_, undo_->found_files);
-    ToggleSetMembership(l0_missing_files_, undo_->l0_missing_files);
-    ToggleSetMembership(non_l0_missing_files_, undo_->non_l0_missing_files);
-    ToggleSetMembership(missing_blob_files_, undo_->missing_blob_files);
-
-    if (!undo_->rolled_back) {
-      // Forward -> pre-edit: peel off the entries appended by this edit.
-      for (size_t i = undo_->intermediate_files_size;
-           i < intermediate_files_.size(); ++i) {
-        undo_->intermediate_tail.push_back(std::move(intermediate_files_[i]));
-      }
-      intermediate_files_.resize(undo_->intermediate_files_size);
-    } else {
-      // Pre-edit -> forward: re-append.
-      for (auto& s : undo_->intermediate_tail) {
-        intermediate_files_.push_back(std::move(s));
-      }
-      undo_->intermediate_tail.clear();
-    }
-
-    // Swap the scalar/flag state (the MutableScalars base subobject).
-    std::swap(up_cast<MutableScalars>(*this), up_cast<MutableScalars>(*undo_));
-
-    undo_->rolled_back = !undo_->rolled_back;
-  }
-
-  // Reverts the most recent Apply() to the pre-edit state.
-  void RollbackLastApply() {
-    assert(undo_);
-    assert(!undo_->rolled_back);
-    ToggleUndo();
-  }
-
-  // Re-applies the most recent Apply() after a RollbackLastApply().
-  void RedoLastApply() {
-    assert(undo_);
-    assert(undo_->rolled_back);
-    ToggleUndo();
-  }
-
-  // Finalizes the most recent Apply(): frees FileMetaData whose deletion was
-  // deferred and drops the undo record. Safe to call with no pending edit.
-  void CommitLastApply() {
-    if (!undo_) {
-      return;
-    }
-    if (undo_->rolled_back) {
-      // Always finalize in the forward (post-edit) state.
-      ToggleUndo();
-    }
-    for (FileMetaData* f : undo_->deferred_unref) {
-      UnrefFile(f);
-    }
-    undo_.reset();
-  }
-
   // Mapping used for checking the consistency of links between SST files and
   // blob files. It is built using the forward links (table file -> blob file),
   // and is subsequently compared with the inverse mapping stored in the
@@ -837,7 +536,7 @@ class VersionBuilder::Rep : public MutableScalars {
 
           if (epoch_number_requirement ==
               EpochNumberRequirement::kMightMissing) {
-            if (!level_zero_cmp_by_seqno_(lhs, rhs)) {
+            if (!level_zero_cmp_by_seqno_->operator()(lhs, rhs)) {
               std::ostringstream oss;
               oss << "L0 files are not sorted properly: files #"
                   << lhs->fd.GetNumber() << " with seqnos (largest, smallest) "
@@ -869,7 +568,7 @@ class VersionBuilder::Rep : public MutableScalars {
               }
             }
 
-            if (!level_zero_cmp_by_epochno_(lhs, rhs)) {
+            if (!level_zero_cmp_by_epochno_->operator()(lhs, rhs)) {
               std::ostringstream oss;
               oss << "L0 files are not sorted properly: files #"
                   << lhs->fd.GetNumber() << " with epoch number "
@@ -898,7 +597,7 @@ class VersionBuilder::Rep : public MutableScalars {
           assert(lhs);
           assert(rhs);
 
-          if (!level_nonzero_cmp_(lhs, rhs)) {
+          if (!level_nonzero_cmp_->operator()(lhs, rhs)) {
             std::ostringstream oss;
             oss << 'L' << level << " files are not sorted properly: files #"
                 << lhs->fd.GetNumber() << ", #" << rhs->fd.GetNumber();
@@ -1196,12 +895,7 @@ class VersionBuilder::Rep : public MutableScalars {
     auto& add_files = level_state.added_files;
     auto add_it = add_files.find(file_number);
     if (add_it != add_files.end()) {
-      if (undo_) {
-        // Defer freeing so a rollback of this edit can reinstall the file.
-        undo_->deferred_unref.push_back(add_it->second);
-      } else {
-        UnrefFile(add_it->second);
-      }
+      UnrefFile(add_it->second);
       add_files.erase(add_it);
     }
 
@@ -1282,7 +976,7 @@ class VersionBuilder::Rep : public MutableScalars {
         delete f;
         s = Status::MemoryLimit(
             "Can't allocate " +
-            kCacheEntryRoleToCamelString[lossless_cast<std::uint32_t>(
+            kCacheEntryRoleToCamelString[static_cast<std::uint32_t>(
                 CacheEntryRole::kFileMetadata)] +
             " due to exceeding the memory limit "
             "based on "
@@ -1360,14 +1054,6 @@ class VersionBuilder::Rep : public MutableScalars {
       }
     }
 
-    // In tracking mode, snapshot how to undo this edit before mutating, so
-    // VersionEditHandlerPointInTime can build a Version from the pre-edit state
-    // (on a valid->invalid negative edge) via RollbackLastApply() instead of
-    // copying the whole Rep on every edit.
-    if (track_found_and_missing_files_) {
-      CaptureUndo(edit);
-    }
-
     // Note: we process the blob file related changes first because the
     // table file addition/deletion logic depends on the blob files
     // already being there.
@@ -1805,13 +1491,13 @@ class VersionBuilder::Rep : public MutableScalars {
     }
 
     if (epoch_number_requirement == EpochNumberRequirement::kMightMissing) {
-      SaveSSTFilesTo(vstorage, /* level */ 0, level_zero_cmp_by_seqno_);
+      SaveSSTFilesTo(vstorage, /* level */ 0, *level_zero_cmp_by_seqno_);
     } else {
-      SaveSSTFilesTo(vstorage, /* level */ 0, level_zero_cmp_by_epochno_);
+      SaveSSTFilesTo(vstorage, /* level */ 0, *level_zero_cmp_by_epochno_);
     }
 
     for (int level = 1; level < num_levels_; ++level) {
-      SaveSSTFilesTo(vstorage, level, level_nonzero_cmp_);
+      SaveSSTFilesTo(vstorage, level, *level_nonzero_cmp_);
     }
   }
 
@@ -1865,13 +1551,13 @@ class VersionBuilder::Rep : public MutableScalars {
 
     if (epoch_number_requirement == EpochNumberRequirement::kMightMissing) {
       MergeUnorderdAddedFilesWithBase(
-          base_files, unordered_added_files, level_zero_cmp_by_seqno_,
+          base_files, unordered_added_files, *level_zero_cmp_by_seqno_,
           [&](FileMetaData* file) {
             expected_sorted_l0_files.push_back(file);
           });
     } else {
       MergeUnorderdAddedFilesWithBase(
-          base_files, unordered_added_files, level_zero_cmp_by_epochno_,
+          base_files, unordered_added_files, *level_zero_cmp_by_epochno_,
           [&](FileMetaData* file) {
             expected_sorted_l0_files.push_back(file);
           });
@@ -2061,11 +1747,11 @@ Status VersionBuilder::LoadTableHandlers(
                                  max_file_size_for_l0_meta_pin, read_options);
 }
 
-void VersionBuilder::RollbackLastApply() { rep_->RollbackLastApply(); }
-
-void VersionBuilder::RedoLastApply() { rep_->RedoLastApply(); }
-
-void VersionBuilder::CommitLastApply() { rep_->CommitLastApply(); }
+void VersionBuilder::CreateOrReplaceSavePoint() {
+  assert(rep_);
+  savepoint_ = std::move(rep_);
+  rep_ = std::make_unique<Rep>(*savepoint_);
+}
 
 bool VersionBuilder::ValidVersionAvailable() {
   return rep_->ValidVersionAvailable();
@@ -2079,6 +1765,29 @@ std::vector<std::string>& VersionBuilder::GetAndClearIntermediateFiles() {
 
 void VersionBuilder::ClearFoundFiles() { return rep_->ClearFoundFiles(); }
 
+Status VersionBuilder::SaveSavePointTo(VersionStorageInfo* vstorage) const {
+  if (!savepoint_ || !savepoint_->ValidVersionAvailable()) {
+    return Status::InvalidArgument();
+  }
+  return savepoint_->SaveTo(vstorage);
+}
+
+Status VersionBuilder::LoadSavePointTableHandlers(
+    InternalStats* internal_stats, int max_threads,
+    bool prefetch_index_and_filter_in_cache, bool is_initial_load,
+    const MutableCFOptions& mutable_cf_options,
+    size_t max_file_size_for_l0_meta_pin, const ReadOptions& read_options) {
+  if (!savepoint_ || !savepoint_->ValidVersionAvailable()) {
+    return Status::InvalidArgument();
+  }
+  return savepoint_->LoadTableHandlers(
+      internal_stats, max_threads, prefetch_index_and_filter_in_cache,
+      is_initial_load, mutable_cf_options, max_file_size_for_l0_meta_pin,
+      read_options);
+}
+
+void VersionBuilder::ClearSavePoint() { savepoint_.reset(nullptr); }
+
 BaseReferencedVersionBuilder::BaseReferencedVersionBuilder(
     ColumnFamilyData* cfd, VersionEditHandler* version_edit_handler,
     bool track_found_and_missing_files, bool allow_incomplete_valid_version)

db/version_builder.h

@@ -64,6 +64,14 @@ class VersionBuilder {
 
   //============APIs only used by VersionEditHandlerPointInTime ============//
 
+  // Creates a save point for the Version that has been built so far. Subsequent
+  // VersionEdits applied to the builder will not affect the Version in this
+  // save point. VersionBuilder currently only supports creating one save point,
+  // so when `CreateOrReplaceSavePoint` is called again, the previous save point
+  // is cleared. `ClearSavePoint` can be called explicitly to clear
+  // the save point too.
+  void CreateOrReplaceSavePoint();
+
   // The builder can find all the files to build a `Version`. Or if
   // `allow_incomplete_valid_version_` is true and the version history is never
   // edited in an atomic group, and only a suffix of L0 SST files and their
@@ -86,23 +94,27 @@ class VersionBuilder {
   // Clearing all the found files in this Version.
   void ClearFoundFiles();
 
-  // Single-edit undo for building a Version reflecting the state *before* the
-  // most recent `Apply()`. On a valid->invalid negative edge,
-  // `RollbackLastApply()` reverts the builder to the pre-edit state (so
-  // `SaveTo`/`LoadTableHandlers` build the pre-edit Version), then
-  // `RedoLastApply()` restores the post-edit state to continue replay.
-  // `CommitLastApply()` finalizes the most recent `Apply()` (freeing files
-  // whose deletion was deferred) and must be called once per `Apply()`; it is a
-  // no-op if there is nothing pending. This replaces the previous per-edit
-  // whole-Rep save-point copy, which was quadratic over a MANIFEST.
-  void RollbackLastApply();
-  void RedoLastApply();
-  void CommitLastApply();
+  // Save the Version in the save point to the provided `vstorage`.
+  // Non-OK status will be returned if there is not a valid save point.
+  Status SaveSavePointTo(VersionStorageInfo* vstorage) const;
+
+  // Load all the table handlers for the Version in the save point.
+  // Non-OK status will be returned if there is not a valid save point.
+  Status LoadSavePointTableHandlers(InternalStats* internal_stats,
+                                    int max_threads,
+                                    bool prefetch_index_and_filter_in_cache,
+                                    bool is_initial_load,
+                                    const MutableCFOptions& mutable_cf_options,
+                                    size_t max_file_size_for_l0_meta_pin,
+                                    const ReadOptions& read_options);
+
+  void ClearSavePoint();
 
   //======= End of APIs only used by VersionEditPointInTime==========//
 
  private:
   class Rep;
+  std::unique_ptr<Rep> savepoint_;
   std::unique_ptr<Rep> rep_;
 };
 

db/version_builder_test.cc

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

db/version_edit_handler.cc

@@ -872,6 +872,7 @@ Status VersionEditHandlerPointInTime::MaybeCreateVersionBeforeApplyEdit(
   assert(builder_iter != builders_.end());
   VersionBuilder* builder = builder_iter->second->version_builder();
   const bool valid_pit_before_edit = builder->ValidVersionAvailable();
+  builder->CreateOrReplaceSavePoint();
   s = builder->Apply(&edit);
   const bool valid_pit_after_edit = builder->ValidVersionAvailable();
 
@@ -887,40 +888,22 @@ Status VersionEditHandlerPointInTime::MaybeCreateVersionBeforeApplyEdit(
   if (s.ok() && !missing_info && !in_atomic_group_ &&
       ((!valid_pit_after_edit && valid_pit_before_edit) ||
        (valid_pit_after_edit && force_create_version))) {
-    // The Version to be created reflects the state *before* this edit. On a
-    // negative edge (valid before, invalid after), roll the just-applied edit
-    // back so the builder reflects the pre-edit state, build the Version from
-    // it, then redo to restore the post-edit state and continue replay. In the
-    // force case the edit is empty, so pre-edit == post-edit and no rollback is
-    // needed.
-    const bool negative_edge = valid_pit_before_edit && !valid_pit_after_edit;
-    if (negative_edge) {
-      builder->RollbackLastApply();
-    }
     const auto& mopts = cfd->GetLatestMutableCFOptions();
     auto* version = new Version(
         cfd, version_set_, version_set_->file_options_, mopts, io_tracer_,
         version_set_->current_version_number_++, epoch_number_requirement_);
-    s = builder->LoadTableHandlers(
+    s = builder->LoadSavePointTableHandlers(
         cfd->internal_stats(),
         version_set_->db_options_->max_file_opening_threads, false, true, mopts,
         MaxFileSizeForL0MetaPin(mopts), read_options_);
     if (!s.ok()) {
       delete version;
-      if (negative_edge) {
-        builder->RedoLastApply();
-      }
       if (s.IsCorruption()) {
-        // This point in time cannot be recovered; skip it and continue.
         s = Status::OK();
       }
-      builder->CommitLastApply();
       return s;
     }
-    s = builder->SaveTo(version->storage_info());
-    if (negative_edge) {
-      builder->RedoLastApply();
-    }
+    s = builder->SaveSavePointTo(version->storage_info());
     if (s.ok()) {
       if (AtomicUpdateVersionsContains(cfd->GetID())) {
         AtomicUpdateVersionsPut(version);
@@ -944,7 +927,7 @@ Status VersionEditHandlerPointInTime::MaybeCreateVersionBeforeApplyEdit(
     }
   }
 
-  builder->CommitLastApply();
+  builder->ClearSavePoint();
   return s;
 }
 

db_stress_tool/db_stress_common.h

@@ -470,7 +470,6 @@ DECLARE_uint32(min_tombstones_for_range_conversion);
 DECLARE_uint32(ingest_wbwi_one_in);
 DECLARE_bool(universal_reduce_file_locking);
 DECLARE_bool(use_multiscan);
-DECLARE_bool(multiscan_reverse);
 DECLARE_bool(multiscan_use_async_io);
 DECLARE_bool(read_scoped_block_buffer_provider);
 DECLARE_uint64(multiscan_max_prefetch_memory_bytes);

db_stress_tool/db_stress_gflags.cc

@@ -1723,11 +1723,6 @@ DEFINE_bool(
 DEFINE_bool(use_multiscan, false,
             "If set, use the batched MultiScan API for scans.");
 
-DEFINE_bool(multiscan_reverse, false,
-            "If set with use_multiscan, scan each MultiScan range in reverse "
-            "using SeekForPrev and Prev. This does not require "
-            "test_backward_scan.");
-
 DEFINE_bool(multiscan_use_async_io, false,
             "If set, enable async_io for MultiScan operations.");
 

db_stress_tool/db_stress_test_base.cc

@@ -1021,14 +1021,14 @@ void StressTest::VerificationAbort(SharedState* shared, int cf, int64_t key,
 // Under --verify_cpu_corruption_dir, right after each write op
 // (put/delete/deleterange), flush, and compaction (compactrange/compactfiles),
 // verifies that op for corruption and, on the first one found, writes a result
-// file into the directory and marks the run as a verification failure. It
-// catches: (a) a corruption returned by the op itself or by the read-back
+// file into the directory and marks the run as a verification failure. It catches:
+// (a) a corruption returned by the op itself or by the read-back
 // (Status::Corruption); or (b) a silent data corruption (SDC) -- a read-back
 // that SUCCEEDS but returns the wrong result vs the committed expected state: a
-// lost key (NotFound), a resurrected key (deleted but present), or a wrong
-// value (bytes differ). Surfacing these with an immediate read right after the
-// op (instead of waiting for a later read or the end-of-run db verification)
-// gives the most immediate and accurate verification when an external CPU-fault
+// lost key (NotFound), a resurrected key (deleted but present), or a wrong value
+// (bytes differ). Surfacing these with an immediate read right after the op
+// (instead of waiting for a later read or the end-of-run db verification) gives
+// the most immediate and accurate verification when an external CPU-fault
 // injector (e.g. gdb) flips a register inside that op.
 //
 // REQUIRES (enforced at startup) --threads=1 and all fault injection off (every
@@ -1037,9 +1037,9 @@ void StressTest::VerificationAbort(SharedState* shared, int cf, int64_t key,
 // would otherwise taint it.
 //
 // OUTPUT CONTRACT (one file per worker thread):
-// <dir>/data_corruption.<thread_id>.json -- a JSON object with fields: kind
-// (one of lost|resurrected|wrong-value|detected-corruption), cf (int), key
-// (int), value_from_db (hex), value_from_expected (hex), op_status (string).
+// <dir>/data_corruption.<thread_id>.json -- a JSON object with fields: kind (one
+// of lost|resurrected|wrong-value|detected-corruption), cf (int), key (int),
+// value_from_db (hex), value_from_expected (hex), op_status (string).
 //
 // PERFORMANCE: the read-back scans the entire keyspace after every op
 // (O(max_key) Get calls per op), so this is meant for single-op
@@ -2028,9 +2028,8 @@ void StressTest::OperateDb(ThreadState* thread) {
           ThreadStatusUtil::SetEnableTracking(FLAGS_enable_thread_tracking);
           ThreadStatusUtil::SetThreadOperation(
               ThreadStatus::OperationType::OP_DBITERATOR);
-          Status s =
-              TestMultiScan(thread, read_opts, rand_column_families, rand_keys);
-          ProcessStatus(shared, "MultiScan", s);
+          Status s;
+          s = TestMultiScan(thread, read_opts, rand_column_families, rand_keys);
           ThreadStatusUtil::ResetThreadStatus();
         } else if (!FLAGS_skip_verifydb &&
                    thread->rand.OneInOpt(
@@ -2237,8 +2236,6 @@ Status StressTest::TestMultiScan(ThreadState* thread,
   std::vector<std::string> end_key_strs;
   // TODO support reverse BytewiseComparator in the stress test
   MultiScanArgs scan_opts(options_.comparator);
-  const bool reverse_multiscan = FLAGS_multiscan_reverse;
-  scan_opts.reverse = reverse_multiscan;
   scan_opts.use_async_io =
       FLAGS_multiscan_use_async_io &&
       CheckFSFeatureSupport(options_.env->GetFileSystem().get(),
@@ -2255,11 +2252,7 @@ Status StressTest::TestMultiScan(ThreadState* thread,
   end_key_strs.reserve(num_scans);
 
   // Will be initialized before Seek() below.
-  Slice lb;
   Slice ub;
-  if (reverse_multiscan) {
-    ro.iterate_lower_bound = &lb;
-  }
   ro.iterate_upper_bound = &ub;
   for (size_t i = 0; i < num_scans * 2; i += 2) {
     assert(rand_keys[i] <= rand_keys[i + 1]);
@@ -2302,9 +2295,7 @@ Status StressTest::TestMultiScan(ThreadState* thread,
       early_exit && thread->rand.OneIn(2);
   bool abandon_prepared_scan = false;
 
-  const std::vector<ScanOptions>& scan_ranges = scan_opts.GetScanRanges();
-  for (size_t scan_idx = 0; scan_idx < scan_ranges.size(); ++scan_idx) {
-    const ScanOptions& scan_opt = scan_ranges[scan_idx];
+  for (const ScanOptions& scan_opt : scan_opts.GetScanRanges()) {
     if (op_logs.size() > kOpLogsLimit) {
       // Shouldn't take too much memory for the history log. Clear it.
       op_logs = "(cleared...)\n";
@@ -2336,27 +2327,14 @@ Status StressTest::TestMultiScan(ThreadState* thread,
 
     assert(scan_opt.range.start);
     assert(scan_opt.range.limit);
-    lb = scan_opt.range.start.value();
+    Slice key = scan_opt.range.start.value();
     ub = scan_opt.range.limit.value();
-    Slice key = reverse_multiscan ? ub : lb;
 
     LastIterateOp last_op;
-    if (reverse_multiscan) {
-      iter->SeekForPrev(key);
-      cmp_iter->SeekForPrev(key);
-      while (cmp_iter->Valid() && options_.comparator->CompareWithoutTimestamp(
-                                      cmp_iter->key(), /*a_has_ts=*/false, ub,
-                                      /*b_has_ts=*/false) >= 0) {
-        cmp_iter->Prev();
-      }
-      last_op = kLastOpSeekForPrev;
-      op_logs += "SFP " + key.ToString(true) + " ";
-    } else {
-      iter->Seek(key);
-      cmp_iter->Seek(key);
-      last_op = kLastOpSeek;
-      op_logs += "S " + key.ToString(true) + " ";
-    }
+    iter->Seek(key);
+    cmp_iter->Seek(key);
+    last_op = kLastOpSeek;
+    op_logs += "S " + key.ToString(true) + " ";
 
     if (iter->Valid() && ro.allow_unprepared_value) {
       op_logs += "*";
@@ -2374,64 +2352,8 @@ Status StressTest::TestMultiScan(ThreadState* thread,
       return cmp_iter->status();
     }
 
-    auto verify_reverse_multiscan = [&]() {
-      if (diverged || ro.iter_start_ts != nullptr) {
-        return;
-      }
-
-      if (!iter->status().ok()) {
-        fprintf(stderr, "Reverse MultiScan error: %s\n",
-                iter->status().ToString().c_str());
-        diverged = true;
-        thread->stats.AddErrors(1);
-        thread->shared->SetVerificationFailure();
-        return;
-      }
-
-      const bool cmp_valid =
-          cmp_iter->Valid() && options_.comparator->CompareWithoutTimestamp(
-                                   cmp_iter->key(), /*a_has_ts=*/false, lb,
-                                   /*b_has_ts=*/false) >= 0;
-      if (iter->Valid() != cmp_valid ||
-          (iter->Valid() && iter->key() != cmp_iter->key())) {
-        fprintf(stderr,
-                "Reverse MultiScan diverged from control iterator %s under "
-                "range [%s, %s)\n",
-                op_logs.c_str(), lb.ToString(true).c_str(),
-                ub.ToString(true).c_str());
-        if (iter->Valid()) {
-          fprintf(stderr, "iterator has key %s\n",
-                  iter->key().ToString(true).c_str());
-        } else {
-          fprintf(stderr, "iterator is not valid with status: %s\n",
-                  iter->status().ToString().c_str());
-        }
-        if (cmp_valid) {
-          fprintf(stderr, "control iterator has key %s\n",
-                  cmp_iter->key().ToString(true).c_str());
-        } else {
-          fprintf(stderr, "control iterator is outside range or invalid\n");
-        }
-        diverged = true;
-        thread->stats.AddErrors(1);
-        thread->shared->SetVerificationFailure();
-        return;
-      }
-
-      if (iter->Valid() && !verify_func(iter.get())) {
-        diverged = true;
-        thread->stats.AddErrors(1);
-        thread->shared->SetVerificationFailure();
-      }
-    };
-
-    if (reverse_multiscan) {
-      verify_reverse_multiscan();
-    } else {
-      VerifyIterator(thread, cmp_cfh, ro, iter.get(), cmp_iter.get(), last_op,
-                     key, rand_column_families, op_logs, verify_func,
-                     &diverged);
-    }
+    VerifyIterator(thread, cmp_cfh, ro, iter.get(), cmp_iter.get(), last_op,
+                   key, rand_column_families, op_logs, verify_func, &diverged);
 
     uint64_t range_iterations = 0;
     while (iter->Valid()) {
@@ -2445,23 +2367,13 @@ Status StressTest::TestMultiScan(ThreadState* thread,
         break;
       }
 
-      if (reverse_multiscan) {
-        iter->Prev();
-        if (!diverged) {
-          assert(cmp_iter->Valid());
-          cmp_iter->Prev();
-        }
-        op_logs += "P";
-      } else {
-        iter->Next();
-        if (!diverged) {
-          assert(cmp_iter->Valid());
-          cmp_iter->Next();
-        }
-        op_logs += "N";
+      iter->Next();
+      if (!diverged) {
+        assert(cmp_iter->Valid());
+        cmp_iter->Next();
       }
+      op_logs += "N";
       ++range_iterations;
-      last_op = kLastOpNextOrPrev;
 
       if (iter->Valid() && ro.allow_unprepared_value) {
         op_logs += "*";
@@ -2479,13 +2391,9 @@ Status StressTest::TestMultiScan(ThreadState* thread,
         return cmp_iter->status();
       }
 
-      if (reverse_multiscan) {
-        verify_reverse_multiscan();
-      } else {
-        VerifyIterator(thread, cmp_cfh, ro, iter.get(), cmp_iter.get(), last_op,
-                       key, rand_column_families, op_logs, verify_func,
-                       &diverged);
-      }
+      VerifyIterator(thread, cmp_cfh, ro, iter.get(), cmp_iter.get(), last_op,
+                     key, rand_column_families, op_logs, verify_func,
+                     &diverged);
 
       if (diverged) {
         if (thread->shared->HasVerificationFailedYet()) {

docs/_posts/2026-06-20-blob-direct-write-partitioned-blob-files.markdown

@@ -1,303 +0,0 @@
----
-title: Blob Direct Write With Partitioned Blob Files
-layout: post
-author: xbw
-category: blog
----
-
-## TL;DR
-
-Blob Direct Write moves large-value separation earlier in RocksDB's write path.
-When `enable_blob_files` and `enable_blob_direct_write` are enabled, values at
-or above `min_blob_size` can be written directly to blob files during a write,
-while the WAL and memtable store a compact `BlobIndex` reference instead of the
-full value.
-
-The companion partitioning support makes this more than a write-path
-optimization. A column family can have multiple direct-write blob partitions,
-and applications can provide a `BlobFilePartitionStrategy` to choose where each
-large value goes. That turns blob files into a policy-controlled grouping unit.
-For example, an application can route values with similar TTLs into the same
-set of blob files while using Universal Compaction for the key and metadata
-part of the LSM.
-
-The reduced-scope v1 implementation landed in
-[pull request #14535](https://github.com/facebook/rocksdb/pull/14535), and
-custom partition selection was added in
-[pull request #14565](https://github.com/facebook/rocksdb/pull/14565).
-
-## Background
-
-Integrated BlobDB already separates large values from the LSM tree. The LSM
-stores keys plus blob references, and blob files store the large value bytes.
-This reduces compaction write amplification because compaction can rewrite
-keys and references without repeatedly copying large values.
-
-Before Blob Direct Write, however, large values still entered RocksDB through
-the normal write path first. They were serialized into a write batch, written
-to the WAL, inserted into the memtable, and later extracted into blob files
-during flush or compaction. That design is simple and broadly compatible, but
-it means large values still consume WAL bandwidth and memtable memory before
-they become out-of-line blobs.
-
-Blob Direct Write changes that placement point. The write path can externalize
-a large value immediately, then publish a `BlobIndex` through the normal WAL
-and memtable machinery.
-
-## Write Path
-
-The core write-path logic lives in `BlobWriteBatchTransformer` and
-`BlobFilePartitionManager`.
-
-For a regular `Put` inside a `WriteBatch`, the transformer does the following:
-
-1. Checks the column family's current blob direct-write settings.
-2. Leaves small values inline when `value.size() < min_blob_size`.
-3. Writes qualifying large values to a blob file through
-   `BlobFilePartitionManager::WriteBlob()`.
-4. Encodes the returned blob file number, offset, size, and compression type
-   into a `BlobIndex`.
-5. Rewrites the batch entry as `PutBlobIndex`.
-
-The rest of RocksDB still sees an ordinary ordered write: the transformed batch
-goes through WAL logging, sequence assignment, and memtable insertion. The
-difference is that the payload traveling through those structures is the blob
-reference rather than the original large value.
-
-The implementation also handles `PutEntity()` for wide-column entities. Large
-columns can be moved to blob files while small columns, such as metadata, stay
-inline in the entity. The serialized entity then uses the V2 wide-column format
-to store `BlobIndex` references for the blob-backed columns.
-
-If a transformed write fails after appending blob bytes but before the
-transformed batch is committed, RocksDB does not try to rewrite the physical
-blob file. Instead, it records those appended records as initial garbage for
-the file. This keeps the append-only blob-file contract simple while preserving
-correct garbage accounting.
-
-## Partitioned Blob Files
-
-Blob Direct Write maintains a per-column-family `BlobFilePartitionManager`.
-The manager owns one active blob writer slot per configured partition:
-
-```cpp
-Options options;
-options.enable_blob_files = true;
-options.enable_blob_direct_write = true;
-options.allow_concurrent_memtable_write = false;
-options.min_blob_size = 1024;
-options.blob_direct_write_partitions = 8;
-```
-
-By default, RocksDB uses a round-robin strategy. Applications that need
-policy-driven grouping can install a custom `BlobFilePartitionStrategy`:
-
-```cpp
-class TtlBucketPartitionStrategy : public rocksdb::BlobFilePartitionStrategy {
- public:
-  using rocksdb::BlobFilePartitionStrategy::SelectPartition;
-
-  const char* Name() const override { return "TtlBucketPartitionStrategy"; }
-
-  uint32_t SelectPartition(uint32_t num_partitions, uint32_t column_family_id,
-                           const rocksdb::Slice& key,
-                           const rocksdb::Slice& value) override {
-    return ExtractTtlBucket(key, value) % num_partitions;
-  }
-};
-
-options.blob_direct_write_partition_strategy =
-    std::make_shared<TtlBucketPartitionStrategy>();
-```
-
-The strategy sees the logical write inputs. If blob compression is enabled, the
-strategy still receives the original uncompressed value. Its return value is
-normalized modulo `blob_direct_write_partitions`, so implementations can return
-a hash or bucket id directly.
-
-For `PutEntity()`, the strategy has a wide-column overload:
-
-```cpp
-uint32_t SelectPartition(uint32_t num_partitions, uint32_t column_family_id,
-                         const rocksdb::Slice& key,
-                         const rocksdb::WideColumns& columns) override;
-```
-
-RocksDB calls this once per entity and reuses the selected partition for all
-blob-backed columns in that entity. This is useful when inline metadata, such
-as a TTL bucket or schema version, should determine placement for the large
-payload columns.
-
-The strategy runs on the write hot path. It must be thread-safe, must not
-throw exceptions into RocksDB, and should avoid I/O, callbacks into RocksDB, or
-other blocking work. It is also an application callback rather than a
-serialized OPTIONS object, so applications that rely on custom placement must
-provide the strategy again every time they open the DB.
-
-## Lifecycle and Manifest Registration
-
-Direct-write blob files have to line up with memtable lifetime. RocksDB uses a
-generation-based lifecycle:
-
-1. While a memtable is mutable, direct writes append to the current generation
-   of active partition files.
-2. When RocksDB switches memtables, `RotateCurrentGeneration()` moves those
-   active files into an immutable generation associated with the memtable that
-   now contains their `BlobIndex` references.
-3. When that memtable is flushed, `PrepareFlushAdditions()` seals the matching
-   blob files, builds `BlobFileAddition` records, and attaches them to the
-   flush's `VersionEdit`.
-4. The MANIFEST commit makes those blob files part of the column family's
-   versioned state.
-
-This is the key correctness boundary. Before flush commits, active
-direct-write blob files can be read by the primary DB, but they are not yet
-normal MANIFEST-visible BlobDB files. After flush commits, the usual versioned
-BlobDB read and file-lifetime machinery owns them.
-
-Old memtables and old `SuperVersion`s can temporarily outlive the flush that
-registered their blob files. RocksDB protects those sealed file numbers until
-the corresponding memtables are released, preventing obsolete-file cleanup from
-racing with delayed reads through older in-memory state.
-
-## Reading Direct-Write Blobs
-
-A read can encounter a `BlobIndex` before its blob file is visible in the
-current `Version`. This happens for values that have been written to an active
-direct-write blob file but whose memtable has not flushed yet.
-
-`BlobFilePartitionManager::ResolveBlobDirectWriteIndex()` handles that by
-trying the normal versioned BlobDB path first. If the blob file is already
-known to the `Version`, RocksDB returns that result directly, including any
-real I/O or corruption error. If the file is not yet versioned, RocksDB falls
-back to opening the blob file through the blob file cache and reading it as an
-in-flight direct-write file.
-
-This fallback also handles an important cache edge case. A reader may have
-cached a blob file reader while the file was still open and smaller. After more
-records are appended or after the file is sealed, that cached reader can have a
-stale view of file size or footer state. The direct-write path evicts and
-reopens the reader when needed so later reads see the finalized file.
-
-## What Partitioning Enables
-
-The most important capability is policy-aware physical grouping.
-
-Consider a large-value workload where each key has a TTL, but different keys
-expire on different horizons. A single DB-wide FIFO lifetime is too coarse:
-short-lived values can remain on disk until the longest-lived values in the
-same file age out. Standard compaction can delete keys individually, but if
-large values are scattered across blob files, the deleted values become
-garbage spread across many files.
-
-With partitioned direct-write blobs, an application can route values into
-coarse TTL buckets:
-
-```text
-TTL bucket 0 -> blob partition 0
-TTL bucket 1 -> blob partition 1
-TTL bucket 2 -> blob partition 2
-...
-```
-
-The LSM can still use Universal Compaction for keys and metadata. A compaction
-filter can remove expired keys based on metadata, and BlobDB can account for
-the corresponding blob bytes as garbage. Because values with similar expiry
-times were colocated, expiration tends to concentrate garbage in a smaller set
-of blob files. Once a blob file contains no live values, it can be deleted as a
-whole file.
-
-That gives RocksDB a middle ground:
-
-* The LSM keeps flexible key-level semantics.
-* Blob files provide bulk physical reclamation.
-* The application controls the grouping policy without creating separate DBs
-  or column families for every group.
-
-TTL bucketing is only one example. The same mechanism can group values by key
-range, tenant, object class, expected lifetime, value size, or any other
-application-level placement policy that can be computed cheaply during the
-write.
-
-## Interaction With Wide Columns
-
-Partitioned Blob Direct Write becomes especially useful with wide columns.
-Small metadata columns can stay inline in the LSM while large payload columns
-move to blob files. That means read paths and compaction filters can inspect
-metadata without always resolving the large blob.
-
-For the TTL example, an entity might keep a compact TTL bucket column inline
-and put the large payload column in a blob file. The partition strategy can use
-the TTL column to choose a blob partition, while a compaction filter can later
-drop expired keys by inspecting the inline metadata. Blob I/O is only needed
-when the value itself is actually required.
-
-This keeps RocksDB byte-oriented. RocksDB manages keys, sequence numbers,
-`BlobIndex` references, blob file lifetime, and byte storage. The application
-or a higher layer remains responsible for schema, TTL interpretation, and value
-decoding.
-
-## Limitations
-
-Blob Direct Write is still a reduced-scope v1 feature. The durable boundary,
-write-thread model, and API compatibility are deliberately conservative.
-
-The most important limitation is crash recovery. Direct-write blob files are
-registered in the MANIFEST only when the memtable containing their `BlobIndex`
-references is flushed. Until that flush commits, the WAL can contain
-references to active blob files that are not part of versioned metadata. v1
-does not support recovering that active direct-write state through WAL replay,
-so applications should not treat WAL logging alone as the crash-recovery
-boundary for direct-written blobs. RocksDB forces a final flush for live
-direct-write column families during clean close, but a process crash, failed
-close-time flush, or shutdown path that cannot complete the flush can leave
-active direct-write blob files unregistered.
-
-There are also configuration and API limits:
-
-* `enable_blob_direct_write` requires `enable_blob_files`.
-* `enable_blob_direct_write`, `blob_direct_write_partitions`, and
-  `blob_direct_write_partition_strategy` are not dynamically changeable
-  through `SetOptions()`.
-* The v1 write path requires the ordered single-memtable-writer mode. It is not
-  compatible with `unordered_write`, `enable_pipelined_write`,
-  `two_write_queues`, or `allow_concurrent_memtable_write`.
-* `DB::IngestWriteBatchWithIndex()` is not supported while any live column
-  family has `enable_blob_direct_write` enabled.
-* MemPurge and user-defined timestamps are not supported with Blob Direct
-  Write.
-* Pre-serialized wide-column entities that already contain blob references are
-  rejected when Blob Direct Write is enabled. RocksDB needs to create and track
-  the direct-write blob references inside the current write path.
-* Checkpoint, backup, and live-file enumeration must flush pending
-  direct-write state first. Calls that intentionally skip the flush, or that
-  run while the WAL is locked and therefore cannot flush, can return
-  `NotSupported`.
-
-One subtle implementation point is that partitions are currently a file
-placement and lifecycle abstraction. The manager has multiple active partition
-files, but v1 still protects partition state with a single manager mutex.
-Future work can add finer-grained per-partition concurrency without changing
-the placement or generation contract.
-
-Custom file systems also need to support the visibility model. The primary DB
-may read an active direct-write blob file while it is still open for writing,
-so appended bytes must be visible to a separate reader before the writer is
-closed. File systems that do not provide that behavior should avoid enabling
-Blob Direct Write until they can satisfy the contract.
-
-## When to Consider It
-
-Blob Direct Write is worth considering when large values dominate the write
-path and there is a useful physical grouping policy for those values. The most
-natural cases are workloads where the key and small metadata should stay cheap
-to compact, scan, or filter, while the large payload should be stored
-out-of-line and reclaimed in policy-controlled groups.
-
-For workloads with mixed TTLs, the combination of Universal Compaction,
-metadata-aware filtering, and partitioned blob placement can provide
-per-value-expiration behavior while preserving whole-file reclamation as the
-physical cleanup unit. More generally, partitioned blob files give RocksDB a
-new placement hook: applications can keep RocksDB's LSM semantics for keys
-while shaping blob-file layout around how data will age, be read, or be
-deleted.

folly.mk

@@ -23,6 +23,7 @@ $(error Please specify only one of USE_FOLLY and USE_FOLLY_LITE)
 endif
 ifneq ($(strip $(FOLLY_PATH)),)
 	BOOST_PATH = $(shell (ls -d $(FOLLY_PATH)/../boost*))
+	DBL_CONV_PATH = $(shell (ls -d $(FOLLY_PATH)/../double-conversion*))
 	GFLAGS_PATH = $(shell (ls -d $(FOLLY_PATH)/../gflags*))
 	GLOG_PATH = $(shell (ls -d $(FOLLY_PATH)/../glog*))
 	LIBEVENT_PATH = $(shell (ls -d $(FOLLY_PATH)/../libevent*))
@@ -30,28 +31,27 @@ ifneq ($(strip $(FOLLY_PATH)),)
 	LIBSODIUM_PATH = $(shell (ls -d $(FOLLY_PATH)/../libsodium*))
 	FMT_PATH = $(shell (ls -d $(FOLLY_PATH)/../fmt*))
 
-	# For some reason, some libraries are under either lib or lib64
+	# For some reason, glog and fmt libraries are under either lib or lib64
 	GLOG_LIB_PATH = $(shell (ls -d $(GLOG_PATH)/lib*))
 	FMT_LIB_PATH = $(shell (ls -d $(FMT_PATH)/lib*))
-	LIBEVENT_LIB_PATH = $(shell (ls -d $(LIBEVENT_PATH)/lib*))
 
 	# AIX: pre-defined system headers are surrounded by an extern "C" block
 	ifeq ($(PLATFORM), OS_AIX)
-		PLATFORM_CCFLAGS += -I$(BOOST_PATH)/include -I$(GLOG_PATH)/include -I$(LIBEVENT_PATH)/include -I$(XZ_PATH)/include -I$(LIBSODIUM_PATH)/include -I$(FOLLY_PATH)/include -I$(FMT_PATH)/include
-		PLATFORM_CXXFLAGS += -I$(BOOST_PATH)/include -I$(GLOG_PATH)/include -I$(LIBEVENT_PATH)/include -I$(XZ_PATH)/include -I$(LIBSODIUM_PATH)/include -I$(FOLLY_PATH)/include -I$(FMT_PATH)/include
+		PLATFORM_CCFLAGS += -I$(BOOST_PATH)/include -I$(DBL_CONV_PATH)/include -I$(GLOG_PATH)/include -I$(LIBEVENT_PATH)/include -I$(XZ_PATH)/include -I$(LIBSODIUM_PATH)/include -I$(FOLLY_PATH)/include -I$(FMT_PATH)/include
+		PLATFORM_CXXFLAGS += -I$(BOOST_PATH)/include -I$(DBL_CONV_PATH)/include -I$(GLOG_PATH)/include -I$(LIBEVENT_PATH)/include -I$(XZ_PATH)/include -I$(LIBSODIUM_PATH)/include -I$(FOLLY_PATH)/include -I$(FMT_PATH)/include
 	else
-		PLATFORM_CCFLAGS += -isystem $(BOOST_PATH)/include -isystem $(GLOG_PATH)/include -isystem $(LIBEVENT_PATH)/include -isystem $(XZ_PATH)/include -isystem $(LIBSODIUM_PATH)/include -isystem $(FOLLY_PATH)/include -isystem $(FMT_PATH)/include
-		PLATFORM_CXXFLAGS += -isystem $(BOOST_PATH)/include -isystem $(GLOG_PATH)/include -isystem $(LIBEVENT_PATH)/include -isystem $(XZ_PATH)/include -isystem $(LIBSODIUM_PATH)/include -isystem $(FOLLY_PATH)/include -isystem $(FMT_PATH)/include
+		PLATFORM_CCFLAGS += -isystem $(BOOST_PATH)/include -isystem $(DBL_CONV_PATH)/include -isystem $(GLOG_PATH)/include -isystem $(LIBEVENT_PATH)/include -isystem $(XZ_PATH)/include -isystem $(LIBSODIUM_PATH)/include -isystem $(FOLLY_PATH)/include -isystem $(FMT_PATH)/include
+		PLATFORM_CXXFLAGS += -isystem $(BOOST_PATH)/include -isystem $(DBL_CONV_PATH)/include -isystem $(GLOG_PATH)/include -isystem $(LIBEVENT_PATH)/include -isystem $(XZ_PATH)/include -isystem $(LIBSODIUM_PATH)/include -isystem $(FOLLY_PATH)/include -isystem $(FMT_PATH)/include
 	endif
 
 	# Add -ldl at the end as gcc resolves a symbol in a library by searching only in libraries specified later
 	# in the command line
 
-	PLATFORM_LDFLAGS += $(FOLLY_PATH)/lib/libfolly.a $(BOOST_PATH)/lib/libboost_context.a $(BOOST_PATH)/lib/libboost_filesystem.a $(BOOST_PATH)/lib/libboost_atomic.a $(BOOST_PATH)/lib/libboost_program_options.a $(BOOST_PATH)/lib/libboost_regex.a $(BOOST_PATH)/lib/libboost_system.a $(BOOST_PATH)/lib/libboost_thread.a $(LIBEVENT_LIB_PATH)/libevent.a $(LIBSODIUM_PATH)/lib/libsodium.a -ldl
+	PLATFORM_LDFLAGS += $(FOLLY_PATH)/lib/libfolly.a $(BOOST_PATH)/lib/libboost_context.a $(BOOST_PATH)/lib/libboost_filesystem.a $(BOOST_PATH)/lib/libboost_atomic.a $(BOOST_PATH)/lib/libboost_program_options.a $(BOOST_PATH)/lib/libboost_regex.a $(BOOST_PATH)/lib/libboost_system.a $(BOOST_PATH)/lib/libboost_thread.a $(DBL_CONV_PATH)/lib/libdouble-conversion.a $(LIBEVENT_PATH)/lib/libevent.a $(LIBSODIUM_PATH)/lib/libsodium.a -ldl
 ifneq ($(DEBUG_LEVEL),0)
-	PLATFORM_LDFLAGS += $(FMT_LIB_PATH)/libfmtd.a $(GLOG_LIB_PATH)/libglogd.so $(GFLAGS_PATH)/lib/libgflags_debug.so.2.3
+	PLATFORM_LDFLAGS += $(FMT_LIB_PATH)/libfmtd.a $(GLOG_LIB_PATH)/libglogd.so $(GFLAGS_PATH)/lib/libgflags_debug.so.2.2
 else
-	PLATFORM_LDFLAGS += $(FMT_LIB_PATH)/libfmt.a $(GLOG_LIB_PATH)/libglog.so $(GFLAGS_PATH)/lib/libgflags.so.2.3
+	PLATFORM_LDFLAGS += $(FMT_LIB_PATH)/libfmt.a $(GLOG_LIB_PATH)/libglog.so $(GFLAGS_PATH)/lib/libgflags.so.2.2
 endif
 	PLATFORM_LDFLAGS += -Wl,-rpath=$(GLOG_LIB_PATH) -Wl,-rpath=$(GFLAGS_PATH)/lib
 endif
@@ -98,7 +98,7 @@ endif  # FMT_SOURCE_PATH
 	PLATFORM_LDFLAGS += -lglog
 endif
 
-FOLLY_COMMIT_HASH = 2a68075b77d958854d895e78fe6c41940dd49f3c
+FOLLY_COMMIT_HASH = 548b16da0b3cc887d69cdb6ae06496ad8a2a9276
 FOLLY_GETDEPS_CACHE_DIR = /tmp/rocksdb-getdeps-cache
 
 define restore_folly_getdeps_downloads

include/rocksdb/db.h

@@ -1828,17 +1828,16 @@ class DB {
   // state, though while the WAL is locked, flushes as part of CreateCheckpoint
   // and simiar are skipped. Other operations allowed on a "read only" DB should
   // work while frozen. Each LockWAL() call that returns OK must eventually be
-  // followed by a corresponding call to UnlockWAL(). It is also expected that
-  // UnlockWAL() is called on the same thread that called LockWAL(). Where
-  // supported, non-OK status is generally only possible with some kind of
-  // corruption or I/O error.
+  // followed by a corresponding call to UnlockWAL(). Where supported, non-OK
+  // status is generally only possible with some kind of corruption or I/O
+  // error.
   virtual Status LockWAL() {
     return Status::NotSupported("LockWAL not implemented");
   }
 
-  // Unfreeze the DB state from a successful LockWAL(). The write stop on the
-  // database will be cleared when UnlockWAL() have been called for each
-  // successful LockWAL(). Must be called from the same thread as LockWAL().
+  // Unfreeze the DB state from a successful LockWAL().
+  // The write stop on the database will be cleared when UnlockWAL() have been
+  // called for each successful LockWAL().
   virtual Status UnlockWAL() {
     return Status::NotSupported("UnlockWAL not implemented");
   }

include/rocksdb/multi_scan.h

@@ -71,15 +71,11 @@ class Scan {
  public:
   class ScanIterator;
 
-  explicit Scan(Iterator* db_iter, bool reverse = false)
-      : db_iter_(db_iter), reverse_(reverse) {}
+  explicit Scan(Iterator* db_iter) : db_iter_(db_iter) {}
 
-  void Reset(Iterator* db_iter, bool reverse = false) {
-    db_iter_ = db_iter;
-    reverse_ = reverse;
-  }
+  void Reset(Iterator* db_iter) { db_iter_ = db_iter; }
 
-  ScanIterator begin() { return ScanIterator(db_iter_, reverse_); }
+  ScanIterator begin() { return ScanIterator(db_iter_); }
 
   std::nullptr_t end() { return nullptr; }
 
@@ -92,8 +88,7 @@ class Scan {
     using difference_type = int;
     using iterator_category = std::input_iterator_tag;
 
-    explicit ScanIterator(Iterator* db_iter, bool reverse = false)
-        : db_iter_(db_iter), reverse_(reverse) {
+    explicit ScanIterator(Iterator* db_iter) : db_iter_(db_iter) {
       valid_ = db_iter_->Valid();
       if (valid_) {
         result_ = value_type(db_iter_->key(), db_iter_->value());
@@ -114,11 +109,7 @@ class Scan {
       if (!valid_) {
         throw std::logic_error("Trying to advance invalid iterator");
       } else {
-        if (reverse_) {
-          db_iter_->Prev();
-        } else {
-          db_iter_->Next();
-        }
+        db_iter_->Next();
         status_ = db_iter_->status();
         if (!status_.ok()) {
           throw MultiScanException(status_);
@@ -151,7 +142,6 @@ class Scan {
 
    private:
     Iterator* db_iter_;
-    bool reverse_{};
     bool valid_;
     Status status_;
     value_type result_;
@@ -159,7 +149,6 @@ class Scan {
 
  private:
   Iterator* db_iter_;
-  bool reverse_;
 };
 
 // A container object encapsulating the scan ranges for a multi scan.
@@ -190,19 +179,15 @@ class MultiScan {
 
     MultiScanIterator(const std::vector<ScanOptions>& scan_opts, DB* db,
                       ColumnFamilyHandle* cfh, ReadOptions& read_options,
-                      Slice* lower_bound, Slice* upper_bound,
-                      std::unique_ptr<Iterator>& db_iter, bool reverse)
+                      Slice* upper_bound, std::unique_ptr<Iterator>& db_iter)
         : scan_opts_(scan_opts),
           db_(db),
           cfh_(cfh),
           read_options_(read_options),
-          lower_bound_(lower_bound),
           upper_bound_(upper_bound),
           idx_(0),
-          reverse_(reverse),
-          valid_(true),
           db_iter_(db_iter),
-          scan_(db_iter_.get(), reverse_) {
+          scan_(db_iter_.get()) {
       if (scan_opts_.empty()) {
         throw std::logic_error("Zero scans in multi-scan");
       }
@@ -210,7 +195,7 @@ class MultiScan {
       if (!status_.ok()) {
         throw MultiScanException(status_);
       }
-      SeekCurrentRange();
+      db_iter_->Seek(*scan_opts_[idx_].range.start);
       status_ = db_iter_->status();
       if (!status_.ok()) {
         throw MultiScanException(status_);
@@ -221,9 +206,13 @@ class MultiScan {
 
     MultiScanIterator& operator++();
 
-    bool operator==(std::nullptr_t /*other*/) const { return !valid_; }
+    bool operator==(std::nullptr_t /*other*/) const {
+      return idx_ >= scan_opts_.size();
+    }
 
-    bool operator!=(std::nullptr_t /*other*/) const { return valid_; }
+    bool operator!=(std::nullptr_t /*other*/) const {
+      return idx_ < scan_opts_.size();
+    }
 
     reference operator*() { return scan_; }
     reference operator->() { return scan_; }
@@ -233,22 +222,16 @@ class MultiScan {
     DB* db_;
     ColumnFamilyHandle* cfh_;
     ReadOptions& read_options_;
-    Slice* lower_bound_;
     Slice* upper_bound_;
     size_t idx_;
-    bool reverse_;
-    bool valid_;
     std::unique_ptr<Iterator>& db_iter_;
     Status status_;
     Scan scan_;
-
-    void SeekCurrentRange();
   };
 
   MultiScanIterator begin() {
     return MultiScanIterator(scan_opts_.GetScanRanges(), db_, cfh_,
-                             read_options_, &lower_bound_, &upper_bound_,
-                             db_iter_, scan_opts_.reverse);
+                             read_options_, &upper_bound_, db_iter_);
   }
 
   std::nullptr_t end() { return nullptr; }
@@ -257,8 +240,7 @@ class MultiScan {
   ReadOptions read_options_;
   const MultiScanArgs scan_opts_;
   DB* db_;
-  ColumnFamilyHandle* cfh_{};
-  Slice lower_bound_;
+  ColumnFamilyHandle* cfh_;
   Slice upper_bound_;
   std::unique_ptr<Iterator> db_iter_;
 };

include/rocksdb/options.h

@@ -1995,14 +1995,12 @@ class MultiScanArgs {
     io_coalesce_threshold = other.io_coalesce_threshold;
     max_prefetch_size = other.max_prefetch_size;
     use_async_io = other.use_async_io;
-    reverse = other.reverse;
     io_dispatcher = other.io_dispatcher;
   }
   MultiScanArgs(MultiScanArgs&& other) noexcept
       : io_coalesce_threshold(other.io_coalesce_threshold),
         max_prefetch_size(other.max_prefetch_size),
         use_async_io(other.use_async_io),
-        reverse(other.reverse),
         io_dispatcher(std::move(other.io_dispatcher)),
         comp_(other.comp_),
         original_ranges_(std::move(other.original_ranges_)) {}
@@ -2013,7 +2011,6 @@ class MultiScanArgs {
     io_coalesce_threshold = other.io_coalesce_threshold;
     max_prefetch_size = other.max_prefetch_size;
     use_async_io = other.use_async_io;
-    reverse = other.reverse;
     io_dispatcher = other.io_dispatcher;
     return *this;
   }
@@ -2025,7 +2022,6 @@ class MultiScanArgs {
       io_coalesce_threshold = other.io_coalesce_threshold;
       max_prefetch_size = other.max_prefetch_size;
       use_async_io = other.use_async_io;
-      reverse = other.reverse;
       io_dispatcher = std::move(other.io_dispatcher);
     }
     return *this;
@@ -2089,7 +2085,6 @@ class MultiScanArgs {
     io_coalesce_threshold = other.io_coalesce_threshold;
     max_prefetch_size = other.max_prefetch_size;
     use_async_io = other.use_async_io;
-    reverse = other.reverse;
     io_dispatcher = other.io_dispatcher;
   }
 
@@ -2112,10 +2107,6 @@ class MultiScanArgs {
   // When false, it will use synchronous MultiRead().
   bool use_async_io = false;
 
-  // Scan ranges in reverse order. Ranges are still specified in comparator
-  // order as [start, limit), and reverse scans require bounded ranges.
-  bool reverse = false;
-
   // Optional IODispatcher for multi-scan operations.
   // If nullptr (default), a new IODispatcher is created internally.
   // Users can provide their own IODispatcher for custom IO scheduling

include/rocksdb/utilities/backup_engine.h

@@ -309,9 +309,6 @@ struct CreateBackupOptions {
   // Flush will always trigger if 2PC is enabled.
   // If write-ahead logs are disabled, set flush_before_backup=true to
   // avoid losing unflushed key/value pairs from the memtable.
-  //
-  // NOTE: If LockWAL() is active and WAL-disabled writes left unpersisted data,
-  // backup may wait for UnlockWAL() before it can flush.
   bool flush_before_backup = false;
 
   // Callback for reporting progress, based on callback_trigger_interval_size.

include/rocksdb/utilities/transaction_db.h

@@ -119,14 +119,6 @@ class RangeLockManagerHandle : public LockManagerHandle {
   // Set the user-provided barrier check function
   virtual void SetEscalationBarrierFunc(EscalationBarrierFunc func) = 0;
 
-  // Predicate returning true if the thread waiting for a lock has been killed.
-  // A range lock wait polls this and returns promptly (DB_LOCK_INTERRUPTED ->
-  // Status::Aborted()) when it fires.
-  using IsKilledCallback = std::function<bool()>;
-
-  // Set the user-provided kill-check function
-  virtual void SetIsKilledCallback(IsKilledCallback func) = 0;
-
   virtual RangeLockStatus GetRangeLockStatusData() = 0;
 
   class Counters {

include/rocksdb/version.h

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

table/block_based/block.h

@@ -465,9 +465,6 @@ class BlockIter : public InternalIteratorBase<TValue> {
   uint32_t restarts_;  // Offset of restart array (list of fixed32)
   // current_ is offset in data_ of current entry.  >= restarts_ if !Valid
   uint32_t current_;
-  // Cached result of GetKeysEndOffset(). Computed once in InitializeBase()
-  // since values_section_, data_, and restarts_ do not change afterward.
-  uint32_t keys_end_offset_{0};
   // Raw key from block.
   IterKey raw_key_;
   // Buffer for key data when global seqno assignment is enabled.
@@ -591,9 +588,6 @@ class BlockIter : public InternalIteratorBase<TValue> {
             (block_restart_interval > 0 || num_restarts == 1)));
 
     values_section_ = values_section;
-    keys_end_offset_ = values_section_
-                           ? static_cast<uint32_t>(values_section_ - data_)
-                           : restarts_;
     current_ = GetKeysEndOffset();
   }
 
@@ -710,7 +704,10 @@ class BlockIter : public InternalIteratorBase<TValue> {
   // Return the offset where the keys section ends.
   // For separated KV storage, this is the start of the values section.
   // Otherwise, it's the start of the restart array.
-  inline uint32_t GetKeysEndOffset() const { return keys_end_offset_; }
+  inline uint32_t GetKeysEndOffset() const {
+    return values_section_ ? static_cast<uint32_t>(values_section_ - data_)
+                           : restarts_;
+  }
 
   uint32_t GetRestartPoint(uint32_t index) const {
     assert(index < num_restarts_);

table/block_based/block_based_table_iterator.cc

@@ -205,9 +205,7 @@ void BlockBasedTableIterator::SeekImpl(const Slice* target,
 }
 
 void BlockBasedTableIterator::SeekForPrev(const Slice& target) {
-  if (!IsReverseMultiScan()) {
-    ResetMultiScan();
-  }
+  ResetMultiScan();
   direction_ = IterDirection::kBackward;
   ResetBlockCacheLookupVar();
   is_out_of_bound_ = false;
@@ -245,11 +243,7 @@ void BlockBasedTableIterator::SeekForPrev(const Slice& target) {
   // first block, rather than the second. However, we don't have the information
   // to distinguish the two unless we read the second block. In this case, we'll
   // end up with reading two blocks.
-  if (IsReverseMultiScan()) {
-    index_iter_->SeekForPrev(target);
-  } else {
-    index_iter_->Seek(target);
-  }
+  index_iter_->Seek(target);
   is_index_at_curr_block_ = true;
 
   if (!index_iter_->Valid()) {
@@ -286,9 +280,7 @@ void BlockBasedTableIterator::SeekForPrev(const Slice& target) {
 }
 
 void BlockBasedTableIterator::SeekToLast() {
-  if (!IsReverseMultiScan()) {
-    ResetMultiScan();
-  }
+  ResetMultiScan();
   direction_ = IterDirection::kBackward;
   ResetBlockCacheLookupVar();
   is_out_of_bound_ = false;
@@ -335,8 +327,7 @@ bool BlockBasedTableIterator::NextAndGetResult(IterateResult* result) {
 }
 
 void BlockBasedTableIterator::Prev() {
-  if ((readahead_cache_lookup_ && !IsIndexAtCurr()) ||
-      (multi_scan_read_set_ && !IsReverseMultiScan())) {
+  if ((readahead_cache_lookup_ && !IsIndexAtCurr()) || multi_scan_read_set_) {
     ResetMultiScan();
     // In case of readahead_cache_lookup_, index_iter_ has moved forward. So we
     // need to reseek the index_iter_ to point to current block by using
@@ -391,7 +382,7 @@ void BlockBasedTableIterator::InitDataBlock() {
         ResetDataIter();
       }
       size_t rs_idx = multi_scan_index_iter_->current_read_set_index();
-      if (rs_idx >= multi_scan_index_iter_->GetPrefetchBlockCount()) {
+      if (rs_idx >= prefetch_max_idx_) {
         if (multi_scan_index_iter_->GetMaxPrefetchSize() == 0) {
           // max_prefetch_size is not set, treat as end of file
           return;
@@ -1009,10 +1000,9 @@ void BlockBasedTableIterator::BlockCacheLookupForReadAheadSize(
 // multiscan with regular iterator usage.
 // - scan ranges should be non-overlapping, and have increasing start keys.
 // If a scan range's limit is not set, then there should only be one scan range.
-// - After Prepare(), a forward scan expects Seek to be called on the start key
-// of each ScanOption in order. A reverse scan expects SeekForPrev to be called
-// on the limit key of each ScanOption in reverse order. If any other Seek is
-// done, an error status is returned.
+// - After Prepare(), the iterator expects Seek to be called on the start key
+// of each ScanOption in order. If any other Seek is done, an error status is
+// returned
 // - Whenever all blocks of a scan opt are exhausted, the iterator will become
 // invalid and UpperBoundCheckResult() will return kOutOfBound. So that the
 // upper layer (LevelIterator) will stop scanning instead thinking EOF is
@@ -1058,37 +1048,24 @@ void BlockBasedTableIterator::Prepare(const MultiScanArgs* multiscan_opts) {
     return;
   }
 
-  // Calculate prefetch range (enforces max_prefetch_size)
-  size_t prefetch_start_idx = 0;
+  // Calculate prefetch_max_idx (enforces max_prefetch_size)
   size_t prefetch_max_idx = scan_block_handles.size();
   if (multiscan_opts->max_prefetch_size > 0) {
     uint64_t total_size = 0;
-    if (multiscan_opts->reverse) {
-      for (size_t i = scan_block_handles.size(); i > 0; --i) {
-        const size_t block_idx = i - 1;
-        total_size += BlockBasedTable::BlockSizeWithTrailer(
-            scan_block_handles[block_idx]);
-        if (total_size > multiscan_opts->max_prefetch_size) {
-          prefetch_start_idx = block_idx + 1;
-          break;
-        }
-      }
-    } else {
-      for (size_t i = 0; i < scan_block_handles.size(); ++i) {
-        total_size +=
-            BlockBasedTable::BlockSizeWithTrailer(scan_block_handles[i]);
-        if (total_size > multiscan_opts->max_prefetch_size) {
-          prefetch_max_idx = i;
-          break;
-        }
+    for (size_t i = 0; i < scan_block_handles.size(); ++i) {
+      total_size +=
+          BlockBasedTable::BlockSizeWithTrailer(scan_block_handles[i]);
+      if (total_size > multiscan_opts->max_prefetch_size) {
+        prefetch_max_idx = i;
+        break;
       }
     }
   }
 
-  // Create block handles vector for IODispatcher (limited to prefetch range)
+  // Create block handles vector for IODispatcher (limited to prefetch_max_idx)
   std::vector<BlockHandle> blocks_to_prefetch;
-  if (prefetch_start_idx < prefetch_max_idx) {
-    blocks_to_prefetch.assign(scan_block_handles.begin() + prefetch_start_idx,
+  if (prefetch_max_idx > 0) {
+    blocks_to_prefetch.assign(scan_block_handles.begin(),
                               scan_block_handles.begin() + prefetch_max_idx);
   }
 
@@ -1122,11 +1099,11 @@ void BlockBasedTableIterator::Prepare(const MultiScanArgs* multiscan_opts) {
   // the index iterator. The original index_iter_ is saved for restoration
   // on backward operations.
   // Note: data_block_separators keeps full size for seek logic, even though
-  // only blocks in [prefetch_start_idx, prefetch_max_idx) are prefetched.
+  // only blocks up to prefetch_max_idx are actually prefetched.
   auto multi_scan_idx_iter = std::make_unique<MultiScanIndexIterator>(
       std::move(scan_block_handles), std::move(data_block_separators),
       std::move(block_index_ranges_per_scan), multiscan_opts, read_set,
-      prefetch_start_idx, prefetch_max_idx, icomp_, table_->GetStatistics());
+      prefetch_max_idx, icomp_, table_->GetStatistics());
   assert(multi_scan_idx_iter->status().ok());
 
   multi_scan_read_set_ = std::move(read_set);

table/block_based/block_based_table_iterator.h

@@ -315,11 +315,6 @@ class BlockBasedTableIterator : public InternalIteratorBase<Slice> {
 
   bool IsIndexAtCurr() const { return is_index_at_curr_block_; }
 
-  inline bool IsReverseMultiScan() const {
-    return multi_scan_read_set_ && multi_scan_index_iter_ &&
-           multi_scan_index_iter_->scan_opts()->reverse;
-  }
-
   const BlockBasedTable* table_;
   const ReadOptions& read_options_;
   const InternalKeyComparator& icomp_;

table/block_based/block_based_table_reader_test.cc

@@ -1152,11 +1152,12 @@ class BlockBasedTableReaderMultiScanAsyncIOTest
     : public BlockBasedTableReaderMultiScanTest {};
 
 // TODO: test no block cache case
-TEST_P(BlockBasedTableReaderMultiScanAsyncIOTest, MultiScanPrepare) {
+TEST_P(BlockBasedTableReaderMultiScanAsyncIOTest, DISABLED_MultiScanPrepare) {
   auto param = GetParam();
   auto fill_cache = param.fill_cache;
   auto use_async_io = param.use_async_io;
 
+  options_.statistics = CreateDBStatistics();
   std::shared_ptr<FileSystem> fs = options_.env->GetFileSystem();
   int64_t supported_ops = 0;
   fs->SupportedOps(supported_ops);
@@ -1170,210 +1171,244 @@ TEST_P(BlockBasedTableReaderMultiScanAsyncIOTest, MultiScanPrepare) {
           100 /* num_block */,
           true /* mixed_with_human_readable_string_value */, ts_sz,
           same_key_diff_ts_, comparator_);
+  std::string table_name = "BlockBasedTableReaderTest_NewIterator" +
+                           CompressionTypeToString(compression_type_) +
+                           "_async" + std::to_string(use_async_io);
+  ImmutableOptions ioptions(options_);
+  // Only insert 60 out of 100 blocks
+  CreateTable(table_name, ioptions, compression_type_,
+              std::vector<std::pair<std::string, std::string>>{
+                  kv.begin() + 20 * kEntriesPerBlock,
+                  kv.begin() + 80 * kEntriesPerBlock},
+              compression_parallel_threads_, compression_dict_bytes_);
 
-  auto user_key = [&](size_t key_idx) {
-    return ExtractUserKey(kv[key_idx].first);
-  };
-  auto reverse_seek_key = [&](size_t key_idx) {
-    std::string target;
-    AppendInternalKey(&target,
-                      ParsedInternalKey(user_key(key_idx), kMaxSequenceNumber,
-                                        kValueTypeForSeekForPrev));
-    return target;
-  };
-  auto new_iter = [&](BlockBasedTable* table) {
-    return std::unique_ptr<InternalIterator>(table->NewIterator(
+  std::unique_ptr<BlockBasedTable> table;
+  FileOptions foptions;
+  foptions.use_direct_reads = use_direct_reads_;
+  InternalKeyComparator comparator(options_.comparator);
+  NewBlockBasedTableReader(foptions, ioptions, comparator, table_name, &table,
+                           true /* bool prefetch_index_and_filter_in_cache */,
+                           nullptr /* status */, persist_udt_);
+
+  // 1. Should coalesce into a single I/O
+  std::unique_ptr<InternalIterator> iter;
+  iter.reset(table->NewIterator(
+      read_opts, options_.prefix_extractor.get(), /*arena=*/nullptr,
+      /*skip_filters=*/false, TableReaderCaller::kUncategorized));
+
+  MultiScanArgs scan_options(comparator_);
+  scan_options.use_async_io = use_async_io;
+  scan_options.insert(ExtractUserKey(kv[30 * kEntriesPerBlock].first),
+                      ExtractUserKey(kv[31 * kEntriesPerBlock].first));
+  scan_options.insert(ExtractUserKey(kv[32 * kEntriesPerBlock].first),
+                      ExtractUserKey(kv[33 * kEntriesPerBlock].first));
+  auto read_count_before =
+      options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
+
+  iter->Prepare(&scan_options);
+  iter->Seek(kv[30 * kEntriesPerBlock].first);
+  for (size_t i = 30 * kEntriesPerBlock; i <= 31 * kEntriesPerBlock; ++i) {
+    ASSERT_TRUE(iter->status().ok()) << iter->status().ToString();
+    ASSERT_TRUE(iter->Valid()) << i;
+    ASSERT_EQ(iter->key().ToString(), kv[i].first);
+    iter->Next();
+  }
+  // Iter may still be valid after scan range. Upper layer (DBIter) handles
+  // exact upper bound checking. So we don't check !iter->Valid() here.
+  ASSERT_OK(iter->status());
+  iter->Seek(kv[32 * kEntriesPerBlock].first);
+  for (size_t i = 32 * kEntriesPerBlock; i < 33 * kEntriesPerBlock; ++i) {
+    ASSERT_TRUE(iter->Valid());
+    ASSERT_EQ(iter->key().ToString(), kv[i].first);
+    iter->Next();
+  }
+  ASSERT_OK(iter->status());
+  auto read_count_after =
+      options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
+  ASSERT_EQ(read_count_before + 1, read_count_after);
+
+  // 2. No IO coalesce, should do MultiRead/ReadAsync with 2 read requests.
+  iter.reset(table->NewIterator(
+      read_opts, options_.prefix_extractor.get(), /*arena=*/nullptr,
+      /*skip_filters=*/false, TableReaderCaller::kUncategorized));
+  scan_options = MultiScanArgs(comparator_);
+  scan_options.insert(ExtractUserKey(kv[40 * kEntriesPerBlock].first),
+                      ExtractUserKey(kv[45 * kEntriesPerBlock].first));
+  scan_options.insert(ExtractUserKey(kv[70 * kEntriesPerBlock].first),
+                      ExtractUserKey(kv[75 * kEntriesPerBlock].first));
+
+  read_count_before =
+      options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
+  iter->Prepare(&scan_options);
+
+  iter->Seek(kv[40 * kEntriesPerBlock].first);
+  for (size_t i = 40 * kEntriesPerBlock; i < 45 * kEntriesPerBlock; ++i) {
+    ASSERT_TRUE(iter->Valid());
+    ASSERT_EQ(iter->key().ToString(), kv[i].first);
+    iter->Next();
+  }
+  ASSERT_OK(iter->status());
+  iter->Seek(kv[70 * kEntriesPerBlock].first);
+  for (size_t i = 70 * kEntriesPerBlock; i < 75 * kEntriesPerBlock; ++i) {
+    ASSERT_TRUE(iter->Valid());
+    ASSERT_EQ(iter->key().ToString(), kv[i].first);
+    iter->Next();
+  }
+  ASSERT_OK(iter->status());
+
+  read_count_after =
+      options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
+  ASSERT_EQ(read_count_before + 2, read_count_after);
+
+  iter.reset(table->NewIterator(
+      read_opts, options_.prefix_extractor.get(), /*arena=*/nullptr,
+      /*skip_filters=*/false, TableReaderCaller::kUncategorized));
+
+  // 3. Tests I/O excludes blocks already in cache.
+  // Reading blocks from 40-79
+  // From reads above, blocks 40-44 and 70-74 already in cache
+  // So we should read 45-69, 75-79 in two I/Os.
+  // If fill_cache is false, then we'll do one giant I/O.
+  scan_options = MultiScanArgs(comparator_);
+  scan_options.use_async_io = use_async_io;
+  scan_options.insert(ExtractUserKey(kv[40 * kEntriesPerBlock].first),
+                      ExtractUserKey(kv[80 * kEntriesPerBlock].first));
+  read_count_before =
+      options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
+  iter->Prepare(&scan_options);
+  read_count_after =
+      options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
+  // When async IO is available on this thread, Prepare() only queues the reads
+  // and stats are updated later during Poll(). Otherwise it falls back to the
+  // synchronous path and records the reads immediately.
+  const uint64_t expected_prepare_reads =
+      async_supported ? 0 : (fill_cache ? 2 : 1);
+  ASSERT_EQ(read_count_before + expected_prepare_reads, read_count_after);
+
+  iter->Seek(kv[40 * kEntriesPerBlock].first);
+  for (size_t i = 40 * kEntriesPerBlock; i < 80 * kEntriesPerBlock; ++i) {
+    ASSERT_TRUE(iter->Valid());
+    ASSERT_EQ(iter->key().ToString(), kv[i].first);
+    iter->Next();
+  }
+  ASSERT_FALSE(iter->Valid());
+  ASSERT_OK(iter->status());
+  read_count_after =
+      options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
+  if (!fill_cache) {
+    ASSERT_EQ(read_count_before + 1, read_count_after);
+  } else {
+    ASSERT_EQ(read_count_before + 2, read_count_after);
+  }
+
+  // 4. Check cases when Seek key does not match start key in ScanOptions
+  iter.reset(table->NewIterator(
+      read_opts, options_.prefix_extractor.get(), /*arena=*/nullptr,
+      /*skip_filters=*/false, TableReaderCaller::kUncategorized));
+  scan_options = MultiScanArgs(comparator_);
+  scan_options.use_async_io = use_async_io;
+  scan_options.insert(ExtractUserKey(kv[30 * kEntriesPerBlock].first),
+                      ExtractUserKey(kv[40 * kEntriesPerBlock].first));
+  scan_options.insert(ExtractUserKey(kv[50 * kEntriesPerBlock].first),
+                      ExtractUserKey(kv[60 * kEntriesPerBlock].first));
+  iter->Prepare(&scan_options);
+  // Match start key
+  iter->Seek(kv[30 * kEntriesPerBlock].first);
+  for (size_t i = 30 * kEntriesPerBlock; i < 40 * kEntriesPerBlock; ++i) {
+    ASSERT_TRUE(iter->Valid());
+    ASSERT_EQ(iter->key().ToString(), kv[i].first);
+    iter->Next();
+  }
+  ASSERT_OK(iter->status());
+
+  // Seek a key that is larger than next start key is allowed, as long as it is
+  // larger than the previous key
+  iter->Seek(kv[50 * kEntriesPerBlock + 1].first);
+  ASSERT_OK(iter->status());
+
+  // Check seek key going backward
+  iter.reset(table->NewIterator(
+      read_opts, options_.prefix_extractor.get(), /*arena=*/nullptr,
+      /*skip_filters=*/false, TableReaderCaller::kUncategorized));
+  scan_options = MultiScanArgs(comparator_);
+  scan_options.use_async_io = use_async_io;
+  scan_options.insert(ExtractUserKey(kv[30 * kEntriesPerBlock].first),
+                      ExtractUserKey(kv[31 * kEntriesPerBlock].first));
+  scan_options.insert(ExtractUserKey(kv[32 * kEntriesPerBlock].first),
+                      ExtractUserKey(kv[33 * kEntriesPerBlock].first));
+  iter->Prepare(&scan_options);
+  iter->Seek(kv[32 * kEntriesPerBlock].first);
+  auto key = iter->key();
+  ASSERT_OK(iter->status());
+  iter->Seek(kv[30 * kEntriesPerBlock].first);
+  // When seek key goes backward, it is adjusted to the last seeked position.
+  // Assert the key read is same as before.
+  ASSERT_EQ(key, iter->key());
+  ASSERT_OK(iter->status());
+
+  // Test prefetch limit reached.
+  iter.reset(table->NewIterator(
+      read_opts, options_.prefix_extractor.get(), /*arena=*/nullptr,
+      /*skip_filters=*/false, TableReaderCaller::kUncategorized));
+  scan_options = MultiScanArgs(comparator_);
+  scan_options.use_async_io = use_async_io;
+  scan_options.max_prefetch_size = 1024;  // less than block size
+  scan_options.insert(ExtractUserKey(kv[30 * kEntriesPerBlock].first),
+                      ExtractUserKey(kv[40 * kEntriesPerBlock].first));
+  iter->Prepare(&scan_options);
+  iter->Seek(kv[31 * kEntriesPerBlock].first);
+  ASSERT_TRUE(iter->status().IsIncomplete());
+
+  // Randomly seek keys on the file, as long as the key is moving forward, it
+  // is allowed
+
+  if (use_async_io) {
+    // Skip following test when async io is enabled. There is some issue with
+    // IO_uring that I am still trying to root cause.
+    // TODO : enable the test again with async IO
+    return;
+  }
+  for (int i = 0; i < 100; i++) {
+    iter.reset(table->NewIterator(
         read_opts, options_.prefix_extractor.get(), /*arena=*/nullptr,
         /*skip_filters=*/false, TableReaderCaller::kUncategorized));
-  };
-  auto add_range = [&](MultiScanArgs* scan_options, size_t start_block,
-                       size_t limit_block) {
-    scan_options->insert(user_key(start_block * kEntriesPerBlock),
-                         user_key(limit_block * kEntriesPerBlock));
-  };
-  auto scan_block_range = [&](InternalIterator* iter, size_t start_block,
-                              size_t limit_block, bool reverse) {
-    const size_t start_idx = start_block * kEntriesPerBlock;
-    const size_t limit_idx = limit_block * kEntriesPerBlock;
-    if (reverse) {
-      const std::string target = reverse_seek_key(limit_idx);
-      iter->SeekForPrev(target);
-      for (size_t i = limit_idx; i > start_idx; --i) {
-        ASSERT_TRUE(iter->status().ok()) << iter->status().ToString();
-        ASSERT_TRUE(iter->Valid()) << i;
-        ASSERT_EQ(iter->key().ToString(), kv[i - 1].first);
-        iter->Prev();
-      }
-    } else {
-      iter->Seek(kv[start_idx].first);
-      for (size_t i = start_idx; i < limit_idx; ++i) {
-        ASSERT_TRUE(iter->status().ok()) << iter->status().ToString();
-        ASSERT_TRUE(iter->Valid()) << i;
-        ASSERT_EQ(iter->key().ToString(), kv[i].first);
+    scan_options = MultiScanArgs(comparator_);
+    scan_options.use_async_io = use_async_io;
+    scan_options.insert(ExtractUserKey(kv[5 * kEntriesPerBlock].first),
+                        ExtractUserKey(kv[10 * kEntriesPerBlock].first));
+    scan_options.insert(ExtractUserKey(kv[25 * kEntriesPerBlock].first),
+                        ExtractUserKey(kv[35 * kEntriesPerBlock].first));
+    scan_options.insert(ExtractUserKey(kv[35 * kEntriesPerBlock].first),
+                        ExtractUserKey(kv[40 * kEntriesPerBlock].first));
+    scan_options.insert(ExtractUserKey(kv[45 * kEntriesPerBlock].first),
+                        ExtractUserKey(kv[50 * kEntriesPerBlock].first));
+    scan_options.insert(ExtractUserKey(kv[75 * kEntriesPerBlock].first),
+                        ExtractUserKey(kv[85 * kEntriesPerBlock].first));
+    scan_options.insert(ExtractUserKey(kv[85 * kEntriesPerBlock].first),
+                        ExtractUserKey(kv[95 * kEntriesPerBlock].first));
+
+    iter->Prepare(&scan_options);
+
+    auto random_seed = static_cast<uint32_t>(
+        std::chrono::duration_cast<std::chrono::nanoseconds>(
+            std::chrono::system_clock::now().time_since_epoch())
+            .count());
+    Random rnd(random_seed);
+    std::cout << random_seed << std::endl;
+    SCOPED_TRACE("Random seed " + std::to_string(random_seed));
+
+    // Search key always start from the start key of first prepared range.
+    int last_read_key_index = rnd.Uniform(100) + 5 * kEntriesPerBlock;
+    while (last_read_key_index < 100 * kEntriesPerBlock) {
+      iter->Seek(kv[last_read_key_index].first);
+      EXPECT_OK(iter->status());
+      // iterate for a few keys
+      while (iter->Valid()) {
         iter->Next();
+        last_read_key_index++;
+        EXPECT_OK(iter->status());
       }
-    }
-    ASSERT_OK(iter->status());
-  };
-  const bool run_reverse =
-      compression_type_ == kNoCompression && !use_direct_reads_ &&
-      param.index_type == BlockBasedTableOptions::IndexType::kBinarySearch &&
-      param.compression_parallel_threads == 1 &&
-      param.compression_dict_bytes == 0;
-
-  for (const bool reverse : {false, true}) {
-    if (reverse && !run_reverse) {
-      continue;
-    }
-    SCOPED_TRACE(reverse ? "reverse" : "forward");
-    options_.statistics = CreateDBStatistics();
-    std::string table_name = "BlockBasedTableReaderTest_NewIterator" +
-                             CompressionTypeToString(compression_type_) +
-                             "_async" + std::to_string(use_async_io) +
-                             "_reverse" + std::to_string(reverse);
-    ImmutableOptions ioptions(options_);
-    CreateTable(table_name, ioptions, compression_type_,
-                std::vector<std::pair<std::string, std::string>>{
-                    kv.begin() + 20 * kEntriesPerBlock,
-                    kv.begin() + 80 * kEntriesPerBlock},
-                compression_parallel_threads_, compression_dict_bytes_);
-
-    std::unique_ptr<BlockBasedTable> table;
-    FileOptions foptions;
-    foptions.use_direct_reads = use_direct_reads_;
-    InternalKeyComparator comparator(options_.comparator);
-    NewBlockBasedTableReader(foptions, ioptions, comparator, table_name, &table,
-                             true /* bool prefetch_index_and_filter_in_cache */,
-                             nullptr /* status */, persist_udt_);
-
-    // 1. Should coalesce into a single I/O
-    std::unique_ptr<InternalIterator> iter = new_iter(table.get());
-    MultiScanArgs scan_options(comparator_);
-    scan_options.use_async_io = use_async_io;
-    scan_options.reverse = reverse;
-    add_range(&scan_options, 30, 31);
-    add_range(&scan_options, 32, 33);
-    auto read_count_before =
-        options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
-
-    iter->Prepare(&scan_options);
-    scan_block_range(iter.get(), 30, 31, reverse);
-    scan_block_range(iter.get(), 32, 33, reverse);
-    auto read_count_after =
-        options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
-    ASSERT_EQ(read_count_before + 1, read_count_after);
-
-    // 2. No IO coalesce, should do MultiRead/ReadAsync with 2 read requests.
-    iter = new_iter(table.get());
-    scan_options = MultiScanArgs(comparator_);
-    scan_options.use_async_io = use_async_io;
-    scan_options.reverse = reverse;
-    add_range(&scan_options, 40, 45);
-    add_range(&scan_options, 70, 75);
-
-    read_count_before =
-        options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
-    iter->Prepare(&scan_options);
-    scan_block_range(iter.get(), 40, 45, reverse);
-    scan_block_range(iter.get(), 70, 75, reverse);
-
-    read_count_after =
-        options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
-    ASSERT_EQ(read_count_before + 2, read_count_after);
-
-    // 3. Tests I/O excludes blocks already in cache.
-    // Reading blocks from 40-79. From reads above, blocks 40-44 and 70-74
-    // are already in cache. So fill_cache=true should read 45-69, 75-79 in
-    // two I/Os. If fill_cache is false, then we'll do one giant I/O.
-    iter = new_iter(table.get());
-    scan_options = MultiScanArgs(comparator_);
-    scan_options.use_async_io = use_async_io;
-    scan_options.reverse = reverse;
-    add_range(&scan_options, 40, 80);
-    read_count_before =
-        options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
-    iter->Prepare(&scan_options);
-    read_count_after =
-        options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
-    const uint64_t expected_prepare_reads =
-        async_supported ? 0 : (fill_cache ? 2 : 1);
-    ASSERT_EQ(read_count_before + expected_prepare_reads, read_count_after);
-
-    scan_block_range(iter.get(), 40, 80, reverse);
-    read_count_after =
-        options_.statistics->getTickerCount(NON_LAST_LEVEL_READ_COUNT);
-    if (!fill_cache) {
-      ASSERT_EQ(read_count_before + 1, read_count_after);
-    } else {
-      ASSERT_EQ(read_count_before + 2, read_count_after);
-    }
-
-    // 4. Check cases when seek key does not match scan range boundary.
-    iter = new_iter(table.get());
-    scan_options = MultiScanArgs(comparator_);
-    scan_options.use_async_io = use_async_io;
-    scan_options.reverse = reverse;
-    add_range(&scan_options, 30, 40);
-    add_range(&scan_options, 50, 60);
-    iter->Prepare(&scan_options);
-    scan_block_range(iter.get(), 30, 40, reverse);
-    if (reverse) {
-      iter->SeekForPrev(kv[50 * kEntriesPerBlock + 1].first);
-    } else {
-      iter->Seek(kv[50 * kEntriesPerBlock + 1].first);
-    }
-    ASSERT_OK(iter->status());
-
-    // Test prefetch limit reached.
-    iter = new_iter(table.get());
-    scan_options = MultiScanArgs(comparator_);
-    scan_options.use_async_io = use_async_io;
-    scan_options.reverse = reverse;
-    scan_options.max_prefetch_size = 1024;  // less than block size
-    add_range(&scan_options, 30, 40);
-    iter->Prepare(&scan_options);
-    if (reverse) {
-      iter->SeekForPrev(reverse_seek_key(40 * kEntriesPerBlock));
-    } else {
-      iter->Seek(kv[31 * kEntriesPerBlock].first);
-    }
-    ASSERT_TRUE(iter->status().IsIncomplete() ||
-                iter->status().IsPrefetchLimitReached());
-
-    if (!reverse) {
-      // Randomly seek keys on the file, as long as the key is moving forward.
-      for (int i = 0; i < 100; i++) {
-        iter = new_iter(table.get());
-        scan_options = MultiScanArgs(comparator_);
-        scan_options.use_async_io = use_async_io;
-        add_range(&scan_options, 5, 10);
-        add_range(&scan_options, 25, 35);
-        add_range(&scan_options, 35, 40);
-        add_range(&scan_options, 45, 50);
-        add_range(&scan_options, 75, 85);
-        add_range(&scan_options, 85, 95);
-
-        iter->Prepare(&scan_options);
-
-        auto random_seed = static_cast<uint32_t>(
-            std::chrono::duration_cast<std::chrono::nanoseconds>(
-                std::chrono::system_clock::now().time_since_epoch())
-                .count());
-        Random rnd(random_seed);
-        SCOPED_TRACE("Random seed " + std::to_string(random_seed));
-
-        int last_read_key_index = rnd.Uniform(100) + 5 * kEntriesPerBlock;
-        while (last_read_key_index < 100 * kEntriesPerBlock) {
-          iter->Seek(kv[last_read_key_index].first);
-          EXPECT_OK(iter->status());
-          while (iter->Valid()) {
-            iter->Next();
-            last_read_key_index++;
-            EXPECT_OK(iter->status());
-          }
-          last_read_key_index += rnd.Uniform(100);
-        }
-      }
+      last_read_key_index += rnd.Uniform(100);
     }
   }
 }
@@ -1576,90 +1611,6 @@ TEST_P(BlockBasedTableReaderMultiScanTest, MultiScanPrefetchSizeLimit) {
   }
 }
 
-TEST_P(BlockBasedTableReaderMultiScanTest, ReverseMultiScanPrepare) {
-  if (std::string(options_.comparator->Name()).find("Reverse") !=
-      std::string::npos) {
-    ROCKSDB_GTEST_BYPASS("This test asserts bytewise iterator order.");
-    return;
-  }
-
-  ReadOptions read_opts;
-  size_t ts_sz = options_.comparator->timestamp_size();
-
-  std::vector<std::pair<std::string, std::string>> kv =
-      BlockBasedTableReaderBaseTest::GenerateKVMap(
-          20 /* num_block */, true /* mixed_with_human_readable_string_value */,
-          ts_sz, same_key_diff_ts_, comparator_);
-
-  std::string table_name = "BlockBasedTableReaderTest_ReverseMultiScanPrepare" +
-                           CompressionTypeToString(compression_type_);
-
-  ImmutableOptions ioptions(options_);
-  CreateTable(table_name, ioptions, compression_type_, kv,
-              compression_parallel_threads_, compression_dict_bytes_);
-
-  std::unique_ptr<BlockBasedTable> table;
-  FileOptions foptions;
-  foptions.use_direct_reads = use_direct_reads_;
-  InternalKeyComparator comparator(options_.comparator);
-  NewBlockBasedTableReader(foptions, ioptions, comparator, table_name, &table,
-                           true /* bool prefetch_index_and_filter_in_cache */,
-                           nullptr /* status */, persist_udt_);
-
-  std::unique_ptr<InternalIterator> iter;
-  iter.reset(table->NewIterator(
-      read_opts, options_.prefix_extractor.get(), /*arena=*/nullptr,
-      /*skip_filters=*/false, TableReaderCaller::kUncategorized));
-
-  MultiScanArgs scan_options(comparator_);
-  scan_options.reverse = true;
-  scan_options.max_prefetch_size = 10 * 1024 * 1024;  // 10MB
-  scan_options.insert(ExtractUserKey(kv[2 * kEntriesPerBlock].first),
-                      ExtractUserKey(kv[4 * kEntriesPerBlock].first));
-  scan_options.insert(ExtractUserKey(kv[8 * kEntriesPerBlock].first),
-                      ExtractUserKey(kv[10 * kEntriesPerBlock].first));
-  scan_options.insert(ExtractUserKey(kv[14 * kEntriesPerBlock].first),
-                      ExtractUserKey(kv[16 * kEntriesPerBlock].first));
-
-  iter->Prepare(&scan_options);
-
-  iter->SeekForPrev(kv[19 * kEntriesPerBlock].first);
-  ASSERT_TRUE(iter->status().ok()) << iter->status().ToString();
-  ASSERT_TRUE(iter->Valid());
-
-  std::string limit_seek_key;
-  AppendInternalKey(
-      &limit_seek_key,
-      ParsedInternalKey(ExtractUserKey(kv[16 * kEntriesPerBlock].first),
-                        kMaxSequenceNumber, kValueTypeForSeekForPrev));
-  iter->SeekForPrev(limit_seek_key);
-  for (size_t i = 16 * kEntriesPerBlock; i > 14 * kEntriesPerBlock; --i) {
-    ASSERT_TRUE(iter->status().ok()) << iter->status().ToString();
-    ASSERT_TRUE(iter->Valid()) << i;
-    ASSERT_EQ(iter->key().ToString(), kv[i - 1].first);
-    iter->Prev();
-  }
-  ASSERT_OK(iter->status());
-
-  iter->SeekForPrev(kv[10 * kEntriesPerBlock - 1].first);
-  for (size_t i = 10 * kEntriesPerBlock; i > 8 * kEntriesPerBlock; --i) {
-    ASSERT_TRUE(iter->status().ok()) << iter->status().ToString();
-    ASSERT_TRUE(iter->Valid()) << i;
-    ASSERT_EQ(iter->key().ToString(), kv[i - 1].first);
-    iter->Prev();
-  }
-  ASSERT_OK(iter->status());
-
-  iter->SeekForPrev(kv[4 * kEntriesPerBlock - 1].first);
-  for (size_t i = 4 * kEntriesPerBlock; i > 2 * kEntriesPerBlock; --i) {
-    ASSERT_TRUE(iter->status().ok()) << iter->status().ToString();
-    ASSERT_TRUE(iter->Valid()) << i;
-    ASSERT_EQ(iter->key().ToString(), kv[i - 1].first);
-    iter->Prev();
-  }
-  ASSERT_OK(iter->status());
-}
-
 TEST_P(BlockBasedTableReaderMultiScanTest, MultiScanUnpinPreviousBlocks) {
   std::vector<std::pair<std::string, std::string>> kv =
       BlockBasedTableReaderBaseTest::GenerateKVMap(
@@ -2407,15 +2358,12 @@ INSTANTIATE_TEST_CASE_P(
 INSTANTIATE_TEST_CASE_P(
     BlockBasedTableReaderMultiScanAsyncIOTest,
     BlockBasedTableReaderMultiScanAsyncIOTest,
-    ::testing::ValuesIn(
-        BlockBasedTableReaderTestParamBuilder()
-            .WithIndexTypes(
-                {BlockBasedTableOptions::IndexType::kBinarySearch,
-                 BlockBasedTableOptions::IndexType::kHashSearch,
-                 BlockBasedTableOptions::IndexType::kTwoLevelIndexSearch})
-            .WithFillCacheFlags(Bool())
-            .WithUseAsyncIoFlags(IOUringFlags())
-            .build()));
+    ::testing::ValuesIn(BlockBasedTableReaderTestParamBuilder()
+                            .WithComparators({BytewiseComparator(),
+                                              ReverseBytewiseComparator()})
+                            .WithFillCacheFlags(Bool())
+                            .WithUseAsyncIoFlags(IOUringFlags())
+                            .build()));
 
 INSTANTIATE_TEST_CASE_P(
     BlockBasedTableReaderMultiScanTest, BlockBasedTableReaderMultiScanTest,

table/block_based/multi_scan_index_iterator.cc

@@ -5,8 +5,6 @@
 
 #include "table/block_based/multi_scan_index_iterator.h"
 
-#include <algorithm>
-
 #include "monitoring/statistics_impl.h"
 #include "rocksdb/options.h"
 
@@ -17,117 +15,37 @@ MultiScanIndexIterator::MultiScanIndexIterator(
     std::vector<std::string>&& data_block_separators,
     std::vector<std::tuple<size_t, size_t>>&& block_index_ranges_per_scan,
     const MultiScanArgs* scan_opts, std::shared_ptr<ReadSet> read_set,
-    size_t prefetch_start_idx, size_t prefetch_max_idx,
-    const InternalKeyComparator& icomp, Statistics* statistics)
+    size_t prefetch_max_idx, const InternalKeyComparator& icomp,
+    Statistics* statistics)
     : block_handles_(std::move(block_handles)),
       data_block_separators_(std::move(data_block_separators)),
       block_index_ranges_per_scan_(std::move(block_index_ranges_per_scan)),
       scan_opts_(scan_opts),
       read_set_(std::move(read_set)),
-      prefetch_start_idx_(prefetch_start_idx),
       prefetch_max_idx_(prefetch_max_idx),
-      first_unreleased_prefetch_idx_(prefetch_start_idx),
-      one_past_last_unreleased_prefetch_idx_(prefetch_max_idx),
-      released_prefetch_blocks_(prefetch_max_idx - prefetch_start_idx, false),
-      unreleased_prefetch_blocks_(prefetch_max_idx - prefetch_start_idx),
       icomp_(icomp),
       user_comparator_(icomp.user_comparator()),
       statistics_(statistics) {}
 
 MultiScanIndexIterator::~MultiScanIndexIterator() {
-  CountUnreleasedPrefetchedBlocksAsWasted();
   if (statistics_ && wasted_blocks_count_ > 0) {
     RecordTick(statistics_, MULTISCAN_PREFETCH_BLOCKS_WASTED,
                wasted_blocks_count_);
   }
   // Release any remaining pinned blocks
   if (read_set_) {
-    for (size_t i = first_unreleased_prefetch_idx_;
-         i < one_past_last_unreleased_prefetch_idx_ &&
-         unreleased_prefetch_blocks_ > 0;
-         ++i) {
-      ReleasePrefetchedBlock(i);
+    for (size_t i = cur_idx_; i < block_handles_.size(); ++i) {
+      read_set_->ReleaseBlock(i);
     }
   }
 }
 
-void MultiScanIndexIterator::CountUnreleasedPrefetchedBlocksAsWasted() {
-  if (unreleased_prefetch_blocks_ == 0) {
-    return;
-  }
-
-  for (size_t i = first_unreleased_prefetch_idx_;
-       i < one_past_last_unreleased_prefetch_idx_; ++i) {
-    if (!IsPrefetchedBlock(i) ||
-        released_prefetch_blocks_[i - prefetch_start_idx_]) {
-      continue;
-    }
-    if (valid_ && !scan_range_exhausted_ && i == cur_idx_) {
-      continue;
-    }
-    ++wasted_blocks_count_;
-  }
-}
-
-bool MultiScanIndexIterator::IsPrefetchedBlock(size_t block_idx) const {
-  return block_idx >= prefetch_start_idx_ && block_idx < prefetch_max_idx_;
-}
-
-size_t MultiScanIndexIterator::current_read_set_index() const {
-  if (!IsPrefetchedBlock(cur_idx_)) {
-    return prefetch_max_idx_ - prefetch_start_idx_;
-  }
-  return cur_idx_ - prefetch_start_idx_;
-}
-
-void MultiScanIndexIterator::ReleaseBlock(size_t block_idx) {
-  ReleasePrefetchedBlock(block_idx);
-}
-
-bool MultiScanIndexIterator::ReleasePrefetchedBlock(size_t block_idx) {
-  if (!IsPrefetchedBlock(block_idx) || unreleased_prefetch_blocks_ == 0) {
-    return false;
-  }
-
-  const size_t read_set_idx = block_idx - prefetch_start_idx_;
-  if (released_prefetch_blocks_[read_set_idx]) {
-    return false;
-  }
-
-  assert(read_set_);
-  read_set_->ReleaseBlock(read_set_idx);
-  released_prefetch_blocks_[read_set_idx] = true;
-  --unreleased_prefetch_blocks_;
-  ShrinkUnreleasedPrefetchBounds();
-  return true;
-}
-
-void MultiScanIndexIterator::ShrinkUnreleasedPrefetchBounds() {
-  while (first_unreleased_prefetch_idx_ <
-             one_past_last_unreleased_prefetch_idx_ &&
-         released_prefetch_blocks_[first_unreleased_prefetch_idx_ -
-                                   prefetch_start_idx_]) {
-    ++first_unreleased_prefetch_idx_;
-  }
-  while (first_unreleased_prefetch_idx_ <
-             one_past_last_unreleased_prefetch_idx_ &&
-         released_prefetch_blocks_[one_past_last_unreleased_prefetch_idx_ - 1 -
-                                   prefetch_start_idx_]) {
-    --one_past_last_unreleased_prefetch_idx_;
-  }
-}
-
 void MultiScanIndexIterator::ReleaseBlocks(size_t from_idx, size_t to_idx) {
-  const size_t release_from = std::max(from_idx, prefetch_start_idx_);
-  const size_t release_to = std::min(to_idx, prefetch_max_idx_);
-  if (release_from >= release_to || unreleased_prefetch_blocks_ == 0) {
-    return;
-  }
-
-  for (size_t i = release_from; i < release_to; ++i) {
-    if (ReleasePrefetchedBlock(i)) {
+  for (size_t i = from_idx; i < to_idx; ++i) {
+    if (i < prefetch_max_idx_) {
       wasted_blocks_count_++;
     }
+    read_set_->ReleaseBlock(i);
   }
 }
 
@@ -135,12 +53,6 @@ void MultiScanIndexIterator::Seek(const Slice& target) {
   if (!status_.ok()) {
     return;
   }
-  if (scan_opts_->reverse) {
-    status_ = Status::InvalidArgument("Seek called on reverse MultiScan");
-    RecordTick(statistics_, MULTISCAN_SEEK_ERRORS);
-    valid_ = false;
-    return;
-  }
 
   // Reset scan range exhaustion flag on Seek
   scan_range_exhausted_ = false;
@@ -274,10 +186,10 @@ void MultiScanIndexIterator::SeekToBlock(const Slice* user_seek_target) {
              *user_seek_target, /*a_has_ts=*/true,
              data_block_separators_[block_idx],
              /*b_has_ts=*/false) > 0) {
-    if (IsPrefetchedBlock(block_idx)) {
+    if (block_idx < prefetch_max_idx_) {
       wasted_blocks_count_++;
     }
-    ReleaseBlock(block_idx);
+    read_set_->ReleaseBlock(block_idx);
     block_idx++;
   }
 
@@ -306,22 +218,6 @@ void MultiScanIndexIterator::SeekToBlockIdx(size_t block_idx) {
   valid_ = true;
 }
 
-void MultiScanIndexIterator::SeekToBlockIdxReverse(size_t block_idx) {
-  if (cur_idx_ > block_idx && cur_idx_ < block_handles_.size()) {
-    ReleaseBlocks(block_idx + 1, cur_idx_ + 1);
-  }
-
-  cur_idx_ = block_idx;
-  if (!IsPrefetchedBlock(cur_idx_)) {
-    valid_ = false;
-    if (scan_opts_->max_prefetch_size > 0) {
-      status_ = Status::PrefetchLimitReached();
-    }
-    return;
-  }
-  valid_ = true;
-}
-
 void MultiScanIndexIterator::SetExhausted() {
   scan_range_exhausted_ = true;
   if (next_scan_idx_ < block_index_ranges_per_scan_.size()) {
@@ -347,7 +243,7 @@ void MultiScanIndexIterator::Next() {
   assert(valid_);
 
   // Release current block
-  ReleaseBlock(cur_idx_);
+  read_set_->ReleaseBlock(cur_idx_);
   ++cur_idx_;
 
   // Check if we've crossed a scan range boundary
@@ -362,7 +258,7 @@ void MultiScanIndexIterator::Next() {
   }
 
   // Check prefetch limit
-  if (!IsPrefetchedBlock(cur_idx_)) {
+  if (cur_idx_ >= prefetch_max_idx_) {
     valid_ = false;
     if (scan_opts_->max_prefetch_size > 0) {
       status_ = Status::PrefetchLimitReached();
@@ -396,109 +292,13 @@ void MultiScanIndexIterator::SeekToFirst() {
   valid_ = true;
 }
 
-void MultiScanIndexIterator::SeekForPrev(const Slice& target) {
-  if (!scan_opts_->reverse || !status_.ok()) {
-    valid_ = false;
-    return;
-  }
-
-  scan_range_exhausted_ = false;
-  if (scan_opts_->size() == 0) {
-    valid_ = false;
-    return;
-  }
-
-  const auto& scan_ranges = scan_opts_->GetScanRanges();
-  Slice user_seek_target = ExtractUserKey(target);
-  size_t scan_idx = scan_ranges.size();
-  for (size_t i = scan_ranges.size(); i > 0; --i) {
-    const size_t candidate = i - 1;
-    const auto& range = scan_ranges[candidate].range;
-    if (!range.start.has_value() || !range.limit.has_value()) {
-      continue;
-    }
-    const int cmp_start = user_comparator_.CompareWithoutTimestamp(
-        user_seek_target, /*a_has_ts=*/true, range.start.value(),
-        /*b_has_ts=*/false);
-    if (cmp_start >= 0) {
-      scan_idx = candidate;
-      break;
-    }
-  }
-
-  if (scan_idx == scan_ranges.size()) {
-    status_ = Status::InvalidArgument(
-        "SeekForPrev target is outside prepared ranges");
-    RecordTick(statistics_, MULTISCAN_SEEK_ERRORS);
-    valid_ = false;
-    return;
-  }
-
-  cur_scan_idx_ = scan_idx;
-  auto [cur_scan_start_idx, cur_scan_end_idx] =
-      block_index_ranges_per_scan_[cur_scan_idx_];
-  if (cur_scan_start_idx >= cur_scan_end_idx) {
-    valid_ = false;
-    return;
-  }
-
-  size_t block_idx = cur_scan_start_idx;
-  while (block_idx + 1 < cur_scan_end_idx &&
-         user_comparator_.CompareWithoutTimestamp(
-             user_seek_target, /*a_has_ts=*/true,
-             data_block_separators_[block_idx], /*b_has_ts=*/false) > 0) {
-    ++block_idx;
-  }
-
-  SeekToBlockIdxReverse(block_idx);
-}
-
-void MultiScanIndexIterator::SeekToLast() {
-  if (!scan_opts_->reverse) {
-    valid_ = false;
-    return;
-  }
-  for (size_t i = block_index_ranges_per_scan_.size(); i > 0; --i) {
-    const size_t scan_idx = i - 1;
-    auto [start, end] = block_index_ranges_per_scan_[scan_idx];
-    if (start < end) {
-      cur_scan_idx_ = scan_idx;
-      SeekToBlockIdxReverse(end - 1);
-      return;
-    }
-  }
+void MultiScanIndexIterator::SeekForPrev(const Slice& /*target*/) {
   valid_ = false;
 }
 
-void MultiScanIndexIterator::Prev() {
-  assert(valid_);
-  if (!scan_opts_->reverse) {
-    valid_ = false;
-    return;
-  }
+void MultiScanIndexIterator::SeekToLast() { valid_ = false; }
 
-  ReleaseBlock(cur_idx_);
-  auto [cur_scan_start_idx, cur_scan_end_idx] =
-      block_index_ranges_per_scan_[cur_scan_idx_];
-  (void)cur_scan_end_idx;
-  if (cur_idx_ <= cur_scan_start_idx) {
-    // Reverse scan range transitions are driven by DBIter's lower bound and
-    // the next SeekForPrev(), so this path does not need the forward
-    // out-of-bound signal from SetExhausted().
-    valid_ = false;
-    return;
-  }
-
-  --cur_idx_;
-  if (!IsPrefetchedBlock(cur_idx_)) {
-    valid_ = false;
-    if (scan_opts_->max_prefetch_size > 0) {
-      status_ = Status::PrefetchLimitReached();
-    }
-    return;
-  }
-  valid_ = true;
-}
+void MultiScanIndexIterator::Prev() { valid_ = false; }
 
 Slice MultiScanIndexIterator::key() const {
   assert(valid_);

table/block_based/multi_scan_index_iterator.h

@@ -5,7 +5,6 @@
 
 #pragma once
 
-#include <cassert>
 #include <string>
 #include <tuple>
 #include <vector>
@@ -27,8 +26,10 @@ class Statistics;
 // to use the same SeekImpl()/FindBlockForward() code path for both
 // regular iteration and MultiScan.
 //
-// The iterator supports forward Seek()/Next() or reverse SeekForPrev()/Prev()
-// depending on MultiScanArgs::reverse.
+// The iterator supports forward-only Seek() and Next(). Seek targets must
+// be non-decreasing (enforced via prev_seek_key_). When a scan range is
+// exhausted, Next() jumps to the start of the next scan range. When all
+// ranges are exhausted, the iterator becomes invalid.
 class MultiScanIndexIterator : public InternalIteratorBase<IndexValue> {
  public:
   // scan_opts and icomp must outlive this iterator. read_set is shared.
@@ -37,8 +38,8 @@ class MultiScanIndexIterator : public InternalIteratorBase<IndexValue> {
       std::vector<std::string>&& data_block_separators,
       std::vector<std::tuple<size_t, size_t>>&& block_index_ranges_per_scan,
       const MultiScanArgs* scan_opts, std::shared_ptr<ReadSet> read_set,
-      size_t prefetch_start_idx, size_t prefetch_max_idx,
-      const InternalKeyComparator& icomp, Statistics* statistics);
+      size_t prefetch_max_idx, const InternalKeyComparator& icomp,
+      Statistics* statistics);
 
   ~MultiScanIndexIterator() override;
 
@@ -57,6 +58,7 @@ class MultiScanIndexIterator : public InternalIteratorBase<IndexValue> {
   // Move to the first block of the first scan range.
   void SeekToFirst() override;
 
+  // Not supported -- sets valid_ = false.
   void SeekForPrev(const Slice& target) override;
   void SeekToLast() override;
   void Prev() override;
@@ -76,30 +78,18 @@ class MultiScanIndexIterator : public InternalIteratorBase<IndexValue> {
 
   Status status() const override { return status_; }
 
-  const MultiScanArgs* scan_opts() const { return scan_opts_; }
-
-  // Returns the current index into the ReadSet, or prefetch size if the
-  // current block was not prefetched.
-  size_t current_read_set_index() const;
+  // Returns the current index into the block_handles/read_set arrays.
+  size_t current_read_set_index() const { return cur_idx_; }
 
   // Returns the max_prefetch_size from scan options.
   uint64_t GetMaxPrefetchSize() const;
 
-  size_t GetPrefetchBlockCount() const {
-    return prefetch_max_idx_ - prefetch_start_idx_;
-  }
-
   // Returns true if the last Next() crossed a scan range boundary.
   // Only valid immediately after Next(); reset to false on the next Seek().
   bool IsScanRangeExhausted() const { return scan_range_exhausted_; }
 
   // Returns true if there are more scan ranges after the current one.
   bool HasMoreScanRanges() const {
-    // Reverse MultiScan advances ranges through SeekForPrev(), not this
-    // forward-only range transition path.
-    if (scan_opts_->reverse) {
-      return false;
-    }
     return next_scan_idx_ < block_index_ranges_per_scan_.size();
   }
 
@@ -107,11 +97,6 @@ class MultiScanIndexIterator : public InternalIteratorBase<IndexValue> {
   // Release blocks from from_idx (inclusive) to to_idx (exclusive),
   // counting wasted prefetched blocks.
   void ReleaseBlocks(size_t from_idx, size_t to_idx);
-  void ReleaseBlock(size_t block_idx);
-  bool ReleasePrefetchedBlock(size_t block_idx);
-  void ShrinkUnreleasedPrefetchBounds();
-  bool IsPrefetchedBlock(size_t block_idx) const;
-  void CountUnreleasedPrefetchedBlocksAsWasted();
 
   // Find the correct scan range and block for an unexpected seek target
   // (target doesn't match expected scan range start).
@@ -119,7 +104,6 @@ class MultiScanIndexIterator : public InternalIteratorBase<IndexValue> {
 
   // Position at block_idx after releasing any skipped blocks.
   void SeekToBlockIdx(size_t block_idx);
-  void SeekToBlockIdxReverse(size_t block_idx);
 
   // Mark the current scan range as exhausted. If more ranges remain,
   // positions at the next range's start (stays valid for out-of-bound
@@ -131,19 +115,13 @@ class MultiScanIndexIterator : public InternalIteratorBase<IndexValue> {
   std::vector<std::tuple<size_t, size_t>> block_index_ranges_per_scan_;
   const MultiScanArgs* scan_opts_;
   std::shared_ptr<ReadSet> read_set_;
-  size_t prefetch_start_idx_;
   size_t prefetch_max_idx_;
-  size_t first_unreleased_prefetch_idx_;
-  size_t one_past_last_unreleased_prefetch_idx_;
-  std::vector<bool> released_prefetch_blocks_;
-  size_t unreleased_prefetch_blocks_;
   const InternalKeyComparator& icomp_;
   UserComparatorWrapper user_comparator_;
   Statistics* statistics_;
 
   size_t cur_idx_ = 0;
   size_t next_scan_idx_ = 0;
-  size_t cur_scan_idx_ = 0;
   bool valid_ = false;
   Status status_;
   std::string prev_seek_key_;

table/table_test.cc

@@ -8256,55 +8256,12 @@ class UserDefinedIndexTestBase : public BlockBasedTableTestBase {
           return Status::OK();
         }
 
-        Status SeekToLastAndGetResult(IterateResult* result) override {
-          if (index_.empty()) {
-            result->bound_check_result = IterBoundCheck::kUnknown;
-            result->key = Slice();
-            return Status::OK();
-          }
-
-          iter_ = index_.end();
-          --iter_;
-          SetPreviousValidResult(result);
-          return Status::OK();
-        }
-
-        Status PrevAndGetResult(IterateResult* result) override {
-          if (iter_ == index_.end() || iter_ == index_.begin()) {
-            iter_ = index_.end();
-            result->bound_check_result = IterBoundCheck::kUnknown;
-            result->key = Slice();
-            return Status::OK();
-          }
-
-          --iter_;
-          SetPreviousValidResult(result);
-          return Status::OK();
-        }
-
         void AdvanceToNextIndexEntry() {
           while (iter_ != index_.end() && iter_->second.second == 0) {
             iter_++;
           }
         }
 
-        void SetPreviousValidResult(IterateResult* result) {
-          while (iter_ != index_.end() && iter_->second.second == 0) {
-            if (iter_ == index_.begin()) {
-              iter_ = index_.end();
-              break;
-            }
-            --iter_;
-          }
-          if (iter_ != index_.end()) {
-            result->bound_check_result = IterBoundCheck::kInbound;
-            result->key = Slice(iter_->first);
-          } else {
-            result->bound_check_result = IterBoundCheck::kUnknown;
-            result->key = Slice();
-          }
-        }
-
         bool IsInbound() {
           if (!scan_opts_) {
             return true;
@@ -9811,16 +9768,14 @@ struct UserDefinedIndexStressTestParam {
   const Comparator* comparator;
   bool enable_udi;
   bool enable_compaction_with_sst_partitioner;
-  bool reverse_scan;
 
   using UserDefinedIndexStressTestTuple =
-      std::tuple<const Comparator*, bool, bool, bool>;
+      std::tuple<const Comparator*, bool, bool>;
 
   UserDefinedIndexStressTestParam(const UserDefinedIndexStressTestTuple& tuple)
       : comparator(std::get<0>(tuple)),
         enable_udi(std::get<1>(tuple)),
-        enable_compaction_with_sst_partitioner(std::get<2>(tuple)),
-        reverse_scan(std::get<3>(tuple)) {}
+        enable_compaction_with_sst_partitioner(std::get<2>(tuple)) {}
 };
 
 std::ostream& operator<<(std::ostream& os,
@@ -9829,8 +9784,7 @@ std::ostream& operator<<(std::ostream& os,
             << (param.comparator ? param.comparator->Name() : "nullptr")
             << ", enable_udi=" << param.enable_udi
             << ", enable_compaction_with_sst_partitioner="
-            << param.enable_compaction_with_sst_partitioner
-            << ", reverse_scan=" << param.reverse_scan << "}";
+            << param.enable_compaction_with_sst_partitioner << "}";
 }
 
 struct DataRange {
@@ -9872,7 +9826,6 @@ class UserDefinedIndexStressTest
     enable_udi_ = param.enable_udi;
     enable_compaction_with_sst_partitioner_ =
         param.enable_compaction_with_sst_partitioner;
-    reverse_scan_ = param.reverse_scan;
     options_.comparator = comparator_;
     is_reverse_comparator_ = comparator_ == ReverseBytewiseComparator();
     options_.compaction_style = kCompactionStyleUniversal;
@@ -9896,7 +9849,6 @@ class UserDefinedIndexStressTest
   static constexpr auto kKeyRange = 100;
   bool enable_udi_{};
   bool enable_compaction_with_sst_partitioner_{};
-  bool reverse_scan_{};
   uint32_t rand_seed_{};
   std::shared_ptr<UserDefinedIndexFactory> user_defined_index_factory_;
   BlockBasedTableOptions table_options_;
@@ -10033,8 +9985,7 @@ class UserDefinedIndexStressTest
   }
 
   void RangeScan(std::unique_ptr<Iterator>& iter,
-                 const std::vector<DataRange>& ranges, Slice& lower_bound,
-                 Slice& upper_bound,
+                 const std::vector<DataRange>& ranges, Slice& upper_bound,
                  std::vector<std::pair<std::string, std::string>>& result,
                  bool use_multi_scan) {
     ASSERT_NE(iter, nullptr);
@@ -10042,7 +9993,6 @@ class UserDefinedIndexStressTest
     ASSERT_TRUE(!ranges.empty());
 
     MultiScanArgs scan_opts(options_.comparator);
-    scan_opts.reverse = reverse_scan_;
     std::unordered_map<std::string, std::string> property_bag;
     if (use_multi_scan) {
       for (auto const& range : ranges) {
@@ -10066,36 +10016,19 @@ class UserDefinedIndexStressTest
         continue;
       }
       size_t scan_key_count = 0;
-      lower_bound = range.start_key;
+      if (kVerbose) {
+        std::cout << "seek key " << range.start_key << std::endl;
+      }
       upper_bound = range.end_key;
-      if (reverse_scan_) {
+      for (iter->Seek(range.start_key);
+           iter->Valid() && scan_key_count < range.scan_key_count_limit;
+           iter->Next()) {
         if (kVerbose) {
-          std::cout << "seek for prev key " << range.end_key << std::endl;
-        }
-        for (iter->SeekForPrev(range.end_key);
-             iter->Valid() && scan_key_count < range.scan_key_count_limit;
-             iter->Prev()) {
-          if (kVerbose) {
-            std::cout << "key " << iter->key().ToString() << " value "
-                      << iter->value().ToString() << std::endl;
-          }
-          result.emplace_back(iter->key().ToString(), iter->value().ToString());
-          scan_key_count++;
-        }
-      } else {
-        if (kVerbose) {
-          std::cout << "seek key " << range.start_key << std::endl;
-        }
-        for (iter->Seek(range.start_key);
-             iter->Valid() && scan_key_count < range.scan_key_count_limit;
-             iter->Next()) {
-          if (kVerbose) {
-            std::cout << "key " << iter->key().ToString() << " value "
-                      << iter->value().ToString() << std::endl;
-          }
-          result.emplace_back(iter->key().ToString(), iter->value().ToString());
-          scan_key_count++;
+          std::cout << "key " << iter->key().ToString() << " value "
+                    << iter->value().ToString() << std::endl;
         }
+        result.emplace_back(iter->key().ToString(), iter->value().ToString());
+        scan_key_count++;
       }
       ASSERT_OK(iter->status());
     }
@@ -10133,53 +10066,36 @@ class UserDefinedIndexStressTest
 
       // Query regular CF
       std::vector<std::pair<std::string, std::string>> expected_result;
-      Slice lower_bound("");
       Slice upper_bound("");
       ReadOptions ro;
-      if (reverse_scan_) {
-        ro.iterate_lower_bound = &lower_bound;
-      }
       ro.iterate_upper_bound = &upper_bound;
 
       std::unique_ptr<Iterator> iter(db_->NewIterator(ro, regular_cfh_));
-      ASSERT_NO_FATAL_FAILURE(RangeScan(iter, ranges, lower_bound, upper_bound,
-                                        expected_result, false));
+      ASSERT_NO_FATAL_FAILURE(
+          RangeScan(iter, ranges, upper_bound, expected_result, false));
       ASSERT_OK(iter->status());
 
       // Query ingest CF
       iter.reset(db_->NewIterator(ro, ingest_cfh_));
       std::vector<std::pair<std::string, std::string>> ingest_cf_result;
-      ASSERT_NO_FATAL_FAILURE(RangeScan(iter, ranges, lower_bound, upper_bound,
-                                        ingest_cf_result, false));
+      ASSERT_NO_FATAL_FAILURE(
+          RangeScan(iter, ranges, upper_bound, ingest_cf_result, false));
 
       ASSERT_EQ(expected_result, ingest_cf_result);
       ASSERT_OK(iter->status());
 
-      // Query ingest CF with UDI if it is enabled.
-      if (enable_udi_ && reverse_scan_) {
+      // Query ingest CF with UDI if it is enabled
+      if (enable_udi_) {
         ro.table_index_factory = user_defined_index_factory_.get();
-        iter.reset(db_->NewIterator(ro, ingest_cfh_));
-        std::vector<std::pair<std::string, std::string>> ingest_cf_udi_result;
-        ASSERT_NO_FATAL_FAILURE(RangeScan(iter, ranges, lower_bound,
-                                          upper_bound, ingest_cf_udi_result,
-                                          false));
-        ASSERT_EQ(expected_result, ingest_cf_udi_result);
-        ASSERT_OK(iter->status());
       }
 
-      if (!reverse_scan_) {
-        if (enable_udi_) {
-          ro.table_index_factory = user_defined_index_factory_.get();
-        }
-        iter.reset(db_->NewIterator(ro, ingest_cfh_));
-        std::vector<std::pair<std::string, std::string>>
-            ingest_cf_multi_scan_result;
-        ASSERT_NO_FATAL_FAILURE(RangeScan(iter, ranges, lower_bound,
-                                          upper_bound,
-                                          ingest_cf_multi_scan_result, true));
-        ASSERT_EQ(expected_result, ingest_cf_multi_scan_result);
-        ASSERT_OK(iter->status());
-      }
+      iter.reset(db_->NewIterator(ro, ingest_cfh_));
+      std::vector<std::pair<std::string, std::string>>
+          ingest_cf_multi_scan_result;
+      ASSERT_NO_FATAL_FAILURE(RangeScan(iter, ranges, upper_bound,
+                                        ingest_cf_multi_scan_result, true));
+      ASSERT_EQ(expected_result, ingest_cf_multi_scan_result);
+      ASSERT_OK(iter->status());
     }
   }
 
@@ -10487,7 +10403,7 @@ INSTANTIATE_TEST_CASE_P(
     UserDefinedIndexStressTest, UserDefinedIndexStressTest,
     testing::Combine(testing::Values(BytewiseComparator(),
                                      ReverseBytewiseComparator()),
-                     testing::Bool(), testing::Bool(), testing::Bool()));
+                     testing::Bool(), testing::Bool()));
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

tools/check_format_compatible.sh

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

tools/db_bench_tool.cc

@@ -4097,8 +4097,6 @@ class Benchmark {
                 FLAGS_io_dispatcher_max_prefetch_memory_bytes);
         fprintf(stderr, "multiscan_max_prefetch_size = %" PRIu64 "\n",
                 FLAGS_multiscan_max_prefetch_size);
-        fprintf(stderr, "reverse_iterator = %s\n",
-                FLAGS_reverse_iterator ? "true" : "false");
         method = &Benchmark::MultiScan;
       } else if (name == "multiscanrandom") {
         int64_t max_range_keys = std::max<int64_t>(
@@ -4116,8 +4114,6 @@ class Benchmark {
                 FLAGS_multiscan_use_async_io ? "true" : "false");
         fprintf(stderr, "use_multiscan = %s\n",
                 FLAGS_use_multiscan ? "true" : "false");
-        fprintf(stderr, "reverse_iterator = %s\n",
-                FLAGS_reverse_iterator ? "true" : "false");
         method = &Benchmark::MultiScanRandom;
       } else if (name == "multireadwhilewriting") {
         fprintf(stderr, "entries_per_batch = %" PRIi64 "\n",
@@ -7370,7 +7366,6 @@ class Benchmark {
       opts.io_coalesce_threshold = FLAGS_multiscan_coalesce_threshold;
       opts.use_async_io = FLAGS_multiscan_use_async_io;
       opts.max_prefetch_size = FLAGS_multiscan_max_prefetch_size;
-      opts.reverse = FLAGS_reverse_iterator;
       if (io_dispatcher) {
         opts.io_dispatcher = io_dispatcher;
       }
@@ -7441,7 +7436,6 @@ class Benchmark {
     }
 
     int64_t multiscans_done = 0;
-    uint64_t rows_scanned = 0;
     auto duration = thread->shared->MakeDuration(FLAGS_duration, reads_);
     int64_t num_keys = 1;
     while (!duration.Done(num_keys)) {
@@ -7506,7 +7500,6 @@ class Benchmark {
         opts.io_coalesce_threshold = FLAGS_multiscan_coalesce_threshold;
         opts.use_async_io = FLAGS_multiscan_use_async_io;
         opts.max_prefetch_size = FLAGS_multiscan_max_prefetch_size;
-        opts.reverse = FLAGS_reverse_iterator;
         if (io_dispatcher) {
           opts.io_dispatcher = io_dispatcher;
         }
@@ -7537,7 +7530,7 @@ class Benchmark {
           fprintf(stderr, "MultiScanException: %s\n", e.what());
         }
       } else {
-        // Normal iterator path: Seek + Next/SeekForPrev + Prev for each range
+        // Normal iterator path: Seek + Next for each range
         std::unique_ptr<const char[]> skey_guard;
         Slice skey = AllocateKey(&skey_guard);
         std::unique_ptr<const char[]> ekey_guard;
@@ -7545,45 +7538,21 @@ class Benchmark {
 
         std::unique_ptr<Iterator> iter(
             db->NewIterator(read_options_, db->DefaultColumnFamily()));
-        auto scan_range = [&](const RangeSpec& r) {
+        for (auto& r : non_overlapping) {
           GenerateKeyFromInt(r.start, FLAGS_num, &skey);
           GenerateKeyFromInt(r.start + r.size, FLAGS_num, &ekey);
-          if (FLAGS_reverse_iterator) {
-            iter->SeekForPrev(ekey);
-            if (iter->Valid() && iter->key().compare(ekey) >= 0) {
-              iter->Prev();
-            }
-            for (; iter->Valid() && iter->key().compare(skey) >= 0;
-                 iter->Prev()) {
-              keys++;
-            }
-          } else {
-            for (iter->Seek(skey);
-                 iter->Valid() && iter->key().compare(ekey) < 0; iter->Next()) {
-              keys++;
-            }
+          for (iter->Seek(skey); iter->Valid() && iter->key().compare(ekey) < 0;
+               iter->Next()) {
+            keys++;
           }
           if (!iter->status().ok()) {
             fprintf(stderr, "Iterator error: %s\n",
                     iter->status().ToString().c_str());
-          }
-        };
-        if (FLAGS_reverse_iterator) {
-          for (auto it = non_overlapping.rbegin();
-               it != non_overlapping.rend() && iter->status().ok(); ++it) {
-            scan_range(*it);
-          }
-        } else {
-          for (auto& r : non_overlapping) {
-            scan_range(r);
-            if (!iter->status().ok()) {
-              break;
-            }
+            break;
           }
         }
       }
       num_keys = std::max<int64_t>(1, keys);
-      rows_scanned += static_cast<uint64_t>(keys);
 
       if (thread->shared->read_rate_limiter.get() != nullptr) {
         thread->shared->read_rate_limiter->Request(
@@ -7594,13 +7563,10 @@ class Benchmark {
       multiscans_done += 1;
     }
 
-    const uint64_t rows_per_multiscan =
-        multiscans_done == 0 ? 0 : rows_scanned / multiscans_done;
-    char msg[200];
+    char msg[100];
     snprintf(msg, sizeof(msg),
-             "(multiscans:%" PRIu64 " rows_scanned:%" PRIu64
-             " rows_per_multiscan:%" PRIu64 " max_range_keys:%" PRId64 ")",
-             multiscans_done, rows_scanned, rows_per_multiscan, max_range_keys);
+             "(multiscans:%" PRIu64 " max_range_keys:%" PRId64 ")",
+             multiscans_done, max_range_keys);
     thread->stats.AddMessage(msg);
   }
 

unreleased_history/bug_fixes/big_endian_fault_injection_build.md

@@ -1 +0,0 @@
-Fixed RocksDB builds on big-endian architectures such as s390x by disabling the optional persistent fault-injection log, whose binary format is little-endian-only.

util/cast_util.h

@@ -38,25 +38,6 @@ inline std::shared_ptr<DestClass> static_cast_with_check(
   return ret;
 }
 
-// A safe, compile-time-checked upcast of a reference to a base class. Unlike a
-// raw static_cast, this cannot silently perform a downcast or an unrelated
-// conversion, and it reads as an explicit "treat this as its base" (e.g. to
-// copy or swap just the base subobject). `Base` must be a public base of
-// `Derived`; a free function cannot access a private/protected base.
-template <class Base, class Derived>
-inline Base& up_cast(Derived& ref) {
-  static_assert(std::is_base_of_v<Base, Derived>,
-                "up_cast requires Base to be a base class of Derived");
-  return ref;
-}
-
-template <class Base, class Derived>
-inline const Base& up_cast(const Derived& ref) {
-  static_assert(std::is_base_of_v<Base, Derived>,
-                "up_cast requires Base to be a base class of Derived");
-  return ref;
-}
-
 // A wrapper around static_cast for lossless conversion between integral
 // types, including enum types, and pointers to such types. For example, this
 // can be used for converting between signed/unsigned or enum type and

utilities/checkpoint/checkpoint_test.cc

@@ -13,7 +13,6 @@
 #ifndef OS_WIN
 #include <unistd.h>
 #endif
-#include <atomic>
 #include <cstdlib>
 #include <iostream>
 #include <thread>
@@ -906,70 +905,6 @@ TEST_F(CheckpointTest, CheckpointWithLockWAL) {
   ASSERT_EQ("foo_value", get_result);
 }
 
-TEST_F(CheckpointTest, CheckpointWithLockWALNoWALSameThreadAborted) {
-  WriteOptions write_options;
-  write_options.disableWAL = true;
-  ASSERT_OK(db_->Put(write_options, "foo", "foo_value"));
-  ASSERT_OK(db_->LockWAL());
-
-  Checkpoint* checkpoint = nullptr;
-  ASSERT_OK(Checkpoint::Create(db_.get(), &checkpoint));
-  std::unique_ptr<Checkpoint> checkpoint_holder(checkpoint);
-
-  Status s = checkpoint->CreateCheckpoint(snapshot_name_);
-  ASSERT_TRUE(s.IsAborted()) << s.ToString();
-  ASSERT_NE(s.ToString().find("Likely deadlock"), std::string::npos)
-      << s.ToString();
-
-  ASSERT_OK(db_->UnlockWAL());
-}
-
-TEST_F(CheckpointTest, CheckpointWithLockWALNoWALCrossThreadFlushes) {
-  Options options = CurrentOptions();
-  WriteOptions write_options;
-  write_options.disableWAL = true;
-  ASSERT_OK(db_->Put(write_options, "foo", "foo_value"));
-  ASSERT_OK(db_->LockWAL());
-
-  Checkpoint* checkpoint = nullptr;
-  ASSERT_OK(Checkpoint::Create(db_.get(), &checkpoint));
-  std::unique_ptr<Checkpoint> checkpoint_holder(checkpoint);
-
-  SyncPoint::GetInstance()->LoadDependency(
-      {{"DBImpl::WaitForPendingWrites:BeforeBlock",
-        "CheckpointTest::CheckpointWithLockWALNoWALCrossThreadFlushes:"
-        "BeforeUnlock"}});
-  SyncPoint::GetInstance()->EnableProcessing();
-
-  std::atomic<bool> checkpoint_done{false};
-  Status checkpoint_status;
-  port::Thread checkpoint_thread([&]() {
-    checkpoint_status = checkpoint->CreateCheckpoint(snapshot_name_);
-    checkpoint_done.store(true);
-  });
-
-  TEST_SYNC_POINT(
-      "CheckpointTest::CheckpointWithLockWALNoWALCrossThreadFlushes:"
-      "BeforeUnlock");
-  const bool finished_early = checkpoint_done.load();
-  Status unlock_status = db_->UnlockWAL();
-  checkpoint_thread.join();
-  SyncPoint::GetInstance()->DisableProcessing();
-  SyncPoint::GetInstance()->LoadDependency({});
-
-  ASSERT_OK(unlock_status);
-  ASSERT_FALSE(finished_early) << checkpoint_status.ToString();
-  ASSERT_OK(checkpoint_status);
-  Close();
-
-  std::unique_ptr<DB> snapshot_db;
-  ASSERT_OK(DB::Open(options, snapshot_name_, &snapshot_db));
-  ReadOptions read_opts;
-  std::string get_result;
-  ASSERT_OK(snapshot_db->Get(read_opts, "foo", &get_result));
-  ASSERT_EQ("foo_value", get_result);
-}
-
 TEST_F(CheckpointTest, CheckpointWithLockWALRejectsBlobDirectWrite) {
   Options options = CurrentOptions();
   options.enable_blob_files = true;

utilities/fault_injection_fs.cc

@@ -26,7 +26,6 @@
 #include "env/io_posix.h"
 #include "monitoring/thread_status_util.h"
 #include "port/lang.h"
-#include "port/port.h"
 #include "port/stack_trace.h"
 #include "rocksdb/env.h"
 #include "rocksdb/io_status.h"
@@ -80,9 +79,7 @@ uint8_t DetailPayloadSize(const InjectedErrorLog::TaggedEntryDetail& detail) {
 
 int OpenInjectedErrorLogFile(const std::string& path) {
 #ifndef OS_WIN
-  // Entry is written in native layout, while the decoder expects little-endian
-  // fields. Keep the optional log disabled on unsupported architectures.
-  if (!port::kLittleEndian || path.empty()) {
+  if (path.empty()) {
     return -1;
   }
   int flags = O_WRONLY | O_CREAT | O_TRUNC;

utilities/fault_injection_fs.h

@@ -44,8 +44,6 @@ namespace ROCKSDB_NAMESPACE {
 // A binary log that records injected errors directly to a file.
 // Thread-safe for concurrent writes. The log file can be flushed on exit or
 // from a signal handler so records survive clean exits and crash paths.
-// Disabled on big-endian platforms because the decoder expects little-endian
-// fields in the native Entry layout.
 class InjectedErrorLog {
  public:
   static constexpr size_t kMaxOpNameLen = 32;
@@ -88,13 +86,12 @@ class InjectedErrorLog {
     char op_name[kMaxOpNameLen];
     char file_name[kMaxFileNameLen];
     char status_message[kMaxStatusMessageLen];
-    // Explicit tail padding so the size does not depend on the alignment of
-    // uint64_t (4 bytes on i386, 8 bytes on most other targets).
-    uint8_t padding[4];
   };
 
   static_assert(sizeof(Entry) == 256,
                 "Injected error log entry size must stay stable");
+  static_assert(port::kLittleEndian,
+                "Injected error log requires little-endian platforms");
 
   explicit InjectedErrorLog(const std::string& path);
   ~InjectedErrorLog();

utilities/fault_injection_fs_test.cc

@@ -10,7 +10,6 @@
 #include <thread>
 #include <vector>
 
-#include "port/port.h"
 #include "test_util/testharness.h"
 
 namespace ROCKSDB_NAMESPACE {
@@ -80,11 +79,6 @@ class CloseCountingWritableFile : public FSWritableFileOwnerWrapper {
 // the expected fixed-width fields. The test writes one entry, flushes the
 // file, and decodes the raw bytes back into an entry struct.
 TEST_F(InjectedErrorLogTest, BasicRecordAndFlush) {
-  if (!port::kLittleEndian) {
-    ROCKSDB_GTEST_BYPASS(
-        "Persistent injected error log requires little-endian");
-    return;
-  }
   std::string path = test::PerThreadDBPath("injected_error_log_basic.bin");
   {
     InjectedErrorLog log(path);
@@ -120,11 +114,6 @@ TEST_F(InjectedErrorLogTest, BasicRecordAndFlush) {
 // to the old in-memory ring size. The test writes more than 1,000 entries and
 // checks that both the first and last ones are still present in the file.
 TEST_F(InjectedErrorLogTest, DirectLogKeepsAllEntries) {
-  if (!port::kLittleEndian) {
-    ROCKSDB_GTEST_BYPASS(
-        "Persistent injected error log requires little-endian");
-    return;
-  }
   std::string path =
       test::PerThreadDBPath("injected_error_log_all_entries.bin");
   {
@@ -155,11 +144,6 @@ TEST_F(InjectedErrorLogTest, DirectLogKeepsAllEntries) {
 // shared file. The test has several threads emit records concurrently and then
 // checks the raw file size matches the total number of writes.
 TEST_F(InjectedErrorLogTest, ConcurrentRecord) {
-  if (!port::kLittleEndian) {
-    ROCKSDB_GTEST_BYPASS(
-        "Persistent injected error log requires little-endian");
-    return;
-  }
   std::string path = test::PerThreadDBPath("injected_error_log_concurrent.bin");
   constexpr int kNumThreads = 4;
   constexpr int kRecordsPerThread = 200;
@@ -196,11 +180,6 @@ TEST_F(InjectedErrorLogTest, ConcurrentRecord) {
 // in the fixed-width record. The test logs a path longer than the file-name
 // field and checks the stored sample matches the expected tail bytes.
 TEST_F(InjectedErrorLogTest, LongFileNameKeepsSuffix) {
-  if (!port::kLittleEndian) {
-    ROCKSDB_GTEST_BYPASS(
-        "Persistent injected error log requires little-endian");
-    return;
-  }
   std::string path =
       test::PerThreadDBPath("injected_error_log_suffix_truncation.bin");
   const std::string long_file_name =

utilities/transactions/lock/range/range_locking_test.cc

@@ -475,57 +475,6 @@ TEST_F(RangeLockingTest, LockWaiteeAccess) {
   delete txn1;
 }
 
-// A killed_callback that fires makes a conflicting range-lock wait return
-// Status::Aborted() (DB_LOCK_INTERRUPTED), not a timeout.
-TEST_F(RangeLockingTest, KilledCallbackAbortsWait) {
-  TransactionOptions txn_options;
-  auto cf = db->DefaultColumnFamily();
-  // Large timeout so a timeout cannot masquerade as the interrupt.
-  txn_options.lock_timeout = 100000;
-
-  std::atomic<bool> killed{false};
-  range_lock_mgr->SetIsKilledCallback([&]() { return killed.load(); });
-
-  Transaction* txn0 = db->BeginTransaction(WriteOptions(), txn_options);
-  Transaction* txn1 = db->BeginTransaction(WriteOptions(), txn_options);
-
-  ASSERT_OK(txn0->GetRangeLock(cf, Endpoint("a"), Endpoint("c")));
-
-  killed.store(true);
-  auto s = txn1->GetRangeLock(cf, Endpoint("b"), Endpoint("z"));
-  ASSERT_TRUE(s.IsAborted());
-  ASSERT_FALSE(s.IsTimedOut());
-
-  txn0->Rollback();
-  txn1->Rollback();
-  delete txn0;
-  delete txn1;
-}
-
-// With a killed_callback installed that never fires, a conflicting wait still
-// times out: the interrupt path must not disturb timeout behavior.
-TEST_F(RangeLockingTest, KilledCallbackFalseStillTimesOut) {
-  TransactionOptions txn_options;
-  auto cf = db->DefaultColumnFamily();
-  txn_options.lock_timeout = 50;
-
-  range_lock_mgr->SetIsKilledCallback([]() { return false; });
-
-  Transaction* txn0 = db->BeginTransaction(WriteOptions(), txn_options);
-  Transaction* txn1 = db->BeginTransaction(WriteOptions(), txn_options);
-
-  ASSERT_OK(txn0->GetRangeLock(cf, Endpoint("a"), Endpoint("c")));
-
-  auto s = txn1->GetRangeLock(cf, Endpoint("b"), Endpoint("z"));
-  ASSERT_TRUE(s.IsTimedOut());
-  ASSERT_FALSE(s.IsAborted());
-
-  txn0->Rollback();
-  txn1->Rollback();
-  delete txn0;
-  delete txn1;
-}
-
 void PointLockManagerTestExternalSetup(PointLockManagerTest* self) {
   self->env_ = Env::Default();
   self->db_dir_ = test::PerThreadDBPath("point_lock_manager_test");

utilities/transactions/lock/range/range_tree/lib/db.h

@@ -58,8 +58,6 @@ typedef struct __toku_engine_status_row {
 #define DB_BUFFER_SMALL -30999
 #define DB_LOCK_DEADLOCK -30995
 #define DB_LOCK_NOTGRANTED -30994
-// wait aborted by killed_callback (vs. timeout)
-#define DB_LOCK_INTERRUPTED -30993
 #define DB_NOTFOUND -30989
 #define DB_KEYEXIST -30996
 #define DB_DBT_MALLOC 8

utilities/transactions/lock/range/range_tree/lib/locktree/lock_request.cc

@@ -234,13 +234,13 @@ int lock_request::start(void) {
 // sleep on the lock request until it becomes resolved or the wait time has
 // elapsed.
 int lock_request::wait(uint64_t wait_time_ms) {
-  return wait(wait_time_ms, 0, nullptr, nullptr);
+  return wait(wait_time_ms, 0, nullptr);
 }
 
 int lock_request::wait(uint64_t wait_time_ms, uint64_t killed_time_ms,
-                       int (*killed_callback)(void*), void* killed_callback_arg,
-                       void (*lock_wait_callback)(void*, lock_wait_infos*),
-                       void* callback_arg) {
+                       int (*killed_callback)(void),
+                       void (*lock_wait_callback)(void *, lock_wait_infos *),
+                       void *callback_arg) {
   uint64_t t_now = toku_current_time_microsec();
   uint64_t t_start = t_now;
   uint64_t t_end = t_start + wait_time_ms * 1000;
@@ -258,9 +258,9 @@ int lock_request::wait(uint64_t wait_time_ms, uint64_t killed_time_ms,
 
   while (m_state == state::PENDING) {
     // check if this thread is killed
-    if (killed_callback && killed_callback(killed_callback_arg)) {
+    if (killed_callback && killed_callback()) {
       remove_from_lock_requests();
-      complete(DB_LOCK_INTERRUPTED);
+      complete(DB_LOCK_NOTGRANTED);
       continue;
     }
 
@@ -440,6 +440,26 @@ void lock_request::report_waits(lock_wait_infos *wait_conflicts,
 
 void *lock_request::get_extra(void) const { return m_extra; }
 
+void lock_request::kill_waiter(void) {
+  remove_from_lock_requests();
+  complete(DB_LOCK_NOTGRANTED);
+  toku_external_cond_broadcast(&m_wait_cond);
+}
+
+void lock_request::kill_waiter(locktree *lt, void *extra) {
+  lt_lock_request_info *info = lt->get_lock_request_info();
+  toku_external_mutex_lock(&info->mutex);
+  for (uint32_t i = 0; i < info->pending_lock_requests.size(); i++) {
+    lock_request *request;
+    int r = info->pending_lock_requests.fetch(i, &request);
+    if (r == 0 && request->get_extra() == extra) {
+      request->kill_waiter();
+      break;
+    }
+  }
+  toku_external_mutex_unlock(&info->mutex);
+}
+
 // find another lock request by txnid. must hold the mutex.
 lock_request *lock_request::find_lock_request(const TXNID &txnid) {
   lock_request *request;

utilities/transactions/lock/range/range_tree/lib/locktree/lock_request.h

@@ -108,14 +108,13 @@ class lock_request {
   int start(void);
 
   // effect: Sleeps until either the request is granted or the wait time
-  // expires. returns: The return code of locktree::acquire_[write,read]_lock(),
-  //          DB_LOCK_NOTGRANTED if the wait time expired, or
-  //          DB_LOCK_INTERRUPTED if killed_callback returned true.
+  // expires. returns: The return code of locktree::acquire_[write,read]_lock()
+  //          or simply DB_LOCK_NOTGRANTED if the wait time expired.
   int wait(uint64_t wait_time_ms);
   int wait(uint64_t wait_time_ms, uint64_t killed_time_ms,
-           int (*killed_callback)(void*), void* killed_callback_arg,
-           void (*lock_wait_callback)(void*, lock_wait_infos*) = nullptr,
-           void* callback_arg = nullptr);
+           int (*killed_callback)(void),
+           void (*lock_wait_callback)(void *, lock_wait_infos *) = nullptr,
+           void *callback_arg = nullptr);
 
   // return: left end-point of the lock range
   const DBT *get_left_key(void) const;
@@ -152,6 +151,9 @@ class lock_request {
 
   void *get_extra(void) const;
 
+  void kill_waiter(void);
+  static void kill_waiter(locktree *lt, void *extra);
+
  private:
   enum state {
     UNINITIALIZED,

utilities/transactions/lock/range/range_tree/lib/locktree/locktree.h

@@ -201,6 +201,8 @@ class locktree_manager {
   // Add time t to the escalator's wait time statistics
   void add_escalator_wait_time(uint64_t t);
 
+  void kill_waiter(void *extra);
+
  private:
   static const uint64_t DEFAULT_MAX_LOCK_MEMORY = 64L * 1024 * 1024;
 

utilities/transactions/lock/range/range_tree/lib/locktree/manager.cc

@@ -507,5 +507,19 @@ void locktree_manager::get_status(LTM_STATUS statp) {
   *statp = ltm_status;
 }
 
+void locktree_manager::kill_waiter(void *extra) {
+  mutex_lock();
+  int r = 0;
+  uint32_t num_locktrees = m_locktree_map.size();
+  for (uint32_t i = 0; i < num_locktrees; i++) {
+    locktree *lt;
+    r = m_locktree_map.fetch(i, &lt);
+    invariant_zero(r);
+    if (r) continue;  // Get rid of "may be used uninitialized" warning
+    lock_request::kill_waiter(lt, extra);
+  }
+  mutex_unlock();
+}
+
 } /* namespace toku */
 #endif  // OS_WIN

utilities/transactions/lock/range/range_tree/range_tree_lock_manager.cc

@@ -87,16 +87,9 @@ Status RangeTreeLockManager::TryLock(PessimisticTransaction* txn,
               exclusive ? toku::lock_request::WRITE : toku::lock_request::READ,
               false /* not a big txn */, &wait_key);
 
-  // Poll for kill only when an embedder installed a kill-check callback;
-  // otherwise IsKilledThunk can never fire and the 100ms wakeups are pure
-  // overhead.
-  constexpr uint64_t kKillPollMs = 100;
+  // This is for "periodically wake up and check if the wait is killed" feature
+  // which we are not using.
   uint64_t killed_time_msec = 0;
-  int (*killed_thunk)(void*) = nullptr;
-  if (killed_callback_) {
-    killed_time_msec = kKillPollMs;
-    killed_thunk = &IsKilledThunk;
-  }
   uint64_t wait_time_msec = txn->GetLockTimeout();
 
   if (wait_time_msec == static_cast<uint64_t>(-1)) {
@@ -123,8 +116,9 @@ Status RangeTreeLockManager::TryLock(PessimisticTransaction* txn,
 
   request.start();
 
-  const int r = request.wait(wait_time_msec, killed_time_msec, killed_thunk,
-                             this, wait_callback_for_locktree, nullptr);
+  const int r = request.wait(wait_time_msec, killed_time_msec,
+                             nullptr,  // killed_callback
+                             wait_callback_for_locktree, nullptr);
 
   // Inform the txn that we are no longer waiting:
   txn->ClearWaitingTxn();
@@ -135,8 +129,6 @@ Status RangeTreeLockManager::TryLock(PessimisticTransaction* txn,
       break;  // fall through
     case DB_LOCK_NOTGRANTED:
       return Status::TimedOut(Status::SubCode::kLockTimeout);
-    case DB_LOCK_INTERRUPTED:
-      return Status::Aborted();
     case TOKUDB_OUT_OF_LOCKS:
       return Status::LockLimit();
     case DB_LOCK_DEADLOCK: {
@@ -153,10 +145,6 @@ Status RangeTreeLockManager::TryLock(PessimisticTransaction* txn,
   return Status::OK();
 }
 
-int RangeTreeLockManager::IsKilledThunk(void* extra) {
-  return static_cast<RangeTreeLockManager*>(extra)->killed_callback_() ? 1 : 0;
-}
-
 // Wait callback that locktree library will call to inform us about
 // the lock waits that are in progress.
 void wait_callback_for_locktree(void*, toku::lock_wait_infos* infos) {

utilities/transactions/lock/range/range_tree/range_tree_lock_manager.h

@@ -94,18 +94,12 @@ class RangeTreeLockManager : public RangeLockManagerBase,
     barrier_func_ = func;
   }
 
-  void SetIsKilledCallback(IsKilledCallback func) override {
-    killed_callback_ = func;
-  }
-
  private:
   toku::locktree_manager ltm_;
 
   EscalationBarrierFunc barrier_func_ =
       [](const Endpoint&, const Endpoint&) -> bool { return false; };
 
-  IsKilledCallback killed_callback_{};
-
   std::shared_ptr<TransactionDBMutexFactory> mutex_factory_;
 
   // Map from cf_id to locktree*. Can only be accessed while holding the
@@ -132,10 +126,6 @@ class RangeTreeLockManager : public RangeLockManagerBase,
                           const toku::range_buffer& buffer, void* extra);
 
   static bool OnEscalationBarrierCheck(const DBT* a, const DBT* b, void* extra);
-
-  // Bridges the C-style toku killed-callback to killed_callback_; extra is the
-  // RangeTreeLockManager*.
-  static int IsKilledThunk(void* extra);
 };
 
 void serialize_endpoint(const Endpoint& endp, std::string* buf);

utilities/transactions/write_unprepared_transaction_test.cc

@@ -1017,58 +1017,6 @@ TEST_P(WriteUnpreparedTransactionTest, RangeTombstoneOwnDeletionsAndRollback) {
   }
 }
 
-TEST_P(WriteUnpreparedTransactionTest,
-       RollbackPreparedAfterCommitWriteFailure) {
-  // Verify that a WriteUnprepared transaction can be rolled back after a
-  // commit write failure. Previously, CommitInternal() would clear
-  // unprep_seqs_ even on failure, making subsequent Rollback() hit
-  // assertion `unprep_seqs.size() > 0`.
-  options.max_bgerror_resume_count = 0;
-  ASSERT_OK(ReOpen());
-
-  // Use fault injection to cause commit write to fail.
-  fault_fs->SetFileTypesExcludedFromFaultInjection({FileType::kInfoLogFile});
-
-  WriteOptions woptions;
-  TransactionOptions txn_options;
-  // Set a low flush threshold to trigger unprepared batches being written to
-  // DB, populating unprep_seqs_.
-  txn_options.write_batch_flush_threshold = 1;
-  std::unique_ptr<Transaction> txn(db->BeginTransaction(woptions, txn_options));
-  ASSERT_NE(nullptr, txn);
-  ASSERT_OK(txn->SetName("wup_commit_fail_rollback"));
-  ASSERT_OK(txn->Put(Slice("key1"), Slice("value1")));
-  // The flush threshold of 1 should cause the batch to be flushed as
-  // unprepared, populating unprep_seqs_.
-  ASSERT_OK(txn->Put(Slice("key2"), Slice("value2")));
-  ASSERT_OK(txn->Prepare());
-
-  // Inject a write error so that Commit() fails.
-  fault_fs->SetThreadLocalErrorContext(FaultInjectionIOType::kWrite,
-                                       /*seed=*/0, /*one_in=*/1,
-                                       /*retryable=*/true,
-                                       /*has_data_loss=*/false);
-  fault_fs->EnableThreadLocalErrorInjection(FaultInjectionIOType::kWrite);
-  const Status commit_s = txn->Commit();
-  fault_fs->DisableThreadLocalErrorInjection(FaultInjectionIOType::kWrite);
-
-  ASSERT_NOK(commit_s);
-  ASSERT_TRUE(FaultInjectionTestFS::IsInjectedError(commit_s))
-      << commit_s.ToString();
-  fault_fs->GetAndResetInjectedThreadLocalErrorCount(
-      FaultInjectionIOType::kWrite);
-
-  // Resume the DB to clear the error state.
-  ASSERT_OK(db->Resume());
-
-  // Rollback should succeed without hitting the assertion failure.
-  ASSERT_OK(txn->Rollback());
-  txn.reset();
-
-  // Verify the DB is usable after rollback.
-  ASSERT_OK(ReOpenNoDelete());
-}
-
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

utilities/transactions/write_unprepared_txn.cc

@@ -608,18 +608,13 @@ Status WriteUnpreparedTxn::CommitInternal() {
       for (const auto& seq : unprep_seqs_) {
         wpt_db_->RemovePrepared(seq.first, seq.second);
       }
-      unprep_seqs_.clear();
-      flushed_save_points_.reset(nullptr);
-      unflushed_save_points_.reset(nullptr);
-    } else if (UNLIKELY(!do_one_write)) {
-      // The commit write failed after add_prepared_callback may have added a
-      // prepared entry for commit_batch_seq. Since commit_batch_seq is never
-      // stored in unprep_seqs_ on this path, a follow-up Rollback() cannot
-      // remove it, so clean it up here. unprep_seqs_ is otherwise preserved so
-      // the transaction can still be rolled back after the caller recovers
-      // from the write error.
+    }
+    if (UNLIKELY(!do_one_write)) {
       wpt_db_->RemovePrepared(commit_batch_seq, commit_batch_cnt);
     }
+    unprep_seqs_.clear();
+    flushed_save_points_.reset(nullptr);
+    unflushed_save_points_.reset(nullptr);
     return s;
   }  // else do the 2nd write to publish seq
 

utilities/trie_index/trie_index_factory.cc

@@ -308,13 +308,12 @@ Status TrieIndexIterator::SeekToLastAndGetResult(IterateResult* result) {
 }
 
 Status TrieIndexIterator::PrevAndGetResult(IterateResult* result) {
-  // Upper-bound checks are based on forward movement. While moving backward,
-  // DBIter's lower-bound handling decides when to stop.
-  // Overflow fast path: key doesn't change within the same run.
+  // Overflow fast path: key doesn't change within the same run, so
+  // current_key_scratch_ can be passed directly to CheckBounds (no copy).
   if (overflow_run_index_ > 0) {
     overflow_run_index_--;
     result->key = Slice(current_key_scratch_);
-    result->bound_check_result = IterBoundCheck::kInbound;
+    result->bound_check_result = CheckBounds(Slice(current_key_scratch_));
     return Status::OK();
   }
 
@@ -333,7 +332,7 @@ Status TrieIndexIterator::PrevAndGetResult(IterateResult* result) {
 
   CopyTrieKeyToResult(result);
   SetupOverflowForCurrentLeaf(/*position_at_last=*/true);
-  result->bound_check_result = IterBoundCheck::kInbound;
+  result->bound_check_result = CheckBounds(Slice(prev_key_scratch_));
   return Status::OK();
 }
 

utilities/trie_index/trie_index_factory.h

@@ -298,10 +298,12 @@ class TrieIndexFactory : public UserDefinedIndexFactory {
   // builds) to surface programming errors immediately.
   UserDefinedIndexBuilder* NewBuilder() const override {
     abort();
+    return nullptr;
   }
   std::unique_ptr<UserDefinedIndexReader> NewReader(
       Slice& /*index_block*/) const override {
     abort();
+    return nullptr;
   }
 
   // New API with comparator.

utilities/trie_index/trie_index_test.cc

@@ -2687,33 +2687,6 @@ TEST_F(TrieIndexFactoryTest, UpperBoundDoesNotDropValidBlocks) {
   ASSERT_EQ(result.bound_check_result, IterBoundCheck::kOutOfBound);
 }
 
-TEST_F(TrieIndexFactoryTest, PrevAfterPrepareDoesNotUseForwardUpperBound) {
-  auto ctx = BuildTrieAndGetIterator({
-      {"az", "b", 0, 500, 0, 0},
-      {"bz", "d", 1000, 500, 0, 0},
-      {"dz", "f", 2000, 500, 0, 0},
-  });
-
-  ScanOptions scans[] = {
-      ScanOptions(Slice("a"), Slice("c")),
-  };
-  ctx.iter->Prepare(scans, 1);
-
-  IterateResult result;
-  ASSERT_OK(ctx.iter->SeekAndGetResult(Slice("a"), &result,
-                                       SeekCtx(kMaxSequenceNumber)));
-  ASSERT_EQ(result.bound_check_result, IterBoundCheck::kInbound);
-  ASSERT_EQ(ctx.iter->value().offset, 0u);
-
-  ASSERT_OK(ctx.iter->NextAndGetResult(&result));
-  ASSERT_EQ(result.bound_check_result, IterBoundCheck::kInbound);
-  ASSERT_EQ(ctx.iter->value().offset, 1000u);
-
-  ASSERT_OK(ctx.iter->PrevAndGetResult(&result));
-  ASSERT_EQ(result.bound_check_result, IterBoundCheck::kInbound);
-  ASSERT_EQ(ctx.iter->value().offset, 0u);
-}
-
 TEST_F(TrieIndexFactoryTest, MultiScanBoundsAdvanceCorrectly) {
   // Validates that current_scan_idx_ advances correctly when
   // the seek target is past the current scan's limit. Otherwise all