Minimum macOS version needed to build v7.2.2 and up is 10.13 (#9976)

Summary:
Some C++ code changes between version 7.1.2 and 7.2.2 now seem to require at least macOS 10.13 (2017) to build successfully, previously we needed 10.12 (2016). I haven't been able to identify the exact commit.

**NOTE**: This needs to be merged to both `main` and `7.2.fb` branches.

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

Reviewed By: jay-zhuang

Differential Revision: D36303226

Pulled By: ajkr

fbshipit-source-id: 589ce3ecf821db3402b0876e76d37b407896c945

.circleci/config.yml

@@ -2,7 +2,6 @@ version: 2.1
 
 orbs:
   win: circleci/windows@2.4.0
-  slack: circleci/slack@3.4.2
 
 aliases:
   - &notify-on-main-failure
@@ -57,7 +56,6 @@ commands:
 
   post-steps:
     steps:
-      - slack/status: *notify-on-main-failure
       - store_test_results: # store test result if there's any
           path: /tmp/test-results
       - store_artifacts: # store LOG for debugging if there's any
@@ -113,12 +111,6 @@ commands:
             cmake .. -GNinja -DCMAKE_BUILD_TYPE=Release -DBENCHMARK_ENABLE_GTEST_TESTS=0
             ninja && sudo ninja install
 
-  install-valgrind:
-    steps:
-      - run:
-          name: Install valgrind
-          command: sudo apt-get update -y && sudo apt-get install -y valgrind
-
   upgrade-cmake:
     steps:
       - run:
@@ -230,16 +222,14 @@ jobs:
       - run: make V=1 J=32 -j32 check
       - post-steps
 
-  build-linux-encrypted_env-no_compression:
+  build-linux-encrypted-env:
     machine:
       image: ubuntu-2004:202111-02
     resource_class: 2xlarge
     steps:
       - pre-steps
       - install-gflags
-      - run: ENCRYPTED_ENV=1 ROCKSDB_DISABLE_SNAPPY=1 ROCKSDB_DISABLE_ZLIB=1 ROCKSDB_DISABLE_BZIP=1 ROCKSDB_DISABLE_LZ4=1 ROCKSDB_DISABLE_ZSTD=1 make V=1 J=32 -j32 check
-      - run: |
-          ./sst_dump --help | egrep -q 'Supported compression types: kNoCompression$' # Verify no compiled in compression
+      - run: ENCRYPTED_ENV=1 make V=1 J=32 -j32 check
       - post-steps
 
   build-linux-shared_lib-alt_namespace-status_checked:
@@ -320,7 +310,7 @@ jobs:
       - pre-steps
       - install-gflags
       - install-clang-10
-      - run: COMPILE_WITH_ASAN=1 CC=clang-10 CXX=clang++-10 ROCKSDB_DISABLE_ALIGNED_NEW=1 USE_CLANG=1 make V=1 -j32 check # aligned new doesn't work for reason we haven't figured out
+      - run: ASAN_OPTIONS=detect_stack_use_after_return=1 COMPILE_WITH_ASAN=1 CC=clang-10 CXX=clang++-10 ROCKSDB_DISABLE_ALIGNED_NEW=1 USE_CLANG=1 make V=1 -j32 check # aligned new doesn't work for reason we haven't figured out
       - post-steps
 
   build-linux-clang10-mini-tsan:
@@ -364,17 +354,6 @@ jobs:
       - run: COMPILE_WITH_UBSAN=1 OPT="-fsanitize-blacklist=.circleci/ubsan_suppression_list.txt" CC=clang-10 CXX=clang++-10 ROCKSDB_DISABLE_ALIGNED_NEW=1 USE_CLANG=1 make V=1 -j32 ubsan_check # aligned new doesn't work for reason we haven't figured out
       - post-steps
 
-  build-linux-valgrind:
-    machine:
-      image: ubuntu-2004:202111-02
-    resource_class: 2xlarge
-    steps:
-      - pre-steps
-      - install-gflags
-      - install-valgrind
-      - run: PORTABLE=1 make V=1 -j32 valgrind_test
-      - post-steps
-
   build-linux-clang10-clang-analyze:
     machine:
       image: ubuntu-2004:202111-02
@@ -474,19 +453,6 @@ jobs:
       - run: CC=clang-13 CXX=clang++-13 USE_CLANG=1 make -j16 all microbench
       - post-steps
 
-  # Ensure ASAN+UBSAN with folly, and full testsuite with clang 13
-  build-linux-clang-13-asan-ubsan-with-folly:
-    machine:
-      image: ubuntu-2004:202111-02
-    resource_class: 2xlarge
-    steps:
-      - pre-steps
-      - install-clang-13
-      - install-gflags
-      - run: make checkout_folly
-      - run: CC=clang-13 CXX=clang++-13 USE_CLANG=1 USE_FOLLY=1 COMPILE_WITH_UBSAN=1 COMPILE_WITH_ASAN=1 make -j32 check
-      - post-steps
-
   # This job is only to make sure the microbench tests are able to run, the benchmark result is not meaningful as the CI host is changing.
   build-linux-run-microbench:
     machine:
@@ -835,9 +801,9 @@ workflows:
     jobs:
       - build-linux-cmake-with-folly
       - build-linux-cmake-with-benchmark
-  build-linux-encrypted_env-no_compression:
+  build-linux-encrypted-env:
     jobs:
-      - build-linux-encrypted_env-no_compression
+      - build-linux-encrypted-env
   build-linux-shared_lib-alt_namespace-status_checked:
     jobs:
       - build-linux-shared_lib-alt_namespace-status_checked
@@ -939,5 +905,3 @@ workflows:
       - build-linux-arm-test-full
       - build-linux-run-microbench
       - build-linux-non-shm
-      - build-linux-clang-13-asan-ubsan-with-folly
-      - build-linux-valgrind

CMakeLists.txt

@@ -796,7 +796,6 @@ set(SOURCES
         trace_replay/trace_record_result.cc
         trace_replay/trace_record.cc
         trace_replay/trace_replay.cc
-        util/cleanable.cc
         util/coding.cc
         util/compaction_job_stats_impl.cc
         util/comparator.cc

HISTORY.md

@@ -1,26 +1,12 @@
 # Rocksdb Change Log
-## Unreleased
+## 7.2.2 (04/28/2022)
 ### Bug Fixes
-* Fixed a bug where manual flush would block forever even though flush options had wait=false.
-* Fixed a bug where RocksDB could corrupt DBs with `avoid_flush_during_recovery == true` by removing valid WALs, leading to `Status::Corruption` with message like "SST file is ahead of WALs" when attempting to reopen.
 * Fixed a bug in async_io path where incorrect length of data is read by FilePrefetchBuffer if data is consumed from two populated buffers and request for more data is sent.
-* Fixed a CompactionFilter bug. Compaction filter used to use `Delete` to remove keys, even if the keys should be removed with `SingleDelete`. Mixing `Delete` and `SingleDelete` may cause undefined behavior.
-
-### New Features
-* DB::GetLiveFilesStorageInfo is ready for production use.
-* Add new stats PREFETCHED_BYTES_DISCARDED which records number of prefetched bytes discarded by RocksDB FilePrefetchBuffer on destruction and POLL_WAIT_MICROS records wait time for FS::Poll API completion.
-
-### Public API changes
-* Add rollback_deletion_type_callback to TransactionDBOptions so that write-prepared transactions know whether to issue a Delete or SingleDelete to cancel a previous key written during prior prepare phase. The PR aims to prevent mixing SingleDeletes and Deletes for the same key that can lead to undefined behaviors for write-prepared transactions.
-* EXPERIMENTAL: Add new API AbortIO in file_system to abort the read requests submitted asynchronously.
-* CompactionFilter::Decision has a new value: kRemoveWithSingleDelete. If CompactionFilter returns this decision, then CompactionIterator will use `SingleDelete` to mark a key as removed.
 
+## 7.2.1 (04/26/2022)
 ### Bug Fixes
+* Fixed a bug where RocksDB could corrupt DBs with `avoid_flush_during_recovery == true` by removing valid WALs, leading to `Status::Corruption` with message like "SST file is ahead of WALs" when attempting to reopen.
 * RocksDB calls FileSystem::Poll API during FilePrefetchBuffer destruction which impacts performance as it waits for read requets completion which is not needed anymore. Calling FileSystem::AbortIO to abort those requests instead fixes that performance issue.
-* Fixed unnecessary block cache contention when queries within a MultiGet batch and across parallel batches access the same data block, which previously could cause severely degraded performance in this unusual case. (In more typical MultiGet cases, this fix is expected to yield a small or negligible performance improvement.)
-
-### Behavior changes
-* Enforce the existing contract of SingleDelete so that SingleDelete cannot be mixed with Delete because it leads to undefined behavior. Fix a number of unit tests that violate the contract but happen to pass.
 
 ## 7.2.0 (04/15/2022)
 ### Bug Fixes

Makefile

@@ -340,8 +340,6 @@ endif
 # ASAN doesn't work well with jemalloc. If we're compiling with ASAN, we should use regular malloc.
 ifdef COMPILE_WITH_ASAN
 	DISABLE_JEMALLOC=1
-	ASAN_OPTIONS?=detect_stack_use_after_return=1
-	export ASAN_OPTIONS
 	EXEC_LDFLAGS += -fsanitize=address
 	PLATFORM_CCFLAGS += -fsanitize=address
 	PLATFORM_CXXFLAGS += -fsanitize=address
@@ -598,6 +596,9 @@ am__v_CCH_1 =
 check-headers: $(HEADER_OK_FILES)
 
 # options_settable_test doesn't pass with UBSAN as we use hack in the test
+ifdef COMPILE_WITH_UBSAN
+TESTS := $(shell echo $(TESTS) | sed 's/\boptions_settable_test\b//g')
+endif
 ifdef ASSERT_STATUS_CHECKED
 # TODO: finish fixing all tests to pass this check
 TESTS_FAILING_ASC = \
@@ -1045,31 +1046,31 @@ ldb_tests: ldb
 include crash_test.mk
 
 asan_check: clean
-	COMPILE_WITH_ASAN=1 $(MAKE) check -j32
+	ASAN_OPTIONS=detect_stack_use_after_return=1 COMPILE_WITH_ASAN=1 $(MAKE) check -j32
 	$(MAKE) clean
 
 asan_crash_test: clean
-	COMPILE_WITH_ASAN=1 $(MAKE) crash_test
+	ASAN_OPTIONS=detect_stack_use_after_return=1 COMPILE_WITH_ASAN=1 $(MAKE) crash_test
 	$(MAKE) clean
 
 whitebox_asan_crash_test: clean
-	COMPILE_WITH_ASAN=1 $(MAKE) whitebox_crash_test
+	ASAN_OPTIONS=detect_stack_use_after_return=1 COMPILE_WITH_ASAN=1 $(MAKE) whitebox_crash_test
 	$(MAKE) clean
 
 blackbox_asan_crash_test: clean
-	COMPILE_WITH_ASAN=1 $(MAKE) blackbox_crash_test
+	ASAN_OPTIONS=detect_stack_use_after_return=1 COMPILE_WITH_ASAN=1 $(MAKE) blackbox_crash_test
 	$(MAKE) clean
 
 asan_crash_test_with_atomic_flush: clean
-	COMPILE_WITH_ASAN=1 $(MAKE) crash_test_with_atomic_flush
+	ASAN_OPTIONS=detect_stack_use_after_return=1 COMPILE_WITH_ASAN=1 $(MAKE) crash_test_with_atomic_flush
 	$(MAKE) clean
 
 asan_crash_test_with_txn: clean
-	COMPILE_WITH_ASAN=1 $(MAKE) crash_test_with_txn
+	ASAN_OPTIONS=detect_stack_use_after_return=1 COMPILE_WITH_ASAN=1 $(MAKE) crash_test_with_txn
 	$(MAKE) clean
 
 asan_crash_test_with_best_efforts_recovery: clean
-	COMPILE_WITH_ASAN=1 $(MAKE) crash_test_with_best_efforts_recovery
+	ASAN_OPTIONS=detect_stack_use_after_return=1 COMPILE_WITH_ASAN=1 $(MAKE) crash_test_with_best_efforts_recovery
 	$(MAKE) clean
 
 ubsan_check: clean

TARGETS

@@ -224,7 +224,6 @@ cpp_library_wrapper(name="rocksdb_lib", srcs=[
         "trace_replay/trace_record_result.cc",
         "trace_replay/trace_replay.cc",
         "util/build_version.cc",
-        "util/cleanable.cc",
         "util/coding.cc",
         "util/compaction_job_stats_impl.cc",
         "util/comparator.cc",
@@ -544,7 +543,6 @@ cpp_library_wrapper(name="rocksdb_whole_archive_lib", srcs=[
         "trace_replay/trace_record_result.cc",
         "trace_replay/trace_replay.cc",
         "util/build_version.cc",
-        "util/cleanable.cc",
         "util/coding.cc",
         "util/compaction_job_stats_impl.cc",
         "util/comparator.cc",

build_tools/build_detect_platform

@@ -662,13 +662,13 @@ else
   fi
 
   if [[ "${PLATFORM}" == "OS_MACOSX" ]]; then
-    # For portability compile for macOS 10.12 (2016) or newer
-    COMMON_FLAGS="$COMMON_FLAGS -mmacosx-version-min=10.12"
-    PLATFORM_LDFLAGS="$PLATFORM_LDFLAGS -mmacosx-version-min=10.12"
+    # For portability compile for macOS 10.13 (2017) or newer
+    COMMON_FLAGS="$COMMON_FLAGS -mmacosx-version-min=10.13"
+    PLATFORM_LDFLAGS="$PLATFORM_LDFLAGS -mmacosx-version-min=10.13"
     # -mmacosx-version-min must come first here.
-    PLATFORM_SHARED_LDFLAGS="-mmacosx-version-min=10.12 $PLATFORM_SHARED_LDFLAGS"
-    PLATFORM_CMAKE_FLAGS="-DCMAKE_OSX_DEPLOYMENT_TARGET=10.12"
-    JAVA_STATIC_DEPS_COMMON_FLAGS="-mmacosx-version-min=10.12"
+    PLATFORM_SHARED_LDFLAGS="-mmacosx-version-min=10.13 $PLATFORM_SHARED_LDFLAGS"
+    PLATFORM_CMAKE_FLAGS="-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13"
+    JAVA_STATIC_DEPS_COMMON_FLAGS="-mmacosx-version-min=10.13"
     JAVA_STATIC_DEPS_LDFLAGS="$JAVA_STATIC_DEPS_COMMON_FLAGS"
     JAVA_STATIC_DEPS_CCFLAGS="$JAVA_STATIC_DEPS_COMMON_FLAGS"
     JAVA_STATIC_DEPS_CXXFLAGS="$JAVA_STATIC_DEPS_COMMON_FLAGS"

build_tools/fbcode_config.sh

@@ -21,48 +21,38 @@ LIBGCC_LIBS=" -L $LIBGCC_BASE/lib"
 GLIBC_INCLUDE="$GLIBC_BASE/include"
 GLIBC_LIBS=" -L $GLIBC_BASE/lib"
 
-if ! test $ROCKSDB_DISABLE_SNAPPY; then
-  # snappy
-  SNAPPY_INCLUDE=" -I $SNAPPY_BASE/include/"
-  if test -z $PIC_BUILD; then
-    SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy.a"
-  else
-    SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy_pic.a"
-  fi
-  CFLAGS+=" -DSNAPPY"
+# snappy
+SNAPPY_INCLUDE=" -I $SNAPPY_BASE/include/"
+if test -z $PIC_BUILD; then
+  SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy.a"
+else
+  SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy_pic.a"
 fi
+CFLAGS+=" -DSNAPPY"
 
 if test -z $PIC_BUILD; then
-  if ! test $ROCKSDB_DISABLE_ZLIB; then
-    # location of zlib headers and libraries
-    ZLIB_INCLUDE=" -I $ZLIB_BASE/include/"
-    ZLIB_LIBS=" $ZLIB_BASE/lib/libz.a"
-    CFLAGS+=" -DZLIB"
-  fi
+  # location of zlib headers and libraries
+  ZLIB_INCLUDE=" -I $ZLIB_BASE/include/"
+  ZLIB_LIBS=" $ZLIB_BASE/lib/libz.a"
+  CFLAGS+=" -DZLIB"
 
-  if ! test $ROCKSDB_DISABLE_BZIP; then
-    # location of bzip headers and libraries
-    BZIP_INCLUDE=" -I $BZIP2_BASE/include/"
-    BZIP_LIBS=" $BZIP2_BASE/lib/libbz2.a"
-    CFLAGS+=" -DBZIP2"
-  fi
+  # location of bzip headers and libraries
+  BZIP_INCLUDE=" -I $BZIP2_BASE/include/"
+  BZIP_LIBS=" $BZIP2_BASE/lib/libbz2.a"
+  CFLAGS+=" -DBZIP2"
 
-  if ! test $ROCKSDB_DISABLE_LZ4; then
-    LZ4_INCLUDE=" -I $LZ4_BASE/include/"
-    LZ4_LIBS=" $LZ4_BASE/lib/liblz4.a"
-    CFLAGS+=" -DLZ4"
-  fi
+  LZ4_INCLUDE=" -I $LZ4_BASE/include/"
+  LZ4_LIBS=" $LZ4_BASE/lib/liblz4.a"
+  CFLAGS+=" -DLZ4"
 fi
 
-if ! test $ROCKSDB_DISABLE_ZSTD; then
-  ZSTD_INCLUDE=" -I $ZSTD_BASE/include/"
-  if test -z $PIC_BUILD; then
-    ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd.a"
-  else
-    ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd_pic.a"
-  fi
-  CFLAGS+=" -DZSTD -DZSTD_STATIC_LINKING_ONLY"
+ZSTD_INCLUDE=" -I $ZSTD_BASE/include/"
+if test -z $PIC_BUILD; then
+  ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd.a"
+else
+  ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd_pic.a"
 fi
+CFLAGS+=" -DZSTD -DZSTD_STATIC_LINKING_ONLY"
 
 # location of gflags headers and libraries
 GFLAGS_INCLUDE=" -I $GFLAGS_BASE/include/"

build_tools/fbcode_config_platform009.sh

@@ -27,38 +27,28 @@ else
   MAYBE_PIC=_pic
 fi
 
-if ! test $ROCKSDB_DISABLE_SNAPPY; then
-  # snappy
-  SNAPPY_INCLUDE=" -I $SNAPPY_BASE/include/"
-  SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy${MAYBE_PIC}.a"
-  CFLAGS+=" -DSNAPPY"
-fi
+# snappy
+SNAPPY_INCLUDE=" -I $SNAPPY_BASE/include/"
+SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy${MAYBE_PIC}.a"
+CFLAGS+=" -DSNAPPY"
 
-if ! test $ROCKSDB_DISABLE_ZLIB; then
-  # location of zlib headers and libraries
-  ZLIB_INCLUDE=" -I $ZLIB_BASE/include/"
-  ZLIB_LIBS=" $ZLIB_BASE/lib/libz${MAYBE_PIC}.a"
-  CFLAGS+=" -DZLIB"
-fi
+# location of zlib headers and libraries
+ZLIB_INCLUDE=" -I $ZLIB_BASE/include/"
+ZLIB_LIBS=" $ZLIB_BASE/lib/libz${MAYBE_PIC}.a"
+CFLAGS+=" -DZLIB"
 
-if ! test $ROCKSDB_DISABLE_BZIP; then
-  # location of bzip headers and libraries
-  BZIP_INCLUDE=" -I $BZIP2_BASE/include/"
-  BZIP_LIBS=" $BZIP2_BASE/lib/libbz2${MAYBE_PIC}.a"
-  CFLAGS+=" -DBZIP2"
-fi
+# location of bzip headers and libraries
+BZIP_INCLUDE=" -I $BZIP2_BASE/include/"
+BZIP_LIBS=" $BZIP2_BASE/lib/libbz2${MAYBE_PIC}.a"
+CFLAGS+=" -DBZIP2"
 
-if ! test $ROCKSDB_DISABLE_LZ4; then
-  LZ4_INCLUDE=" -I $LZ4_BASE/include/"
-  LZ4_LIBS=" $LZ4_BASE/lib/liblz4${MAYBE_PIC}.a"
-  CFLAGS+=" -DLZ4"
-fi
+LZ4_INCLUDE=" -I $LZ4_BASE/include/"
+LZ4_LIBS=" $LZ4_BASE/lib/liblz4${MAYBE_PIC}.a"
+CFLAGS+=" -DLZ4"
 
-if ! test $ROCKSDB_DISABLE_ZSTD; then
-  ZSTD_INCLUDE=" -I $ZSTD_BASE/include/"
-  ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd${MAYBE_PIC}.a"
-  CFLAGS+=" -DZSTD"
-fi
+ZSTD_INCLUDE=" -I $ZSTD_BASE/include/"
+ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd${MAYBE_PIC}.a"
+CFLAGS+=" -DZSTD"
 
 # location of gflags headers and libraries
 GFLAGS_INCLUDE=" -I $GFLAGS_BASE/include/"

build_tools/fbcode_config_platform010.sh

@@ -29,38 +29,28 @@ else
   MAYBE_PIC=_pic
 fi
 
-if ! test $ROCKSDB_DISABLE_SNAPPY; then
-  # snappy
-  SNAPPY_INCLUDE=" -I $SNAPPY_BASE/include/"
-  SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy${MAYBE_PIC}.a"
-  CFLAGS+=" -DSNAPPY"
-fi
+# snappy
+SNAPPY_INCLUDE=" -I $SNAPPY_BASE/include/"
+SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy${MAYBE_PIC}.a"
+CFLAGS+=" -DSNAPPY"
 
-if ! test $ROCKSDB_DISABLE_ZLIB; then
-  # location of zlib headers and libraries
-  ZLIB_INCLUDE=" -I $ZLIB_BASE/include/"
-  ZLIB_LIBS=" $ZLIB_BASE/lib/libz${MAYBE_PIC}.a"
-  CFLAGS+=" -DZLIB"
-fi
+# location of zlib headers and libraries
+ZLIB_INCLUDE=" -I $ZLIB_BASE/include/"
+ZLIB_LIBS=" $ZLIB_BASE/lib/libz${MAYBE_PIC}.a"
+CFLAGS+=" -DZLIB"
 
-if ! test $ROCKSDB_DISABLE_BZIP; then
-  # location of bzip headers and libraries
-  BZIP_INCLUDE=" -I $BZIP2_BASE/include/"
-  BZIP_LIBS=" $BZIP2_BASE/lib/libbz2${MAYBE_PIC}.a"
-  CFLAGS+=" -DBZIP2"
-fi
+# location of bzip headers and libraries
+BZIP_INCLUDE=" -I $BZIP2_BASE/include/"
+BZIP_LIBS=" $BZIP2_BASE/lib/libbz2${MAYBE_PIC}.a"
+CFLAGS+=" -DBZIP2"
 
-if ! test $ROCKSDB_DISABLE_LZ4; then
-  LZ4_INCLUDE=" -I $LZ4_BASE/include/"
-  LZ4_LIBS=" $LZ4_BASE/lib/liblz4${MAYBE_PIC}.a"
-  CFLAGS+=" -DLZ4"
-fi
+LZ4_INCLUDE=" -I $LZ4_BASE/include/"
+LZ4_LIBS=" $LZ4_BASE/lib/liblz4${MAYBE_PIC}.a"
+CFLAGS+=" -DLZ4"
 
-if ! test $ROCKSDB_DISABLE_ZSTD; then
-  ZSTD_INCLUDE=" -I $ZSTD_BASE/include/"
-  ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd${MAYBE_PIC}.a"
-  CFLAGS+=" -DZSTD"
-fi
+ZSTD_INCLUDE=" -I $ZSTD_BASE/include/"
+ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd${MAYBE_PIC}.a"
+CFLAGS+=" -DZSTD"
 
 # location of gflags headers and libraries
 GFLAGS_INCLUDE=" -I $GFLAGS_BASE/include/"
@@ -118,7 +108,7 @@ if [ -z "$USE_CLANG" ]; then
   CXX="$GCC_BASE/bin/g++"
   AR="$GCC_BASE/bin/gcc-ar"
 
-
+  
   CFLAGS+=" -B$BINUTILS"
   CFLAGS+=" -isystem $LIBGCC_INCLUDE"
   CFLAGS+=" -isystem $GLIBC_INCLUDE"

db/c.cc

@@ -4193,14 +4193,6 @@ rocksdb_cache_t* rocksdb_cache_create_lru(size_t capacity) {
   return c;
 }
 
-rocksdb_cache_t* rocksdb_cache_create_lru_with_strict_capacity_limit(
-    size_t capacity) {
-  rocksdb_cache_t* c = new rocksdb_cache_t;
-  c->rep = NewLRUCache(capacity);
-  c->rep->SetStrictCapacityLimit(true);
-  return c;
-}
-
 rocksdb_cache_t* rocksdb_cache_create_lru_opts(
     rocksdb_lru_cache_options_t* opt) {
   rocksdb_cache_t* c = new rocksdb_cache_t;

db/compaction/compaction_iterator.cc

@@ -307,14 +307,6 @@ bool CompactionIterator::InvokeFilterIfNeeded(bool* need_skip,
     // no value associated with delete
     value_.clear();
     iter_stats_.num_record_drop_user++;
-  } else if (filter == CompactionFilter::Decision::kRemoveWithSingleDelete) {
-    // convert the current key to a single delete; key_ is pointing into
-    // current_key_ at this point, so updating current_key_ updates key()
-    ikey_.type = kTypeSingleDeletion;
-    current_key_.UpdateInternalKey(ikey_.sequence, kTypeSingleDeletion);
-    // no value associated with single delete
-    value_.clear();
-    iter_stats_.num_record_drop_user++;
   } else if (filter == CompactionFilter::Decision::kChangeValue) {
     if (ikey_.type == kTypeBlobIndex) {
       // value transfer from blob file to inlined data
@@ -633,34 +625,24 @@ void CompactionIterator::NextFromInput() {
 
           TEST_SYNC_POINT_CALLBACK(
               "CompactionIterator::NextFromInput:SingleDelete:2", nullptr);
-          if (next_ikey.type == kTypeSingleDeletion) {
+          if (next_ikey.type == kTypeSingleDeletion ||
+              next_ikey.type == kTypeDeletion) {
             // We encountered two SingleDeletes for same key in a row. This
             // could be due to unexpected user input. If write-(un)prepared
             // transaction is used, this could also be due to releasing an old
             // snapshot between a Put and its matching SingleDelete.
+            // Furthermore, if write-(un)prepared transaction is rolled back
+            // after prepare, we will write a Delete to cancel a prior Put. If
+            // old snapshot is released between a later Put and its matching
+            // SingleDelete, we will end up with a Delete followed by
+            // SingleDelete.
             // Skip the first SingleDelete and let the next iteration decide
-            // how to handle the second SingleDelete.
+            // how to handle the second SingleDelete or Delete.
 
             // First SingleDelete has been skipped since we already called
             // input_.Next().
             ++iter_stats_.num_record_drop_obsolete;
             ++iter_stats_.num_single_del_mismatch;
-          } else if (next_ikey.type == kTypeDeletion) {
-            std::ostringstream oss;
-            oss << "Found SD and type: " << static_cast<int>(next_ikey.type)
-                << " on the same key, violating the contract "
-                   "of SingleDelete. Check your application to make sure the "
-                   "application does not mix SingleDelete and Delete for "
-                   "the same key. If you are using "
-                   "write-prepared/write-unprepared transactions, and use "
-                   "SingleDelete to delete certain keys, then make sure "
-                   "TransactionDBOptions::rollback_deletion_type_callback is "
-                   "configured properly. Mixing SD and DEL can lead to "
-                   "undefined behaviors";
-            ROCKS_LOG_ERROR(info_log_, "%s", oss.str().c_str());
-            valid_ = false;
-            status_ = Status::Corruption(oss.str());
-            return;
           } else if (!is_timestamp_eligible_for_gc) {
             // We cannot drop the SingleDelete as timestamp is enabled, and
             // timestamp of this key is greater than or equal to

db/compaction/compaction_job_test.cc

@@ -890,10 +890,10 @@ TEST_F(CompactionJobTest, MultiSingleDelete) {
   //      -> Snapshot Put
   // K: SDel SDel Put SDel Put Put Snapshot SDel Put SDel SDel Put SDel
   //      -> Snapshot Put Snapshot SDel
-  // L: SDel Put SDel Put SDel Snapshot SDel Put SDel SDel Put SDel
-  //      -> Snapshot SDel Put SDel
-  // M: (Put) SDel Put SDel Put SDel Snapshot Put SDel SDel Put SDel SDel
-  //      -> SDel Snapshot Put SDel
+  // L: SDel Put Del Put SDel Snapshot Del Put Del SDel Put SDel
+  //      -> Snapshot SDel
+  // M: (Put) SDel Put Del Put SDel Snapshot Put Del SDel Put SDel Del
+  //      -> SDel Snapshot Del
   NewDB();
 
   auto file1 = mock::MakeMockFile({
@@ -924,14 +924,14 @@ TEST_F(CompactionJobTest, MultiSingleDelete) {
       {KeyStr("L", 16U, kTypeSingleDeletion), ""},
       {KeyStr("L", 15U, kTypeValue), "val"},
       {KeyStr("L", 14U, kTypeSingleDeletion), ""},
-      {KeyStr("L", 13U, kTypeSingleDeletion), ""},
+      {KeyStr("L", 13U, kTypeDeletion), ""},
       {KeyStr("L", 12U, kTypeValue), "val"},
-      {KeyStr("L", 11U, kTypeSingleDeletion), ""},
-      {KeyStr("M", 16U, kTypeSingleDeletion), ""},
+      {KeyStr("L", 11U, kTypeDeletion), ""},
+      {KeyStr("M", 16U, kTypeDeletion), ""},
       {KeyStr("M", 15U, kTypeSingleDeletion), ""},
       {KeyStr("M", 14U, kTypeValue), "val"},
       {KeyStr("M", 13U, kTypeSingleDeletion), ""},
-      {KeyStr("M", 12U, kTypeSingleDeletion), ""},
+      {KeyStr("M", 12U, kTypeDeletion), ""},
       {KeyStr("M", 11U, kTypeValue), "val"},
   });
   AddMockFile(file1);
@@ -972,12 +972,12 @@ TEST_F(CompactionJobTest, MultiSingleDelete) {
       {KeyStr("K", 1U, kTypeSingleDeletion), ""},
       {KeyStr("L", 5U, kTypeSingleDeletion), ""},
       {KeyStr("L", 4U, kTypeValue), "val"},
-      {KeyStr("L", 3U, kTypeSingleDeletion), ""},
+      {KeyStr("L", 3U, kTypeDeletion), ""},
       {KeyStr("L", 2U, kTypeValue), "val"},
       {KeyStr("L", 1U, kTypeSingleDeletion), ""},
       {KeyStr("M", 10U, kTypeSingleDeletion), ""},
       {KeyStr("M", 7U, kTypeValue), "val"},
-      {KeyStr("M", 5U, kTypeSingleDeletion), ""},
+      {KeyStr("M", 5U, kTypeDeletion), ""},
       {KeyStr("M", 4U, kTypeValue), "val"},
       {KeyStr("M", 3U, kTypeSingleDeletion), ""},
   });
@@ -1019,9 +1019,7 @@ TEST_F(CompactionJobTest, MultiSingleDelete) {
                           {KeyStr("K", 8U, kTypeValue), "val3"},
                           {KeyStr("L", 16U, kTypeSingleDeletion), ""},
                           {KeyStr("L", 15U, kTypeValue), ""},
-                          {KeyStr("L", 11U, kTypeSingleDeletion), ""},
-                          {KeyStr("M", 15U, kTypeSingleDeletion), ""},
-                          {KeyStr("M", 14U, kTypeValue), ""},
+                          {KeyStr("M", 16U, kTypeDeletion), ""},
                           {KeyStr("M", 3U, kTypeSingleDeletion), ""}});
 
   SetLastSequence(22U);

db/convenience.cc

@@ -40,8 +40,7 @@ Status VerifySstFileChecksum(const Options& options,
 Status VerifySstFileChecksum(const Options& options,
                              const EnvOptions& env_options,
                              const ReadOptions& read_options,
-                             const std::string& file_path,
-                             const SequenceNumber& largest_seqno) {
+                             const std::string& file_path) {
   std::unique_ptr<FSRandomAccessFile> file;
   uint64_t file_size;
   InternalKeyComparator internal_comparator(options.comparator);
@@ -62,13 +61,12 @@ Status VerifySstFileChecksum(const Options& options,
           nullptr /* stats */, 0 /* hist_type */, nullptr /* file_read_hist */,
           ioptions.rate_limiter.get()));
   const bool kImmortal = true;
-  auto reader_options = TableReaderOptions(
-      ioptions, options.prefix_extractor, env_options, internal_comparator,
-      false /* skip_filters */, !kImmortal, false /* force_direct_prefetch */,
-      -1 /* level */);
-  reader_options.largest_seqno = largest_seqno;
   s = ioptions.table_factory->NewTableReader(
-      reader_options, std::move(file_reader), file_size, &table_reader,
+      TableReaderOptions(ioptions, options.prefix_extractor, env_options,
+                         internal_comparator, false /* skip_filters */,
+                         !kImmortal, false /* force_direct_prefetch */,
+                         -1 /* level */),
+      std::move(file_reader), file_size, &table_reader,
       false /* prefetch_index_and_filter_in_cache */);
   if (!s.ok()) {
     return s;

db/corruption_test.cc

@@ -354,7 +354,6 @@ TEST_F(CorruptionTest, PostPITRCorruptionWALsRetained) {
     options_.create_missing_column_families = true;
     std::vector<ColumnFamilyHandle*> cfhs;
     ASSERT_OK(DB::Open(options_, dbname_, cf_descs, &cfhs, &db_));
-    assert(db_ != nullptr);  // suppress false clang-analyze report
 
     ASSERT_OK(db_->Put(WriteOptions(), cfhs[0], "k", "v"));
     ASSERT_OK(db_->Put(WriteOptions(), cfhs[1], "k", "v"));
@@ -376,8 +375,6 @@ TEST_F(CorruptionTest, PostPITRCorruptionWALsRetained) {
     options_.avoid_flush_during_recovery = true;
     std::vector<ColumnFamilyHandle*> cfhs;
     ASSERT_OK(DB::Open(options_, dbname_, cf_descs, &cfhs, &db_));
-    assert(db_ != nullptr);  // suppress false clang-analyze report
-
     // Flush one but not both CFs and write some data so there's a seqno gap
     // between the PITR corruption and the next DB session's first WAL.
     ASSERT_OK(db_->Put(WriteOptions(), cfhs[1], "k2", "v2"));
@@ -394,7 +391,6 @@ TEST_F(CorruptionTest, PostPITRCorruptionWALsRetained) {
   for (int i = 0; i < 2; ++i) {
     std::vector<ColumnFamilyHandle*> cfhs;
     ASSERT_OK(DB::Open(options_, dbname_, cf_descs, &cfhs, &db_));
-    assert(db_ != nullptr);  // suppress false clang-analyze report
 
     for (auto* cfh : cfhs) {
       delete cfh;

db/db_bloom_filter_test.cc

@@ -111,7 +111,6 @@ TEST_P(DBBloomFilterTestDefFormatVersion, KeyMayExist) {
     options_override.filter_policy = Create(20, bfp_impl_);
     options_override.partition_filters = partition_filters_;
     options_override.metadata_block_size = 32;
-    options_override.full_block_cache = true;
     Options options = CurrentOptions(options_override);
     if (partition_filters_) {
       auto* table_options =

db/db_compaction_filter_test.cc

@@ -968,71 +968,6 @@ TEST_F(DBTestCompactionFilter, IgnoreSnapshotsFalseRecovery) {
   ASSERT_TRUE(TryReopen(options).IsNotSupported());
 }
 
-TEST_F(DBTestCompactionFilter, DropKeyWithSingleDelete) {
-  Options options = GetDefaultOptions();
-  options.create_if_missing = true;
-
-  Reopen(options);
-
-  ASSERT_OK(Put("a", "v0"));
-  ASSERT_OK(Put("b", "v0"));
-  const Snapshot* snapshot = db_->GetSnapshot();
-
-  ASSERT_OK(SingleDelete("b"));
-  ASSERT_OK(Flush());
-
-  {
-    CompactRangeOptions cro;
-    cro.change_level = true;
-    cro.target_level = options.num_levels - 1;
-    ASSERT_OK(db_->CompactRange(cro, nullptr, nullptr));
-  }
-
-  db_->ReleaseSnapshot(snapshot);
-  Close();
-
-  class DeleteFilterV2 : public CompactionFilter {
-   public:
-    Decision FilterV2(int /*level*/, const Slice& key, ValueType /*value_type*/,
-                      const Slice& /*existing_value*/,
-                      std::string* /*new_value*/,
-                      std::string* /*skip_until*/) const override {
-      if (key.starts_with("b")) {
-        return Decision::kRemoveWithSingleDelete;
-      }
-      return Decision::kRemove;
-    }
-
-    const char* Name() const override { return "DeleteFilterV2"; }
-  } delete_filter_v2;
-
-  options.compaction_filter = &delete_filter_v2;
-  options.level0_file_num_compaction_trigger = 2;
-  Reopen(options);
-
-  ASSERT_OK(Put("b", "v1"));
-  ASSERT_OK(Put("x", "v1"));
-  ASSERT_OK(Flush());
-
-  ASSERT_OK(Put("r", "v1"));
-  ASSERT_OK(Put("z", "v1"));
-  ASSERT_OK(Flush());
-
-  ASSERT_OK(dbfull()->TEST_WaitForCompact());
-
-  Close();
-
-  options.compaction_filter = nullptr;
-  Reopen(options);
-  ASSERT_OK(SingleDelete("b"));
-  ASSERT_OK(Flush());
-  {
-    CompactRangeOptions cro;
-    cro.bottommost_level_compaction = BottommostLevelCompaction::kForce;
-    ASSERT_OK(db_->CompactRange(cro, nullptr, nullptr));
-  }
-}
-
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

db/db_compaction_test.cc

@@ -2409,30 +2409,6 @@ TEST_P(DBCompactionTestWithParam, LevelCompactionCFPathUse) {
 
   check_getvalues();
 
-  {  // Also verify GetLiveFilesStorageInfo with db_paths / cf_paths
-    std::vector<LiveFileStorageInfo> new_infos;
-    LiveFilesStorageInfoOptions lfsio;
-    lfsio.wal_size_for_flush = UINT64_MAX;  // no flush
-    ASSERT_OK(db_->GetLiveFilesStorageInfo(lfsio, &new_infos));
-    std::unordered_map<std::string, int> live_sst_by_dir;
-    for (auto& info : new_infos) {
-      if (info.file_type == kTableFile) {
-        live_sst_by_dir[info.directory]++;
-        // Verify file on disk (no directory confusion)
-        uint64_t size;
-        ASSERT_OK(env_->GetFileSize(
-            info.directory + "/" + info.relative_filename, &size));
-        ASSERT_EQ(info.size, size);
-      }
-    }
-    ASSERT_EQ(3U * 3U, live_sst_by_dir.size());
-    for (auto& paths : {options.db_paths, cf_opt1.cf_paths, cf_opt2.cf_paths}) {
-      ASSERT_EQ(1, live_sst_by_dir[paths[0].path]);
-      ASSERT_EQ(4, live_sst_by_dir[paths[1].path]);
-      ASSERT_EQ(2, live_sst_by_dir[paths[2].path]);
-    }
-  }
-
   ReopenWithColumnFamilies({"default", "one", "two"}, option_vector);
 
   check_getvalues();

db/db_impl/db_impl.cc

@@ -5123,8 +5123,8 @@ Status DBImpl::VerifyChecksumInternal(const ReadOptions& read_options,
                                      fmeta->file_checksum_func_name, fname,
                                      read_options);
         } else {
-          s = ROCKSDB_NAMESPACE::VerifySstFileChecksum(
-              opts, file_options_, read_options, fname, fd.largest_seqno);
+          s = ROCKSDB_NAMESPACE::VerifySstFileChecksum(opts, file_options_,
+                                                       read_options, fname);
         }
         RecordTick(stats_, VERIFY_CHECKSUM_READ_BYTES,
                    IOSTATS(bytes_read) - prev_bytes_read);

db/db_impl/db_impl_compaction_flush.cc

@@ -1704,11 +1704,9 @@ Status DBImpl::Flush(const FlushOptions& flush_options,
   Status s;
   if (immutable_db_options_.atomic_flush) {
     s = AtomicFlushMemTables({cfh->cfd()}, flush_options,
-                             FlushReason::kManualFlush,
-                             write_controller().IsStopped());
+                             FlushReason::kManualFlush);
   } else {
-    s = FlushMemTable(cfh->cfd(), flush_options, FlushReason::kManualFlush,
-                      write_controller().IsStopped());
+    s = FlushMemTable(cfh->cfd(), flush_options, FlushReason::kManualFlush);
   }
 
   ROCKS_LOG_INFO(immutable_db_options_.info_log,

db/db_impl/db_impl_open.cc

@@ -947,6 +947,7 @@ Status DBImpl::RecoverLogFiles(const std::vector<uint64_t>& wal_numbers,
     // Read all the records and add to a memtable
     std::string scratch;
     Slice record;
+    WriteBatch batch;
 
     TEST_SYNC_POINT_CALLBACK("DBImpl::RecoverLogFiles:BeforeReadWal",
                              /*arg=*/nullptr);
@@ -960,15 +961,10 @@ Status DBImpl::RecoverLogFiles(const std::vector<uint64_t>& wal_numbers,
         continue;
       }
 
-      // We create a new batch and initialize with a valid prot_info_ to store
-      // the data checksums
-      WriteBatch batch(0, 0, 8, 0);
-
       status = WriteBatchInternal::SetContents(&batch, record);
       if (!status.ok()) {
         return status;
       }
-
       SequenceNumber sequence = WriteBatchInternal::Sequence(&batch);
 
       if (immutable_db_options_.wal_recovery_mode ==

db/db_iterator_test.cc

@@ -111,12 +111,9 @@ TEST_P(DBIteratorTest, PersistedTierOnIterator) {
 TEST_P(DBIteratorTest, NonBlockingIteration) {
   do {
     ReadOptions non_blocking_opts, regular_opts;
-    anon::OptionsOverride options_override;
-    options_override.full_block_cache = true;
-    Options options = CurrentOptions(options_override);
+    Options options = CurrentOptions();
     options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
     non_blocking_opts.read_tier = kBlockCacheTier;
-
     CreateAndReopenWithCF({"pikachu"}, options);
     // write one kv to the database.
     ASSERT_OK(Put(1, "a", "b"));

db/db_test.cc

@@ -2427,8 +2427,8 @@ TEST_F(DBTest, SnapshotFiles) {
 
     // Also test GetLiveFilesStorageInfo
     std::vector<LiveFileStorageInfo> new_infos;
-    ASSERT_OK(db_->GetLiveFilesStorageInfo(LiveFilesStorageInfoOptions(),
-                                           &new_infos));
+    ASSERT_OK(dbfull()->GetLiveFilesStorageInfo(LiveFilesStorageInfoOptions(),
+                                                &new_infos));
 
     // Close DB (while deletions disabled)
     Close();
@@ -7034,51 +7034,6 @@ TEST_F(DBTest, MemoryUsageWithMaxWriteBufferSizeToMaintain) {
   }
 }
 
-TEST_F(DBTest, ManualFlushWithStoppedWritesTest) {
-  Options options = CurrentOptions();
-
-  // Setting a small write buffer size. This will block writes
-  // rather quickly until a flush is made.
-  constexpr unsigned int memtableSize = 1000;
-  options.db_write_buffer_size = memtableSize;
-  options.max_background_flushes = 1;
-  Reopen(options);
-
-  std::atomic<bool> done(false);
-
-  // Will suppress future flushes.
-  // This simulates a situation where the writes will be stopped for any reason.
-  ASSERT_OK(dbfull()->PauseBackgroundWork());
-
-  port::Thread backgroundWriter([&]() {
-    Random rnd(301);
-    // These writes won't finish.
-    ASSERT_OK(Put("k1", rnd.RandomString(memtableSize / 2)));
-    ASSERT_OK(Put("k2", rnd.RandomString(memtableSize / 2)));
-    ASSERT_OK(Put("k3", rnd.RandomString(memtableSize / 2)));
-    ASSERT_OK(Put("k4", rnd.RandomString(memtableSize / 2)));
-    done.store(true);
-  });
-
-  env_->SleepForMicroseconds(1000000 / 2);
-  // make sure thread is stuck waiting for flush.
-  ASSERT_FALSE(done.load());
-
-  FlushOptions flushOptions;
-  flushOptions.wait = false;
-  flushOptions.allow_write_stall = true;
-
-  // This is the main goal of the test. This manual flush should not deadlock
-  // as we use wait=false, and even allow_write_stall=true for extra safety.
-  ASSERT_OK(dbfull()->Flush(flushOptions));
-
-  // This will re-allow background flushes.
-  ASSERT_OK(dbfull()->ContinueBackgroundWork());
-
-  backgroundWriter.join();
-  ASSERT_TRUE(done.load());
-}
-
 #endif
 
 }  // namespace ROCKSDB_NAMESPACE

db/db_test_util.cc

@@ -11,8 +11,6 @@
 
 #include "db/forward_iterator.h"
 #include "env/mock_env.h"
-#include "port/lang.h"
-#include "rocksdb/cache.h"
 #include "rocksdb/convenience.h"
 #include "rocksdb/env_encryption.h"
 #include "rocksdb/unique_id.h"
@@ -362,17 +360,6 @@ Options DBTestBase::GetOptions(
   ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearCallBack(
       "NewWritableFile:O_DIRECT");
 #endif
-  // kMustFreeHeapAllocations -> indicates ASAN build
-  if (kMustFreeHeapAllocations && !options_override.full_block_cache) {
-    // Detecting block cache use-after-free is normally difficult in unit
-    // tests, because as a cache, it tends to keep unreferenced entries in
-    // memory, and we normally want unit tests to take advantage of block
-    // cache for speed. However, we also want a strong chance of detecting
-    // block cache use-after-free in unit tests in ASAN builds, so for ASAN
-    // builds we use a trivially small block cache to which entries can be
-    // added but are immediately freed on no more references.
-    table_options.block_cache = NewLRUCache(/* too small */ 1);
-  }
 
   bool can_allow_mmap = IsMemoryMappedAccessSupported();
   switch (option_config) {
@@ -844,7 +831,7 @@ std::vector<std::string> DBTestBase::MultiGet(std::vector<int> cfs,
   std::vector<Status> s;
   if (!batched) {
     s = db_->MultiGet(options, handles, keys, &result);
-    for (size_t i = 0; i < s.size(); ++i) {
+    for (unsigned int i = 0; i < s.size(); ++i) {
       if (s[i].IsNotFound()) {
         result[i] = "NOT_FOUND";
       } else if (!s[i].ok()) {
@@ -857,16 +844,13 @@ std::vector<std::string> DBTestBase::MultiGet(std::vector<int> cfs,
     s.resize(cfs.size());
     db_->MultiGet(options, cfs.size(), handles.data(), keys.data(),
                   pin_values.data(), s.data());
-    for (size_t i = 0; i < s.size(); ++i) {
+    for (unsigned int i = 0; i < s.size(); ++i) {
       if (s[i].IsNotFound()) {
         result[i] = "NOT_FOUND";
       } else if (!s[i].ok()) {
         result[i] = s[i].ToString();
       } else {
         result[i].assign(pin_values[i].data(), pin_values[i].size());
-        // Increase likelihood of detecting potential use-after-free bugs with
-        // PinnableSlices tracking the same resource
-        pin_values[i].Reset();
       }
     }
   }
@@ -879,25 +863,23 @@ std::vector<std::string> DBTestBase::MultiGet(const std::vector<std::string>& k,
   options.verify_checksums = true;
   options.snapshot = snapshot;
   std::vector<Slice> keys;
-  std::vector<std::string> result(k.size());
+  std::vector<std::string> result;
   std::vector<Status> statuses(k.size());
   std::vector<PinnableSlice> pin_values(k.size());
 
-  for (size_t i = 0; i < k.size(); ++i) {
+  for (unsigned int i = 0; i < k.size(); ++i) {
     keys.push_back(k[i]);
   }
   db_->MultiGet(options, dbfull()->DefaultColumnFamily(), keys.size(),
                 keys.data(), pin_values.data(), statuses.data());
-  for (size_t i = 0; i < statuses.size(); ++i) {
+  result.resize(k.size());
+  for (auto iter = result.begin(); iter != result.end(); ++iter) {
+    iter->assign(pin_values[iter - result.begin()].data(),
+                 pin_values[iter - result.begin()].size());
+  }
+  for (unsigned int i = 0; i < statuses.size(); ++i) {
     if (statuses[i].IsNotFound()) {
       result[i] = "NOT_FOUND";
-    } else if (!statuses[i].ok()) {
-      result[i] = statuses[i].ToString();
-    } else {
-      result[i].assign(pin_values[i].data(), pin_values[i].size());
-      // Increase likelihood of detecting potential use-after-free bugs with
-      // PinnableSlices tracking the same resource
-      pin_values[i].Reset();
     }
   }
   return result;

db/db_test_util.h

@@ -104,9 +104,6 @@ struct OptionsOverride {
   std::shared_ptr<const FilterPolicy> filter_policy = nullptr;
   // These will be used only if filter_policy is set
   bool partition_filters = false;
-  // Force using a default block cache. (Setting to false allows ASAN build
-  // use a trivially small block cache for better UAF error detection.)
-  bool full_block_cache = false;
   uint64_t metadata_block_size = 1024;
 
   // Used as a bit mask of individual enums in which to skip an XF test point

db/external_sst_file_basic_test.cc

@@ -1844,28 +1844,6 @@ TEST_F(ExternalSSTFileBasicTest, FailIfNotBottommostLevel) {
   }
 }
 
-TEST_F(ExternalSSTFileBasicTest, VerifyChecksum) {
-  const std::string kPutVal = "put_val";
-  const std::string kIngestedVal = "ingested_val";
-
-  ASSERT_OK(Put("k", kPutVal, WriteOptions()));
-  ASSERT_OK(Flush());
-
-  std::string external_file = sst_files_dir_ + "/file_to_ingest.sst";
-  {
-    SstFileWriter sst_file_writer{EnvOptions(), CurrentOptions()};
-
-    ASSERT_OK(sst_file_writer.Open(external_file));
-    ASSERT_OK(sst_file_writer.Put("k", kIngestedVal));
-    ASSERT_OK(sst_file_writer.Finish());
-  }
-
-  ASSERT_OK(db_->IngestExternalFile(db_->DefaultColumnFamily(), {external_file},
-                                    IngestExternalFileOptions()));
-
-  ASSERT_OK(db_->VerifyChecksum());
-}
-
 INSTANTIATE_TEST_CASE_P(ExternalSSTFileBasicTest, ExternalSSTFileBasicTest,
                         testing::Values(std::make_tuple(true, true),
                                         std::make_tuple(true, false),

db/write_batch.cc

@@ -152,6 +152,14 @@ struct SavePoints {
   std::stack<SavePoint, autovector<SavePoint>> stack;
 };
 
+WriteBatch::WriteBatch(size_t reserved_bytes, size_t max_bytes)
+    : content_flags_(0), max_bytes_(max_bytes), rep_() {
+  rep_.reserve((reserved_bytes > WriteBatchInternal::kHeader)
+                   ? reserved_bytes
+                   : WriteBatchInternal::kHeader);
+  rep_.resize(WriteBatchInternal::kHeader);
+}
+
 WriteBatch::WriteBatch(size_t reserved_bytes, size_t max_bytes,
                        size_t protection_bytes_per_key, size_t default_cf_ts_sz)
     : content_flags_(0),
@@ -572,16 +580,14 @@ Status WriteBatchInternal::Iterate(const WriteBatch* wb,
         s = handler->MarkBeginPrepare();
         assert(s.ok());
         empty_batch = false;
-        if (handler->WriteAfterCommit() ==
-            WriteBatch::Handler::OptionState::kDisabled) {
+        if (!handler->WriteAfterCommit()) {
           s = Status::NotSupported(
               "WriteCommitted txn tag when write_after_commit_ is disabled (in "
               "WritePrepared/WriteUnprepared mode). If it is not due to "
               "corruption, the WAL must be emptied before changing the "
               "WritePolicy.");
         }
-        if (handler->WriteBeforePrepare() ==
-            WriteBatch::Handler::OptionState::kEnabled) {
+        if (handler->WriteBeforePrepare()) {
           s = Status::NotSupported(
               "WriteCommitted txn tag when write_before_prepare_ is enabled "
               "(in WriteUnprepared mode). If it is not due to corruption, the "
@@ -594,8 +600,7 @@ Status WriteBatchInternal::Iterate(const WriteBatch* wb,
         s = handler->MarkBeginPrepare();
         assert(s.ok());
         empty_batch = false;
-        if (handler->WriteAfterCommit() ==
-            WriteBatch::Handler::OptionState::kEnabled) {
+        if (handler->WriteAfterCommit()) {
           s = Status::NotSupported(
               "WritePrepared/WriteUnprepared txn tag when write_after_commit_ "
               "is enabled (in default WriteCommitted mode). If it is not due "
@@ -609,15 +614,13 @@ Status WriteBatchInternal::Iterate(const WriteBatch* wb,
         s = handler->MarkBeginPrepare(true /* unprepared */);
         assert(s.ok());
         empty_batch = false;
-        if (handler->WriteAfterCommit() ==
-            WriteBatch::Handler::OptionState::kEnabled) {
+        if (handler->WriteAfterCommit()) {
           s = Status::NotSupported(
               "WriteUnprepared txn tag when write_after_commit_ is enabled (in "
               "default WriteCommitted mode). If it is not due to corruption, "
               "the WAL must be emptied before changing the WritePolicy.");
         }
-        if (handler->WriteBeforePrepare() ==
-            WriteBatch::Handler::OptionState::kDisabled) {
+        if (!handler->WriteBeforePrepare()) {
           s = Status::NotSupported(
               "WriteUnprepared txn tag when write_before_prepare_ is disabled "
               "(in WriteCommitted/WritePrepared mode). If it is not due to "
@@ -1491,8 +1494,6 @@ Status WriteBatch::UpdateTimestamps(
   return s;
 }
 
-namespace {
-
 class MemTableInserter : public WriteBatch::Handler {
 
   SequenceNumber sequence_;
@@ -1580,24 +1581,9 @@ class MemTableInserter : public WriteBatch::Handler {
     return res;
   }
 
-  void DecrementProtectionInfoIdxForTryAgain() {
-    if (prot_info_ != nullptr) --prot_info_idx_;
-  }
-
-  void ResetProtectionInfo() {
-    prot_info_idx_ = 0;
-    prot_info_ = nullptr;
-  }
-
  protected:
-  Handler::OptionState WriteBeforePrepare() const override {
-    return write_before_prepare_ ? Handler::OptionState::kEnabled
-                                 : Handler::OptionState::kDisabled;
-  }
-  Handler::OptionState WriteAfterCommit() const override {
-    return write_after_commit_ ? Handler::OptionState::kEnabled
-                               : Handler::OptionState::kDisabled;
-  }
+  bool WriteBeforePrepare() const override { return write_before_prepare_; }
+  bool WriteAfterCommit() const override { return write_after_commit_; }
 
  public:
   // cf_mems should not be shared with concurrent inserters
@@ -1885,25 +1871,15 @@ class MemTableInserter : public WriteBatch::Handler {
   Status PutCF(uint32_t column_family_id, const Slice& key,
                const Slice& value) override {
     const auto* kv_prot_info = NextProtectionInfo();
-    Status ret_status;
     if (kv_prot_info != nullptr) {
       // Memtable needs seqno, doesn't need CF ID
       auto mem_kv_prot_info =
           kv_prot_info->StripC(column_family_id).ProtectS(sequence_);
-      ret_status = PutCFImpl(column_family_id, key, value, kTypeValue,
-                             &mem_kv_prot_info);
-    } else {
-      ret_status = PutCFImpl(column_family_id, key, value, kTypeValue,
-                             nullptr /* kv_prot_info */);
+      return PutCFImpl(column_family_id, key, value, kTypeValue,
+                       &mem_kv_prot_info);
     }
-    // TODO: this assumes that if TryAgain status is returned to the caller,
-    // the operation is actually tried again. The proper way to do this is to
-    // pass a `try_again` parameter to the operation itself and decrement
-    // prot_info_idx_ based on that
-    if (UNLIKELY(ret_status.IsTryAgain())) {
-      DecrementProtectionInfoIdxForTryAgain();
-    }
-    return ret_status;
+    return PutCFImpl(column_family_id, key, value, kTypeValue,
+                     nullptr /* kv_prot_info */);
   }
 
   Status DeleteImpl(uint32_t /*column_family_id*/, const Slice& key,
@@ -1950,9 +1926,6 @@ class MemTableInserter : public WriteBatch::Handler {
       } else if (ret_status.ok()) {
         MaybeAdvanceSeq(false /* batch_boundary */);
       }
-      if (UNLIKELY(ret_status.IsTryAgain())) {
-        DecrementProtectionInfoIdxForTryAgain();
-      }
       return ret_status;
     }
 
@@ -1984,9 +1957,6 @@ class MemTableInserter : public WriteBatch::Handler {
       ret_status =
           WriteBatchInternal::Delete(rebuilding_trx_, column_family_id, key);
     }
-    if (UNLIKELY(ret_status.IsTryAgain())) {
-      DecrementProtectionInfoIdxForTryAgain();
-    }
     return ret_status;
   }
 
@@ -2015,9 +1985,6 @@ class MemTableInserter : public WriteBatch::Handler {
       } else if (ret_status.ok()) {
         MaybeAdvanceSeq(false /* batch_boundary */);
       }
-      if (UNLIKELY(ret_status.IsTryAgain())) {
-        DecrementProtectionInfoIdxForTryAgain();
-      }
       return ret_status;
     }
     assert(ret_status.ok());
@@ -2042,9 +2009,6 @@ class MemTableInserter : public WriteBatch::Handler {
       ret_status = WriteBatchInternal::SingleDelete(rebuilding_trx_,
                                                     column_family_id, key);
     }
-    if (UNLIKELY(ret_status.IsTryAgain())) {
-      DecrementProtectionInfoIdxForTryAgain();
-    }
     return ret_status;
   }
 
@@ -2074,9 +2038,6 @@ class MemTableInserter : public WriteBatch::Handler {
       } else if (ret_status.ok()) {
         MaybeAdvanceSeq(false /* batch_boundary */);
       }
-      if (UNLIKELY(ret_status.IsTryAgain())) {
-        DecrementProtectionInfoIdxForTryAgain();
-      }
       return ret_status;
     }
     assert(ret_status.ok());
@@ -2131,9 +2092,6 @@ class MemTableInserter : public WriteBatch::Handler {
       ret_status = WriteBatchInternal::DeleteRange(
           rebuilding_trx_, column_family_id, begin_key, end_key);
     }
-    if (UNLIKELY(ret_status.IsTryAgain())) {
-      DecrementProtectionInfoIdxForTryAgain();
-    }
     return ret_status;
   }
 
@@ -2163,9 +2121,6 @@ class MemTableInserter : public WriteBatch::Handler {
       } else if (ret_status.ok()) {
         MaybeAdvanceSeq(false /* batch_boundary */);
       }
-      if (UNLIKELY(ret_status.IsTryAgain())) {
-        DecrementProtectionInfoIdxForTryAgain();
-      }
       return ret_status;
     }
     assert(ret_status.ok());
@@ -2287,31 +2242,23 @@ class MemTableInserter : public WriteBatch::Handler {
       ret_status = WriteBatchInternal::Merge(rebuilding_trx_, column_family_id,
                                              key, value);
     }
-    if (UNLIKELY(ret_status.IsTryAgain())) {
-      DecrementProtectionInfoIdxForTryAgain();
-    }
     return ret_status;
   }
 
   Status PutBlobIndexCF(uint32_t column_family_id, const Slice& key,
                         const Slice& value) override {
     const auto* kv_prot_info = NextProtectionInfo();
-    Status ret_status;
     if (kv_prot_info != nullptr) {
       // Memtable needs seqno, doesn't need CF ID
       auto mem_kv_prot_info =
           kv_prot_info->StripC(column_family_id).ProtectS(sequence_);
       // Same as PutCF except for value type.
-      ret_status = PutCFImpl(column_family_id, key, value, kTypeBlobIndex,
-                             &mem_kv_prot_info);
+      return PutCFImpl(column_family_id, key, value, kTypeBlobIndex,
+                       &mem_kv_prot_info);
     } else {
-      ret_status = PutCFImpl(column_family_id, key, value, kTypeBlobIndex,
-                             nullptr /* kv_prot_info */);
+      return PutCFImpl(column_family_id, key, value, kTypeBlobIndex,
+                       nullptr /* kv_prot_info */);
     }
-    if (UNLIKELY(ret_status.IsTryAgain())) {
-      DecrementProtectionInfoIdxForTryAgain();
-    }
-    return ret_status;
   }
 
   void CheckMemtableFull() {
@@ -2454,7 +2401,6 @@ class MemTableInserter : public WriteBatch::Handler {
           const auto& batch_info = trx->batches_.begin()->second;
           // all inserts must reference this trx log number
           log_number_ref_ = batch_info.log_number_;
-          ResetProtectionInfo();
           s = batch_info.batch_->Iterate(this);
           log_number_ref_ = 0;
         }
@@ -2476,10 +2422,6 @@ class MemTableInserter : public WriteBatch::Handler {
     const bool batch_boundry = true;
     MaybeAdvanceSeq(batch_boundry);
 
-    if (UNLIKELY(s.IsTryAgain())) {
-      DecrementProtectionInfoIdxForTryAgain();
-    }
-
     return s;
   }
 
@@ -2524,7 +2466,6 @@ class MemTableInserter : public WriteBatch::Handler {
                 return ucmp->timestamp_size();
               });
           if (s.ok()) {
-            ResetProtectionInfo();
             s = batch_info.batch_->Iterate(this);
             log_number_ref_ = 0;
           }
@@ -2547,10 +2488,6 @@ class MemTableInserter : public WriteBatch::Handler {
     constexpr bool batch_boundary = true;
     MaybeAdvanceSeq(batch_boundary);
 
-    if (UNLIKELY(s.IsTryAgain())) {
-      DecrementProtectionInfoIdxForTryAgain();
-    }
-
     return s;
   }
 
@@ -2586,8 +2523,6 @@ class MemTableInserter : public WriteBatch::Handler {
   }
 };
 
-}  // namespace
-
 // This function can only be called in these conditions:
 // 1) During Recovery()
 // 2) During Write(), in a single-threaded write thread
@@ -2678,94 +2613,11 @@ Status WriteBatchInternal::InsertInto(
   return s;
 }
 
-namespace {
-
-// This class updates protection info for a WriteBatch.
-class ProtectionInfoUpdater : public WriteBatch::Handler {
- public:
-  explicit ProtectionInfoUpdater(WriteBatch::ProtectionInfo* prot_info)
-      : prot_info_(prot_info) {}
-
-  ~ProtectionInfoUpdater() override {}
-
-  Status PutCF(uint32_t cf, const Slice& key, const Slice& val) override {
-    return UpdateProtInfo(cf, key, val, kTypeValue);
-  }
-
-  Status DeleteCF(uint32_t cf, const Slice& key) override {
-    return UpdateProtInfo(cf, key, "", kTypeDeletion);
-  }
-
-  Status SingleDeleteCF(uint32_t cf, const Slice& key) override {
-    return UpdateProtInfo(cf, key, "", kTypeSingleDeletion);
-  }
-
-  Status DeleteRangeCF(uint32_t cf, const Slice& begin_key,
-                       const Slice& end_key) override {
-    return UpdateProtInfo(cf, begin_key, end_key, kTypeRangeDeletion);
-  }
-
-  Status MergeCF(uint32_t cf, const Slice& key, const Slice& val) override {
-    return UpdateProtInfo(cf, key, val, kTypeMerge);
-  }
-
-  Status PutBlobIndexCF(uint32_t cf, const Slice& key,
-                        const Slice& val) override {
-    return UpdateProtInfo(cf, key, val, kTypeBlobIndex);
-  }
-
-  Status MarkBeginPrepare(bool /* unprepare */) override {
-    return Status::OK();
-  }
-
-  Status MarkEndPrepare(const Slice& /* xid */) override {
-    return Status::OK();
-  }
-
-  Status MarkCommit(const Slice& /* xid */) override { return Status::OK(); }
-
-  Status MarkCommitWithTimestamp(const Slice& /* xid */,
-                                 const Slice& /* ts */) override {
-    return Status::OK();
-  }
-
-  Status MarkRollback(const Slice& /* xid */) override { return Status::OK(); }
-
-  Status MarkNoop(bool /* empty_batch */) override { return Status::OK(); }
-
- private:
-  Status UpdateProtInfo(uint32_t cf, const Slice& key, const Slice& val,
-                        const ValueType op_type) {
-    if (prot_info_) {
-      prot_info_->entries_.emplace_back(
-          ProtectionInfo64().ProtectKVO(key, val, op_type).ProtectC(cf));
-    }
-    return Status::OK();
-  }
-
-  // No copy or move.
-  ProtectionInfoUpdater(const ProtectionInfoUpdater&) = delete;
-  ProtectionInfoUpdater(ProtectionInfoUpdater&&) = delete;
-  ProtectionInfoUpdater& operator=(const ProtectionInfoUpdater&) = delete;
-  ProtectionInfoUpdater& operator=(ProtectionInfoUpdater&&) = delete;
-
-  WriteBatch::ProtectionInfo* const prot_info_ = nullptr;
-};
-
-}  // namespace
-
 Status WriteBatchInternal::SetContents(WriteBatch* b, const Slice& contents) {
   assert(contents.size() >= WriteBatchInternal::kHeader);
-
+  assert(b->prot_info_ == nullptr);
   b->rep_.assign(contents.data(), contents.size());
   b->content_flags_.store(ContentFlags::DEFERRED, std::memory_order_relaxed);
-
-  // If we have a prot_info_, update protection info entries for the batch.
-  if (b->prot_info_) {
-    ProtectionInfoUpdater prot_info_updater(b->prot_info_.get());
-    return b->Iterate(&prot_info_updater);
-  }
-
   return Status::OK();
 }
 

db_stress_tool/db_stress_compaction_filter.h

@@ -46,13 +46,11 @@ class DbStressCompactionFilter : public CompactionFilter {
     // Reaching here means we acquired the lock.
 
     bool key_exists = state_->Exists(cf_id_, key_num);
-    const bool allow_overwrite = state_->AllowsOverwrite(key_num);
 
     key_mutex->Unlock();
 
     if (!key_exists) {
-      return allow_overwrite ? Decision::kRemove
-                             : Decision::kRemoveWithSingleDelete;
+      return Decision::kRemove;
     }
     return Decision::kKeep;
   }

db_stress_tool/db_stress_driver.cc

@@ -58,8 +58,8 @@ void ThreadBody(void* v) {
 
 bool RunStressTest(StressTest* stress) {
   SystemClock* clock = db_stress_env->GetSystemClock().get();
+  stress->InitDb();
   SharedState shared(db_stress_env, stress);
-  stress->InitDb(&shared);
   stress->FinishInitDb(&shared);
 
 #ifndef NDEBUG

db_stress_tool/db_stress_shared_state.cc

@@ -12,6 +12,8 @@
 #include "db_stress_tool/db_stress_shared_state.h"
 
 namespace ROCKSDB_NAMESPACE {
+const uint32_t SharedState::UNKNOWN_SENTINEL = 0xfffffffe;
+const uint32_t SharedState::DELETION_SENTINEL = 0xffffffff;
 #if defined(ROCKSDB_SUPPORT_THREAD_LOCAL)
 #if defined(OS_SOLARIS)
 __thread bool SharedState::ignore_read_error;

db_stress_tool/db_stress_shared_state.h

@@ -45,9 +45,9 @@ class SharedState {
  public:
   // indicates a key may have any value (or not be present) as an operation on
   // it is incomplete.
-  static constexpr uint32_t UNKNOWN_SENTINEL = 0xfffffffe;
+  static const uint32_t UNKNOWN_SENTINEL;
   // indicates a key should definitely be deleted
-  static constexpr uint32_t DELETION_SENTINEL = 0xffffffff;
+  static const uint32_t DELETION_SENTINEL;
 
   // Errors when reading filter blocks are ignored, so we use a thread
   // local variable updated via sync points to keep track of errors injected
@@ -81,9 +81,36 @@ class SharedState {
         stress_test_(stress_test),
         verification_failure_(false),
         should_stop_test_(false),
-        no_overwrite_ids_(GenerateNoOverwriteIds()),
+        no_overwrite_ids_(FLAGS_column_families),
         expected_state_manager_(nullptr),
         printing_verification_results_(false) {
+    // Pick random keys in each column family that will not experience
+    // overwrite
+
+    fprintf(stdout, "Choosing random keys with no overwrite\n");
+    Random64 rnd(seed_);
+    // Start with the identity permutation. Subsequent iterations of
+    // for loop below will start with perm of previous for loop
+    int64_t* permutation = new int64_t[max_key_];
+    for (int64_t i = 0; i < max_key_; i++) {
+      permutation[i] = i;
+    }
+    // Now do the Knuth shuffle
+    int64_t num_no_overwrite_keys = (max_key_ * FLAGS_nooverwritepercent) / 100;
+    // Only need to figure out first num_no_overwrite_keys of permutation
+    no_overwrite_ids_.reserve(num_no_overwrite_keys);
+    for (int64_t i = 0; i < num_no_overwrite_keys; i++) {
+      int64_t rand_index = i + rnd.Next() % (max_key_ - i);
+      // Swap i and rand_index;
+      int64_t temp = permutation[i];
+      permutation[i] = permutation[rand_index];
+      permutation[rand_index] = temp;
+      // Fill no_overwrite_ids_ with the first num_no_overwrite_keys of
+      // permutation
+      no_overwrite_ids_.insert(permutation[i]);
+    }
+    delete[] permutation;
+
     Status status;
     // TODO: We should introduce a way to explicitly disable verification
     // during shutdown. When that is disabled and FLAGS_expected_values_dir
@@ -266,7 +293,7 @@ class SharedState {
                                                 pending);
   }
 
-  bool AllowsOverwrite(int64_t key) const {
+  bool AllowsOverwrite(int64_t key) {
     return no_overwrite_ids_.find(key) == no_overwrite_ids_.end();
   }
 
@@ -308,36 +335,6 @@ class SharedState {
     ignore_read_error = true;
   }
 
-  // Pick random keys in each column family that will not experience overwrite.
-  std::unordered_set<int64_t> GenerateNoOverwriteIds() const {
-    fprintf(stdout, "Choosing random keys with no overwrite\n");
-    // Start with the identity permutation. Subsequent iterations of
-    // for loop below will start with perm of previous for loop
-    std::vector<int64_t> permutation(max_key_);
-    for (int64_t i = 0; i < max_key_; ++i) {
-      permutation[i] = i;
-    }
-    // Now do the Knuth shuffle
-    const int64_t num_no_overwrite_keys =
-        (max_key_ * FLAGS_nooverwritepercent) / 100;
-    // Only need to figure out first num_no_overwrite_keys of permutation
-    std::unordered_set<int64_t> ret;
-    ret.reserve(num_no_overwrite_keys);
-    Random64 rnd(seed_);
-    for (int64_t i = 0; i < num_no_overwrite_keys; i++) {
-      assert(i < max_key_);
-      int64_t rand_index = i + rnd.Next() % (max_key_ - i);
-      // Swap i and rand_index;
-      int64_t temp = permutation[i];
-      permutation[i] = permutation[rand_index];
-      permutation[rand_index] = temp;
-      // Fill no_overwrite_ids_ with the first num_no_overwrite_keys of
-      // permutation
-      ret.insert(permutation[i]);
-    }
-    return ret;
-  }
-
   port::Mutex mu_;
   port::CondVar cv_;
   const uint32_t seed_;
@@ -358,7 +355,7 @@ class SharedState {
   std::atomic<bool> should_stop_test_;
 
   // Keys that should not be overwritten
-  const std::unordered_set<int64_t> no_overwrite_ids_;
+  std::unordered_set<size_t> no_overwrite_ids_;
 
   std::unique_ptr<ExpectedStateManager> expected_state_manager_;
   // Cannot store `port::Mutex` directly in vector since it is not copyable

db_stress_tool/db_stress_test_base.cc

@@ -281,12 +281,12 @@ bool StressTest::BuildOptionsTable() {
   return true;
 }
 
-void StressTest::InitDb(SharedState* shared) {
+void StressTest::InitDb() {
   uint64_t now = clock_->NowMicros();
   fprintf(stdout, "%s Initializing db_stress\n",
           clock_->TimeToString(now / 1000000).c_str());
   PrintEnv();
-  Open(shared);
+  Open();
   BuildOptionsTable();
 }
 
@@ -568,7 +568,7 @@ void StressTest::PreloadDbAndReopenAsReadOnly(int64_t number_of_keys,
     fprintf(stdout, "%s Reopening database in read-only\n",
             clock_->TimeToString(now / 1000000).c_str());
     // Reopen as read-only, can ignore all options related to updates
-    Open(shared);
+    Open();
   } else {
     fprintf(stderr, "Failed to preload db");
     exit(1);
@@ -2302,12 +2302,10 @@ void StressTest::PrintEnv() const {
   fprintf(stdout, "------------------------------------------------\n");
 }
 
-void StressTest::Open(SharedState* shared) {
+void StressTest::Open() {
   assert(db_ == nullptr);
 #ifndef ROCKSDB_LITE
   assert(txn_db_ == nullptr);
-#else
-  (void)shared;
 #endif
   if (FLAGS_options_file.empty()) {
     BlockBasedTableOptions block_based_options;
@@ -2615,7 +2613,6 @@ void StressTest::Open(SharedState* shared) {
     options_.listeners.emplace_back(new DbStressListener(
         FLAGS_db, options_.db_paths, cf_descriptors, db_stress_listener_env));
 #endif  // !ROCKSDB_LITE
-    RegisterAdditionalListeners();
     options_.create_missing_column_families = true;
     if (!FLAGS_use_txn) {
 #ifndef NDEBUG
@@ -2754,7 +2751,6 @@ void StressTest::Open(SharedState* shared) {
           static_cast<size_t>(FLAGS_wp_snapshot_cache_bits);
       txn_db_options.wp_commit_cache_bits =
           static_cast<size_t>(FLAGS_wp_commit_cache_bits);
-      PrepareTxnDbOptions(shared, txn_db_options);
       s = TransactionDB::Open(options_, txn_db_options, FLAGS_db,
                               cf_descriptors, &column_families_, &txn_db_);
       if (!s.ok()) {
@@ -2914,7 +2910,7 @@ void StressTest::Reopen(ThreadState* thread) {
   auto now = clock_->NowMicros();
   fprintf(stdout, "%s Reopening database for the %dth time\n",
           clock_->TimeToString(now / 1000000).c_str(), num_times_reopened_);
-  Open(thread->shared);
+  Open();
 
   if ((FLAGS_sync_fault_injection || FLAGS_disable_wal) && IsStateTracked()) {
     Status s = thread->shared->SaveAtAndAfter(db_);

db_stress_tool/db_stress_test_base.h

@@ -16,7 +16,6 @@ namespace ROCKSDB_NAMESPACE {
 class SystemClock;
 class Transaction;
 class TransactionDB;
-struct TransactionDBOptions;
 
 class StressTest {
  public:
@@ -30,7 +29,7 @@ class StressTest {
 
   bool BuildOptionsTable();
 
-  void InitDb(SharedState*);
+  void InitDb();
   // The initialization work is split into two parts to avoid a circular
   // dependency with `SharedState`.
   virtual void FinishInitDb(SharedState*);
@@ -219,19 +218,12 @@ class StressTest {
 
   void PrintEnv() const;
 
-  void Open(SharedState* shared);
+  void Open();
 
   void Reopen(ThreadState* thread);
 
   void CheckAndSetOptionsForUserTimestamp();
 
-  virtual void RegisterAdditionalListeners() {}
-
-#ifndef ROCKSDB_LITE
-  virtual void PrepareTxnDbOptions(SharedState* /*shared*/,
-                                   TransactionDBOptions& /*txn_db_opts*/) {}
-#endif
-
   std::shared_ptr<Cache> cache_;
   std::shared_ptr<Cache> compressed_cache_;
   std::shared_ptr<const FilterPolicy> filter_policy_;

db_stress_tool/multi_ops_txns_stress.cc

@@ -15,7 +15,6 @@
 #ifndef NDEBUG
 #include "utilities/fault_injection_fs.h"
 #endif  // NDEBUG
-#include "utilities/transactions/write_prepared_txn_db.h"
 
 namespace ROCKSDB_NAMESPACE {
 
@@ -32,21 +31,6 @@ DEFINE_int32(delay_snapshot_read_one_in, 0,
              "With a chance of 1/N, inject a random delay between taking "
              "snapshot and read.");
 
-DEFINE_int32(rollback_one_in, 0,
-             "If non-zero, rollback non-read-only transactions with a "
-             "probability of 1/N.");
-
-DEFINE_int32(clear_wp_commit_cache_one_in, 0,
-             "If non-zero, evict all commit entries from commit cache with a "
-             "probability of 1/N. This options applies to write-prepared and "
-             "write-unprepared transactions.");
-
-extern "C" bool rocksdb_write_prepared_TEST_ShouldClearCommitCache(void) {
-  static Random rand(static_cast<uint32_t>(db_stress_env->NowMicros()));
-  return FLAGS_clear_wp_commit_cache_one_in > 0 &&
-         rand.OneIn(FLAGS_clear_wp_commit_cache_one_in);
-}
-
 // MultiOpsTxnsStressTest can either operate on a database with pre-populated
 // data (possibly from previous ones), or create a new db and preload it with
 // data specified via `-lb_a`, `-ub_a`, `-lb_c`, `-ub_c`, etc. Among these, we
@@ -91,9 +75,8 @@ void MultiOpsTxnsStressTest::KeyGenerator::FinishInit() {
         "Cannot allocate key in [%u, %u)\nStart with a new DB or try change "
         "the number of threads for testing via -threads=<#threads>\n",
         static_cast<unsigned int>(low_), static_cast<unsigned int>(high_));
-    fflush(stdout);
     fflush(stderr);
-    assert(false);
+    std::terminate();
   }
   initialized_ = true;
 }
@@ -148,43 +131,33 @@ void MultiOpsTxnsStressTest::KeyGenerator::UndoAllocation(uint32_t new_val) {
 }
 
 std::string MultiOpsTxnsStressTest::Record::EncodePrimaryKey(uint32_t a) {
-  std::string ret;
-  PutFixed32(&ret, kPrimaryIndexId);
-  PutFixed32(&ret, a);
-
-  char* const buf = &ret[0];
-  std::reverse(buf, buf + sizeof(kPrimaryIndexId));
-  std::reverse(buf + sizeof(kPrimaryIndexId),
-               buf + sizeof(kPrimaryIndexId) + sizeof(a));
-  return ret;
+  char buf[8];
+  EncodeFixed32(buf, kPrimaryIndexId);
+  std::reverse(buf, buf + 4);
+  EncodeFixed32(buf + 4, a);
+  std::reverse(buf + 4, buf + 8);
+  return std::string(buf, sizeof(buf));
 }
 
 std::string MultiOpsTxnsStressTest::Record::EncodeSecondaryKey(uint32_t c) {
-  std::string ret;
-  PutFixed32(&ret, kSecondaryIndexId);
-  PutFixed32(&ret, c);
-
-  char* const buf = &ret[0];
-  std::reverse(buf, buf + sizeof(kSecondaryIndexId));
-  std::reverse(buf + sizeof(kSecondaryIndexId),
-               buf + sizeof(kSecondaryIndexId) + sizeof(c));
-  return ret;
+  char buf[8];
+  EncodeFixed32(buf, kSecondaryIndexId);
+  std::reverse(buf, buf + 4);
+  EncodeFixed32(buf + 4, c);
+  std::reverse(buf + 4, buf + 8);
+  return std::string(buf, sizeof(buf));
 }
 
 std::string MultiOpsTxnsStressTest::Record::EncodeSecondaryKey(uint32_t c,
                                                                uint32_t a) {
-  std::string ret;
-  PutFixed32(&ret, kSecondaryIndexId);
-  PutFixed32(&ret, c);
-  PutFixed32(&ret, a);
-
-  char* const buf = &ret[0];
-  std::reverse(buf, buf + sizeof(kSecondaryIndexId));
-  std::reverse(buf + sizeof(kSecondaryIndexId),
-               buf + sizeof(kSecondaryIndexId) + sizeof(c));
-  std::reverse(buf + sizeof(kSecondaryIndexId) + sizeof(c),
-               buf + sizeof(kSecondaryIndexId) + sizeof(c) + sizeof(a));
-  return ret;
+  char buf[12];
+  EncodeFixed32(buf, kSecondaryIndexId);
+  std::reverse(buf, buf + 4);
+  EncodeFixed32(buf + 4, c);
+  EncodeFixed32(buf + 8, a);
+  std::reverse(buf + 4, buf + 8);
+  std::reverse(buf + 8, buf + 12);
+  return std::string(buf, sizeof(buf));
 }
 
 std::tuple<Status, uint32_t, uint32_t>
@@ -228,26 +201,40 @@ std::string MultiOpsTxnsStressTest::Record::EncodePrimaryKey() const {
 }
 
 std::string MultiOpsTxnsStressTest::Record::EncodePrimaryIndexValue() const {
-  std::string ret;
-  PutFixed32(&ret, b_);
-  PutFixed32(&ret, c_);
-  return ret;
+  char buf[8];
+  EncodeFixed32(buf, b_);
+  EncodeFixed32(buf + 4, c_);
+  return std::string(buf, sizeof(buf));
 }
 
 std::pair<std::string, std::string>
 MultiOpsTxnsStressTest::Record::EncodeSecondaryIndexEntry() const {
-  std::string secondary_index_key = EncodeSecondaryKey(c_, a_);
+  std::string secondary_index_key;
+  char buf[12];
+  EncodeFixed32(buf, kSecondaryIndexId);
+  std::reverse(buf, buf + 4);
+  EncodeFixed32(buf + 4, c_);
+  EncodeFixed32(buf + 8, a_);
+  std::reverse(buf + 4, buf + 8);
+  std::reverse(buf + 8, buf + 12);
+  secondary_index_key.assign(buf, sizeof(buf));
 
   // Secondary index value is always 4-byte crc32 of the secondary key
   std::string secondary_index_value;
-  uint32_t crc =
-      crc32c::Value(secondary_index_key.data(), secondary_index_key.size());
+  uint32_t crc = crc32c::Value(buf, sizeof(buf));
   PutFixed32(&secondary_index_value, crc);
-  return std::make_pair(std::move(secondary_index_key), secondary_index_value);
+  return std::make_pair(secondary_index_key, secondary_index_value);
 }
 
 std::string MultiOpsTxnsStressTest::Record::EncodeSecondaryKey() const {
-  return EncodeSecondaryKey(c_, a_);
+  char buf[12];
+  EncodeFixed32(buf, kSecondaryIndexId);
+  std::reverse(buf, buf + 4);
+  EncodeFixed32(buf + 4, c_);
+  EncodeFixed32(buf + 8, a_);
+  std::reverse(buf + 4, buf + 8);
+  std::reverse(buf + 8, buf + 12);
+  return std::string(buf, sizeof(buf));
 }
 
 Status MultiOpsTxnsStressTest::Record::DecodePrimaryIndexEntry(
@@ -257,22 +244,27 @@ Status MultiOpsTxnsStressTest::Record::DecodePrimaryIndexEntry(
     return Status::Corruption("Primary index key length is not 8");
   }
 
-  uint32_t index_id = 0;
-
-  [[maybe_unused]] bool res = GetFixed32(&primary_index_key, &index_id);
-  assert(res);
-  index_id = EndianSwapValue(index_id);
-
+  const char* const index_id_buf = primary_index_key.data();
+  uint32_t index_id =
+      static_cast<uint32_t>(static_cast<unsigned char>(index_id_buf[0])) << 24;
+  index_id += static_cast<uint32_t>(static_cast<unsigned char>(index_id_buf[1]))
+              << 16;
+  index_id += static_cast<uint32_t>(static_cast<unsigned char>(index_id_buf[2]))
+              << 8;
+  index_id +=
+      static_cast<uint32_t>(static_cast<unsigned char>(index_id_buf[3]));
+  primary_index_key.remove_prefix(sizeof(uint32_t));
   if (index_id != kPrimaryIndexId) {
     std::ostringstream oss;
     oss << "Unexpected primary index id: " << index_id;
     return Status::Corruption(oss.str());
   }
 
-  res = GetFixed32(&primary_index_key, &a_);
-  assert(res);
-  a_ = EndianSwapValue(a_);
-  assert(primary_index_key.empty());
+  const char* const buf = primary_index_key.data();
+  a_ = static_cast<uint32_t>(static_cast<unsigned char>(buf[0])) << 24;
+  a_ += static_cast<uint32_t>(static_cast<unsigned char>(buf[1])) << 16;
+  a_ += static_cast<uint32_t>(static_cast<unsigned char>(buf[2])) << 8;
+  a_ += static_cast<uint32_t>(static_cast<unsigned char>(buf[3]));
 
   if (primary_index_value.size() != 8) {
     return Status::Corruption("Primary index value length is not 8");
@@ -290,28 +282,33 @@ Status MultiOpsTxnsStressTest::Record::DecodeSecondaryIndexEntry(
   uint32_t crc =
       crc32c::Value(secondary_index_key.data(), secondary_index_key.size());
 
-  uint32_t index_id = 0;
-
-  [[maybe_unused]] bool res = GetFixed32(&secondary_index_key, &index_id);
-  assert(res);
-  index_id = EndianSwapValue(index_id);
-
+  const char* const index_id_buf = secondary_index_key.data();
+  uint32_t index_id =
+      static_cast<uint32_t>(static_cast<unsigned char>(index_id_buf[0])) << 24;
+  index_id += static_cast<uint32_t>(static_cast<unsigned char>(index_id_buf[1]))
+              << 16;
+  index_id += static_cast<uint32_t>(static_cast<unsigned char>(index_id_buf[2]))
+              << 8;
+  index_id +=
+      static_cast<uint32_t>(static_cast<unsigned char>(index_id_buf[3]));
+  secondary_index_key.remove_prefix(sizeof(uint32_t));
   if (index_id != kSecondaryIndexId) {
     std::ostringstream oss;
     oss << "Unexpected secondary index id: " << index_id;
     return Status::Corruption(oss.str());
   }
 
+  const char* const buf = secondary_index_key.data();
   assert(secondary_index_key.size() == 8);
-  res = GetFixed32(&secondary_index_key, &c_);
-  assert(res);
-  c_ = EndianSwapValue(c_);
+  c_ = static_cast<uint32_t>(static_cast<unsigned char>(buf[0])) << 24;
+  c_ += static_cast<uint32_t>(static_cast<unsigned char>(buf[1])) << 16;
+  c_ += static_cast<uint32_t>(static_cast<unsigned char>(buf[2])) << 8;
+  c_ += static_cast<uint32_t>(static_cast<unsigned char>(buf[3]));
 
-  assert(secondary_index_key.size() == 4);
-  res = GetFixed32(&secondary_index_key, &a_);
-  assert(res);
-  a_ = EndianSwapValue(a_);
-  assert(secondary_index_key.empty());
+  a_ = static_cast<uint32_t>(static_cast<unsigned char>(buf[4])) << 24;
+  a_ += static_cast<uint32_t>(static_cast<unsigned char>(buf[5])) << 16;
+  a_ += static_cast<uint32_t>(static_cast<unsigned char>(buf[6])) << 8;
+  a_ += static_cast<uint32_t>(static_cast<unsigned char>(buf[7]));
 
   if (secondary_index_value.size() != 4) {
     return Status::Corruption("Secondary index value length is not 4");
@@ -523,35 +520,9 @@ Status MultiOpsTxnsStressTest::TestCustomOperations(
     // Should never reach here.
     assert(false);
   }
-
   return s;
 }
 
-void MultiOpsTxnsStressTest::RegisterAdditionalListeners() {
-  options_.listeners.emplace_back(new MultiOpsTxnsStressListener(this));
-}
-
-#ifndef ROCKSDB_LITE
-void MultiOpsTxnsStressTest::PrepareTxnDbOptions(
-    SharedState* /*shared*/, TransactionDBOptions& txn_db_opts) {
-  // MultiOpsTxnStressTest uses SingleDelete to delete secondary keys, thus we
-  // register this callback to let TxnDb know that when rolling back
-  // a transaction, use only SingleDelete to cancel prior Put from the same
-  // transaction if applicable.
-  txn_db_opts.rollback_deletion_type_callback =
-      [](TransactionDB* /*db*/, ColumnFamilyHandle* /*column_family*/,
-         const Slice& key) {
-        Slice ks = key;
-        uint32_t index_id = 0;
-        [[maybe_unused]] bool res = GetFixed32(&ks, &index_id);
-        assert(res);
-        index_id = EndianSwapValue(index_id);
-        assert(index_id <= Record::kSecondaryIndexId);
-        return index_id == Record::kSecondaryIndexId;
-      };
-}
-#endif  // !ROCKSDB_LITE
-
 Status MultiOpsTxnsStressTest::PrimaryKeyUpdateTxn(ThreadState* thread,
                                                    uint32_t old_a,
                                                    uint32_t old_a_pos,
@@ -590,10 +561,8 @@ Status MultiOpsTxnsStressTest::PrimaryKeyUpdateTxn(ThreadState* thread,
     }
     if (s.IsNotFound()) {
       thread->stats.AddGets(/*ngets=*/1, /*nfounds=*/0);
-    } else if (s.IsBusy() || s.IsIncomplete()) {
+    } else if (s.IsBusy()) {
       // ignore.
-      // Incomplete also means rollback by application. See the transaction
-      // implementations.
     } else {
       thread->stats.AddErrors(1);
     }
@@ -662,16 +631,6 @@ Status MultiOpsTxnsStressTest::PrimaryKeyUpdateTxn(ThreadState* thread,
     return s;
   }
 
-  if (FLAGS_rollback_one_in > 0 && thread->rand.OneIn(FLAGS_rollback_one_in)) {
-    s = Status::Incomplete();
-    return s;
-  }
-
-  s = WriteToCommitTimeWriteBatch(*txn);
-  if (!s.ok()) {
-    return s;
-  }
-
   s = txn->Commit();
 
   auto& key_gen = key_gen_for_a_.at(thread->tid);
@@ -718,12 +677,11 @@ Status MultiOpsTxnsStressTest::SecondaryKeyUpdateTxn(ThreadState* thread,
           Record::kPrimaryIndexEntrySize + Record::kSecondaryIndexEntrySize);
       return;
     } else if (s.IsBusy() || s.IsTimedOut() || s.IsTryAgain() ||
-               s.IsMergeInProgress() || s.IsIncomplete()) {
+               s.IsMergeInProgress()) {
       // ww-conflict detected, or
       // lock cannot be acquired, or
       // memtable history is not large enough for conflict checking, or
-      // Merge operation cannot be resolved, or
-      // application rollback.
+      // Merge operation cannot be resolved.
       // TODO (yanqin) add stats for other cases?
     } else if (s.IsNotFound()) {
       // ignore.
@@ -769,9 +727,8 @@ Status MultiOpsTxnsStressTest::SecondaryKeyUpdateTxn(ThreadState* thread,
     Record record;
     s = record.DecodeSecondaryIndexEntry(it->key(), it->value());
     if (!s.ok()) {
-      fprintf(stderr, "Cannot decode secondary key (%s => %s): %s\n",
-              it->key().ToString(true).c_str(),
-              it->value().ToString(true).c_str(), s.ToString().c_str());
+      fprintf(stderr, "Cannot decode secondary key: %s\n",
+              s.ToString().c_str());
       assert(false);
       break;
     }
@@ -792,31 +749,21 @@ Status MultiOpsTxnsStressTest::SecondaryKeyUpdateTxn(ThreadState* thread,
     } else if (s.IsNotFound()) {
       // We can also fail verification here.
       std::ostringstream oss;
-      auto* dbimpl = static_cast_with_check<DBImpl>(db_->GetRootDB());
-      assert(dbimpl);
-      oss << "snap " << read_opts.snapshot->GetSequenceNumber()
-          << " (published " << dbimpl->GetLastPublishedSequence()
-          << "), pk should exist: " << Slice(pk).ToString(true);
+      oss << "pk should exist: " << Slice(pk).ToString(true);
       fprintf(stderr, "%s\n", oss.str().c_str());
       assert(false);
       break;
     }
     if (!s.ok()) {
-      std::ostringstream oss;
-      auto* dbimpl = static_cast_with_check<DBImpl>(db_->GetRootDB());
-      assert(dbimpl);
-      oss << "snap " << read_opts.snapshot->GetSequenceNumber()
-          << " (published " << dbimpl->GetLastPublishedSequence() << "), "
-          << s.ToString();
-      fprintf(stderr, "%s\n", oss.str().c_str());
+      fprintf(stderr, "%s\n", s.ToString().c_str());
       assert(false);
       break;
     }
     auto result = Record::DecodePrimaryIndexValue(value);
     s = std::get<0>(result);
     if (!s.ok()) {
-      fprintf(stderr, "Cannot decode primary index value %s: %s\n",
-              Slice(value).ToString(true).c_str(), s.ToString().c_str());
+      fprintf(stderr, "Cannot decode primary index value: %s\n",
+              s.ToString().c_str());
       assert(false);
       break;
     }
@@ -824,12 +771,8 @@ Status MultiOpsTxnsStressTest::SecondaryKeyUpdateTxn(ThreadState* thread,
     uint32_t c = std::get<2>(result);
     if (c != old_c) {
       std::ostringstream oss;
-      auto* dbimpl = static_cast_with_check<DBImpl>(db_->GetRootDB());
-      assert(dbimpl);
-      oss << "snap " << read_opts.snapshot->GetSequenceNumber()
-          << " (published " << dbimpl->GetLastPublishedSequence()
-          << "), pk/sk mismatch. pk: (a=" << record.a_value() << ", "
-          << "c=" << c << "), sk: (c=" << old_c << ")";
+      oss << "c in primary index does not match secondary index: " << c
+          << " != " << old_c;
       s = Status::Corruption();
       fprintf(stderr, "%s\n", oss.str().c_str());
       assert(false);
@@ -868,16 +811,6 @@ Status MultiOpsTxnsStressTest::SecondaryKeyUpdateTxn(ThreadState* thread,
     return s;
   }
 
-  if (FLAGS_rollback_one_in > 0 && thread->rand.OneIn(FLAGS_rollback_one_in)) {
-    s = Status::Incomplete();
-    return s;
-  }
-
-  s = WriteToCommitTimeWriteBatch(*txn);
-  if (!s.ok()) {
-    return s;
-  }
-
   s = txn->Commit();
 
   if (s.ok()) {
@@ -923,7 +856,7 @@ Status MultiOpsTxnsStressTest::UpdatePrimaryIndexValueTxn(ThreadState* thread,
     } else if (s.IsInvalidArgument()) {
       // ignored.
     } else if (s.IsBusy() || s.IsTimedOut() || s.IsTryAgain() ||
-               s.IsMergeInProgress() || s.IsIncomplete()) {
+               s.IsMergeInProgress()) {
       // ignored.
     } else {
       thread->stats.AddErrors(1);
@@ -941,8 +874,8 @@ Status MultiOpsTxnsStressTest::UpdatePrimaryIndexValueTxn(ThreadState* thread,
   auto result = Record::DecodePrimaryIndexValue(value);
   if (!std::get<0>(result).ok()) {
     s = std::get<0>(result);
-    fprintf(stderr, "Cannot decode primary index value %s: %s\n",
-            Slice(value).ToString(true).c_str(), s.ToString().c_str());
+    fprintf(stderr, "Cannot decode primary index value: %s\n",
+            s.ToString().c_str());
     assert(false);
     return s;
   }
@@ -959,17 +892,6 @@ Status MultiOpsTxnsStressTest::UpdatePrimaryIndexValueTxn(ThreadState* thread,
   if (!s.ok()) {
     return s;
   }
-
-  if (FLAGS_rollback_one_in > 0 && thread->rand.OneIn(FLAGS_rollback_one_in)) {
-    s = Status::Incomplete();
-    return s;
-  }
-
-  s = WriteToCommitTimeWriteBatch(*txn);
-  if (!s.ok()) {
-    return s;
-  }
-
   s = txn->Commit();
   if (s.ok()) {
     delete txn;
@@ -1128,15 +1050,12 @@ void MultiOpsTxnsStressTest::VerifyDb(ThreadState* thread) const {
   // First, iterate primary index.
   size_t primary_index_entries_count = 0;
   {
-    std::string iter_ub_str;
-    PutFixed32(&iter_ub_str, Record::kPrimaryIndexId + 1);
-    std::reverse(iter_ub_str.begin(), iter_ub_str.end());
+    char buf[4];
+    EncodeFixed32(buf, Record::kPrimaryIndexId + 1);
+    std::reverse(buf, buf + sizeof(buf));
+    std::string iter_ub_str(buf, sizeof(buf));
     Slice iter_ub = iter_ub_str;
 
-    std::string start_key;
-    PutFixed32(&start_key, Record::kPrimaryIndexId);
-    std::reverse(start_key.begin(), start_key.end());
-
     // This `ReadOptions` is for validation purposes. Ignore
     // `FLAGS_rate_limit_user_ops` to avoid slowing any validation.
     ReadOptions ropts;
@@ -1145,7 +1064,7 @@ void MultiOpsTxnsStressTest::VerifyDb(ThreadState* thread) const {
     ropts.iterate_upper_bound = &iter_ub;
 
     std::unique_ptr<Iterator> it(db_->NewIterator(ropts));
-    for (it->Seek(start_key); it->Valid(); it->Next()) {
+    for (it->SeekToFirst(); it->Valid(); it->Next()) {
       Record record;
       Status s = record.DecodePrimaryIndexEntry(it->key(), it->value());
       if (!s.ok()) {
@@ -1182,9 +1101,10 @@ void MultiOpsTxnsStressTest::VerifyDb(ThreadState* thread) const {
   // Second, iterate secondary index.
   size_t secondary_index_entries_count = 0;
   {
-    std::string start_key;
-    PutFixed32(&start_key, Record::kSecondaryIndexId);
-    std::reverse(start_key.begin(), start_key.end());
+    char buf[4];
+    EncodeFixed32(buf, Record::kSecondaryIndexId);
+    std::reverse(buf, buf + sizeof(buf));
+    const std::string start_key(buf, sizeof(buf));
 
     // This `ReadOptions` is for validation purposes. Ignore
     // `FLAGS_rate_limit_user_ops` to avoid slowing any validation.
@@ -1198,8 +1118,7 @@ void MultiOpsTxnsStressTest::VerifyDb(ThreadState* thread) const {
       Record record;
       Status s = record.DecodeSecondaryIndexEntry(it->key(), it->value());
       if (!s.ok()) {
-        oss << "Cannot decode secondary index entry "
-            << it->key().ToString(true) << "=>" << it->value().ToString(true);
+        oss << "Cannot decode secondary index entry";
         VerificationAbort(thread->shared, oss.str(), s);
         assert(false);
         return;
@@ -1213,7 +1132,7 @@ void MultiOpsTxnsStressTest::VerifyDb(ThreadState* thread) const {
       s = db_->Get(ropts, pk, &value);
       if (!s.ok()) {
         oss << "Error searching pk " << Slice(pk).ToString(true) << ". "
-            << s.ToString() << ". sk " << it->key().ToString(true);
+            << s.ToString();
         VerificationAbort(thread->shared, oss.str(), s);
         assert(false);
         return;
@@ -1229,10 +1148,8 @@ void MultiOpsTxnsStressTest::VerifyDb(ThreadState* thread) const {
       }
       uint32_t c_in_primary = std::get<2>(result);
       if (c_in_primary != record.c_value()) {
-        oss << "Pk/sk mismatch. pk: " << Slice(pk).ToString(true) << "=>"
-            << Slice(value).ToString(true) << " (a=" << record.a_value()
-            << ", c=" << c_in_primary << "), sk: " << it->key().ToString(true)
-            << " (c=" << record.c_value() << ")";
+        oss << "Pk/sk mismatch. pk: (c=" << c_in_primary
+            << "), sk: (c=" << record.c_value() << ")";
         VerificationAbort(thread->shared, oss.str(), s);
         assert(false);
         return;
@@ -1250,75 +1167,6 @@ void MultiOpsTxnsStressTest::VerifyDb(ThreadState* thread) const {
   }
 }
 
-void MultiOpsTxnsStressTest::VerifyPkSkFast(int job_id) {
-  const Snapshot* const snapshot = db_->GetSnapshot();
-  assert(snapshot);
-  ManagedSnapshot snapshot_guard(db_, snapshot);
-
-  std::ostringstream oss;
-  auto* dbimpl = static_cast_with_check<DBImpl>(db_->GetRootDB());
-  assert(dbimpl);
-
-  oss << "Job " << job_id << ": [" << snapshot->GetSequenceNumber() << ","
-      << dbimpl->GetLastPublishedSequence() << "] ";
-
-  std::string start_key;
-  PutFixed32(&start_key, Record::kSecondaryIndexId);
-  std::reverse(start_key.begin(), start_key.end());
-
-  // This `ReadOptions` is for validation purposes. Ignore
-  // `FLAGS_rate_limit_user_ops` to avoid slowing any validation.
-  ReadOptions ropts;
-  ropts.snapshot = snapshot;
-  ropts.total_order_seek = true;
-
-  std::unique_ptr<Iterator> it(db_->NewIterator(ropts));
-  for (it->Seek(start_key); it->Valid(); it->Next()) {
-    Record record;
-    Status s = record.DecodeSecondaryIndexEntry(it->key(), it->value());
-    if (!s.ok()) {
-      oss << "Cannot decode secondary index entry " << it->key().ToString(true)
-          << "=>" << it->value().ToString(true);
-      fprintf(stderr, "%s\n", oss.str().c_str());
-      fflush(stderr);
-      assert(false);
-    }
-    // After decoding secondary index entry, we know a and c. Crc is verified
-    // in decoding phase.
-    //
-    // Form a primary key and search in the primary index.
-    std::string pk = Record::EncodePrimaryKey(record.a_value());
-    std::string value;
-    s = db_->Get(ropts, pk, &value);
-    if (!s.ok()) {
-      oss << "Error searching pk " << Slice(pk).ToString(true) << ". "
-          << s.ToString() << ". sk " << it->key().ToString(true);
-      fprintf(stderr, "%s\n", oss.str().c_str());
-      fflush(stderr);
-      assert(false);
-    }
-    auto result = Record::DecodePrimaryIndexValue(value);
-    s = std::get<0>(result);
-    if (!s.ok()) {
-      oss << "Error decoding primary index value "
-          << Slice(value).ToString(true) << ". " << s.ToString();
-      fprintf(stderr, "%s\n", oss.str().c_str());
-      fflush(stderr);
-      assert(false);
-    }
-    uint32_t c_in_primary = std::get<2>(result);
-    if (c_in_primary != record.c_value()) {
-      oss << "Pk/sk mismatch. pk: " << Slice(pk).ToString(true) << "=>"
-          << Slice(value).ToString(true) << " (a=" << record.a_value()
-          << ", c=" << c_in_primary << "), sk: " << it->key().ToString(true)
-          << " (c=" << record.c_value() << ")";
-      fprintf(stderr, "%s\n", oss.str().c_str());
-      fflush(stderr);
-      assert(false);
-    }
-  }
-}
-
 std::pair<uint32_t, uint32_t> MultiOpsTxnsStressTest::ChooseExistingA(
     ThreadState* thread) {
   uint32_t tid = thread->tid;
@@ -1345,22 +1193,6 @@ uint32_t MultiOpsTxnsStressTest::GenerateNextC(ThreadState* thread) {
   return key_gen->Allocate();
 }
 
-#ifndef ROCKSDB_LITE
-Status MultiOpsTxnsStressTest::WriteToCommitTimeWriteBatch(Transaction& txn) {
-  WriteBatch* ctwb = txn.GetCommitTimeWriteBatch();
-  assert(ctwb);
-  // Do not change the content in key_buf.
-  static constexpr char key_buf[sizeof(Record::kMetadataPrefix) + 4] = {
-      '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\xff'};
-
-  uint64_t counter_val = counter_.Next();
-  char val_buf[sizeof(counter_val)];
-  EncodeFixed64(val_buf, counter_val);
-  return ctwb->Put(Slice(key_buf, sizeof(key_buf)),
-                   Slice(val_buf, sizeof(val_buf)));
-}
-#endif  // !ROCKSDB_LITE
-
 std::string MultiOpsTxnsStressTest::KeySpaces::EncodeTo() const {
   std::string result;
   PutFixed32(&result, lb_a);
@@ -1596,9 +1428,8 @@ void MultiOpsTxnsStressTest::ScanExistingDb(SharedState* shared, int threads) {
       Record record;
       Status s = record.DecodePrimaryIndexEntry(it->key(), it->value());
       if (!s.ok()) {
-        fprintf(stderr, "Cannot decode primary index entry (%s => %s): %s\n",
-                it->key().ToString(true).c_str(),
-                it->value().ToString(true).c_str(), s.ToString().c_str());
+        fprintf(stderr, "Cannot decode primary index entry: %s\n",
+                s.ToString().c_str());
         assert(false);
       }
       uint32_t a = record.a_value();

db_stress_tool/multi_ops_txns_stress.h

@@ -111,7 +111,6 @@ class MultiOpsTxnsStressTest : public StressTest {
  public:
   class Record {
    public:
-    static constexpr uint32_t kMetadataPrefix = 0;
     static constexpr uint32_t kPrimaryIndexId = 1;
     static constexpr uint32_t kSecondaryIndexId = 2;
 
@@ -262,13 +261,6 @@ class MultiOpsTxnsStressTest : public StressTest {
       ThreadState* thread,
       const std::vector<int>& rand_column_families) override;
 
-  void RegisterAdditionalListeners() override;
-
-#ifndef ROCKSDB_LITE
-  void PrepareTxnDbOptions(SharedState* /*shared*/,
-                           TransactionDBOptions& txn_db_opts) override;
-#endif  // !ROCKSDB_LITE
-
   Status PrimaryKeyUpdateTxn(ThreadState* thread, uint32_t old_a,
                              uint32_t old_a_pos, uint32_t new_a);
 
@@ -288,17 +280,7 @@ class MultiOpsTxnsStressTest : public StressTest {
     VerifyDb(thread);
   }
 
-  void VerifyPkSkFast(int job_id);
-
  protected:
-  class Counter {
-   public:
-    uint64_t Next() { return value_.fetch_add(1); }
-
-   private:
-    std::atomic<uint64_t> value_ = Env::Default()->NowNanos();
-  };
-
   using KeySet = std::set<uint32_t>;
   class KeyGenerator {
    public:
@@ -348,21 +330,9 @@ class MultiOpsTxnsStressTest : public StressTest {
 
   uint32_t GenerateNextC(ThreadState* thread);
 
-#ifndef ROCKSDB_LITE
-  // Some applications, e.g. MyRocks writes a KV pair to the database via
-  // commit-time-write-batch (ctwb) in additional to the transaction's regular
-  // write batch. The key is usually constant representing some system
-  // metadata, while the value is monoticailly increasing which represents the
-  // actual value of the metadata. Method WriteToCommitTimeWriteBatch()
-  // emulates this scenario.
-  Status WriteToCommitTimeWriteBatch(Transaction& txn);
-#endif  //! ROCKSDB_LITE
-
   std::vector<std::unique_ptr<KeyGenerator>> key_gen_for_a_;
   std::vector<std::unique_ptr<KeyGenerator>> key_gen_for_c_;
 
-  Counter counter_{};
-
  private:
   struct KeySpaces {
     uint32_t lb_a = 0;
@@ -400,38 +370,5 @@ class InvariantChecker {
                 "MultiOpsTxnsStressTest::Record::c_ must be 4 bytes");
 };
 
-class MultiOpsTxnsStressListener : public EventListener {
- public:
-  explicit MultiOpsTxnsStressListener(MultiOpsTxnsStressTest* stress_test)
-      : stress_test_(stress_test) {
-    assert(stress_test_);
-  }
-
-#ifndef ROCKSDB_LITE
-  ~MultiOpsTxnsStressListener() override {}
-
-  void OnFlushCompleted(DB* db, const FlushJobInfo& info) override {
-    assert(db);
-#ifdef NDEBUG
-    (void)db;
-#endif
-    assert(info.cf_id == 0);
-    stress_test_->VerifyPkSkFast(info.job_id);
-  }
-
-  void OnCompactionCompleted(DB* db, const CompactionJobInfo& info) override {
-    assert(db);
-#ifdef NDEBUG
-    (void)db;
-#endif
-    assert(info.cf_id == 0);
-    stress_test_->VerifyPkSkFast(info.job_id);
-  }
-#endif  //! ROCKSDB_LITE
-
- private:
-  MultiOpsTxnsStressTest* const stress_test_ = nullptr;
-};
-
 }  // namespace ROCKSDB_NAMESPACE
 #endif  // GFLAGS

db_stress_tool/no_batched_ops_stress.cc

@@ -12,7 +12,6 @@
 #ifndef NDEBUG
 #include "utilities/fault_injection_fs.h"
 #endif // NDEBUG
-#include "rocksdb/utilities/transaction_db.h"
 
 namespace ROCKSDB_NAMESPACE {
 class NonBatchedOpsStressTest : public StressTest {
@@ -85,8 +84,8 @@ class NonBatchedOpsStressTest : public StressTest {
             // move to the next item in the iterator
             s = Status::NotFound();
           }
-          VerifyOrSyncValue(static_cast<int>(cf), i, options, shared, from_db,
-                            s, true);
+          VerifyValue(static_cast<int>(cf), i, options, shared, from_db, s,
+                      true);
           if (from_db.length()) {
             PrintKeyValue(static_cast<int>(cf), static_cast<uint32_t>(i),
                           from_db.data(), from_db.length());
@@ -102,8 +101,8 @@ class NonBatchedOpsStressTest : public StressTest {
           std::string keystr = Key(i);
           Slice k = keystr;
           Status s = db_->Get(options, column_families_[cf], k, &from_db);
-          VerifyOrSyncValue(static_cast<int>(cf), i, options, shared, from_db,
-                            s, true);
+          VerifyValue(static_cast<int>(cf), i, options, shared, from_db, s,
+                      true);
           if (from_db.length()) {
             PrintKeyValue(static_cast<int>(cf), static_cast<uint32_t>(i),
                           from_db.data(), from_db.length());
@@ -131,8 +130,8 @@ class NonBatchedOpsStressTest : public StressTest {
           for (size_t j = 0; j < batch_size; ++j) {
             Status s = statuses[j];
             std::string from_db = values[j].ToString();
-            VerifyOrSyncValue(static_cast<int>(cf), i + j, options, shared,
-                              from_db, s, true);
+            VerifyValue(static_cast<int>(cf), i + j, options, shared, from_db,
+                        s, true);
             if (from_db.length()) {
               PrintKeyValue(static_cast<int>(cf), static_cast<uint32_t>(i + j),
                             from_db.data(), from_db.length());
@@ -175,8 +174,8 @@ class NonBatchedOpsStressTest : public StressTest {
           if (number_of_operands) {
             from_db = values[number_of_operands - 1].ToString();
           }
-          VerifyOrSyncValue(static_cast<int>(cf), i, options, shared, from_db,
-                            s, true);
+          VerifyValue(static_cast<int>(cf), i, options, shared, from_db, s,
+                      true);
           if (from_db.length()) {
             PrintKeyValue(static_cast<int>(cf), static_cast<uint32_t>(i),
                           from_db.data(), from_db.length());
@@ -563,8 +562,8 @@ class NonBatchedOpsStressTest : public StressTest {
       Slice k = key_str2;
       std::string from_db;
       Status s = db_->Get(read_opts, cfh, k, &from_db);
-      if (!VerifyOrSyncValue(rand_column_family, rand_key, read_opts, shared,
-                             from_db, s, true)) {
+      if (!VerifyValue(rand_column_family, rand_key, read_opts, shared, from_db,
+                       s, true)) {
         return s;
       }
     }
@@ -632,14 +631,33 @@ class NonBatchedOpsStressTest : public StressTest {
   Status TestDelete(ThreadState* thread, WriteOptions& write_opts,
                     const std::vector<int>& rand_column_families,
                     const std::vector<int64_t>& rand_keys,
-                    std::unique_ptr<MutexLock>& /* lock */) override {
+                    std::unique_ptr<MutexLock>& lock) override {
     int64_t rand_key = rand_keys[0];
     int rand_column_family = rand_column_families[0];
     auto shared = thread->shared;
+    int64_t max_key = shared->GetMaxKey();
 
     // OPERATION delete
-    std::string write_ts_str = NowNanosStr();
-    Slice write_ts = write_ts_str;
+    // If the chosen key does not allow overwrite and it does not exist,
+    // choose another key.
+    std::string write_ts_str;
+    Slice write_ts;
+    while (!shared->AllowsOverwrite(rand_key) &&
+           !shared->Exists(rand_column_family, rand_key)) {
+      lock.reset();
+      rand_key = thread->rand.Next() % max_key;
+      rand_column_family = thread->rand.Next() % FLAGS_column_families;
+      lock.reset(
+          new MutexLock(shared->GetMutexForKey(rand_column_family, rand_key)));
+      if (FLAGS_user_timestamp_size > 0) {
+        write_ts_str = NowNanosStr();
+        write_ts = write_ts_str;
+      }
+    }
+    if (write_ts.size() == 0 && FLAGS_user_timestamp_size) {
+      write_ts_str = NowNanosStr();
+      write_ts = write_ts_str;
+    }
 
     std::string key_str = Key(rand_key);
     Slice key = key_str;
@@ -864,24 +882,16 @@ class NonBatchedOpsStressTest : public StressTest {
   }
 #endif  // ROCKSDB_LITE
 
-  bool VerifyOrSyncValue(int cf, int64_t key, const ReadOptions& /*opts*/,
-                         SharedState* shared, const std::string& value_from_db,
-                         const Status& s, bool strict = false) const {
+  bool VerifyValue(int cf, int64_t key, const ReadOptions& /*opts*/,
+                   SharedState* shared, const std::string& value_from_db,
+                   const Status& s, bool strict = false) const {
     if (shared->HasVerificationFailedYet()) {
       return false;
     }
     // compare value_from_db with the value in the shared state
+    char value[kValueMaxLen];
     uint32_t value_base = shared->Get(cf, key);
     if (value_base == SharedState::UNKNOWN_SENTINEL) {
-      if (s.ok()) {
-        // Value exists in db, update state to reflect that
-        Slice slice(value_from_db);
-        value_base = GetValueBase(slice);
-        shared->Put(cf, key, value_base, false);
-      } else if (s.IsNotFound()) {
-        // Value doesn't exist in db, update state to reflect that
-        shared->SingleDelete(cf, key, false);
-      }
       return true;
     }
     if (value_base == SharedState::DELETION_SENTINEL && !strict) {
@@ -889,7 +899,6 @@ class NonBatchedOpsStressTest : public StressTest {
     }
 
     if (s.ok()) {
-      char value[kValueMaxLen];
       if (value_base == SharedState::DELETION_SENTINEL) {
         VerificationAbort(shared, "Unexpected value found", cf, key);
         return false;
@@ -912,21 +921,6 @@ class NonBatchedOpsStressTest : public StressTest {
     }
     return true;
   }
-
-#ifndef ROCKSDB_LITE
-  void PrepareTxnDbOptions(SharedState* shared,
-                           TransactionDBOptions& txn_db_opts) override {
-    txn_db_opts.rollback_deletion_type_callback =
-        [shared](TransactionDB*, ColumnFamilyHandle*, const Slice& key) {
-          assert(shared);
-          uint64_t key_num = 0;
-          bool ok = GetIntVal(key.ToString(), &key_num);
-          assert(ok);
-          (void)ok;
-          return !shared->AllowsOverwrite(key_num);
-        };
-  }
-#endif  // ROCKSDB_LITE
 };
 
 StressTest* CreateNonBatchedOpsStressTest() {

include/rocksdb/c.h

@@ -1766,8 +1766,6 @@ extern ROCKSDB_LIBRARY_API void rocksdb_lru_cache_options_set_memory_allocator(
 
 extern ROCKSDB_LIBRARY_API rocksdb_cache_t* rocksdb_cache_create_lru(
     size_t capacity);
-extern ROCKSDB_LIBRARY_API rocksdb_cache_t*
-rocksdb_cache_create_lru_with_strict_capacity_limit(size_t capacity);
 extern ROCKSDB_LIBRARY_API rocksdb_cache_t* rocksdb_cache_create_lru_opts(
     rocksdb_lru_cache_options_t*);
 extern ROCKSDB_LIBRARY_API void rocksdb_cache_destroy(rocksdb_cache_t* cache);

include/rocksdb/cleanable.h

@@ -19,12 +19,11 @@ class Cleanable {
   Cleanable(Cleanable&) = delete;
   Cleanable& operator=(Cleanable&) = delete;
 
-  // Executes all the registered cleanups
   ~Cleanable();
 
   // Move constructor and move assignment is allowed.
-  Cleanable(Cleanable&&) noexcept;
-  Cleanable& operator=(Cleanable&&) noexcept;
+  Cleanable(Cleanable&&);
+  Cleanable& operator=(Cleanable&&);
 
   // Clients are allowed to register function/arg1/arg2 triples that
   // will be invoked when this iterator is destroyed.
@@ -32,14 +31,8 @@ class Cleanable {
   // Note that unlike all of the preceding methods, this method is
   // not abstract and therefore clients should not override it.
   using CleanupFunction = void (*)(void* arg1, void* arg2);
-
-  // Add another Cleanup to the list
   void RegisterCleanup(CleanupFunction function, void* arg1, void* arg2);
-
-  // Move the cleanups owned by this Cleanable to another Cleanable, adding to
-  // any existing cleanups it has
   void DelegateCleanupsTo(Cleanable* other);
-
   // DoCleanup and also resets the pointers for reuse
   inline void Reset() {
     DoCleanup();
@@ -47,8 +40,6 @@ class Cleanable {
     cleanup_.next = nullptr;
   }
 
-  inline bool HasCleanups() { return cleanup_.function != nullptr; }
-
  protected:
   struct Cleanup {
     CleanupFunction function;
@@ -77,52 +68,4 @@ class Cleanable {
   }
 };
 
-// A copyable, reference-counted pointer to a simple Cleanable that only
-// performs registered cleanups after all copies are destroy. This is like
-// shared_ptr<Cleanable> but works more efficiently with wrapping the pointer
-// in an outer Cleanable (see RegisterCopyWith() and MoveAsCleanupTo()).
-// WARNING: if you create a reference cycle, for example:
-//   SharedCleanablePtr scp;
-//   scp.Allocate();
-//   scp.RegisterCopyWith(&*scp);
-// It will prevent cleanups from ever happening!
-class SharedCleanablePtr {
- public:
-  // Empy/null pointer
-  SharedCleanablePtr() {}
-  // Copy and move constructors and assignment
-  SharedCleanablePtr(const SharedCleanablePtr& from);
-  SharedCleanablePtr(SharedCleanablePtr&& from) noexcept;
-  SharedCleanablePtr& operator=(const SharedCleanablePtr& from);
-  SharedCleanablePtr& operator=(SharedCleanablePtr&& from) noexcept;
-  // Destructor (decrement refcount if non-null)
-  ~SharedCleanablePtr();
-  // Create a new simple Cleanable and make this assign this pointer to it.
-  // (Reset()s first if necessary.)
-  void Allocate();
-  // Reset to empty/null (decrement refcount if previously non-null)
-  void Reset();
-  // Dereference to pointed-to Cleanable
-  Cleanable& operator*();
-  Cleanable* operator->();
-  // Get as raw pointer to Cleanable
-  Cleanable* get();
-
-  // Creates a (virtual) copy of this SharedCleanablePtr and registers its
-  // destruction with target, so that the cleanups registered with the
-  // Cleanable pointed to by this can only happen after the cleanups in the
-  // target Cleanable are run.
-  // No-op if this is empty (nullptr).
-  void RegisterCopyWith(Cleanable* target);
-
-  // Moves (virtually) this shared pointer to a new cleanup in the target.
-  // This is essentilly a move semantics version of RegisterCopyWith(), for
-  // performance optimization. No-op if this is empty (nullptr).
-  void MoveAsCleanupTo(Cleanable* target);
-
- private:
-  struct Impl;
-  Impl* ptr_ = nullptr;
-};
-
 }  // namespace ROCKSDB_NAMESPACE

include/rocksdb/compaction_filter.h

@@ -39,7 +39,6 @@ class CompactionFilter : public Customizable {
   enum class Decision {
     kKeep,
     kRemove,
-    kRemoveWithSingleDelete,
     kChangeValue,
     kRemoveAndSkipUntil,
     kChangeBlobIndex,  // used internally by BlobDB.

include/rocksdb/convenience.h

@@ -518,8 +518,7 @@ Status VerifySstFileChecksum(const Options& options,
 Status VerifySstFileChecksum(const Options& options,
                              const EnvOptions& env_options,
                              const ReadOptions& read_options,
-                             const std::string& file_path,
-                             const SequenceNumber& largest_seqno = 0);
+                             const std::string& file_path);
 #endif  // ROCKSDB_LITE
 
 }  // namespace ROCKSDB_NAMESPACE

include/rocksdb/db.h

@@ -1440,6 +1440,39 @@ class DB {
   virtual Status EnableFileDeletions(bool force = true) = 0;
 
 #ifndef ROCKSDB_LITE
+  // GetLiveFiles followed by GetSortedWalFiles can generate a lossless backup
+
+  // Retrieve the list of all files in the database. The files are
+  // relative to the dbname and are not absolute paths. Despite being relative
+  // paths, the file names begin with "/". The valid size of the manifest file
+  // is returned in manifest_file_size. The manifest file is an ever growing
+  // file, but only the portion specified by manifest_file_size is valid for
+  // this snapshot. Setting flush_memtable to true does Flush before recording
+  // the live files. Setting flush_memtable to false is useful when we don't
+  // want to wait for flush which may have to wait for compaction to complete
+  // taking an indeterminate time.
+  //
+  // In case you have multiple column families, even if flush_memtable is true,
+  // you still need to call GetSortedWalFiles after GetLiveFiles to compensate
+  // for new data that arrived to already-flushed column families while other
+  // column families were flushing
+  virtual Status GetLiveFiles(std::vector<std::string>&,
+                              uint64_t* manifest_file_size,
+                              bool flush_memtable = true) = 0;
+
+  // Retrieve the sorted list of all wal files with earliest file first
+  virtual Status GetSortedWalFiles(VectorLogPtr& files) = 0;
+
+  // Retrieve information about the current wal file
+  //
+  // Note that the log might have rolled after this call in which case
+  // the current_log_file would not point to the current log file.
+  //
+  // Additionally, for the sake of optimization current_log_file->StartSequence
+  // would always be set to 0
+  virtual Status GetCurrentWalFile(
+      std::unique_ptr<LogFile>* current_log_file) = 0;
+
   // Retrieves the creation time of the oldest file in the DB.
   // This API only works if max_open_files = -1, if it is not then
   // Status returned is Status::NotSupported()
@@ -1484,30 +1517,26 @@ class DB {
   // path relative to the db directory. eg. 000001.sst, /archive/000003.log
   virtual Status DeleteFile(std::string name) = 0;
 
-  // Obtains a list of all live table (SST) files and how they fit into the
-  // LSM-trees, such as column family, level, key range, etc.
-  // This builds a de-normalized form of GetAllColumnFamilyMetaData().
-  // For information about all files in a DB, use GetLiveFilesStorageInfo().
+  // Returns a list of all table files with their level, start key
+  // and end key
   virtual void GetLiveFilesMetaData(
       std::vector<LiveFileMetaData>* /*metadata*/) {}
 
-  // Return a list of all table (SST) and blob files checksum info.
+  // Return a list of all table and blob files checksum info.
   // Note: This function might be of limited use because it cannot be
-  // synchronized with other "live files" APIs. GetLiveFilesStorageInfo()
-  // is recommended instead.
+  // synchronized with GetLiveFiles.
   virtual Status GetLiveFilesChecksumInfo(FileChecksumList* checksum_list) = 0;
 
+  // EXPERIMENTAL: This function is not yet feature-complete.
   // Get information about all live files that make up a DB, for making
   // live copies (Checkpoint, backups, etc.) or other storage-related purposes.
-  // If creating a live copy, use DisableFileDeletions() before and
-  // EnableFileDeletions() after to prevent deletions.
-  // For LSM-tree metadata, use Get*MetaData() functions instead.
+  // Use DisableFileDeletions() before and EnableFileDeletions() after to
+  // preserve the files for live copy.
   virtual Status GetLiveFilesStorageInfo(
       const LiveFilesStorageInfoOptions& opts,
       std::vector<LiveFileStorageInfo>* files) = 0;
 
-  // Obtains the LSM-tree meta data of the specified column family of the DB,
-  // including metadata for each live table (SST) file in that column family.
+  // Obtains the meta data of the specified column family of the DB.
   virtual void GetColumnFamilyMetaData(ColumnFamilyHandle* /*column_family*/,
                                        ColumnFamilyMetaData* /*metadata*/) {}
 
@@ -1516,43 +1545,12 @@ class DB {
     GetColumnFamilyMetaData(DefaultColumnFamily(), metadata);
   }
 
-  // Obtains the LSM-tree meta data of all column families of the DB,
-  // including metadata for each live table (SST) file in the DB.
+  // Obtains the meta data of all column families for the DB.
+  // The returned map contains one entry for each column family indexed by the
+  // name of the column family.
   virtual void GetAllColumnFamilyMetaData(
       std::vector<ColumnFamilyMetaData>* /*metadata*/) {}
 
-  // Retrieve the list of all files in the database except WAL files. The files
-  // are relative to the dbname (or db_paths/cf_paths), not absolute paths.
-  // (Not recommended with db_paths/cf_paths because that information is not
-  // returned.) Despite being relative paths, the file names begin with "/".
-  // The valid size of the manifest file is returned in manifest_file_size.
-  // The manifest file is an ever growing file, but only the portion specified
-  // by manifest_file_size is valid for this snapshot. Setting flush_memtable
-  // to true does Flush before recording the live files. Setting flush_memtable
-  // to false is useful when we don't want to wait for flush which may have to
-  // wait for compaction to complete taking an indeterminate time.
-  //
-  // NOTE: Although GetLiveFiles() followed by GetSortedWalFiles() can generate
-  // a lossless backup, GetLiveFilesStorageInfo() is strongly recommended
-  // instead, because it ensures a single consistent view of all files is
-  // captured in one call.
-  virtual Status GetLiveFiles(std::vector<std::string>&,
-                              uint64_t* manifest_file_size,
-                              bool flush_memtable = true) = 0;
-
-  // Retrieve the sorted list of all wal files with earliest file first
-  virtual Status GetSortedWalFiles(VectorLogPtr& files) = 0;
-
-  // Retrieve information about the current wal file
-  //
-  // Note that the log might have rolled after this call in which case
-  // the current_log_file would not point to the current log file.
-  //
-  // Additionally, for the sake of optimization current_log_file->StartSequence
-  // would always be set to 0
-  virtual Status GetCurrentWalFile(
-      std::unique_ptr<LogFile>* current_log_file) = 0;
-
   // IngestExternalFile() will load a list of external SST files (1) into the DB
   // Two primary modes are supported:
   // - Duplicate keys in the new files will overwrite exiting keys (default)

include/rocksdb/options.h

@@ -767,14 +767,6 @@ struct DBOptions {
 
   // Allow the OS to mmap file for reading sst tables.
   // Not recommended for 32-bit OS.
-  // When the option is set to true and compression is disabled, the blocks
-  // will not be copied and will be read directly from the mmap-ed memory
-  // area, and the block will not be inserted into the block cache. However,
-  // checksums will still be checked if ReadOptions.verify_checksums is set
-  // to be true. It means a checksum check every time a block is read, more
-  // than the setup where the option is set to false and the block cache is
-  // used. The common use of the options is to run RocksDB on ramfs, where
-  // checksum verification is usually not needed.
   // Default: false
   bool allow_mmap_reads = false;
 

include/rocksdb/utilities/ldb_cmd.h

@@ -70,7 +70,6 @@ class LDBCommand {
   static const std::string ARG_BLOB_GARBAGE_COLLECTION_FORCE_THRESHOLD;
   static const std::string ARG_BLOB_COMPACTION_READAHEAD_SIZE;
   static const std::string ARG_DECODE_BLOB_INDEX;
-  static const std::string ARG_DUMP_UNCOMPRESSED_BLOBS;
 
   struct ParsedParams {
     std::string cmd;

include/rocksdb/utilities/transaction_db.h

@@ -222,20 +222,6 @@ struct TransactionDBOptions {
   // pending writes into the database. A value of 0 or less means no limit.
   int64_t default_write_batch_flush_threshold = 0;
 
-  // This option is valid only for write-prepared/write-unprepared. Transaction
-  // will rely on this callback to determine if a key should be rolled back
-  // with Delete or SingleDelete when necessary. If the callback returns true,
-  // then SingleDelete should be used. If the callback is not callable or the
-  // callback returns false, then a Delete is used.
-  // The application should ensure thread-safety of this callback.
-  // The callback should not throw because RocksDB is not exception-safe.
-  // The callback may be removed if we allow mixing Delete and SingleDelete in
-  // the future.
-  std::function<bool(TransactionDB* /*db*/,
-                     ColumnFamilyHandle* /*column_family*/,
-                     const Slice& /*key*/)>
-      rollback_deletion_type_callback;
-
  private:
   // 128 entries
   // Should the default value change, please also update wp_snapshot_cache_bits

include/rocksdb/version.h

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

include/rocksdb/write_batch.h

@@ -63,9 +63,7 @@ struct SavePoint {
 
 class WriteBatch : public WriteBatchBase {
  public:
-  explicit WriteBatch(size_t reserved_bytes = 0, size_t max_bytes = 0)
-      : WriteBatch(reserved_bytes, max_bytes, 0, 0) {}
-
+  explicit WriteBatch(size_t reserved_bytes = 0, size_t max_bytes = 0);
   // `protection_bytes_per_key` is the number of bytes used to store
   // protection information for each key entry. Currently supported values are
   // zero (disabled) and eight.
@@ -320,17 +318,8 @@ class WriteBatch : public WriteBatchBase {
 
    protected:
     friend class WriteBatchInternal;
-    enum class OptionState {
-      kUnknown,
-      kDisabled,
-      kEnabled,
-    };
-    virtual OptionState WriteAfterCommit() const {
-      return OptionState::kUnknown;
-    }
-    virtual OptionState WriteBeforePrepare() const {
-      return OptionState::kUnknown;
-    }
+    virtual bool WriteAfterCommit() const { return true; }
+    virtual bool WriteBeforePrepare() const { return false; }
   };
   Status Iterate(Handler* handler) const;
 
@@ -413,9 +402,6 @@ class WriteBatch : public WriteBatchBase {
   struct ProtectionInfo;
   size_t GetProtectionBytesPerKey() const;
 
-  // Clears prot_info_ if there are no entries.
-  void ClearProtectionInfoIfEmpty();
-
  private:
   friend class WriteBatchInternal;
   friend class LocalSavePoint;

microbench/db_basic_bench.cc

@@ -525,7 +525,6 @@ static void DBGet(benchmark::State& state) {
   bool enable_statistics = state.range(3);
   bool negative_query = state.range(4);
   bool enable_filter = state.range(5);
-  bool mmap = state.range(6);
   uint64_t key_num = max_data / per_key_size;
 
   // setup DB
@@ -534,21 +533,13 @@ static void DBGet(benchmark::State& state) {
   if (enable_statistics) {
     options.statistics = CreateDBStatistics();
   }
-  if (mmap) {
-    options.allow_mmap_reads = true;
-    options.compression = kNoCompression;
-  }
   options.compaction_style = compaction_style;
 
-  BlockBasedTableOptions table_options;
   if (enable_filter) {
+    BlockBasedTableOptions table_options;
     table_options.filter_policy.reset(NewBloomFilterPolicy(10, false));
+    options.table_factory.reset(NewBlockBasedTableFactory(table_options));
   }
-  if (mmap) {
-    table_options.no_block_cache = true;
-    table_options.block_restart_interval = 1;
-  }
-  options.table_factory.reset(NewBlockBasedTableFactory(table_options));
 
   auto rnd = Random(301 + state.thread_index());
   KeyGenerator kg(&rnd, key_num);
@@ -582,9 +573,6 @@ static void DBGet(benchmark::State& state) {
   }
 
   auto ro = ReadOptions();
-  if (mmap) {
-    ro.verify_checksums = false;
-  }
   size_t not_found = 0;
   if (negative_query) {
     for (auto _ : state) {
@@ -628,10 +616,8 @@ static void DBGetArguments(benchmark::internal::Benchmark* b) {
         for (bool enable_statistics : {false, true}) {
           for (bool negative_query : {false, true}) {
             for (bool enable_filter : {false, true}) {
-              for (bool mmap : {false, true}) {
-                b->Args({comp_style, max_data, per_key_size, enable_statistics,
-                         negative_query, enable_filter, mmap});
-              }
+              b->Args({comp_style, max_data, per_key_size, enable_statistics,
+                       negative_query, enable_filter});
             }
           }
         }
@@ -639,10 +625,10 @@ static void DBGetArguments(benchmark::internal::Benchmark* b) {
     }
   }
   b->ArgNames({"comp_style", "max_data", "per_key_size", "enable_statistics",
-               "negative_query", "enable_filter", "mmap"});
+               "negative_query", "enable_filter"});
 }
 
-static constexpr uint64_t kDBGetNum = 1l << 20;
+static constexpr uint64_t kDBGetNum = 10l << 10;
 BENCHMARK(DBGet)->Threads(1)->Iterations(kDBGetNum)->Apply(DBGetArguments);
 BENCHMARK(DBGet)->Threads(8)->Iterations(kDBGetNum / 8)->Apply(DBGetArguments);
 

options/options_settable_test.cc

@@ -34,7 +34,6 @@ namespace ROCKSDB_NAMESPACE {
 #ifndef ROCKSDB_LITE
 #if defined OS_LINUX || defined OS_WIN
 #ifndef __clang__
-#ifndef ROCKSDB_UBSAN_RUN
 
 class OptionsSettableTest : public testing::Test {
  public:
@@ -582,7 +581,6 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) {
   delete[] mcfo2_ptr;
   delete[] cfo_clean_ptr;
 }
-#endif  // !ROCKSDB_UBSAN_RUN
 #endif  // !__clang__
 #endif  // OS_LINUX || OS_WIN
 #endif  // !ROCKSDB_LITE

src.mk

@@ -211,7 +211,6 @@ LIB_SOURCES =                                                   \
   trace_replay/block_cache_tracer.cc                            \
   trace_replay/io_tracer.cc                                     \
   util/build_version.cc                                         \
-  util/cleanable.cc                                             \
   util/coding.cc                                                \
   util/compaction_job_stats_impl.cc                             \
   util/comparator.cc                                            \

table/block_based/block_based_table_reader.cc

@@ -117,6 +117,14 @@ Status ReadBlockFromFile(
   return s;
 }
 
+// Release the cached entry and decrement its ref count.
+// Do not force erase
+void ReleaseCachedEntry(void* arg, void* h) {
+  Cache* cache = reinterpret_cast<Cache*>(arg);
+  Cache::Handle* handle = reinterpret_cast<Cache::Handle*>(h);
+  cache->Release(handle, false /* erase_if_last_ref */);
+}
+
 // For hash based index, return false if table_properties->prefix_extractor_name
 // and prefix_extractor both exist and match, otherwise true.
 inline bool PrefixExtractorChangedHelper(
@@ -2562,11 +2570,10 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,
       iiter_unique_ptr.reset(iiter);
     }
 
-    uint64_t prev_offset = std::numeric_limits<uint64_t>::max();
+    uint64_t offset = std::numeric_limits<uint64_t>::max();
     autovector<BlockHandle, MultiGetContext::MAX_BATCH_SIZE> block_handles;
     autovector<CachableEntry<Block>, MultiGetContext::MAX_BATCH_SIZE> results;
     autovector<Status, MultiGetContext::MAX_BATCH_SIZE> statuses;
-    MultiGetContext::Mask reused_mask = 0;
     char stack_buf[kMultiGetReadStackBufSize];
     std::unique_ptr<char[]> block_buf;
     {
@@ -2628,19 +2635,16 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,
 
         statuses.emplace_back();
         results.emplace_back();
-        if (v.handle.offset() == prev_offset) {
-          // This key can reuse the previous block (later on).
-          // Mark previous as "reused"
-          reused_mask |= MultiGetContext::Mask{1} << (block_handles.size() - 1);
-          // Use null handle to indicate this one reuses same block as
-          // previous.
+        if (v.handle.offset() == offset) {
+          // We're going to reuse the block for this key later on. No need to
+          // look it up now. Place a null handle
           block_handles.emplace_back(BlockHandle::NullBlockHandle());
           continue;
         }
         // Lookup the cache for the given data block referenced by an index
         // iterator value (i.e BlockHandle). If it exists in the cache,
         // initialize block to the contents of the data block.
-        prev_offset = v.handle.offset();
+        offset = v.handle.offset();
         BlockHandle handle = v.handle;
         BlockCacheLookupContext lookup_data_block_context(
             TableReaderCaller::kUserMultiGet);
@@ -2744,7 +2748,6 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,
     DataBlockIter first_biter;
     DataBlockIter next_biter;
     size_t idx_in_batch = 0;
-    SharedCleanablePtr shared_cleanable;
     for (auto miter = sst_file_range.begin(); miter != sst_file_range.end();
          ++miter) {
       Status s;
@@ -2755,8 +2758,7 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,
       bool first_block = true;
       do {
         DataBlockIter* biter = nullptr;
-        bool reusing_prev_block;
-        bool later_reused;
+        bool reusing_block = true;
         uint64_t referenced_data_size = 0;
         bool does_referenced_key_exist = false;
         BlockCacheLookupContext lookup_data_block_context(
@@ -2770,16 +2772,13 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,
             NewDataBlockIterator<DataBlockIter>(
                 read_options, results[idx_in_batch], &first_biter,
                 statuses[idx_in_batch]);
-            reusing_prev_block = false;
+            reusing_block = false;
           } else {
             // If handler is null and result is empty, then the status is never
             // set, which should be the initial value: ok().
             assert(statuses[idx_in_batch].ok());
-            reusing_prev_block = true;
           }
           biter = &first_biter;
-          later_reused =
-              (reused_mask & (MultiGetContext::Mask{1} << idx_in_batch)) != 0;
           idx_in_batch++;
         } else {
           IndexValue v = iiter->value();
@@ -2799,8 +2798,7 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,
               BlockType::kData, get_context, &lookup_data_block_context,
               Status(), nullptr);
           biter = &next_biter;
-          reusing_prev_block = false;
-          later_reused = false;
+          reusing_block = false;
         }
 
         if (read_options.read_tier == kBlockCacheTier &&
@@ -2825,56 +2823,28 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,
           break;
         }
 
-        // Reusing blocks complicates pinning/Cleanable, because the cache
-        // entry referenced by biter can only be released once all returned
-        // pinned values are released. This code previously did an extra
-        // block_cache Ref for each reuse, but that unnecessarily increases
-        // block cache contention. Instead we can use a variant of shared_ptr
-        // to release in block cache only once.
-        //
-        // Although the biter loop below might SaveValue multiple times for
-        // merges, just one value_pinner suffices, as MultiGet will merge
-        // the operands before returning to the API user.
-        Cleanable* value_pinner;
-        if (biter->IsValuePinned()) {
-          if (reusing_prev_block) {
-            // Note that we don't yet know if the MultiGet results will need
-            // to pin this block, so we might wrap a block for sharing and
-            // still end up with 1 (or 0) pinning ref. Not ideal but OK.
-            //
-            // Here we avoid adding redundant cleanups if we didn't end up
-            // delegating the cleanup from last time around.
-            if (!biter->HasCleanups()) {
-              assert(shared_cleanable.get());
-              if (later_reused) {
-                shared_cleanable.RegisterCopyWith(biter);
-              } else {
-                shared_cleanable.MoveAsCleanupTo(biter);
-              }
-            }
-          } else if (later_reused) {
-            assert(biter->HasCleanups());
-            // Make the existing cleanups on `biter` sharable:
-            shared_cleanable.Allocate();
-            // Move existing `biter` cleanup(s) to `shared_cleanable`
-            biter->DelegateCleanupsTo(&*shared_cleanable);
-            // Reference `shared_cleanable` as new cleanup for `biter`
-            shared_cleanable.RegisterCopyWith(biter);
-          }
-          assert(biter->HasCleanups());
-          value_pinner = biter;
-        } else {
-          value_pinner = nullptr;
-        }
-
         // Call the *saver function on each entry/block until it returns false
         for (; biter->Valid(); biter->Next()) {
           ParsedInternalKey parsed_key;
+          Cleanable dummy;
+          Cleanable* value_pinner = nullptr;
           Status pik_status = ParseInternalKey(
               biter->key(), &parsed_key, false /* log_err_key */);  // TODO
           if (!pik_status.ok()) {
             s = pik_status;
           }
+          if (biter->IsValuePinned()) {
+            if (reusing_block) {
+              Cache* block_cache = rep_->table_options.block_cache.get();
+              assert(biter->cache_handle() != nullptr);
+              block_cache->Ref(biter->cache_handle());
+              dummy.RegisterCleanup(&ReleaseCachedEntry, block_cache,
+                                    biter->cache_handle());
+              value_pinner = &dummy;
+            } else {
+              value_pinner = biter;
+            }
+          }
           if (!get_context->SaveValue(parsed_key, biter->value(), &matched,
                                       value_pinner)) {
             if (get_context->State() == GetContext::GetState::kFound) {
@@ -2888,10 +2858,7 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,
           s = biter->status();
         }
         // Write the block cache access.
-        // XXX: There appear to be 'break' statements above that bypass this
-        // writing of the block cache trace record
-        if (block_cache_tracer_ && block_cache_tracer_->is_tracing_enabled() &&
-            !reusing_prev_block) {
+        if (block_cache_tracer_ && block_cache_tracer_->is_tracing_enabled()) {
           // Avoid making copy of block_key, cf_name, and referenced_key when
           // constructing the access record.
           Slice referenced_key;

table/cleanable_test.cc

@@ -3,10 +3,6 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
-#include "rocksdb/cleanable.h"
-
-#include <gtest/gtest.h>
-
 #include <functional>
 
 #include "port/port.h"
@@ -272,115 +268,6 @@ TEST_F(CleanableTest, PinnableSlice) {
   }
 }
 
-static void Decrement(void* intptr, void*) { --*static_cast<int*>(intptr); }
-
-// Allow unit testing moved-from data
-template <class T>
-void MarkInitializedForClangAnalyze(T& t) {
-  // No net effect, but confuse analyzer. (Published advice doesn't work.)
-  char* p = reinterpret_cast<char*>(&t);
-  std::swap(*p, *p);
-}
-
-TEST_F(CleanableTest, SharedWrapCleanables) {
-  int val = 5;
-  Cleanable c1, c2;
-  c1.RegisterCleanup(&Decrement, &val, nullptr);
-  c1.RegisterCleanup(&Decrement, &val, nullptr);
-  ASSERT_TRUE(c1.HasCleanups());
-  ASSERT_FALSE(c2.HasCleanups());
-
-  SharedCleanablePtr scp1;
-  ASSERT_EQ(scp1.get(), nullptr);
-
-  // No-ops
-  scp1.RegisterCopyWith(&c2);
-  scp1.MoveAsCleanupTo(&c2);
-
-  ASSERT_FALSE(c2.HasCleanups());
-  c2.RegisterCleanup(&Decrement, &val, nullptr);
-  c2.RegisterCleanup(&Decrement, &val, nullptr);
-  c2.RegisterCleanup(&Decrement, &val, nullptr);
-
-  scp1.Allocate();
-  ASSERT_NE(scp1.get(), nullptr);
-  ASSERT_FALSE(scp1->HasCleanups());
-
-  // Copy ctor (alias scp2 = scp1)
-  SharedCleanablePtr scp2{scp1};
-  ASSERT_EQ(scp1.get(), scp2.get());
-
-  c1.DelegateCleanupsTo(&*scp1);
-  ASSERT_TRUE(scp1->HasCleanups());
-  ASSERT_TRUE(scp2->HasCleanups());
-  ASSERT_FALSE(c1.HasCleanups());
-
-  SharedCleanablePtr scp3;
-  ASSERT_EQ(scp3.get(), nullptr);
-
-  // Copy operator (alias scp3 = scp2 = scp1)
-  scp3 = scp2;
-
-  // Make scp2 point elsewhere
-  scp2.Allocate();
-  c2.DelegateCleanupsTo(&*scp2);
-
-  ASSERT_EQ(val, 5);
-  // Move operator, invoke old c2 cleanups
-  scp2 = std::move(scp1);
-  ASSERT_EQ(val, 2);
-  MarkInitializedForClangAnalyze(scp1);
-  ASSERT_EQ(scp1.get(), nullptr);
-
-  // Move ctor
-  {
-    SharedCleanablePtr scp4{std::move(scp3)};
-    MarkInitializedForClangAnalyze(scp3);
-    ASSERT_EQ(scp3.get(), nullptr);
-    ASSERT_EQ(scp4.get(), scp2.get());
-
-    scp2.Reset();
-    ASSERT_EQ(val, 2);
-    // invoke old c1 cleanups
-  }
-  ASSERT_EQ(val, 0);
-}
-
-TEST_F(CleanableTest, CleanableWrapShared) {
-  int val = 5;
-  SharedCleanablePtr scp1, scp2;
-  scp1.Allocate();
-  scp1->RegisterCleanup(&Decrement, &val, nullptr);
-  scp1->RegisterCleanup(&Decrement, &val, nullptr);
-
-  scp2.Allocate();
-  scp2->RegisterCleanup(&Decrement, &val, nullptr);
-  scp2->RegisterCleanup(&Decrement, &val, nullptr);
-  scp2->RegisterCleanup(&Decrement, &val, nullptr);
-
-  {
-    Cleanable c1;
-    {
-      Cleanable c2, c3;
-      scp1.RegisterCopyWith(&c1);
-      scp1.MoveAsCleanupTo(&c2);
-      ASSERT_TRUE(c1.HasCleanups());
-      ASSERT_TRUE(c2.HasCleanups());
-      ASSERT_EQ(scp1.get(), nullptr);
-      scp2.MoveAsCleanupTo(&c3);
-      ASSERT_TRUE(c3.HasCleanups());
-      ASSERT_EQ(scp2.get(), nullptr);
-      c2.Reset();
-      ASSERT_FALSE(c2.HasCleanups());
-      ASSERT_EQ(val, 5);
-      // invoke cleanups from scp2
-    }
-    ASSERT_EQ(val, 2);
-    // invoke cleanups from scp1
-  }
-  ASSERT_EQ(val, 0);
-}
-
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

table/iterator.cc

@@ -8,13 +8,93 @@
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 
 #include "rocksdb/iterator.h"
-
 #include "memory/arena.h"
 #include "table/internal_iterator.h"
 #include "table/iterator_wrapper.h"
 
 namespace ROCKSDB_NAMESPACE {
 
+Cleanable::Cleanable() {
+  cleanup_.function = nullptr;
+  cleanup_.next = nullptr;
+}
+
+Cleanable::~Cleanable() { DoCleanup(); }
+
+Cleanable::Cleanable(Cleanable&& other) {
+  *this = std::move(other);
+}
+
+Cleanable& Cleanable::operator=(Cleanable&& other) {
+  if (this != &other) {
+    cleanup_ = other.cleanup_;
+    other.cleanup_.function = nullptr;
+    other.cleanup_.next = nullptr;
+  }
+  return *this;
+}
+
+// If the entire linked list was on heap we could have simply add attach one
+// link list to another. However the head is an embeded object to avoid the cost
+// of creating objects for most of the use cases when the Cleanable has only one
+// Cleanup to do. We could put evernything on heap if benchmarks show no
+// negative impact on performance.
+// Also we need to iterate on the linked list since there is no pointer to the
+// tail. We can add the tail pointer but maintainin it might negatively impact
+// the perforamnce for the common case of one cleanup where tail pointer is not
+// needed. Again benchmarks could clarify that.
+// Even without a tail pointer we could iterate on the list, find the tail, and
+// have only that node updated without the need to insert the Cleanups one by
+// one. This however would be redundant when the source Cleanable has one or a
+// few Cleanups which is the case most of the time.
+// TODO(myabandeh): if the list is too long we should maintain a tail pointer
+// and have the entire list (minus the head that has to be inserted separately)
+// merged with the target linked list at once.
+void Cleanable::DelegateCleanupsTo(Cleanable* other) {
+  assert(other != nullptr);
+  if (cleanup_.function == nullptr) {
+    return;
+  }
+  Cleanup* c = &cleanup_;
+  other->RegisterCleanup(c->function, c->arg1, c->arg2);
+  c = c->next;
+  while (c != nullptr) {
+    Cleanup* next = c->next;
+    other->RegisterCleanup(c);
+    c = next;
+  }
+  cleanup_.function = nullptr;
+  cleanup_.next = nullptr;
+}
+
+void Cleanable::RegisterCleanup(Cleanable::Cleanup* c) {
+  assert(c != nullptr);
+  if (cleanup_.function == nullptr) {
+    cleanup_.function = c->function;
+    cleanup_.arg1 = c->arg1;
+    cleanup_.arg2 = c->arg2;
+    delete c;
+  } else {
+    c->next = cleanup_.next;
+    cleanup_.next = c;
+  }
+}
+
+void Cleanable::RegisterCleanup(CleanupFunction func, void* arg1, void* arg2) {
+  assert(func != nullptr);
+  Cleanup* c;
+  if (cleanup_.function == nullptr) {
+    c = &cleanup_;
+  } else {
+    c = new Cleanup;
+    c->next = cleanup_.next;
+    cleanup_.next = c;
+  }
+  c->function = func;
+  c->arg1 = arg1;
+  c->arg2 = arg2;
+}
+
 Status Iterator::GetProperty(std::string prop_name, std::string* prop) {
   if (prop == nullptr) {
     return Status::InvalidArgument("prop is nullptr");

table/multiget_context.h

@@ -97,10 +97,7 @@ class MultiGetContext {
   // that need to be performed
   static const int MAX_BATCH_SIZE = 32;
 
-  // A bitmask of at least MAX_BATCH_SIZE - 1 bits, so that
-  // Mask{1} << MAX_BATCH_SIZE is well defined
-  using Mask = uint64_t;
-  static_assert(MAX_BATCH_SIZE < sizeof(Mask) * 8);
+  static_assert(MAX_BATCH_SIZE < 64, "MAX_BATCH_SIZE cannot exceed 63");
 
   MultiGetContext(autovector<KeyContext*, MAX_BATCH_SIZE>* sorted_keys,
                   size_t begin, size_t num_keys, SequenceNumber snapshot,
@@ -141,7 +138,7 @@ class MultiGetContext {
     char lookup_key_stack_buf[sizeof(LookupKey) * MAX_LOOKUP_KEYS_ON_STACK];
   std::array<KeyContext*, MAX_BATCH_SIZE> sorted_keys_;
   size_t num_keys_;
-  Mask value_mask_;
+  uint64_t value_mask_;
   uint64_t value_size_;
   std::unique_ptr<char[]> lookup_key_heap_buf;
   LookupKey* lookup_key_ptr_;
@@ -174,7 +171,7 @@ class MultiGetContext {
       Iterator(const Range* range, size_t idx)
           : range_(range), ctx_(range->ctx_), index_(idx) {
         while (index_ < range_->end_ &&
-               (Mask{1} << index_) &
+               (uint64_t{1} << index_) &
                    (range_->ctx_->value_mask_ | range_->skip_mask_))
           index_++;
       }
@@ -184,7 +181,7 @@ class MultiGetContext {
 
       Iterator& operator++() {
         while (++index_ < range_->end_ &&
-               (Mask{1} << index_) &
+               (uint64_t{1} << index_) &
                    (range_->ctx_->value_mask_ | range_->skip_mask_))
           ;
         return *this;
@@ -238,22 +235,22 @@ class MultiGetContext {
 
     bool empty() const { return RemainingMask() == 0; }
 
-    void SkipIndex(size_t index) { skip_mask_ |= Mask{1} << index; }
+    void SkipIndex(size_t index) { skip_mask_ |= uint64_t{1} << index; }
 
     void SkipKey(const Iterator& iter) { SkipIndex(iter.index_); }
 
     bool IsKeySkipped(const Iterator& iter) const {
-      return skip_mask_ & (Mask{1} << iter.index_);
+      return skip_mask_ & (uint64_t{1} << iter.index_);
     }
 
     // Update the value_mask_ in MultiGetContext so its
     // immediately reflected in all the Range Iterators
     void MarkKeyDone(Iterator& iter) {
-      ctx_->value_mask_ |= (Mask{1} << iter.index_);
+      ctx_->value_mask_ |= (uint64_t{1} << iter.index_);
     }
 
     bool CheckKeyDone(Iterator& iter) const {
-      return ctx_->value_mask_ & (Mask{1} << iter.index_);
+      return ctx_->value_mask_ & (uint64_t{1} << iter.index_);
     }
 
     uint64_t KeysLeft() const { return BitsSetToOne(RemainingMask()); }
@@ -272,15 +269,15 @@ class MultiGetContext {
     MultiGetContext* ctx_;
     size_t start_;
     size_t end_;
-    Mask skip_mask_;
+    uint64_t skip_mask_;
 
     Range(MultiGetContext* ctx, size_t num_keys)
         : ctx_(ctx), start_(0), end_(num_keys), skip_mask_(0) {
       assert(num_keys < 64);
     }
 
-    Mask RemainingMask() const {
-      return (((Mask{1} << end_) - 1) & ~((Mask{1} << start_) - 1) &
+    uint64_t RemainingMask() const {
+      return (((uint64_t{1} << end_) - 1) & ~((uint64_t{1} << start_) - 1) &
               ~(ctx_->value_mask_ | skip_mask_));
     }
   };

test_util/transaction_test_util.cc

@@ -91,7 +91,7 @@ Status RandomTransactionInserter::DBGet(
   Status s;
   // Five digits (since the largest uint16_t is 65535) plus the NUL
   // end char.
-  char prefix_buf[6] = {0};
+  char prefix_buf[6];
   // Pad prefix appropriately so we can iterate over each set
   assert(set_i + 1 <= 9999);
   snprintf(prefix_buf, sizeof(prefix_buf), "%.4u", set_i + 1);
@@ -165,11 +165,7 @@ bool RandomTransactionInserter::DoInsert(DB* db, Transaction* txn,
       // Increment key
       std::string sum = ToString(int_value + incr);
       if (txn != nullptr) {
-        if ((set_i % 4) != 0) {
-          s = txn->SingleDelete(key);
-        } else {
-          s = txn->Delete(key);
-        }
+        s = txn->SingleDelete(key);
         if (!get_for_update && (s.IsBusy() || s.IsTimedOut())) {
           // If the initial get was not for update, then the key is not locked
           // before put and put could fail due to concurrent writes.

test_util/transaction_test_util.h

@@ -33,23 +33,6 @@ class Random64;
 // RandomTransactionInserter with similar arguments using the same DB.
 class RandomTransactionInserter {
  public:
-  static bool RollbackDeletionTypeCallback(const Slice& key) {
-    // These are hard-coded atm. See how RandomTransactionInserter::DoInsert()
-    // determines whether to use SingleDelete or Delete for a key.
-    assert(key.size() >= 4);
-    const char* ptr = key.data();
-    assert(ptr);
-    while (ptr && ptr < key.data() + 4 && *ptr == '0') {
-      ++ptr;
-    }
-    std::string prefix(ptr, 4 - (ptr - key.data()));
-    unsigned long set_i = std::stoul(prefix);
-    assert(set_i > 0);
-    assert(set_i <= 9999);
-    --set_i;
-    return ((set_i % 4) != 0);
-  }
-
   // num_keys is the number of keys in each set.
   // num_sets is the number of sets of keys.
   // cmt_delay_ms is the delay between prepare (if there is any) and commit

tools/check_format_compatible.sh

@@ -122,7 +122,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=("6.6.fb" "6.7.fb" "6.8.fb" "6.9.fb" "6.10.fb" "6.11.fb" "6.12.fb" "6.13.fb" "6.14.fb" "6.15.fb" "6.16.fb" "6.17.fb" "6.18.fb" "6.19.fb" "6.20.fb" "6.21.fb" "6.22.fb" "6.23.fb" "6.24.fb" "6.25.fb" "6.26.fb" "6.27.fb" "6.28.fb" "6.29.fb" "7.0.fb" "7.1.fb" "7.2.fb")
+declare -a db_forward_with_options_refs=("6.6.fb" "6.7.fb" "6.8.fb" "6.9.fb" "6.10.fb" "6.11.fb" "6.12.fb" "6.13.fb" "6.14.fb" "6.15.fb" "6.16.fb" "6.17.fb" "6.18.fb" "6.19.fb" "6.20.fb" "6.21.fb" "6.22.fb" "6.23.fb" "6.24.fb" "6.25.fb" "6.26.fb" "6.27.fb" "6.28.fb" "6.29.fb" "7.0.fb" "7.1.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

@@ -410,10 +410,6 @@ DEFINE_double(read_random_exp_range, 0.0,
 
 DEFINE_bool(histogram, false, "Print histogram of operation timings");
 
-DEFINE_bool(confidence_interval_only, false,
-            "Print 95% confidence interval upper and lower bounds only for "
-            "aggregate stats.");
-
 DEFINE_bool(enable_numa, false,
             "Make operations aware of NUMA architecture and bind memory "
             "and cpus corresponding to nodes together. In NUMA, memory "
@@ -2323,83 +2319,28 @@ class CombinedStats {
   }
 
   void Report(const std::string& bench_name) {
-    if (throughput_ops_.size() < 2) {
-      // skip if there are not enough samples
-      return;
-    }
-
     const char* name = bench_name.c_str();
     int num_runs = static_cast<int>(throughput_ops_.size());
 
     if (throughput_mbs_.size() == throughput_ops_.size()) {
       fprintf(stdout,
-              "%s [AVG %d runs] : %d (± %d) ops/sec; %6.1f (± %.1f) MB/sec\n",
-              name, num_runs, static_cast<int>(CalcAvg(throughput_ops_)),
-              static_cast<int>(CalcConfidence95(throughput_ops_)),
-              CalcAvg(throughput_mbs_), CalcConfidence95(throughput_mbs_));
-    } else {
-      fprintf(stdout, "%s [AVG %d runs] : %d (± %d) ops/sec\n", name, num_runs,
-              static_cast<int>(CalcAvg(throughput_ops_)),
-              static_cast<int>(CalcConfidence95(throughput_ops_)));
-    }
-  }
-
-  void ReportWithConfidenceIntervals(const std::string& bench_name) {
-    if (throughput_ops_.size() < 2) {
-      // skip if there are not enough samples
-      return;
-    }
-
-    const char* name = bench_name.c_str();
-    int num_runs = static_cast<int>(throughput_ops_.size());
-
-    int ops_avg = static_cast<int>(CalcAvg(throughput_ops_));
-    int ops_confidence_95 = static_cast<int>(CalcConfidence95(throughput_ops_));
-
-    if (throughput_mbs_.size() == throughput_ops_.size()) {
-      double mbs_avg = CalcAvg(throughput_mbs_);
-      double mbs_confidence_95 = CalcConfidence95(throughput_mbs_);
-      fprintf(stdout,
-              "%s [CI95 %d runs] : (%d, %d) ops/sec; (%.1f, %.1f) MB/sec\n",
-              name, num_runs, ops_avg - ops_confidence_95,
-              ops_avg + ops_confidence_95, mbs_avg - mbs_confidence_95,
-              mbs_avg + mbs_confidence_95);
-    } else {
-      fprintf(stdout, "%s [CI95 %d runs] : (%d, %d) ops/sec\n", name, num_runs,
-              ops_avg - ops_confidence_95, ops_avg + ops_confidence_95);
-    }
-  }
-
-  void ReportFinal(const std::string& bench_name) {
-    if (throughput_ops_.size() < 2) {
-      // skip if there are not enough samples
-      return;
-    }
-
-    const char* name = bench_name.c_str();
-    int num_runs = static_cast<int>(throughput_ops_.size());
-
-    if (throughput_mbs_.size() == throughput_ops_.size()) {
-      fprintf(stdout,
-              "%s [AVG    %d runs] : %d (± %d) ops/sec; %6.1f (± %.1f) MB/sec\n"
+              "%s [AVG    %d runs] : %d ops/sec; %6.1f MB/sec\n"
               "%s [MEDIAN %d runs] : %d ops/sec; %6.1f MB/sec\n",
               name, num_runs, static_cast<int>(CalcAvg(throughput_ops_)),
-              static_cast<int>(CalcConfidence95(throughput_ops_)),
-              CalcAvg(throughput_mbs_), CalcConfidence95(throughput_mbs_), name,
-              num_runs, static_cast<int>(CalcMedian(throughput_ops_)),
+              CalcAvg(throughput_mbs_), name, num_runs,
+              static_cast<int>(CalcMedian(throughput_ops_)),
               CalcMedian(throughput_mbs_));
     } else {
       fprintf(stdout,
-              "%s [AVG    %d runs] : %d (± %d) ops/sec\n"
+              "%s [AVG    %d runs] : %d ops/sec\n"
               "%s [MEDIAN %d runs] : %d ops/sec\n",
-              name, num_runs, static_cast<int>(CalcAvg(throughput_ops_)),
-              static_cast<int>(CalcConfidence95(throughput_ops_)), name,
+              name, num_runs, static_cast<int>(CalcAvg(throughput_ops_)), name,
               num_runs, static_cast<int>(CalcMedian(throughput_ops_)));
     }
   }
 
  private:
-  double CalcAvg(std::vector<double>& data) {
+  double CalcAvg(std::vector<double> data) {
     double avg = 0;
     for (double x : data) {
       avg += x;
@@ -2408,20 +2349,7 @@ class CombinedStats {
     return avg;
   }
 
-  // Calculates 95% CI assuming a normal distribution of samples.
-  // Samples are not from a normal distribution, but it still
-  // provides useful approximation.
-  double CalcConfidence95(std::vector<double>& data) {
-    assert(data.size() > 1);
-    double avg = CalcAvg(data);
-    double std_error = CalcStdDev(data, avg) / std::sqrt(data.size());
-
-    // Z score for the 97.5 percentile
-    // see https://en.wikipedia.org/wiki/1.96
-    return 1.959964 * std_error;
-  }
-
-  double CalcMedian(std::vector<double>& data) {
+  double CalcMedian(std::vector<double> data) {
     assert(data.size() > 0);
     std::sort(data.begin(), data.end());
 
@@ -2435,18 +2363,6 @@ class CombinedStats {
     }
   }
 
-  double CalcStdDev(std::vector<double>& data, double average) {
-    assert(data.size() > 1);
-    double squared_sum = 0.0;
-    for (double x : data) {
-      squared_sum += std::pow(x - average, 2);
-    }
-
-    // using samples count - 1 following Bessel's correction
-    // see https://en.wikipedia.org/wiki/Bessel%27s_correction
-    return std::sqrt(squared_sum / (data.size() - 1));
-  }
-
   std::vector<double> throughput_ops_;
   std::vector<double> throughput_mbs_;
 };
@@ -3609,14 +3525,9 @@ class Benchmark {
         for (int i = 0; i < num_repeat; i++) {
           Stats stats = RunBenchmark(num_threads, name, method);
           combined_stats.AddStats(stats);
-          if (FLAGS_confidence_interval_only) {
-            combined_stats.ReportWithConfidenceIntervals(name);
-          } else {
-            combined_stats.Report(name);
-          }
         }
         if (num_repeat > 1) {
-          combined_stats.ReportFinal(name);
+          combined_stats.Report(name);
         }
       }
       if (post_process_method != nullptr) {
@@ -3946,26 +3857,6 @@ class Benchmark {
     assert(db_.db == nullptr);
 
     options.env = FLAGS_env;
-    options.wal_dir = FLAGS_wal_dir;
-    options.dump_malloc_stats = FLAGS_dump_malloc_stats;
-    options.stats_dump_period_sec =
-        static_cast<unsigned int>(FLAGS_stats_dump_period_sec);
-    options.stats_persist_period_sec =
-        static_cast<unsigned int>(FLAGS_stats_persist_period_sec);
-    options.persist_stats_to_disk = FLAGS_persist_stats_to_disk;
-    options.stats_history_buffer_size =
-        static_cast<size_t>(FLAGS_stats_history_buffer_size);
-    options.avoid_flush_during_recovery = FLAGS_avoid_flush_during_recovery;
-
-    options.compression_opts.level = FLAGS_compression_level;
-    options.compression_opts.max_dict_bytes = FLAGS_compression_max_dict_bytes;
-    options.compression_opts.zstd_max_train_bytes =
-        FLAGS_compression_zstd_max_train_bytes;
-    options.compression_opts.parallel_threads =
-        FLAGS_compression_parallel_threads;
-    options.compression_opts.max_dict_buffer_bytes =
-        FLAGS_compression_max_dict_buffer_bytes;
-
     options.max_open_files = FLAGS_open_files;
     if (FLAGS_cost_write_buffer_to_cache || FLAGS_db_write_buffer_size != 0) {
       options.write_buffer_manager.reset(
@@ -4394,104 +4285,94 @@ class Benchmark {
   }
 
   void InitializeOptionsGeneral(Options* opts) {
-    // Be careful about what is set here to avoid accidentally overwriting
-    // settings already configured by OPTIONS file. Only configure settings that
-    // are needed for the benchmark to run, settings for shared objects that
-    // were not configured already, settings that require dynamically invoking
-    // APIs, and settings for the benchmark itself.
     Options& options = *opts;
 
-    // Always set these since they are harmless when not needed and prevent
-    // a guaranteed failure when they are needed.
-    options.create_missing_column_families = true;
-    options.create_if_missing = true;
-
-    if (options.statistics == nullptr) {
-      options.statistics = dbstats;
-    }
+    options.create_missing_column_families = FLAGS_num_column_families > 1;
+    options.statistics = dbstats;
+    options.wal_dir = FLAGS_wal_dir;
+    options.create_if_missing = !FLAGS_use_existing_db;
+    options.dump_malloc_stats = FLAGS_dump_malloc_stats;
+    options.stats_dump_period_sec =
+        static_cast<unsigned int>(FLAGS_stats_dump_period_sec);
+    options.stats_persist_period_sec =
+        static_cast<unsigned int>(FLAGS_stats_persist_period_sec);
+    options.persist_stats_to_disk = FLAGS_persist_stats_to_disk;
+    options.stats_history_buffer_size =
+        static_cast<size_t>(FLAGS_stats_history_buffer_size);
+    options.avoid_flush_during_recovery = FLAGS_avoid_flush_during_recovery;
 
+    options.compression_opts.level = FLAGS_compression_level;
+    options.compression_opts.max_dict_bytes = FLAGS_compression_max_dict_bytes;
+    options.compression_opts.zstd_max_train_bytes =
+        FLAGS_compression_zstd_max_train_bytes;
+    options.compression_opts.parallel_threads =
+        FLAGS_compression_parallel_threads;
+    options.compression_opts.max_dict_buffer_bytes =
+        FLAGS_compression_max_dict_buffer_bytes;
+    // If this is a block based table, set some related options
     auto table_options =
         options.table_factory->GetOptions<BlockBasedTableOptions>();
     if (table_options != nullptr) {
-      if (FLAGS_cache_size > 0) {
-        // This violates this function's rules on when to set options. But we
-        // have to do it because the case of unconfigured block cache in OPTIONS
-        // file is indistinguishable (it is sanitized to 8MB by this point, not
-        // nullptr), and our regression tests assume this will be the shared
-        // block cache, even with OPTIONS file provided.
+      if (FLAGS_cache_size) {
         table_options->block_cache = cache_;
       }
-      if (table_options->filter_policy == nullptr) {
-        if (FLAGS_bloom_bits < 0) {
-          table_options->filter_policy = BlockBasedTableOptions().filter_policy;
-        } else if (FLAGS_bloom_bits == 0) {
-          table_options->filter_policy.reset();
-        } else if (FLAGS_use_block_based_filter) {
-          // Use back-door way of enabling obsolete block-based Bloom
-          Status s = FilterPolicy::CreateFromString(
-              ConfigOptions(),
-              "rocksdb.internal.DeprecatedBlockBasedBloomFilter:" +
-                  ROCKSDB_NAMESPACE::ToString(FLAGS_bloom_bits),
-              &table_options->filter_policy);
-          if (!s.ok()) {
-            fprintf(stderr,
-                    "failure creating obsolete block-based filter: %s\n",
-                    s.ToString().c_str());
-            exit(1);
-          }
-        } else {
-          table_options->filter_policy.reset(
-              FLAGS_use_ribbon_filter ? NewRibbonFilterPolicy(FLAGS_bloom_bits)
-                                      : NewBloomFilterPolicy(FLAGS_bloom_bits));
+      if (FLAGS_bloom_bits < 0) {
+        table_options->filter_policy = BlockBasedTableOptions().filter_policy;
+      } else if (FLAGS_bloom_bits == 0) {
+        table_options->filter_policy.reset();
+      } else if (FLAGS_use_block_based_filter) {
+        // Use back-door way of enabling obsolete block-based Bloom
+        Status s = FilterPolicy::CreateFromString(
+            ConfigOptions(),
+            "rocksdb.internal.DeprecatedBlockBasedBloomFilter:" +
+                ROCKSDB_NAMESPACE::ToString(FLAGS_bloom_bits),
+            &table_options->filter_policy);
+        if (!s.ok()) {
+          fprintf(stderr, "failure creating obsolete block-based filter: %s\n",
+                  s.ToString().c_str());
+          exit(1);
         }
+      } else {
+        table_options->filter_policy.reset(
+            FLAGS_use_ribbon_filter ? NewRibbonFilterPolicy(FLAGS_bloom_bits)
+                                    : NewBloomFilterPolicy(FLAGS_bloom_bits));
       }
     }
-
-    if (options.row_cache == nullptr) {
-      if (FLAGS_row_cache_size) {
-        if (FLAGS_cache_numshardbits >= 1) {
-          options.row_cache =
-              NewLRUCache(FLAGS_row_cache_size, FLAGS_cache_numshardbits);
-        } else {
-          options.row_cache = NewLRUCache(FLAGS_row_cache_size);
-        }
+    if (FLAGS_row_cache_size) {
+      if (FLAGS_cache_numshardbits >= 1) {
+        options.row_cache =
+            NewLRUCache(FLAGS_row_cache_size, FLAGS_cache_numshardbits);
+      } else {
+        options.row_cache = NewLRUCache(FLAGS_row_cache_size);
       }
     }
-
-    if (options.env == Env::Default()) {
-      options.env = FLAGS_env;
-    }
     if (FLAGS_enable_io_prio) {
-      options.env->LowerThreadPoolIOPriority(Env::LOW);
-      options.env->LowerThreadPoolIOPriority(Env::HIGH);
+      FLAGS_env->LowerThreadPoolIOPriority(Env::LOW);
+      FLAGS_env->LowerThreadPoolIOPriority(Env::HIGH);
     }
     if (FLAGS_enable_cpu_prio) {
-      options.env->LowerThreadPoolCPUPriority(Env::LOW);
-      options.env->LowerThreadPoolCPUPriority(Env::HIGH);
+      FLAGS_env->LowerThreadPoolCPUPriority(Env::LOW);
+      FLAGS_env->LowerThreadPoolCPUPriority(Env::HIGH);
     }
-
+    options.env = FLAGS_env;
     if (FLAGS_sine_write_rate) {
       FLAGS_benchmark_write_rate_limit = static_cast<uint64_t>(SineRate(0));
     }
 
-    if (options.rate_limiter == nullptr) {
-      if (FLAGS_rate_limiter_bytes_per_sec > 0) {
-        options.rate_limiter.reset(NewGenericRateLimiter(
-            FLAGS_rate_limiter_bytes_per_sec,
-            FLAGS_rate_limiter_refill_period_us, 10 /* fairness */,
-            FLAGS_rate_limit_bg_reads ? RateLimiter::Mode::kReadsOnly
-                                      : RateLimiter::Mode::kWritesOnly,
-            FLAGS_rate_limiter_auto_tuned));
-      }
+    if (FLAGS_rate_limiter_bytes_per_sec > 0) {
+      options.rate_limiter.reset(NewGenericRateLimiter(
+          FLAGS_rate_limiter_bytes_per_sec, FLAGS_rate_limiter_refill_period_us,
+          10 /* fairness */,
+          FLAGS_rate_limit_bg_reads ? RateLimiter::Mode::kReadsOnly
+                                    : RateLimiter::Mode::kWritesOnly,
+          FLAGS_rate_limiter_auto_tuned));
     }
 
     options.listeners.emplace_back(listener_);
 
-    if (options.file_checksum_gen_factory == nullptr) {
-      if (FLAGS_file_checksum) {
-        options.file_checksum_gen_factory.reset(
-            new FileChecksumGenCrc32cFactory());
-      }
+    if (FLAGS_file_checksum) {
+      options.file_checksum_gen_factory.reset(
+          new FileChecksumGenCrc32cFactory());
     }
 
     if (FLAGS_num_multi_db <= 1) {
@@ -4510,11 +4391,9 @@ class Benchmark {
     }
 
     // KeepFilter is a noop filter, this can be used to test compaction filter
-    if (options.compaction_filter == nullptr) {
-      if (FLAGS_use_keep_filter) {
-        options.compaction_filter = new KeepFilter();
-        fprintf(stdout, "A noop compaction filter is used\n");
-      }
+    if (FLAGS_use_keep_filter) {
+      options.compaction_filter = new KeepFilter();
+      fprintf(stdout, "A noop compaction filter is used\n");
     }
 
     if (FLAGS_use_existing_keys) {
@@ -8021,27 +7900,14 @@ class Benchmark {
     flush_opt.wait = true;
 
     if (db_.db != nullptr) {
-      Status s;
-      if (FLAGS_num_column_families > 1) {
-        s = db_.db->Flush(flush_opt, db_.cfh);
-      } else {
-        s = db_.db->Flush(flush_opt, db_.db->DefaultColumnFamily());
-      }
-
+      Status s = db_.db->Flush(flush_opt, db_.cfh);
       if (!s.ok()) {
         fprintf(stderr, "Flush failed: %s\n", s.ToString().c_str());
         exit(1);
       }
     } else {
       for (const auto& db_with_cfh : multi_dbs_) {
-        Status s;
-        if (FLAGS_num_column_families > 1) {
-          s = db_with_cfh.db->Flush(flush_opt, db_with_cfh.cfh);
-        } else {
-          s = db_with_cfh.db->Flush(flush_opt,
-                                    db_with_cfh.db->DefaultColumnFamily());
-        }
-
+        Status s = db_with_cfh.db->Flush(flush_opt, db_with_cfh.cfh);
         if (!s.ok()) {
           fprintf(stderr, "Flush failed: %s\n", s.ToString().c_str());
           exit(1);

tools/db_crashtest.py

@@ -383,9 +383,6 @@ multiops_txn_default_params = {
     # compactions.
     "flush_one_in": 1000,
     "key_spaces_path": setup_multiops_txn_key_spaces_file(),
-    "rollback_one_in":  4,
-    # Re-enable once we have a compaction for MultiOpsTxnStressTest
-    "enable_compaction_filter": 0,
 }
 
 multiops_wc_txn_params = {
@@ -404,10 +401,6 @@ multiops_wp_txn_params = {
     "enable_pipelined_write": 0,
     # OpenReadOnly after checkpoint is not currnetly compatible with WritePrepared txns
     "checkpoint_one_in": 0,
-    # Required to be 1 in order to use commit-time-batch
-    "use_only_the_last_commit_time_batch_for_recovery": 1,
-    "recycle_log_file_num": 0,
-    "clear_wp_commit_cache_one_in": 10,
 }
 
 def finalize_and_sanitize(src_params):

tools/ldb_cmd.cc

@@ -18,7 +18,6 @@
 #include <stdexcept>
 #include <string>
 
-#include "db/blob/blob_index.h"
 #include "db/db_impl/db_impl.h"
 #include "db/dbformat.h"
 #include "db/log_reader.h"
@@ -45,7 +44,6 @@
 #include "util/file_checksum_helper.h"
 #include "util/stderr_logger.h"
 #include "util/string_util.h"
-#include "utilities/blob_db/blob_dump_tool.h"
 #include "utilities/merge_operators.h"
 #include "utilities/ttl/db_ttl_impl.h"
 
@@ -101,8 +99,6 @@ const std::string LDBCommand::ARG_BLOB_GARBAGE_COLLECTION_FORCE_THRESHOLD =
 const std::string LDBCommand::ARG_BLOB_COMPACTION_READAHEAD_SIZE =
     "blob_compaction_readahead_size";
 const std::string LDBCommand::ARG_DECODE_BLOB_INDEX = "decode_blob_index";
-const std::string LDBCommand::ARG_DUMP_UNCOMPRESSED_BLOBS =
-    "dump_uncompressed_blobs";
 
 const char* LDBCommand::DELIM = " ==> ";
 
@@ -114,9 +110,6 @@ void DumpWalFile(Options options, std::string wal_file, bool print_header,
 
 void DumpSstFile(Options options, std::string filename, bool output_hex,
                  bool show_properties, bool decode_blob_index);
-
-void DumpBlobFile(const std::string& filename, bool is_key_hex,
-                  bool is_value_hex, bool dump_uncompressed_blobs);
 };
 
 LDBCommand* LDBCommand::InitFromCmdLineArgs(
@@ -1703,11 +1696,11 @@ InternalDumpCommand::InternalDumpCommand(
     const std::vector<std::string>& /*params*/,
     const std::map<std::string, std::string>& options,
     const std::vector<std::string>& flags)
-    : LDBCommand(options, flags, true,
-                 BuildCmdLineOptions(
-                     {ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_FROM, ARG_TO,
-                      ARG_MAX_KEYS, ARG_COUNT_ONLY, ARG_COUNT_DELIM, ARG_STATS,
-                      ARG_INPUT_KEY_HEX, ARG_DECODE_BLOB_INDEX})),
+    : LDBCommand(
+          options, flags, true,
+          BuildCmdLineOptions({ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_FROM,
+                               ARG_TO, ARG_MAX_KEYS, ARG_COUNT_ONLY,
+                               ARG_COUNT_DELIM, ARG_STATS, ARG_INPUT_KEY_HEX})),
       has_from_(false),
       has_to_(false),
       max_keys_(-1),
@@ -1715,8 +1708,7 @@ InternalDumpCommand::InternalDumpCommand(
       count_only_(false),
       count_delim_(false),
       print_stats_(false),
-      is_input_key_hex_(false),
-      decode_blob_index_(false) {
+      is_input_key_hex_(false) {
   has_from_ = ParseStringOption(options, ARG_FROM, &from_);
   has_to_ = ParseStringOption(options, ARG_TO, &to_);
 
@@ -1734,7 +1726,6 @@ InternalDumpCommand::InternalDumpCommand(
   print_stats_ = IsFlagPresent(flags, ARG_STATS);
   count_only_ = IsFlagPresent(flags, ARG_COUNT_ONLY);
   is_input_key_hex_ = IsFlagPresent(flags, ARG_INPUT_KEY_HEX);
-  decode_blob_index_ = IsFlagPresent(flags, ARG_DECODE_BLOB_INDEX);
 
   if (is_input_key_hex_) {
     if (has_from_) {
@@ -1755,7 +1746,6 @@ void InternalDumpCommand::Help(std::string& ret) {
   ret.append(" [--" + ARG_COUNT_ONLY + "]");
   ret.append(" [--" + ARG_COUNT_DELIM + "=<char>]");
   ret.append(" [--" + ARG_STATS + "]");
-  ret.append(" [--" + ARG_DECODE_BLOB_INDEX + "]");
   ret.append("\n");
 }
 
@@ -1787,8 +1777,8 @@ void InternalDumpCommand::DoCommand() {
 
   long long count = 0;
   for (auto& key_version : key_versions) {
-    ValueType value_type = static_cast<ValueType>(key_version.type);
-    InternalKey ikey(key_version.user_key, key_version.sequence, value_type);
+    InternalKey ikey(key_version.user_key, key_version.sequence,
+                     static_cast<ValueType>(key_version.type));
     if (has_to_ && ikey.user_key() == to_) {
       // GetAllKeyVersions() includes keys with user key `to_`, but idump has
       // traditionally excluded such keys.
@@ -1822,21 +1812,8 @@ void InternalDumpCommand::DoCommand() {
 
     if (!count_only_ && !count_delim_) {
       std::string key = ikey.DebugString(is_key_hex_);
-      Slice value(key_version.value);
-      if (!decode_blob_index_ || value_type != kTypeBlobIndex) {
-        fprintf(stdout, "%s => %s\n", key.c_str(),
-                value.ToString(is_value_hex_).c_str());
-      } else {
-        BlobIndex blob_index;
-
-        const Status s = blob_index.DecodeFrom(value);
-        if (!s.ok()) {
-          fprintf(stderr, "%s => error decoding blob index =>\n", key.c_str());
-        } else {
-          fprintf(stdout, "%s => %s\n", key.c_str(),
-                  blob_index.DebugString(is_value_hex_).c_str());
-        }
-      }
+      std::string value = Slice(key_version.value).ToString(is_value_hex_);
+      std::cout << key << " => " << value << "\n";
     }
 
     // Terminate if maximum number of keys have been dumped
@@ -1859,20 +1836,18 @@ DBDumperCommand::DBDumperCommand(
     const std::vector<std::string>& /*params*/,
     const std::map<std::string, std::string>& options,
     const std::vector<std::string>& flags)
-    : LDBCommand(
-          options, flags, true,
-          BuildCmdLineOptions(
-              {ARG_TTL, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_FROM, ARG_TO,
-               ARG_MAX_KEYS, ARG_COUNT_ONLY, ARG_COUNT_DELIM, ARG_STATS,
-               ARG_TTL_START, ARG_TTL_END, ARG_TTL_BUCKET, ARG_TIMESTAMP,
-               ARG_PATH, ARG_DECODE_BLOB_INDEX, ARG_DUMP_UNCOMPRESSED_BLOBS})),
+    : LDBCommand(options, flags, true,
+                 BuildCmdLineOptions(
+                     {ARG_TTL, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_FROM,
+                      ARG_TO, ARG_MAX_KEYS, ARG_COUNT_ONLY, ARG_COUNT_DELIM,
+                      ARG_STATS, ARG_TTL_START, ARG_TTL_END, ARG_TTL_BUCKET,
+                      ARG_TIMESTAMP, ARG_PATH})),
       null_from_(true),
       null_to_(true),
       max_keys_(-1),
       count_only_(false),
       count_delim_(false),
-      print_stats_(false),
-      decode_blob_index_(false) {
+      print_stats_(false) {
   auto itr = options.find(ARG_FROM);
   if (itr != options.end()) {
     null_from_ = false;
@@ -1912,8 +1887,6 @@ DBDumperCommand::DBDumperCommand(
 
   print_stats_ = IsFlagPresent(flags, ARG_STATS);
   count_only_ = IsFlagPresent(flags, ARG_COUNT_ONLY);
-  decode_blob_index_ = IsFlagPresent(flags, ARG_DECODE_BLOB_INDEX);
-  dump_uncompressed_blobs_ = IsFlagPresent(flags, ARG_DUMP_UNCOMPRESSED_BLOBS);
 
   if (is_key_hex_) {
     if (!null_from_) {
@@ -1947,8 +1920,6 @@ void DBDumperCommand::Help(std::string& ret) {
   ret.append(" [--" + ARG_TTL_START + "=<N>:- is inclusive]");
   ret.append(" [--" + ARG_TTL_END + "=<N>:- is exclusive]");
   ret.append(" [--" + ARG_PATH + "=<path_to_a_file>]");
-  ret.append(" [--" + ARG_DECODE_BLOB_INDEX + "]");
-  ret.append(" [--" + ARG_DUMP_UNCOMPRESSED_BLOBS + "]");
   ret.append("\n");
 }
 
@@ -1987,16 +1958,12 @@ void DBDumperCommand::DoCommand() {
         break;
       case kTableFile:
         DumpSstFile(options_, path_, is_key_hex_, /* show_properties */ true,
-                    decode_blob_index_);
+                    /* decode_blob_index */ false);
         break;
       case kDescriptorFile:
         DumpManifestFile(options_, path_, /* verbose_ */ false, is_key_hex_,
                          /*  json_ */ false);
         break;
-      case kBlobFile:
-        DumpBlobFile(path_, is_key_hex_, is_value_hex_,
-                     dump_uncompressed_blobs_);
-        break;
       default:
         exec_state_ = LDBCommandExecuteResult::Failed(
             "File type not supported: " + path_);
@@ -2538,10 +2505,7 @@ class InMemoryHandler : public WriteBatch::Handler {
   ~InMemoryHandler() override {}
 
  protected:
-  Handler::OptionState WriteAfterCommit() const override {
-    return write_after_commit_ ? Handler::OptionState::kEnabled
-                               : Handler::OptionState::kDisabled;
-  }
+  bool WriteAfterCommit() const override { return write_after_commit_; }
 
  private:
   std::stringstream& row_;
@@ -3549,27 +3513,6 @@ void DumpSstFile(Options options, std::string filename, bool output_hex,
   }
 }
 
-void DumpBlobFile(const std::string& filename, bool is_key_hex,
-                  bool is_value_hex, bool dump_uncompressed_blobs) {
-  using ROCKSDB_NAMESPACE::blob_db::BlobDumpTool;
-  BlobDumpTool tool;
-  BlobDumpTool::DisplayType blob_type = is_value_hex
-                                            ? BlobDumpTool::DisplayType::kHex
-                                            : BlobDumpTool::DisplayType::kRaw;
-  BlobDumpTool::DisplayType show_uncompressed_blob =
-      dump_uncompressed_blobs ? blob_type : BlobDumpTool::DisplayType::kNone;
-  BlobDumpTool::DisplayType show_blob =
-      dump_uncompressed_blobs ? BlobDumpTool::DisplayType::kNone : blob_type;
-
-  BlobDumpTool::DisplayType show_key = is_key_hex
-                                           ? BlobDumpTool::DisplayType::kHex
-                                           : BlobDumpTool::DisplayType::kRaw;
-  Status s = tool.Run(filename, show_key, show_blob, show_uncompressed_blob,
-                      /* show_summary */ true);
-  if (!s.ok()) {
-    fprintf(stderr, "Failed: %s\n", s.ToString().c_str());
-  }
-}
 }  // namespace
 
 DBFileDumperCommand::DBFileDumperCommand(
@@ -3577,17 +3520,13 @@ DBFileDumperCommand::DBFileDumperCommand(
     const std::map<std::string, std::string>& options,
     const std::vector<std::string>& flags)
     : LDBCommand(options, flags, true,
-                 BuildCmdLineOptions(
-                     {ARG_DECODE_BLOB_INDEX, ARG_DUMP_UNCOMPRESSED_BLOBS})),
-      decode_blob_index_(IsFlagPresent(flags, ARG_DECODE_BLOB_INDEX)),
-      dump_uncompressed_blobs_(
-          IsFlagPresent(flags, ARG_DUMP_UNCOMPRESSED_BLOBS)) {}
+                 BuildCmdLineOptions({ARG_DECODE_BLOB_INDEX})),
+      decode_blob_index_(IsFlagPresent(flags, ARG_DECODE_BLOB_INDEX)) {}
 
 void DBFileDumperCommand::Help(std::string& ret) {
   ret.append("  ");
   ret.append(DBFileDumperCommand::Name());
   ret.append(" [--" + ARG_DECODE_BLOB_INDEX + "] ");
-  ret.append(" [--" + ARG_DUMP_UNCOMPRESSED_BLOBS + "] ");
   ret.append("\n");
 }
 
@@ -3598,8 +3537,6 @@ void DBFileDumperCommand::DoCommand() {
   }
   Status s;
 
-  // TODO: Use --hex, --key_hex, --value_hex flags consistently for
-  // dumping manifest file, sst files and blob files.
   std::cout << "Manifest File" << std::endl;
   std::cout << "==============================" << std::endl;
   std::string manifest_filename;
@@ -3622,42 +3559,20 @@ void DBFileDumperCommand::DoCommand() {
   DumpManifestFile(options_, manifest_filepath, false, false, false);
   std::cout << std::endl;
 
-  std::vector<ColumnFamilyMetaData> column_families;
-  db_->GetAllColumnFamilyMetaData(&column_families);
-  for (const auto& column_family : column_families) {
-    std::cout << "Column family name: " << column_family.name << std::endl;
-    std::cout << "==============================" << std::endl;
+  std::cout << "SST Files" << std::endl;
+  std::cout << "==============================" << std::endl;
+  std::vector<LiveFileMetaData> metadata;
+  db_->GetLiveFilesMetaData(&metadata);
+  for (auto& fileMetadata : metadata) {
+    std::string filename = fileMetadata.db_path + "/" + fileMetadata.name;
+    // Correct concatenation of filepath and filename:
+    // Check that there is no double slashes (or more!) when concatenation
+    // happens.
+    filename = NormalizePath(filename);
+    std::cout << filename << " level:" << fileMetadata.level << std::endl;
+    std::cout << "------------------------------" << std::endl;
+    DumpSstFile(options_, filename, false, true, decode_blob_index_);
     std::cout << std::endl;
-    std::cout << "SST Files" << std::endl;
-    std::cout << "==============================" << std::endl;
-    for (const LevelMetaData& level : column_family.levels) {
-      for (const SstFileMetaData& sst_file : level.files) {
-        std::string filename = sst_file.db_path + "/" + sst_file.name;
-        // Correct concatenation of filepath and filename:
-        // Check that there is no double slashes (or more!) when concatenation
-        // happens.
-        filename = NormalizePath(filename);
-        std::cout << filename << " level:" << level.level << std::endl;
-        std::cout << "------------------------------" << std::endl;
-        DumpSstFile(options_, filename, false, true, decode_blob_index_);
-        std::cout << std::endl;
-      }
-    }
-    std::cout << "Blob Files" << std::endl;
-    std::cout << "==============================" << std::endl;
-    for (const BlobMetaData& blob_file : column_family.blob_files) {
-      std::string filename =
-          blob_file.blob_file_path + "/" + blob_file.blob_file_name;
-      // Correct concatenation of filepath and filename:
-      // Check that there is no double slashes (or more!) when concatenation
-      // happens.
-      filename = NormalizePath(filename);
-      std::cout << filename << std::endl;
-      std::cout << "------------------------------" << std::endl;
-      DumpBlobFile(filename, /* is_key_hex */ false, /* is_value_hex */ false,
-                   dump_uncompressed_blobs_);
-      std::cout << std::endl;
-    }
   }
   std::cout << std::endl;
 

tools/ldb_cmd_impl.h

@@ -47,7 +47,6 @@ class DBFileDumperCommand : public LDBCommand {
 
  private:
   bool decode_blob_index_;
-  bool dump_uncompressed_blobs_;
 };
 
 class DBLiveFilesMetadataDumperCommand : public LDBCommand {
@@ -108,8 +107,6 @@ class DBDumperCommand : public LDBCommand {
   bool count_delim_;
   bool print_stats_;
   std::string path_;
-  bool decode_blob_index_;
-  bool dump_uncompressed_blobs_;
 
   static const std::string ARG_COUNT_ONLY;
   static const std::string ARG_COUNT_DELIM;
@@ -140,7 +137,6 @@ class InternalDumpCommand : public LDBCommand {
   bool count_delim_;
   bool print_stats_;
   bool is_input_key_hex_;
-  bool decode_blob_index_;
 
   static const std::string ARG_DELIM;
   static const std::string ARG_COUNT_ONLY;

tools/ldb_test.py

@@ -393,20 +393,6 @@ class LDBTestCase(unittest.TestCase):
                                                              hex(ord('b'))),
                 "'a' seq:1, type:1 => val\nInternal keys in range: 1")
 
-    def testIDumpDecodeBlobIndex(self):
-        print("Running testIDumpDecodeBlobIndex...")
-        self.assertRunOK("put a val --create_if_missing", "OK")
-        self.assertRunOK("put b val --enable_blob_files", "OK")
-
-        # Pattern to expect from dump with decode_blob_index flag enabled.
-        regex = ".*\[blob ref\].*"
-        expected_pattern = re.compile(regex)
-        cmd = "idump %s --decode_blob_index"
-        self.assertRunOKFull((cmd)
-                             % (self.dbParam(self.DB_NAME)),
-                             expected_pattern, unexpected=False,
-                             isPattern=True)
-
     def testMiscAdminTask(self):
         print("Running testMiscAdminTask...")
         # These tests need to be improved; for example with asserts about
@@ -488,7 +474,7 @@ class LDBTestCase(unittest.TestCase):
             dbPath += "/"
 
         # Call the dump_live_files function with the edited dbPath name.
-        self.assertTrue(self.dumpLiveFiles("--db=%s --decode_blob_index --dump_uncompressed_blobs" % dbPath, dumpFilePath))
+        self.assertTrue(self.dumpLiveFiles("--db=%s --decode_blob_index" % dbPath, dumpFilePath))
 
         # Investigate the output
         with open(dumpFilePath, "r") as tmp:
@@ -496,22 +482,13 @@ class LDBTestCase(unittest.TestCase):
 
         # Check that all the SST filenames have a correct full path (no multiple '/').
         sstFileList = re.findall(r"%s.*\d+.sst" % dbPath, data)
-        self.assertTrue(len(sstFileList) >= 1)
         for sstFilename in sstFileList:
             filenumber = re.findall(r"\d+.sst", sstFilename)[0]
             self.assertEqual(sstFilename, dbPath+filenumber)
 
-        # Check that all the Blob filenames have a correct full path (no multiple '/').
-        blobFileList = re.findall(r"%s.*\d+.blob" % dbPath, data)
-        self.assertTrue(len(blobFileList) >= 1)
-        for blobFilename in blobFileList:
-            filenumber = re.findall(r"\d+.blob", blobFilename)[0]
-            self.assertEqual(blobFilename, dbPath+filenumber)
-
         # Check that all the manifest filenames
         # have a correct full path (no multiple '/').
         manifestFileList = re.findall(r"%s.*MANIFEST-\d+" % dbPath, data)
-        self.assertTrue(len(manifestFileList) >= 1)
         for manifestFilename in manifestFileList:
             filenumber = re.findall(r"(?<=MANIFEST-)\d+", manifestFilename)[0]
             self.assertEqual(manifestFilename, dbPath+"MANIFEST-"+filenumber)
@@ -710,38 +687,21 @@ class LDBTestCase(unittest.TestCase):
 
         dbPath = os.path.join(self.TMP_DIR, self.DB_NAME)
         self.assertRunOK("put sst1 sst1_val --create_if_missing", "OK")
-        self.assertRunOK("put sst2 sst2_val --enable_blob_files", "OK")
+        self.assertRunOK("put sst2 sst2_val", "OK")
         self.assertRunOK("get sst1", "sst1_val")
 
         # Pattern to expect from SST dump.
-        regex = ".*Sst file format:.*\n.*\[blob ref\].*"
+        regex = ".*Sst file format:.*"
         expected_pattern = re.compile(regex)
 
         sst_files = self.getSSTFiles(dbPath)
         self.assertTrue(len(sst_files) >= 1)
-        cmd = "dump --path=%s --decode_blob_index"
+        cmd = "dump --path=%s"
         self.assertRunOKFull((cmd)
                              % (sst_files[0]),
                              expected_pattern, unexpected=False,
                              isPattern=True)
 
-    def testBlobDump(self):
-        print("Running testBlobDump")
-        dbPath = os.path.join(self.TMP_DIR, self.DB_NAME)
-        self.assertRunOK("batchput x1 y1 --create_if_missing --enable_blob_files", "OK")
-        self.assertRunOK("batchput --enable_blob_files x2 y2 x3 y3 \"x4 abc\" \"y4 xyz\"", "OK")
-
-        # Pattern to expect from blob file dump.
-        regex = ".*Blob log header[\s\S]*Blob log footer[\s\S]*Read record[\s\S]*Summary"
-        expected_pattern = re.compile(regex)
-        blob_files = self.getBlobFiles(dbPath)
-        self.assertTrue(len(blob_files) >= 1)
-        cmd = "dump --path=%s --dump_uncompressed_blobs"
-        self.assertRunOKFull((cmd)
-                             % (blob_files[0]),
-                             expected_pattern, unexpected=False,
-                             isPattern=True)
-
     def testWALDump(self):
         print("Running testWALDump...")
 

tools/sst_dump_tool.cc

@@ -10,9 +10,7 @@
 #include <cinttypes>
 #include <iostream>
 
-#include "options/options_helper.h"
 #include "port/port.h"
-#include "rocksdb/convenience.h"
 #include "rocksdb/utilities/ldb_cmd.h"
 #include "table/sst_file_dumper.h"
 
@@ -32,16 +30,6 @@ static const std::vector<std::pair<CompressionType, const char*>>
 namespace {
 
 void print_help(bool to_stderr) {
-  std::string supported_compressions;
-  for (CompressionType ct : GetSupportedCompressions()) {
-    if (!supported_compressions.empty()) {
-      supported_compressions += ", ";
-    }
-    std::string str;
-    Status s = GetStringFromCompressionType(&str, ct);
-    assert(s.ok());
-    supported_compressions += str;
-  }
   fprintf(
       to_stderr ? stderr : stdout,
       R"(sst_dump --file=<data_dir_OR_sst_file> [--command=check|scan|raw|recompress|identify]
@@ -100,7 +88,6 @@ void print_help(bool to_stderr) {
       kSnappyCompression>
       Can be combined with --command=recompress to run recompression for this
       list of compression types
-      Supported compression types: %s
 
     --parse_internal_key=<0xKEY>
       Convenience option to parse an internal key on the command line. Dumps the
@@ -122,8 +109,7 @@ void print_help(bool to_stderr) {
 
     --compression_max_dict_buffer_bytes=<int64_t>
       Limit on buffer size from which we collect samples for dictionary generation.
-)",
-      supported_compressions.c_str());
+)");
 }
 
 // arg_name would include all prefix, e.g. "--my_arg="

util/cleanable.cc

@@ -1,180 +0,0 @@
-//  Copyright (c) 2011-present, Facebook, Inc.  All rights reserved.
-//  This source code is licensed under both the GPLv2 (found in the
-//  COPYING file in the root directory) and Apache 2.0 License
-//  (found in the LICENSE.Apache file in the root directory).
-//
-// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file. See the AUTHORS file for names of contributors.
-
-#include "rocksdb/cleanable.h"
-
-#include <atomic>
-#include <cassert>
-
-namespace ROCKSDB_NAMESPACE {
-
-Cleanable::Cleanable() {
-  cleanup_.function = nullptr;
-  cleanup_.next = nullptr;
-}
-
-Cleanable::~Cleanable() { DoCleanup(); }
-
-Cleanable::Cleanable(Cleanable&& other) noexcept { *this = std::move(other); }
-
-Cleanable& Cleanable::operator=(Cleanable&& other) noexcept {
-  assert(this != &other);  // https://stackoverflow.com/a/9322542/454544
-  cleanup_ = other.cleanup_;
-  other.cleanup_.function = nullptr;
-  other.cleanup_.next = nullptr;
-  return *this;
-}
-
-// If the entire linked list was on heap we could have simply add attach one
-// link list to another. However the head is an embeded object to avoid the cost
-// of creating objects for most of the use cases when the Cleanable has only one
-// Cleanup to do. We could put evernything on heap if benchmarks show no
-// negative impact on performance.
-// Also we need to iterate on the linked list since there is no pointer to the
-// tail. We can add the tail pointer but maintainin it might negatively impact
-// the perforamnce for the common case of one cleanup where tail pointer is not
-// needed. Again benchmarks could clarify that.
-// Even without a tail pointer we could iterate on the list, find the tail, and
-// have only that node updated without the need to insert the Cleanups one by
-// one. This however would be redundant when the source Cleanable has one or a
-// few Cleanups which is the case most of the time.
-// TODO(myabandeh): if the list is too long we should maintain a tail pointer
-// and have the entire list (minus the head that has to be inserted separately)
-// merged with the target linked list at once.
-void Cleanable::DelegateCleanupsTo(Cleanable* other) {
-  assert(other != nullptr);
-  if (cleanup_.function == nullptr) {
-    return;
-  }
-  Cleanup* c = &cleanup_;
-  other->RegisterCleanup(c->function, c->arg1, c->arg2);
-  c = c->next;
-  while (c != nullptr) {
-    Cleanup* next = c->next;
-    other->RegisterCleanup(c);
-    c = next;
-  }
-  cleanup_.function = nullptr;
-  cleanup_.next = nullptr;
-}
-
-void Cleanable::RegisterCleanup(Cleanable::Cleanup* c) {
-  assert(c != nullptr);
-  if (cleanup_.function == nullptr) {
-    cleanup_.function = c->function;
-    cleanup_.arg1 = c->arg1;
-    cleanup_.arg2 = c->arg2;
-    delete c;
-  } else {
-    c->next = cleanup_.next;
-    cleanup_.next = c;
-  }
-}
-
-void Cleanable::RegisterCleanup(CleanupFunction func, void* arg1, void* arg2) {
-  assert(func != nullptr);
-  Cleanup* c;
-  if (cleanup_.function == nullptr) {
-    c = &cleanup_;
-  } else {
-    c = new Cleanup;
-    c->next = cleanup_.next;
-    cleanup_.next = c;
-  }
-  c->function = func;
-  c->arg1 = arg1;
-  c->arg2 = arg2;
-}
-
-struct SharedCleanablePtr::Impl : public Cleanable {
-  std::atomic<unsigned> ref_count{1};  // Start with 1 ref
-  void Ref() { ref_count.fetch_add(1, std::memory_order_relaxed); }
-  void Unref() {
-    if (ref_count.fetch_sub(1, std::memory_order_relaxed) == 1) {
-      // Last ref
-      delete this;
-    }
-  }
-  static void UnrefWrapper(void* arg1, void* /*arg2*/) {
-    static_cast<SharedCleanablePtr::Impl*>(arg1)->Unref();
-  }
-};
-
-void SharedCleanablePtr::Reset() {
-  if (ptr_) {
-    ptr_->Unref();
-    ptr_ = nullptr;
-  }
-}
-
-void SharedCleanablePtr::Allocate() {
-  Reset();
-  ptr_ = new Impl();
-}
-
-SharedCleanablePtr::SharedCleanablePtr(const SharedCleanablePtr& from) {
-  *this = from;
-}
-
-SharedCleanablePtr::SharedCleanablePtr(SharedCleanablePtr&& from) noexcept {
-  *this = std::move(from);
-}
-
-SharedCleanablePtr& SharedCleanablePtr::operator=(
-    const SharedCleanablePtr& from) {
-  if (this != &from) {
-    Reset();
-    ptr_ = from.ptr_;
-    if (ptr_) {
-      ptr_->Ref();
-    }
-  }
-  return *this;
-}
-
-SharedCleanablePtr& SharedCleanablePtr::operator=(
-    SharedCleanablePtr&& from) noexcept {
-  assert(this != &from);  // https://stackoverflow.com/a/9322542/454544
-  Reset();
-  ptr_ = from.ptr_;
-  from.ptr_ = nullptr;
-  return *this;
-}
-
-SharedCleanablePtr::~SharedCleanablePtr() { Reset(); }
-
-Cleanable& SharedCleanablePtr::operator*() {
-  return *ptr_;  // implicit upcast
-}
-
-Cleanable* SharedCleanablePtr::operator->() {
-  return ptr_;  // implicit upcast
-}
-
-Cleanable* SharedCleanablePtr::get() {
-  return ptr_;  // implicit upcast
-}
-
-void SharedCleanablePtr::RegisterCopyWith(Cleanable* target) {
-  if (ptr_) {
-    // "Virtual" copy of the pointer
-    ptr_->Ref();
-    target->RegisterCleanup(&Impl::UnrefWrapper, ptr_, nullptr);
-  }
-}
-
-void SharedCleanablePtr::MoveAsCleanupTo(Cleanable* target) {
-  if (ptr_) {
-    // "Virtual" move of the pointer
-    target->RegisterCleanup(&Impl::UnrefWrapper, ptr_, nullptr);
-    ptr_ = nullptr;
-  }
-}
-
-}  // namespace ROCKSDB_NAMESPACE

utilities/transactions/lock/point/point_lock_tracker.cc

@@ -44,12 +44,25 @@ class TrackedKeysIterator : public LockTracker::KeyIterator {
 
 void PointLockTracker::Track(const PointLockRequest& r) {
   auto& keys = tracked_keys_[r.column_family_id];
+#ifdef __cpp_lib_unordered_map_try_emplace
+  // use c++17's try_emplace if available, to avoid rehashing the key
+  // in case it is not already in the map
   auto result = keys.try_emplace(r.key, r.seq);
   auto it = result.first;
   if (!result.second && r.seq < it->second.seq) {
     // Now tracking this key with an earlier sequence number
     it->second.seq = r.seq;
   }
+#else
+  auto it = keys.find(r.key);
+  if (it == keys.end()) {
+    auto result = keys.emplace(r.key, TrackedKeyInfo(r.seq));
+    it = result.first;
+  } else if (r.seq < it->second.seq) {
+    // Now tracking this key with an earlier sequence number
+    it->second.seq = r.seq;
+  }
+#endif
   // else we do not update the seq. The smaller the tracked seq, the stronger it
   // the guarantee since it implies from the seq onward there has not been a
   // concurrent update to the key. So we update the seq if it implies stronger

utilities/transactions/lock/range/range_locking_test.cc

@@ -369,46 +369,6 @@ TEST_F(RangeLockingTest, LockWaitCount) {
   delete txn1;
 }
 
-TEST_F(RangeLockingTest, LockWaiteeAccess) {
-  TransactionOptions txn_options;
-  auto cf = db->DefaultColumnFamily();
-  txn_options.lock_timeout = 60;
-  Transaction* txn0 = db->BeginTransaction(WriteOptions(), txn_options);
-  Transaction* txn1 = db->BeginTransaction(WriteOptions(), txn_options);
-
-  // Get a range lock
-  ASSERT_OK(txn0->GetRangeLock(cf, Endpoint("a"), Endpoint("c")));
-
-  std::atomic<bool> reached(false);
-  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
-      "RangeTreeLockManager::TryRangeLock:EnterWaitingTxn", [&](void* /*arg*/) {
-        reached.store(true);
-        std::this_thread::sleep_for(std::chrono::milliseconds(2000));
-      });
-  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
-
-  port::Thread t([&]() {
-    // Attempt to get a conflicting lock
-    auto s = txn1->GetRangeLock(cf, Endpoint("b"), Endpoint("z"));
-    ASSERT_TRUE(s.ok());
-    txn1->Rollback();
-  });
-
-  while (!reached.load()) {
-    std::this_thread::sleep_for(std::chrono::milliseconds(100));
-  }
-  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->DisableProcessing();
-  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearAllCallBacks();
-
-  // Release locks and free the transaction
-  txn0->Rollback();
-  delete txn0;
-
-  t.join();
-
-  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/locktree/lock_request.cc

@@ -368,6 +368,8 @@ void lock_request::retry_all_lock_requests(
 
   toku_mutex_lock(&info->retry_mutex);
 
+  lock_wait_infos conflicts_collector;
+
   // here is the group retry algorithm.
   // get the latest retry_want count and use it as the generation number of
   // this retry operation. if this retry generation is > the last retry
@@ -379,7 +381,7 @@ void lock_request::retry_all_lock_requests(
         info->running_retry = true;
         info->retry_done = info->retry_want;
         toku_mutex_unlock(&info->retry_mutex);
-        retry_all_lock_requests_info(info, lock_wait_callback, callback_arg);
+        retry_all_lock_requests_info(info, &conflicts_collector);
         if (after_retry_all_test_callback) after_retry_all_test_callback();
         toku_mutex_lock(&info->retry_mutex);
         info->running_retry = false;
@@ -391,14 +393,14 @@ void lock_request::retry_all_lock_requests(
     }
   }
   toku_mutex_unlock(&info->retry_mutex);
+
+  report_waits(&conflicts_collector, lock_wait_callback, callback_arg);
 }
 
-void lock_request::retry_all_lock_requests_info(
-    lt_lock_request_info *info,
-    void (*lock_wait_callback)(void *, lock_wait_infos *), void *callback_arg) {
+void lock_request::retry_all_lock_requests_info(lt_lock_request_info *info,
+                                                lock_wait_infos *collector) {
   toku_external_mutex_lock(&info->mutex);
   // retry all of the pending lock requests.
-  lock_wait_infos conflicts_collector;
   for (uint32_t i = 0; i < info->pending_lock_requests.size();) {
     lock_request *request;
     int r = info->pending_lock_requests.fetch(i, &request);
@@ -408,16 +410,12 @@ void lock_request::retry_all_lock_requests_info(
     // move on to the next lock request. otherwise
     // the request is gone from the list so we may
     // read the i'th entry for the next one.
-    r = request->retry(&conflicts_collector);
+    r = request->retry(collector);
     if (r != 0) {
       i++;
     }
   }
 
-  // call report_waits while holding the pending queue lock since
-  // the waiter object is still valid while it's in the queue
-  report_waits(&conflicts_collector, lock_wait_callback, callback_arg);
-
   // future threads should only retry lock requests if some still exist
   info->should_retry_lock_requests = info->pending_lock_requests.size() > 0;
   toku_external_mutex_unlock(&info->mutex);

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

@@ -140,10 +140,8 @@ class lock_request {
       void (*lock_wait_callback)(void *, lock_wait_infos *) = nullptr,
       void *callback_arg = nullptr,
       void (*after_retry_test_callback)(void) = nullptr);
-  static void retry_all_lock_requests_info(
-      lt_lock_request_info *info,
-      void (*lock_wait_callback)(void *, lock_wait_infos *),
-      void *callback_arg);
+  static void retry_all_lock_requests_info(lt_lock_request_info *info,
+                                           lock_wait_infos *collector);
 
   void set_start_test_callback(void (*f)(void));
   void set_start_before_pending_test_callback(void (*f)(void));

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

@@ -111,7 +111,8 @@ Status RangeTreeLockManager::TryLock(PessimisticTransaction* txn,
     deserialize_endpoint(start_dbt, &start);
     deserialize_endpoint(end_dbt, &end);
 
-    di_path.push_back({txnid, column_family_id, is_exclusive, std::move(start),
+    di_path.push_back({((PessimisticTransaction*)txnid)->GetID(),
+                       column_family_id, is_exclusive, std::move(start),
                        std::move(end)});
   };
 
@@ -149,16 +150,13 @@ Status RangeTreeLockManager::TryLock(PessimisticTransaction* txn,
 // 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) {
-  TEST_SYNC_POINT("RangeTreeLockManager::TryRangeLock:EnterWaitingTxn");
   for (auto wait_info : *infos) {
-    // As long as we hold the lock on the locktree's pending request queue
-    // this should be safe.
     auto txn = (PessimisticTransaction*)wait_info.waiter;
     auto cf_id = (ColumnFamilyId)wait_info.ltree->get_dict_id().dictid;
 
     autovector<TransactionID> waitee_ids;
     for (auto waitee : wait_info.waitees) {
-      waitee_ids.push_back(waitee);
+      waitee_ids.push_back(((PessimisticTransaction*)waitee)->GetID());
     }
     txn->SetWaitingTxn(waitee_ids, cf_id, (std::string*)wait_info.m_extra);
   }
@@ -477,10 +475,12 @@ static void push_into_lock_status_data(void* param, const DBT* left,
   deserialize_endpoint(right, &info.end);
 
   if (txnid_arg != TXNID_SHARED) {
-    info.ids.push_back(txnid_arg);
+    TXNID txnid = ((PessimisticTransaction*)txnid_arg)->GetID();
+    info.ids.push_back(txnid);
   } else {
     for (auto it : *owners) {
-      info.ids.push_back(it);
+      TXNID real_id = ((PessimisticTransaction*)it)->GetID();
+      info.ids.push_back(real_id);
     }
   }
   ctx->data->insert({ctx->cfh_id, info});

utilities/transactions/pessimistic_transaction.cc

@@ -61,14 +61,7 @@ PessimisticTransaction::PessimisticTransaction(
 }
 
 void PessimisticTransaction::Initialize(const TransactionOptions& txn_options) {
-  // Range lock manager uses address of transaction object as TXNID
-  const TransactionDBOptions& db_options = txn_db_impl_->GetTxnDBOptions();
-  if (db_options.lock_mgr_handle &&
-      db_options.lock_mgr_handle->getLockManager()->IsRangeLockSupported()) {
-    txn_id_ = reinterpret_cast<TransactionID>(this);
-  } else {
-    txn_id_ = GenTxnID();
-  }
+  txn_id_ = GenTxnID();
 
   txn_state_ = STARTED;
 

utilities/transactions/transaction_test.cc

@@ -5469,10 +5469,6 @@ Status TransactionStressTestInserter(
 TEST_P(MySQLStyleTransactionTest, TransactionStressTest) {
   // Small write buffer to trigger more compactions
   options.write_buffer_size = 1024;
-  txn_db_options.rollback_deletion_type_callback =
-      [](TransactionDB*, ColumnFamilyHandle*, const Slice& key) {
-        return RandomTransactionInserter::RollbackDeletionTypeCallback(key);
-      };
   ASSERT_OK(ReOpenNoDelete());
   constexpr size_t num_workers = 4;        // worker threads count
   constexpr size_t num_checkers = 2;       // checker threads count

utilities/transactions/write_prepared_transaction_test.cc

@@ -3202,8 +3202,6 @@ TEST_P(WritePreparedTransactionTest, ReleaseEarliestSnapshotAfterSeqZeroing2) {
 TEST_P(WritePreparedTransactionTest, SingleDeleteAfterRollback) {
   constexpr size_t kSnapshotCacheBits = 7;  // same as default
   constexpr size_t kCommitCacheBits = 0;    // minimum commit cache
-  txn_db_options.rollback_deletion_type_callback =
-      [](TransactionDB*, ColumnFamilyHandle*, const Slice&) { return true; };
   UpdateTransactionDBOptions(kSnapshotCacheBits, kCommitCacheBits);
   options.disable_auto_compactions = true;
   ASSERT_OK(ReOpen());
@@ -3972,62 +3970,6 @@ TEST_P(WritePreparedTransactionTest, AtomicCommit) {
   }
 }
 
-TEST_P(WritePreparedTransactionTest, BasicRollbackDeletionTypeCb) {
-  options.level0_file_num_compaction_trigger = 2;
-  // Always use SingleDelete to rollback Put.
-  txn_db_options.rollback_deletion_type_callback =
-      [](TransactionDB*, ColumnFamilyHandle*, const Slice&) { return true; };
-
-  const auto write_to_db = [&]() {
-    assert(db);
-    std::unique_ptr<Transaction> txn0(
-        db->BeginTransaction(WriteOptions(), TransactionOptions()));
-    ASSERT_OK(txn0->SetName("txn0"));
-    ASSERT_OK(txn0->Put("a", "v0"));
-    ASSERT_OK(txn0->Prepare());
-
-    // Generate sst1: [PUT('a')]
-    ASSERT_OK(db->Flush(FlushOptions()));
-
-    {
-      CompactRangeOptions cro;
-      cro.change_level = true;
-      cro.target_level = options.num_levels - 1;
-      cro.bottommost_level_compaction = BottommostLevelCompaction::kForce;
-      ASSERT_OK(db->CompactRange(cro, /*begin=*/nullptr, /*end=*/nullptr));
-    }
-
-    ASSERT_OK(txn0->Rollback());
-    txn0.reset();
-
-    ASSERT_OK(db->Put(WriteOptions(), "a", "v1"));
-
-    ASSERT_OK(db->SingleDelete(WriteOptions(), "a"));
-    // Generate another SST with a SD to cover the oldest PUT('a')
-    ASSERT_OK(db->Flush(FlushOptions()));
-
-    auto* dbimpl = static_cast_with_check<DBImpl>(db->GetRootDB());
-    assert(dbimpl);
-    ASSERT_OK(dbimpl->TEST_WaitForCompact());
-
-    {
-      CompactRangeOptions cro;
-      cro.bottommost_level_compaction = BottommostLevelCompaction::kForce;
-      ASSERT_OK(db->CompactRange(cro, /*begin=*/nullptr, /*end=*/nullptr));
-    }
-
-    {
-      std::string value;
-      const Status s = db->Get(ReadOptions(), "a", &value);
-      ASSERT_TRUE(s.IsNotFound());
-    }
-  };
-
-  // Destroy and reopen
-  ASSERT_OK(ReOpen());
-  write_to_db();
-}
-
 // Test that we can change write policy from WriteCommitted to WritePrepared
 // after a clean shutdown (which would empty the WAL)
 TEST_P(WritePreparedTransactionTest, WP_WC_DBBackwardCompatibility) {

utilities/transactions/write_prepared_txn.cc

@@ -263,10 +263,6 @@ Status WritePreparedTxn::CommitInternal() {
 Status WritePreparedTxn::RollbackInternal() {
   ROCKS_LOG_WARN(db_impl_->immutable_db_options().info_log,
                  "RollbackInternal prepare_seq: %" PRIu64, GetId());
-
-  assert(db_impl_);
-  assert(wpt_db_);
-
   WriteBatch rollback_batch;
   assert(GetId() != kMaxSequenceNumber);
   assert(GetId() > 0);
@@ -277,9 +273,8 @@ Status WritePreparedTxn::RollbackInternal() {
   // to prevent callback's seq to be overrriden inside DBImpk::Get
   roptions.snapshot = wpt_db_->GetMaxSnapshot();
   struct RollbackWriteBatchBuilder : public WriteBatch::Handler {
-    DBImpl* const db_;
-    WritePreparedTxnDB* const wpt_db_;
-    WritePreparedTxnReadCallback callback_;
+    DBImpl* db_;
+    WritePreparedTxnReadCallback callback;
     WriteBatch* rollback_batch_;
     std::map<uint32_t, const Comparator*>& comparators_;
     std::map<uint32_t, ColumnFamilyHandle*>& handles_;
@@ -287,16 +282,14 @@ Status WritePreparedTxn::RollbackInternal() {
     std::map<uint32_t, CFKeys> keys_;
     bool rollback_merge_operands_;
     ReadOptions roptions_;
-
     RollbackWriteBatchBuilder(
         DBImpl* db, WritePreparedTxnDB* wpt_db, SequenceNumber snap_seq,
         WriteBatch* dst_batch,
         std::map<uint32_t, const Comparator*>& comparators,
         std::map<uint32_t, ColumnFamilyHandle*>& handles,
-        bool rollback_merge_operands, const ReadOptions& _roptions)
+        bool rollback_merge_operands, ReadOptions _roptions)
         : db_(db),
-          wpt_db_(wpt_db),
-          callback_(wpt_db, snap_seq),  // disable min_uncommitted optimization
+          callback(wpt_db, snap_seq),  // disable min_uncommitted optimization
           rollback_batch_(dst_batch),
           comparators_(comparators),
           handles_(handles),
@@ -311,8 +304,8 @@ Status WritePreparedTxn::RollbackInternal() {
         keys_[cf] = CFKeys(SetComparator(cmp));
       }
       auto it = cf_keys.insert(key);
-      // second is false if a element already existed.
-      if (it.second == false) {
+      if (it.second ==
+          false) {  // second is false if a element already existed.
         return s;
       }
 
@@ -323,7 +316,7 @@ Status WritePreparedTxn::RollbackInternal() {
       get_impl_options.column_family = cf_handle;
       get_impl_options.value = &pinnable_val;
       get_impl_options.value_found = &not_used;
-      get_impl_options.callback = &callback_;
+      get_impl_options.callback = &callback;
       s = db_->GetImpl(roptions_, key, get_impl_options);
       assert(s.ok() || s.IsNotFound());
       if (s.ok()) {
@@ -332,11 +325,7 @@ Status WritePreparedTxn::RollbackInternal() {
       } else if (s.IsNotFound()) {
         // There has been no readable value before txn. By adding a delete we
         // make sure that there will be none afterwards either.
-        if (wpt_db_->ShouldRollbackWithSingleDelete(cf_handle, key)) {
-          s = rollback_batch_->SingleDelete(cf_handle, key);
-        } else {
-          s = rollback_batch_->Delete(cf_handle, key);
-        }
+        s = rollback_batch_->Delete(cf_handle, key);
         assert(s.ok());
       } else {
         // Unexpected status. Return it to the user.
@@ -374,9 +363,7 @@ Status WritePreparedTxn::RollbackInternal() {
     }
 
    protected:
-    Handler::OptionState WriteAfterCommit() const override {
-      return Handler::OptionState::kDisabled;
-    }
+    bool WriteAfterCommit() const override { return false; }
   } rollback_handler(db_impl_, wpt_db_, read_at_seq, &rollback_batch,
                      *cf_comp_map_shared_ptr.get(), *cf_map_shared_ptr.get(),
                      wpt_db_->txn_db_options_.rollback_merge_operands,

utilities/transactions/write_prepared_txn_db.cc

@@ -26,18 +26,6 @@
 #include "utilities/transactions/pessimistic_transaction.h"
 #include "utilities/transactions/transaction_db_mutex_impl.h"
 
-// This function is for testing only. If it returns true, then all entries in
-// the commit cache will be evicted. Unit and/or stress tests (db_stress)
-// can implement this function and customize how frequently commit cache
-// eviction occurs.
-// TODO: remove this function once we can configure commit cache to be very
-// small so that eviction occurs very frequently. This requires the commit
-// cache entry to be able to encode prepare and commit sequence numbers so that
-// the commit sequence number does not have to be within a certain range of
-// prepare sequence number.
-extern "C" bool rocksdb_write_prepared_TEST_ShouldClearCommitCache(void)
-    __attribute__((__weak__));
-
 namespace ROCKSDB_NAMESPACE {
 
 Status WritePreparedTxnDB::Initialize(
@@ -406,7 +394,7 @@ Status WritePreparedTxnDB::NewIterators(
   return Status::OK();
 }
 
-void WritePreparedTxnDB::Init(const TransactionDBOptions& txn_db_opts) {
+void WritePreparedTxnDB::Init(const TransactionDBOptions& /* unused */) {
   // Adcance max_evicted_seq_ no more than 100 times before the cache wraps
   // around.
   INC_STEP_FOR_MAX_EVICTED =
@@ -416,8 +404,6 @@ void WritePreparedTxnDB::Init(const TransactionDBOptions& txn_db_opts) {
   commit_cache_ = std::unique_ptr<std::atomic<CommitEntry64b>[]>(
       new std::atomic<CommitEntry64b>[COMMIT_CACHE_SIZE] {});
   dummy_max_snapshot_.number_ = kMaxSequenceNumber;
-  rollback_deletion_type_callback_ =
-      txn_db_opts.rollback_deletion_type_callback;
 }
 
 void WritePreparedTxnDB::CheckPreparedAgainstMax(SequenceNumber new_max,
@@ -447,7 +433,7 @@ void WritePreparedTxnDB::CheckPreparedAgainstMax(SequenceNumber new_max,
       delayed_prepared_.insert(to_be_popped);
       ROCKS_LOG_WARN(info_log_,
                      "prepared_mutex_ overhead %" PRIu64 " (prep=%" PRIu64
-                     " new_max=%" PRIu64 ")",
+                     " new_max=%" PRIu64,
                      static_cast<uint64_t>(delayed_prepared_.size()),
                      to_be_popped, new_max);
       delayed_prepared_empty_.store(false, std::memory_order_release);
@@ -517,12 +503,6 @@ void WritePreparedTxnDB::AddCommitted(uint64_t prepare_seq, uint64_t commit_seq,
         // legit when a commit entry in a write batch overwrite the previous one
         max_evicted_seq = evicted.commit_seq;
       }
-#ifdef OS_LINUX
-      if (rocksdb_write_prepared_TEST_ShouldClearCommitCache &&
-          rocksdb_write_prepared_TEST_ShouldClearCommitCache()) {
-        max_evicted_seq = last;
-      }
-#endif  // OS_LINUX
       ROCKS_LOG_DETAILS(info_log_,
                         "%lu Evicting %" PRIu64 ",%" PRIu64 " with max %" PRIu64
                         " => %lu",

utilities/transactions/write_prepared_txn_db.h

@@ -465,16 +465,6 @@ class WritePreparedTxnDB : public PessimisticTransactionDB {
   // Get a dummy snapshot that refers to kMaxSequenceNumber
   Snapshot* GetMaxSnapshot() { return &dummy_max_snapshot_; }
 
-  bool ShouldRollbackWithSingleDelete(ColumnFamilyHandle* column_family,
-                                      const Slice& key) {
-    return rollback_deletion_type_callback_
-               ? rollback_deletion_type_callback_(this, column_family, key)
-               : false;
-  }
-
-  std::function<bool(TransactionDB*, ColumnFamilyHandle*, const Slice&)>
-      rollback_deletion_type_callback_;
-
  private:
   friend class AddPreparedCallback;
   friend class PreparedHeap_BasicsTest_Test;
@@ -513,9 +503,8 @@ class WritePreparedTxnDB : public PessimisticTransactionDB {
   friend class WriteUnpreparedTxn;
   friend class WriteUnpreparedTxnDB;
   friend class WriteUnpreparedTransactionTest_RecoveryTest_Test;
-  friend class MultiOpsTxnsStressTest;
 
-  void Init(const TransactionDBOptions& txn_db_opts);
+  void Init(const TransactionDBOptions& /* unused */);
 
   void WPRecordTick(uint32_t ticker_type) const {
     RecordTick(db_impl_->immutable_db_options_.statistics.get(), ticker_type);
@@ -1082,9 +1071,7 @@ struct SubBatchCounter : public WriteBatch::Handler {
   }
   Status MarkBeginPrepare(bool) override { return Status::OK(); }
   Status MarkRollback(const Slice&) override { return Status::OK(); }
-  Handler::OptionState WriteAfterCommit() const override {
-    return Handler::OptionState::kDisabled;
-  }
+  bool WriteAfterCommit() const override { return false; }
 };
 
 SnapshotBackup WritePreparedTxnDB::AssignMinMaxSeqs(const Snapshot* snapshot,

utilities/transactions/write_unprepared_txn.cc

@@ -675,11 +675,7 @@ Status WriteUnpreparedTxn::WriteRollbackKeys(
       s = rollback_batch->Put(cf_handle, key, pinnable_val);
       assert(s.ok());
     } else if (s.IsNotFound()) {
-      if (wupt_db_->ShouldRollbackWithSingleDelete(cf_handle, key)) {
-        s = rollback_batch->SingleDelete(cf_handle, key);
-      } else {
-        s = rollback_batch->Delete(cf_handle, key);
-      }
+      s = rollback_batch->Delete(cf_handle, key);
       assert(s.ok());
     } else {
       return s;