Bottommost level-based compactions in bottom-pri pool

2026-11-07 03:22:34 -05:00 · 2018-05-10 12:10:30 -07:00
83 changed files with 797 additions and 2574 deletions
@@ -845,7 +845,6 @@ if(WITH_TESTS)
        db/db_inplace_update_test.cc
        db/db_io_failure_test.cc
        db/db_iter_test.cc
-        db/db_iter_stress_test.cc
        db/db_iterator_test.cc
        db/db_log_iter_test.cc
        db/db_memtable_test.cc
@@ -1,22 +1,5 @@
 # Rocksdb Change Log
-
-## 5.14.3 (8/21/2018)
-### Public API Change
-* The merge operands are passed to `MergeOperator::ShouldMerge` in the reversed order relative to how they were merged (passed to FullMerge or FullMergeV2) for performance reasons
-### Bug Fixes
-* Fixes DBImpl::FindObsoleteFiles() calling GetChildren() on the same path
-
-## 5.14.2 (7/3/2018)
-### Bug Fixes
-* Change default value of `bytes_max_delete_chunk` to 0 in NewSstFileManager() as it doesn't work well with checkpoints.
-* Set DEBUG_LEVEL=0 for RocksJava Mac Release build.
-
-## 5.14.1 (6/20/2018)
-### Bug Fixes
-* Fix block-based table reader pinning blocks throughout its lifetime, causing memory usage increase.
-* Fix bug with prefix search in partition filters where a shared prefix would be ignored from the later partitions. The bug could report an eixstent key as missing. The bug could be triggered if prefix_extractor is set and partition filters is enabled.
-
-## 5.14.0 (5/16/2018)
+## Unreleased
 ### Public API Change
 * Add a BlockBasedTableOption to align uncompressed data blocks on the smaller of block size or page size boundary, to reduce flash reads by avoiding reads spanning 4K pages.
 * The background thread naming convention changed (on supporting platforms) to "rocksdb:<thread pool priority><thread number>", e.g., "rocksdb:low0".
@@ -24,8 +7,6 @@
 * Touch-up to write-related counters in PerfContext. New counters added: write_scheduling_flushes_compactions_time, write_thread_wait_nanos. Counters whose behavior was fixed or modified: write_memtable_time, write_pre_and_post_process_time, write_delay_time.
 * Posix Env's NewRandomRWFile() will fail if the file doesn't exist.
 * Now, `DBOptions::use_direct_io_for_flush_and_compaction` only applies to background writes, and `DBOptions::use_direct_reads` applies to both user reads and background reads. This conforms with Linux's `open(2)` manpage, which advises against simultaneously reading a file in buffered and direct modes, due to possibly undefined behavior and degraded performance.
-* Iterator::Valid() always returns false if !status().ok(). So, now when doing a Seek() followed by some Next()s, there's no need to check status() after every operation.
-* Iterator::Seek()/SeekForPrev()/SeekToFirst()/SeekToLast() always resets status().

 ### New Features
 * Introduce TTL for level compaction so that all files older than ttl go through the compaction process to get rid of old data.
@@ -41,7 +22,6 @@
 * Fix `BackupableDBOptions::max_valid_backups_to_open` to not delete backup files when refcount cannot be accurately determined.
 * Fix memory leak when pin_l0_filter_and_index_blocks_in_cache is used with partitioned filters
 * Disable rollback of merge operands in WritePrepared transactions to work around an issue in MyRocks. It can be enabled back by setting TransactionDBOptions::rollback_merge_operands to true.
-* Fix bug with prefix search in partition filters where a shared prefix would be ignored from the later partitions. The bug could report an eixstent key as missing. The bug could be triggered if prefix_extractor is set and partition filters is enabled.

 ### Java API Changes
 * Add `BlockBasedTableConfig.setBlockCache` to allow sharing a block cache across DB instances.
@@ -85,10 +85,6 @@ ifeq ($(MAKECMDGOALS),rocksdbjavastaticrelease)
 	DEBUG_LEVEL=0
 endif

-ifeq ($(MAKECMDGOALS),rocksdbjavastaticreleasedocker)
-        DEBUG_LEVEL=0
-endif
-
 ifeq ($(MAKECMDGOALS),rocksdbjavastaticpublish)
 	DEBUG_LEVEL=0
 endif
@@ -406,7 +402,6 @@ TESTS = \
 	db_blob_index_test \
 	db_bloom_filter_test \
 	db_iter_test \
-	db_iter_stress_test \
 	db_log_iter_test \
 	db_compaction_filter_test \
 	db_compaction_test \
@@ -1200,9 +1195,6 @@ db_tailing_iter_test: db/db_tailing_iter_test.o db/db_test_util.o $(LIBOBJECTS)
 db_iter_test: db/db_iter_test.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)

-db_iter_stress_test: db/db_iter_stress_test.o $(LIBOBJECTS) $(TESTHARNESS)
-	$(AM_LINK)
-
 db_universal_compaction_test: db/db_universal_compaction_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)

@@ -1587,7 +1579,7 @@ ZLIB_SHA256 ?= c3e5e9fdd5004dcb542feda5ee4f0ff0744628baf8ed2dd5d66f8ca1197cb1a1
 ZLIB_DOWNLOAD_BASE ?= http://zlib.net
 BZIP2_VER ?= 1.0.6
 BZIP2_SHA256 ?= a2848f34fcd5d6cf47def00461fcb528a0484d8edef8208d6d2e2909dc61d9cd
-BZIP2_DOWNLOAD_BASE ?= https://web.archive.org/web/20180624184835/http://www.bzip.org
+BZIP2_DOWNLOAD_BASE ?= http://www.bzip.org
 SNAPPY_VER ?= 1.1.4
 SNAPPY_SHA256 ?= 134bfe122fd25599bb807bb8130e7ba6d9bdb851e0b16efcb83ac4f5d0b70057
 SNAPPY_DOWNLOAD_BASE ?= https://github.com/google/snappy/releases/download
@@ -28,7 +28,7 @@ rocksdb_compiler_flags = [
    # Added missing flags from output of build_detect_platform
    "-DROCKSDB_PTHREAD_ADAPTIVE_MUTEX",
    "-DROCKSDB_BACKTRACE",
-    "-Wnarrowing",
+    "-Wshorten-64-to-32",
 ]

 rocksdb_external_deps = [
@@ -555,11 +555,6 @@ ROCKS_TESTS = [
        "db/db_io_failure_test.cc",
        "serial",
    ],
-    [
-        "db_iter_stress_test",
-        "db/db_iter_stress_test.cc",
-        "serial",
-    ],
    [
        "db_iter_test",
        "db/db_iter_test.cc",
@@ -1069,3 +1064,14 @@ if not is_opt_mode:
          command = [TEST_RUNNER, BUCK_BINS + test_bin]
        )

+custom_unittest(
+    name = "make_rocksdbjavastatic",
+    command = ["internal_repo_rocksdb/make_rocksdbjavastatic.sh"],
+    type = "simple",
+)
+
+custom_unittest(
+    name = "make_rocksdb_lite_release",
+    command = ["internal_repo_rocksdb/make_rocksdb_lite_release.sh"],
+    type = "simple",
+)
@@ -32,7 +32,7 @@ rocksdb_compiler_flags = [
    # Added missing flags from output of build_detect_platform
    "-DROCKSDB_PTHREAD_ADAPTIVE_MUTEX",
    "-DROCKSDB_BACKTRACE",
-    "-Wnarrowing",
+    "-Wshorten-64-to-32",
 ]

 rocksdb_external_deps = [
@@ -135,4 +135,15 @@ if not is_opt_mode:
          command = [TEST_RUNNER, BUCK_BINS + test_bin]
        )

+custom_unittest(
+    name = "make_rocksdbjavastatic",
+    command = ["internal_repo_rocksdb/make_rocksdbjavastatic.sh"],
+    type = "simple",
+)
+
+custom_unittest(
+    name = "make_rocksdb_lite_release",
+    command = ["internal_repo_rocksdb/make_rocksdb_lite_release.sh"],
+    type = "simple",
+)
 """
@@ -53,13 +53,11 @@ if [ -z "$ROCKSDB_NO_FBCODE" -a -d /mnt/gvfs/third-party ]; then
    FBCODE_BUILD="true"
    # If we're compiling with TSAN we need pic build
    PIC_BUILD=$COMPILE_WITH_TSAN
-    if [ -n "$ROCKSDB_FBCODE_BUILD_WITH_481" ]; then
-      # we need this to build with MySQL. Don't use for other purposes.
-      source "$PWD/build_tools/fbcode_config4.8.1.sh"
-    elif [ -n "$ROCKSDB_FBCODE_BUILD_WITH_5xx" ]; then
+    if [ -z "$ROCKSDB_FBCODE_BUILD_WITH_481" ]; then
      source "$PWD/build_tools/fbcode_config.sh"
    else
-      source "$PWD/build_tools/fbcode_config_platform007.sh"
+      # we need this to build with MySQL. Don't use for other purposes.
+      source "$PWD/build_tools/fbcode_config4.8.1.sh"
    fi
 fi

@@ -1,18 +0,0 @@
-GCC_BASE=/mnt/gvfs/third-party2/gcc/6e8e715624fd15256a7970073387793dfcf79b46/7.x/centos7-native/b2ef2b6
-CLANG_BASE=/mnt/gvfs/third-party2/llvm-fb/ef37e1faa1c29782abfac1ae65a291b9b7966f6d/stable/centos7-native/c9f9104
-LIBGCC_BASE=/mnt/gvfs/third-party2/libgcc/c67031f0f739ac61575a061518d6ef5038f99f90/7.x/platform007/5620abc
-GLIBC_BASE=/mnt/gvfs/third-party2/glibc/60d6f124a78798b73944f5ba87c2306ae3460153/2.26/platform007/f259413
-SNAPPY_BASE=/mnt/gvfs/third-party2/snappy/7f9bdaada18f59bc27ec2b0871eb8a6144343aef/1.1.3/platform007/ca4da3d
-ZLIB_BASE=/mnt/gvfs/third-party2/zlib/22c2d65676fb7c23cfa797c4f6937f38b026f3cf/1.2.8/platform007/ca4da3d
-BZIP2_BASE=/mnt/gvfs/third-party2/bzip2/dc49a21c5fceec6456a7a28a94dcd16690af1337/1.0.6/platform007/ca4da3d
-LZ4_BASE=/mnt/gvfs/third-party2/lz4/907b498203d297947f3bb70b9466f47e100f1873/r131/platform007/ca4da3d
-ZSTD_BASE=/mnt/gvfs/third-party2/zstd/3ee276cbacfad3074e3f07bf826ac47f06970f4e/1.3.5/platform007/15a3614
-GFLAGS_BASE=/mnt/gvfs/third-party2/gflags/0b9929d2588991c65a57168bf88aff2db87c5d48/2.2.0/platform007/ca4da3d
-JEMALLOC_BASE=/mnt/gvfs/third-party2/jemalloc/9c910d36d6235cc40e8ff559358f1833452300ca/master/platform007/5b0f53e
-NUMA_BASE=/mnt/gvfs/third-party2/numa/9cbf2460284c669ed19c3ccb200a71f7dd7e53c7/2.0.11/platform007/ca4da3d
-LIBUNWIND_BASE=/mnt/gvfs/third-party2/libunwind/bf3d7497fe4e6d007354f0adffa16ce3003f8338/1.3/platform007/6f3e0a9
-TBB_BASE=/mnt/gvfs/third-party2/tbb/ff4e0b093534704d8abab678a4fd7f5ea7b094c7/2018_U5/platform007/ca4da3d
-KERNEL_HEADERS_BASE=/mnt/gvfs/third-party2/kernel-headers/b5c4a61a5c483ba24722005ae07895971a2ac707/fb/platform007/da39a3e
-BINUTILS_BASE=/mnt/gvfs/third-party2/binutils/92ff90349e2f43ea0a8246d8b1cf17b6869013e3/2.29.1/centos7-native/da39a3e
-VALGRIND_BASE=/mnt/gvfs/third-party2/valgrind/f3f697a28122e6bcd513273dd9c1ff23852fc59f/3.13.0/platform007/ca4da3d
-LUA_BASE=/mnt/gvfs/third-party2/lua/f0cd714433206d5139df61659eb7b28b1dea6683/5.3.4/platform007/5007832
@@ -1,157 +0,0 @@
-#!/bin/sh
-#
-# Set environment variables so that we can compile rocksdb using
-# fbcode settings.  It uses the latest g++ and clang compilers and also
-# uses jemalloc
-# Environment variables that change the behavior of this script:
-# PIC_BUILD -- if true, it will only take pic versions of libraries from fbcode. libraries that don't have pic variant will not be included
-
-
-BASEDIR=`dirname $BASH_SOURCE`
-source "$BASEDIR/dependencies_platform007.sh"
-
-CFLAGS=""
-
-# libgcc
-LIBGCC_INCLUDE="$LIBGCC_BASE/include/c++/7.3.0"
-LIBGCC_LIBS=" -L $LIBGCC_BASE/lib"
-
-# glibc
-GLIBC_INCLUDE="$GLIBC_BASE/include"
-GLIBC_LIBS=" -L $GLIBC_BASE/lib"
-
-# 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
-  # location of zlib headers and libraries
-  ZLIB_INCLUDE=" -I $ZLIB_BASE/include/"
-  ZLIB_LIBS=" $ZLIB_BASE/lib/libz.a"
-  CFLAGS+=" -DZLIB"
-
-  # location of bzip headers and libraries
-  BZIP_INCLUDE=" -I $BZIP2_BASE/include/"
-  BZIP_LIBS=" $BZIP2_BASE/lib/libbz2.a"
-  CFLAGS+=" -DBZIP2"
-
-  LZ4_INCLUDE=" -I $LZ4_BASE/include/"
-  LZ4_LIBS=" $LZ4_BASE/lib/liblz4.a"
-  CFLAGS+=" -DLZ4"
-fi
-
-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"
-
-# location of gflags headers and libraries
-GFLAGS_INCLUDE=" -I $GFLAGS_BASE/include/"
-if test -z $PIC_BUILD; then
-  GFLAGS_LIBS=" $GFLAGS_BASE/lib/libgflags.a"
-else
-  GFLAGS_LIBS=" $GFLAGS_BASE/lib/libgflags_pic.a"
-fi
-CFLAGS+=" -DGFLAGS=gflags"
-
-# location of jemalloc
-JEMALLOC_INCLUDE=" -I $JEMALLOC_BASE/include/"
-JEMALLOC_LIB=" $JEMALLOC_BASE/lib/libjemalloc.a"
-
-if test -z $PIC_BUILD; then
-  # location of numa
-  NUMA_INCLUDE=" -I $NUMA_BASE/include/"
-  NUMA_LIB=" $NUMA_BASE/lib/libnuma.a"
-  CFLAGS+=" -DNUMA"
-
-  # location of libunwind
-  LIBUNWIND="$LIBUNWIND_BASE/lib/libunwind.a"
-fi
-
-# location of TBB
-TBB_INCLUDE=" -isystem $TBB_BASE/include/"
-if test -z $PIC_BUILD; then
-  TBB_LIBS="$TBB_BASE/lib/libtbb.a"
-else
-  TBB_LIBS="$TBB_BASE/lib/libtbb_pic.a"
-fi
-CFLAGS+=" -DTBB"
-
-# use Intel SSE support for checksum calculations
-export USE_SSE=1
-export PORTABLE=1
-
-BINUTILS="$BINUTILS_BASE/bin"
-AR="$BINUTILS/ar"
-
-DEPS_INCLUDE="$SNAPPY_INCLUDE $ZLIB_INCLUDE $BZIP_INCLUDE $LZ4_INCLUDE $ZSTD_INCLUDE $GFLAGS_INCLUDE $NUMA_INCLUDE $TBB_INCLUDE"
-
-STDLIBS="-L $GCC_BASE/lib64"
-
-CLANG_BIN="$CLANG_BASE/bin"
-CLANG_LIB="$CLANG_BASE/lib"
-CLANG_SRC="$CLANG_BASE/../../src"
-
-CLANG_ANALYZER="$CLANG_BIN/clang++"
-CLANG_SCAN_BUILD="$CLANG_SRC/llvm/tools/clang/tools/scan-build/bin/scan-build"
-
-if [ -z "$USE_CLANG" ]; then
-  # gcc
-  CC="$GCC_BASE/bin/gcc"
-  CXX="$GCC_BASE/bin/g++"
-
-  CFLAGS+=" -B$BINUTILS/gold"
-  CFLAGS+=" -isystem $LIBGCC_INCLUDE"
-  CFLAGS+=" -isystem $GLIBC_INCLUDE"
-  JEMALLOC=1
-else
-  # clang
-  CLANG_INCLUDE="$CLANG_LIB/clang/stable/include"
-  CC="$CLANG_BIN/clang"
-  CXX="$CLANG_BIN/clang++"
-
-  KERNEL_HEADERS_INCLUDE="$KERNEL_HEADERS_BASE/include"
-
-  CFLAGS+=" -B$BINUTILS/gold -nostdinc -nostdlib"
-  CFLAGS+=" -isystem $LIBGCC_BASE/include/c++/7.x "
-  CFLAGS+=" -isystem $LIBGCC_BASE/include/c++/7.x/x86_64-facebook-linux "
-  CFLAGS+=" -isystem $GLIBC_INCLUDE"
-  CFLAGS+=" -isystem $LIBGCC_INCLUDE"
-  CFLAGS+=" -isystem $CLANG_INCLUDE"
-  CFLAGS+=" -isystem $KERNEL_HEADERS_INCLUDE/linux "
-  CFLAGS+=" -isystem $KERNEL_HEADERS_INCLUDE "
-  CFLAGS+=" -Wno-expansion-to-defined "
-  CXXFLAGS="-nostdinc++"
-fi
-
-CFLAGS+=" $DEPS_INCLUDE"
-CFLAGS+=" -DROCKSDB_PLATFORM_POSIX -DROCKSDB_LIB_IO_POSIX -DROCKSDB_FALLOCATE_PRESENT -DROCKSDB_MALLOC_USABLE_SIZE -DROCKSDB_RANGESYNC_PRESENT -DROCKSDB_SCHED_GETCPU_PRESENT -DROCKSDB_SUPPORT_THREAD_LOCAL -DHAVE_SSE42"
-CXXFLAGS+=" $CFLAGS"
-
-EXEC_LDFLAGS=" $SNAPPY_LIBS $ZLIB_LIBS $BZIP_LIBS $LZ4_LIBS $ZSTD_LIBS $GFLAGS_LIBS $NUMA_LIB $TBB_LIBS"
-EXEC_LDFLAGS+=" -B$BINUTILS/gold"
-EXEC_LDFLAGS+=" -Wl,--dynamic-linker,/usr/local/fbcode/platform007/lib/ld.so"
-EXEC_LDFLAGS+=" $LIBUNWIND"
-EXEC_LDFLAGS+=" -Wl,-rpath=/usr/local/fbcode/platform007/lib"
-# required by libtbb
-EXEC_LDFLAGS+=" -ldl"
-
-PLATFORM_LDFLAGS="$LIBGCC_LIBS $GLIBC_LIBS $STDLIBS -lgcc -lstdc++"
-
-EXEC_LDFLAGS_SHARED="$SNAPPY_LIBS $ZLIB_LIBS $BZIP_LIBS $LZ4_LIBS $ZSTD_LIBS $GFLAGS_LIBS $TBB_LIBS"
-
-VALGRIND_VER="$VALGRIND_BASE/bin/"
-
-# lua not supported because it's on track for deprecation, I think
-LUA_PATH=
-LUA_LIB=
-
-export CC CXX AR CFLAGS CXXFLAGS EXEC_LDFLAGS EXEC_LDFLAGS_SHARED VALGRIND_VER JEMALLOC_LIB JEMALLOC_INCLUDE CLANG_ANALYZER CLANG_SCAN_BUILD LUA_PATH LUA_LIB
@@ -85,9 +85,8 @@ NON_SHM="TMPD=/tmp/rocksdb_test_tmp"
 GCC_481="ROCKSDB_FBCODE_BUILD_WITH_481=1"
 ASAN="COMPILE_WITH_ASAN=1"
 CLANG="USE_CLANG=1"
-# in gcc-5 there are known problems with TSAN like https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71090.
-# using platform007 gives us gcc-8 or higher which has that bug fixed.
-TSAN="ROCKSDB_FBCODE_BUILD_WITH_PLATFORM007=1 COMPILE_WITH_TSAN=1"
+LITE="OPT=\"-DROCKSDB_LITE -g\""
+TSAN="COMPILE_WITH_TSAN=1"
 UBSAN="COMPILE_WITH_UBSAN=1"
 DISABLE_JEMALLOC="DISABLE_JEMALLOC=1"
 HTTP_PROXY="https_proxy=http://fwdproxy.29.prn1:8080 http_proxy=http://fwdproxy.29.prn1:8080 ftp_proxy=http://fwdproxy.29.prn1:8080"
@@ -53,45 +53,6 @@ function get_lib_base()
  log_variable $__res_var
 }

-###########################################################
-#                platform007 dependencies                 #
-###########################################################
-
-OUTPUT="$BASEDIR/dependencies_platform007.sh"
-
-rm -f "$OUTPUT"
-touch "$OUTPUT"
-
-echo "Writing dependencies to $OUTPUT"
-
-# Compilers locations
-GCC_BASE=`readlink -f $TP2_LATEST/gcc/7.x/centos7-native/*/`
-CLANG_BASE=`readlink -f $TP2_LATEST/llvm-fb/stable/centos7-native/*/`
-
-log_variable GCC_BASE
-log_variable CLANG_BASE
-
-# Libraries locations
-get_lib_base libgcc     7.x     platform007
-get_lib_base glibc      2.26    platform007
-get_lib_base snappy     LATEST  platform007
-get_lib_base zlib       LATEST  platform007
-get_lib_base bzip2      LATEST  platform007
-get_lib_base lz4        LATEST  platform007
-get_lib_base zstd       LATEST  platform007
-get_lib_base gflags     LATEST  platform007
-get_lib_base jemalloc   LATEST  platform007
-get_lib_base numa       LATEST  platform007
-get_lib_base libunwind  LATEST  platform007
-get_lib_base tbb        LATEST  platform007
-
-get_lib_base kernel-headers fb platform007
-get_lib_base binutils   LATEST centos7-native
-get_lib_base valgrind   LATEST platform007
-get_lib_base lua        5.3.4  platform007
-
-git diff $OUTPUT
-
 ###########################################################
 #                   5.x dependencies                      #
 ###########################################################
@@ -2824,27 +2824,6 @@ TEST_F(ColumnFamilyTest, CreateAndDestoryOptions) {
  ASSERT_OK(db_->DestroyColumnFamilyHandle(cfh));
 }

-TEST_F(ColumnFamilyTest, CreateDropAndDestroy) {
-  ColumnFamilyHandle* cfh;
-  Open();
-  ASSERT_OK(db_->CreateColumnFamily(ColumnFamilyOptions(), "yoyo", &cfh));
-  ASSERT_OK(db_->Put(WriteOptions(), cfh, "foo", "bar"));
-  ASSERT_OK(db_->Flush(FlushOptions(), cfh));
-  ASSERT_OK(db_->DropColumnFamily(cfh));
-  ASSERT_OK(db_->DestroyColumnFamilyHandle(cfh));
-}
-
-TEST_F(ColumnFamilyTest, CreateDropAndDestroyWithoutFileDeletion) {
-  ColumnFamilyHandle* cfh;
-  Open();
-  ASSERT_OK(db_->CreateColumnFamily(ColumnFamilyOptions(), "yoyo", &cfh));
-  ASSERT_OK(db_->Put(WriteOptions(), cfh, "foo", "bar"));
-  ASSERT_OK(db_->Flush(FlushOptions(), cfh));
-  ASSERT_OK(db_->DisableFileDeletions());
-  ASSERT_OK(db_->DropColumnFamily(cfh));
-  ASSERT_OK(db_->DestroyColumnFamilyHandle(cfh));
-}
-
 #ifndef ROCKSDB_LITE
 TEST_F(ColumnFamilyTest, FlushCloseWALFiles) {
  SpecialEnv env(Env::Default());
@@ -477,7 +477,7 @@ bool CompactionPicker::SetupOtherInputs(
      ROCKS_LOG_INFO(ioptions_.info_log,
                     "[%s] Expanding@%d %" ROCKSDB_PRIszt "+%" ROCKSDB_PRIszt
                     "(%" PRIu64 "+%" PRIu64 " bytes) to %" ROCKSDB_PRIszt
-                     "+%" ROCKSDB_PRIszt " (%" PRIu64 "+%" PRIu64 " bytes)\n",
+                     "+%" ROCKSDB_PRIszt " (%" PRIu64 "+%" PRIu64 "bytes)\n",
                     cf_name.c_str(), input_level, inputs->size(),
                     output_level_inputs->size(), inputs_size,
                     output_level_inputs_size, expanded_inputs.size(),
@@ -333,9 +333,9 @@ TEST_F(CorruptionTest, TableFileIndexData) {
  Corrupt(kTableFile, -2000, 500);
  Reopen();
  dbi = reinterpret_cast<DBImpl*>(db_);
-  // one full file may be readable, since only one was corrupted
+  // one full file should be readable, since only one was corrupted
  // the other file should be fully non-readable, since index was corrupted
-  Check(0, 5000);
+  Check(5000, 5000);
  ASSERT_NOK(dbi->VerifyChecksum());
 }

@@ -3128,48 +3128,6 @@ TEST_P(DBCompactionTestWithParam, IntraL0CompactionDoesNotObsoleteDeletions) {
  ASSERT_TRUE(db_->Get(roptions, Key(0), &result).IsNotFound());
 }

-TEST_P(DBCompactionTestWithParam, FullCompactionInBottomPriThreadPool) {
-  const int kNumFilesTrigger = 3;
-  Env::Default()->SetBackgroundThreads(1, Env::Priority::BOTTOM);
-  for (bool use_universal_compaction : {false, true}) {
-    Options options = CurrentOptions();
-    if (use_universal_compaction) {
-      options.compaction_style = kCompactionStyleUniversal;
-    } else {
-      options.compaction_style = kCompactionStyleLevel;
-      options.level_compaction_dynamic_level_bytes = true;
-    }
-    options.num_levels = 4;
-    options.write_buffer_size = 100 << 10;     // 100KB
-    options.target_file_size_base = 32 << 10;  // 32KB
-    options.level0_file_num_compaction_trigger = kNumFilesTrigger;
-    // Trigger compaction if size amplification exceeds 110%
-    options.compaction_options_universal.max_size_amplification_percent = 110;
-    DestroyAndReopen(options);
-
-    int num_bottom_pri_compactions = 0;
-    SyncPoint::GetInstance()->SetCallBack(
-        "DBImpl::BGWorkBottomCompaction",
-        [&](void* /*arg*/) { ++num_bottom_pri_compactions; });
-    SyncPoint::GetInstance()->EnableProcessing();
-
-    Random rnd(301);
-    for (int num = 0; num < kNumFilesTrigger; num++) {
-      ASSERT_EQ(NumSortedRuns(), num);
-      int key_idx = 0;
-      GenerateNewFile(&rnd, &key_idx);
-    }
-    dbfull()->TEST_WaitForCompact();
-
-    ASSERT_EQ(1, num_bottom_pri_compactions);
-
-    // Verify that size amplification did occur
-    ASSERT_EQ(NumSortedRuns(), 1);
-    rocksdb::SyncPoint::GetInstance()->DisableProcessing();
-  }
-  Env::Default()->SetBackgroundThreads(0, Env::Priority::BOTTOM);
-}
-
 TEST_F(DBCompactionTest, OptimizedDeletionObsoleting) {
  // Deletions can be dropped when compacted to non-last level if they fall
  // outside the lower-level files' key-ranges.
@@ -718,11 +718,6 @@ Status DBImpl::FlushWAL(bool sync) {
    if (!s.ok()) {
      ROCKS_LOG_ERROR(immutable_db_options_.info_log, "WAL flush error %s",
                      s.ToString().c_str());
-      // In case there is a fs error we should set it globally to prevent the
-      // future writes
-      WriteStatusCheck(s);
-      // whether sync or not, we should abort the rest of function upon error
-      return s;
    }
    if (!sync) {
      ROCKS_LOG_DEBUG(immutable_db_options_.info_log, "FlushWAL sync=false");
@@ -810,8 +805,6 @@ void DBImpl::MarkLogsSynced(uint64_t up_to, bool synced_dir,
    assert(log.getting_synced);
    if (status.ok() && logs_.size() > 1) {
      logs_to_free_.push_back(log.ReleaseWriter());
-      // To modify logs_ both mutex_ and log_write_mutex_ must be held
-      InstrumentedMutexLock l(&log_write_mutex_);
      it = logs_.erase(it);
    } else {
      log.getting_synced = false;
@@ -221,7 +221,6 @@ class DBImpl : public DB {
  virtual Status Flush(const FlushOptions& options,
                       ColumnFamilyHandle* column_family) override;
  virtual Status FlushWAL(bool sync) override;
-  bool TEST_WALBufferIsEmpty();
  virtual Status SyncWAL() override;

  virtual SequenceNumber GetLatestSequenceNumber() const override;
@@ -25,12 +25,6 @@ void DBImpl::TEST_SwitchWAL() {
  SwitchWAL(&write_context);
 }

-bool DBImpl::TEST_WALBufferIsEmpty() {
-  InstrumentedMutexLock wl(&log_write_mutex_);
-  log::Writer* cur_log_writer = logs_.back().writer;
-  return cur_log_writer->TEST_BufferIsEmpty();
-}
-
 int64_t DBImpl::TEST_MaxNextLevelOverlappingBytes(
    ColumnFamilyHandle* column_family) {
  ColumnFamilyData* cfd;
@@ -12,7 +12,6 @@
 #define __STDC_FORMAT_MACROS
 #endif
 #include <inttypes.h>
-#include <set>
 #include <unordered_set>
 #include "db/event_helpers.h"
 #include "db/memtable_list.h"
@@ -100,10 +99,10 @@ void DBImpl::FindObsoleteFiles(JobContext* job_context, bool force,
  if (doing_the_full_scan) {
    InfoLogPrefix info_log_prefix(!immutable_db_options_.db_log_dir.empty(),
        dbname_);
-    std::set<std::string> paths;
+    std::vector<std::string> paths;
    for (size_t path_id = 0; path_id < immutable_db_options_.db_paths.size();
         path_id++) {
-      paths.insert(immutable_db_options_.db_paths[path_id].path);
+      paths.emplace_back(immutable_db_options_.db_paths[path_id].path);
    }

    // Note that if cf_paths is not specified in the ColumnFamilyOptions
@@ -114,11 +113,7 @@ void DBImpl::FindObsoleteFiles(JobContext* job_context, bool force,
    for (auto cfd : *versions_->GetColumnFamilySet()) {
      for (size_t path_id = 0; path_id < cfd->ioptions()->cf_paths.size();
           path_id++) {
-        auto& path = cfd->ioptions()->cf_paths[path_id].path;
-
-        if (paths.find(path) == paths.end()) {
-          paths.insert(path);
-        }
+        paths.emplace_back(cfd->ioptions()->cf_paths[path_id].path);
      }
    }

@@ -1090,8 +1090,7 @@ Status DBImpl::Open(const DBOptions& db_options, const std::string& dbname,
            new_log_number,
            new log::Writer(
                std::move(file_writer), new_log_number,
-                impl->immutable_db_options_.recycle_log_file_num > 0,
-                impl->immutable_db_options_.manual_wal_flush));
+                impl->immutable_db_options_.recycle_log_file_num > 0));
      }

      // set column family handles
@@ -1218,9 +1217,6 @@ Status DBImpl::Open(const DBOptions& db_options, const std::string& dbname,
  if (s.ok()) {
    ROCKS_LOG_INFO(impl->immutable_db_options_.info_log, "DB pointer %p", impl);
    LogFlush(impl->immutable_db_options_.info_log);
-    assert(impl->TEST_WALBufferIsEmpty());
-    // If the assert above fails then we need to FlushWAL before returning
-    // control back to the user.
    if (!persist_options_status.ok()) {
      s = Status::IOError(
          "DB::Open() failed --- Unable to persist Options file",
@@ -52,13 +52,8 @@ class DBIter final: public Iterator {
 public:
  // The following is grossly complicated. TODO: clean it up
  // Which direction is the iterator currently moving?
-  // (1) When moving forward:
-  //   (1a) if current_entry_is_merged_ = false, the internal iterator is
-  //        positioned at the exact entry that yields this->key(), this->value()
-  //   (1b) if current_entry_is_merged_ = true, the internal iterator is
-  //        positioned immediately after the last entry that contributed to the
-  //        current this->value(). That entry may or may not have key equal to
-  //        this->key().
+  // (1) When moving forward, the internal iterator is positioned at
+  //     the exact entry that yields this->key(), this->value()
  // (2) When moving backwards, the internal iterator is positioned
  //     just before all entries whose user key == this->key().
  enum Direction {
@@ -199,7 +194,6 @@ class DBIter final: public Iterator {
    if (status_.ok()) {
      return iter_->status();
    } else {
-      assert(!valid_);
      return status_;
    }
  }
@@ -241,21 +235,18 @@ class DBIter final: public Iterator {
  void set_valid(bool v) { valid_ = v; }

 private:
-  // For all methods in this block:
-  // PRE: iter_->Valid() && status_.ok()
-  // Return false if there was an error, and status() is non-ok, valid_ = false;
-  // in this case callers would usually stop what they were doing and return.
-  bool ReverseToForward();
-  bool ReverseToBackward();
+  void ReverseToForward();
+  void ReverseToBackward();
+  void PrevInternal();
+  void FindParseableKey(ParsedInternalKey* ikey, Direction direction);
  bool FindValueForCurrentKey();
  bool FindValueForCurrentKeyUsingSeek();
-  bool FindUserKeyBeforeSavedKey();
-  inline bool FindNextUserEntry(bool skipping, bool prefix_check);
-  bool FindNextUserEntryInternal(bool skipping, bool prefix_check);
+  void FindPrevUserKey();
+  void FindNextUserKey();
+  inline void FindNextUserEntry(bool skipping, bool prefix_check);
+  void FindNextUserEntryInternal(bool skipping, bool prefix_check);
  bool ParseKey(ParsedInternalKey* key);
-  bool MergeValuesNewToOld();
-
-  void PrevInternal();
+  void MergeValuesNewToOld();
  bool TooManyInternalKeysSkipped(bool increment = true);
  bool IsVisible(SequenceNumber sequence);

@@ -345,7 +336,6 @@ class DBIter final: public Iterator {
 inline bool DBIter::ParseKey(ParsedInternalKey* ikey) {
  if (!ParseInternalKey(iter_->key(), ikey)) {
    status_ = Status::Corruption("corrupted internal key in DBIter");
-    valid_ = false;
    ROCKS_LOG_ERROR(logger_, "corrupted internal key in DBIter: %s",
                    iter_->key().ToString(true).c_str());
    return false;
@@ -356,16 +346,12 @@ inline bool DBIter::ParseKey(ParsedInternalKey* ikey) {

 void DBIter::Next() {
  assert(valid_);
-  assert(status_.ok());

  // Release temporarily pinned blocks from last operation
  ReleaseTempPinnedData();
  ResetInternalKeysSkippedCounter();
-  bool ok = true;
  if (direction_ == kReverse) {
-    if (!ReverseToForward()) {
-      ok = false;
-    }
+    ReverseToForward();
  } else if (iter_->Valid() && !current_entry_is_merged_) {
    // If the current value is not a merge, the iter position is the
    // current key, which is already returned. We can safely issue a
@@ -379,12 +365,13 @@ void DBIter::Next() {
  if (statistics_ != nullptr) {
    local_stats_.next_count_++;
  }
-  if (ok && iter_->Valid()) {
-    FindNextUserEntry(true /* skipping the current user key */,
-                      prefix_same_as_start_);
-  } else {
+  // Now we point to the next internal position, for both of merge and
+  // not merge cases.
+  if (!iter_->Valid()) {
    valid_ = false;
+    return;
  }
+  FindNextUserEntry(true /* skipping the current user key */, prefix_same_as_start_);
  if (statistics_ != nullptr && valid_) {
    local_stats_.next_found_count_++;
    local_stats_.bytes_read_ += (key().size() + value().size());
@@ -405,16 +392,15 @@ void DBIter::Next() {
 // keys against the prefix of the seeked key. Set to false when
 // performing a seek without a key (e.g. SeekToFirst). Set to
 // prefix_same_as_start_ for other iterations.
-inline bool DBIter::FindNextUserEntry(bool skipping, bool prefix_check) {
+inline void DBIter::FindNextUserEntry(bool skipping, bool prefix_check) {
  PERF_TIMER_GUARD(find_next_user_entry_time);
-  return FindNextUserEntryInternal(skipping, prefix_check);
+  FindNextUserEntryInternal(skipping, prefix_check);
 }

 // Actual implementation of DBIter::FindNextUserEntry()
-bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
+void DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
  // Loop until we hit an acceptable entry to yield
  assert(iter_->Valid());
-  assert(status_.ok());
  assert(direction_ == kForward);
  current_entry_is_merged_ = false;

@@ -434,7 +420,9 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {

  do {
    if (!ParseKey(&ikey_)) {
-      return false;
+      // Skip corrupted keys.
+      iter_->Next();
+      continue;
    }

    if (iterate_upper_bound_ != nullptr &&
@@ -449,7 +437,7 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
    }

    if (TooManyInternalKeysSkipped()) {
-      return false;
+      return;
    }

    if (IsVisible(ikey_.sequence)) {
@@ -467,18 +455,18 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
            // if iterartor specified start_seqnum we
            // 1) return internal key, including the type
            // 2) return ikey only if ikey.seqnum >= start_seqnum_
-            // note that if deletion seqnum is < start_seqnum_ we
+            // not that if deletion seqnum is < start_seqnum_ we
            // just skip it like in normal iterator.
            if (start_seqnum_ > 0 && ikey_.sequence >= start_seqnum_)  {
              saved_key_.SetInternalKey(ikey_);
-              valid_ = true;
-              return true;
+              valid_=true;
+              return;
            } else {
              saved_key_.SetUserKey(
                ikey_.user_key,
                !pin_thru_lifetime_ || !iter_->IsKeyPinned() /* copy */);
-              skipping = true;
-              PERF_COUNTER_ADD(internal_delete_skipped_count, 1);
+                skipping = true;
+                PERF_COUNTER_ADD(internal_delete_skipped_count, 1);
            }
            break;
          case kTypeValue:
@@ -492,7 +480,7 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
              if (ikey_.sequence >= start_seqnum_) {
                saved_key_.SetInternalKey(ikey_);
                valid_ = true;
-                return true;
+                return;
              } else {
                // this key and all previous versions shouldn't be included,
                // skipping
@@ -519,15 +507,14 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
                      "Encounter unexpected blob index. Please open DB with "
                      "rocksdb::blob_db::BlobDB instead.");
                  valid_ = false;
-                  return false;
+                } else {
+                  is_blob_ = true;
+                  valid_ = true;
                }
-
-                is_blob_ = true;
-                valid_ = true;
-                return true;
+                return;
              } else {
                valid_ = true;
-                return true;
+                return;
              }
            }
            break;
@@ -548,7 +535,8 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
              // value
              current_entry_is_merged_ = true;
              valid_ = true;
-              return MergeValuesNewToOld();  // Go to a different state machine
+              MergeValuesNewToOld();  // Go to a different state machine
+              return;
            }
            break;
          default:
@@ -557,14 +545,13 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
        }
      }
    } else {
+      // This key was inserted after our snapshot was taken.
      PERF_COUNTER_ADD(internal_recent_skipped_count, 1);

-      // This key was inserted after our snapshot was taken.
-      // If this happens too many times in a row for the same user key, we want
-      // to seek to the target sequence number.
-      int cmp =
-          user_comparator_->Compare(ikey_.user_key, saved_key_.GetUserKey());
-      if (cmp == 0 || (skipping && cmp <= 0)) {
+      // Here saved_key_ may contain some old key, or the default empty key, or
+      // key assigned by some random other method. We don't care.
+      if (user_comparator_->Compare(ikey_.user_key, saved_key_.GetUserKey()) <=
+          0) {
        num_skipped++;
      } else {
        saved_key_.SetUserKey(
@@ -604,9 +591,7 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
      iter_->Next();
    }
  } while (iter_->Valid());
-
  valid_ = false;
-  return iter_->status().ok();
 }

 // Merge values of the same user key starting from the current iter_ position
@@ -615,12 +600,12 @@ bool DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
 //      saved_key_ stores the user key
 // POST: saved_value_ has the merged value for the user key
 //       iter_ points to the next entry (or invalid)
-bool DBIter::MergeValuesNewToOld() {
+void DBIter::MergeValuesNewToOld() {
  if (!merge_operator_) {
    ROCKS_LOG_ERROR(logger_, "Options::merge_operator is null.");
    status_ = Status::InvalidArgument("merge_operator_ must be set.");
    valid_ = false;
-    return false;
+    return;
  }

  // Temporarily pin the blocks that hold merge operands
@@ -636,7 +621,8 @@ bool DBIter::MergeValuesNewToOld() {
  for (iter_->Next(); iter_->Valid(); iter_->Next()) {
    TEST_SYNC_POINT("DBIter::MergeValuesNewToOld:SteppedToNextOperand");
    if (!ParseKey(&ikey)) {
-      return false;
+      // skip corrupted key
+      continue;
    }

    if (!user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) {
@@ -653,6 +639,7 @@ bool DBIter::MergeValuesNewToOld() {
    } else if (kTypeValue == ikey.type) {
      // hit a put, merge the put value with operands and store the
      // final result in saved_value_. We are done!
+      // ignore corruption if there is any.
      const Slice val = iter_->value();
      s = MergeHelper::TimedFullMerge(
          merge_operator_, ikey.user_key, &val, merge_context_.GetOperands(),
@@ -660,15 +647,10 @@ bool DBIter::MergeValuesNewToOld() {
      if (!s.ok()) {
        valid_ = false;
        status_ = s;
-        return false;
      }
      // iter_ is positioned after put
      iter_->Next();
-      if (!iter_->status().ok()) {
-        valid_ = false;
-        return false;
-      }
-      return true;
+      return;
    } else if (kTypeMerge == ikey.type) {
      // hit a merge, add the value as an operand and run associative merge.
      // when complete, add result to operands and continue.
@@ -686,17 +668,12 @@ bool DBIter::MergeValuesNewToOld() {
            Status::NotSupported("Blob DB does not support merge operator.");
      }
      valid_ = false;
-      return false;
+      return;
    } else {
      assert(false);
    }
  }

-  if (!iter_->status().ok()) {
-    valid_ = false;
-    return false;
-  }
-
  // we either exhausted all internal keys under this user key, or hit
  // a deletion marker.
  // feed null as the existing value to the merge operator, such that
@@ -708,27 +685,17 @@ bool DBIter::MergeValuesNewToOld() {
  if (!s.ok()) {
    valid_ = false;
    status_ = s;
-    return false;
  }
-
-  assert(status_.ok());
-  return true;
 }

 void DBIter::Prev() {
  assert(valid_);
-  assert(status_.ok());
  ReleaseTempPinnedData();
  ResetInternalKeysSkippedCounter();
-  bool ok = true;
  if (direction_ == kForward) {
-    if (!ReverseToBackward()) {
-      ok = false;
-    }
-  }
-  if (ok) {
-    PrevInternal();
+    ReverseToBackward();
  }
+  PrevInternal();
  if (statistics_ != nullptr) {
    local_stats_.prev_count_++;
    if (valid_) {
@@ -738,76 +705,71 @@ void DBIter::Prev() {
  }
 }

-bool DBIter::ReverseToForward() {
-  assert(iter_->status().ok());
-
-  // When moving backwards, iter_ is positioned on _previous_ key, which may
-  // not exist or may have different prefix than the current key().
-  // If that's the case, seek iter_ to current key.
-  if ((prefix_extractor_ != nullptr && !total_order_seek_) || !iter_->Valid()) {
+void DBIter::ReverseToForward() {
+  if (prefix_extractor_ != nullptr && !total_order_seek_) {
    IterKey last_key;
    last_key.SetInternalKey(ParsedInternalKey(
        saved_key_.GetUserKey(), kMaxSequenceNumber, kValueTypeForSeek));
    iter_->Seek(last_key.GetInternalKey());
  }
-
+  FindNextUserKey();
  direction_ = kForward;
-  // Skip keys less than the current key() (a.k.a. saved_key_).
-  while (iter_->Valid()) {
-    ParsedInternalKey ikey;
-    if (!ParseKey(&ikey)) {
-      return false;
-    }
-    if (user_comparator_->Compare(ikey.user_key, saved_key_.GetUserKey()) >=
-        0) {
-      return true;
-    }
-    iter_->Next();
+  if (!iter_->Valid()) {
+    iter_->SeekToFirst();
+    range_del_agg_.InvalidateTombstoneMapPositions();
  }
-
-  if (!iter_->status().ok()) {
-    valid_ = false;
-    return false;
-  }
-
-  return true;
 }

-// Move iter_ to the key before saved_key_.
-bool DBIter::ReverseToBackward() {
-  assert(iter_->status().ok());
-
-  // When current_entry_is_merged_ is true, iter_ may be positioned on the next
-  // key, which may not exist or may have prefix different from current.
-  // If that's the case, seek to saved_key_.
-  if (current_entry_is_merged_ &&
-      ((prefix_extractor_ != nullptr && !total_order_seek_) ||
-       !iter_->Valid())) {
+void DBIter::ReverseToBackward() {
+  if (prefix_extractor_ != nullptr && !total_order_seek_) {
    IterKey last_key;
-    // Using kMaxSequenceNumber and kValueTypeForSeek
-    // (not kValueTypeForSeekForPrev) to seek to a key strictly smaller
-    // than saved_key_.
-    last_key.SetInternalKey(ParsedInternalKey(
-        saved_key_.GetUserKey(), kMaxSequenceNumber, kValueTypeForSeek));
-    if (prefix_extractor_ != nullptr && !total_order_seek_) {
-      iter_->SeekForPrev(last_key.GetInternalKey());
-    } else {
-      // Some iterators may not support SeekForPrev(), so we avoid using it
-      // when prefix seek mode is disabled. This is somewhat expensive
-      // (an extra Prev(), as well as an extra change of direction of iter_),
-      // so we may need to reconsider it later.
-      iter_->Seek(last_key.GetInternalKey());
-      if (!iter_->Valid() && iter_->status().ok()) {
-        iter_->SeekToLast();
+    last_key.SetInternalKey(ParsedInternalKey(saved_key_.GetUserKey(), 0,
+                                              kValueTypeForSeekForPrev));
+    iter_->SeekForPrev(last_key.GetInternalKey());
+  }
+  if (current_entry_is_merged_) {
+    // Not placed in the same key. Need to call Prev() until finding the
+    // previous key.
+    if (!iter_->Valid()) {
+      iter_->SeekToLast();
+      range_del_agg_.InvalidateTombstoneMapPositions();
+    }
+    ParsedInternalKey ikey;
+    FindParseableKey(&ikey, kReverse);
+    while (iter_->Valid() &&
+           user_comparator_->Compare(ikey.user_key, saved_key_.GetUserKey()) >
+               0) {
+      assert(ikey.sequence != kMaxSequenceNumber);
+      if (!IsVisible(ikey.sequence)) {
+        PERF_COUNTER_ADD(internal_recent_skipped_count, 1);
+      } else {
+        PERF_COUNTER_ADD(internal_key_skipped_count, 1);
      }
+      iter_->Prev();
+      FindParseableKey(&ikey, kReverse);
    }
  }
+#ifndef NDEBUG
+  if (iter_->Valid()) {
+    ParsedInternalKey ikey;
+    assert(ParseKey(&ikey));
+    assert(user_comparator_->Compare(ikey.user_key, saved_key_.GetUserKey()) <=
+           0);
+  }
+#endif

+  FindPrevUserKey();
  direction_ = kReverse;
-  return FindUserKeyBeforeSavedKey();
 }

 void DBIter::PrevInternal() {
+  if (!iter_->Valid()) {
+    valid_ = false;
+    return;
+  }
+
+  ParsedInternalKey ikey;
+
  while (iter_->Valid()) {
    saved_key_.SetUserKey(
        ExtractUserKey(iter_->key()),
@@ -829,41 +791,38 @@ void DBIter::PrevInternal() {
      return;
    }

-    if (!FindValueForCurrentKey()) {  // assigns valid_
-      return;
-    }
-
-    // Whether or not we found a value for current key, we need iter_ to end up
-    // on a smaller key.
-    if (!FindUserKeyBeforeSavedKey()) {
-      return;
-    }
-
-    if (valid_) {
-      // Found the value.
+    if (FindValueForCurrentKey()) {
+      if (!iter_->Valid()) {
+        return;
+      }
+      FindParseableKey(&ikey, kReverse);
+      if (user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) {
+        FindPrevUserKey();
+      }
      return;
    }

    if (TooManyInternalKeysSkipped(false)) {
      return;
    }
-  }

+    if (!iter_->Valid()) {
+      break;
+    }
+    FindParseableKey(&ikey, kReverse);
+    if (user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) {
+      FindPrevUserKey();
+    }
+  }
  // We haven't found any key - iterator is not valid
+  // Or the prefix is different than start prefix
+  assert(!iter_->Valid());
  valid_ = false;
 }

-// Used for backwards iteration.
-// Looks at the entries with user key saved_key_ and finds the most up-to-date
-// value for it, or executes a merge, or determines that the value was deleted.
-// Sets valid_ to true if the value is found and is ready to be presented to
-// the user through value().
-// Sets valid_ to false if the value was deleted, and we should try another key.
-// Returns false if an error occurred, and !status().ok() and !valid_.
-//
-// PRE: iter_ is positioned on the last entry with user key equal to saved_key_.
-// POST: iter_ is positioned on one of the entries equal to saved_key_, or on
-//       the entry just before them, or on the entry just after them.
+// This function checks, if the entry with biggest sequence_number <= sequence_
+// is non kTypeDeletion or kTypeSingleDeletion. If it's not, we save value in
+// saved_value_
 bool DBIter::FindValueForCurrentKey() {
  assert(iter_->Valid());
  merge_context_.Clear();
@@ -873,27 +832,20 @@ bool DBIter::FindValueForCurrentKey() {
  ValueType last_not_merge_type = kTypeDeletion;
  ValueType last_key_entry_type = kTypeDeletion;

+  ParsedInternalKey ikey;
+  FindParseableKey(&ikey, kReverse);
+
  // Temporarily pin blocks that hold (merge operands / the value)
  ReleaseTempPinnedData();
  TempPinData();
  size_t num_skipped = 0;
-  while (iter_->Valid()) {
-    ParsedInternalKey ikey;
-    if (!ParseKey(&ikey)) {
-      return false;
-    }
-
-    if (!IsVisible(ikey.sequence) ||
-        !user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) {
-      break;
-    }
+  while (iter_->Valid() && IsVisible(ikey.sequence) &&
+         user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) {
    if (TooManyInternalKeysSkipped()) {
      return false;
    }

-    // This user key has lots of entries.
-    // We're going from old to new, and it's taking too long. Let's do a Seek()
-    // and go from new to old. This helps when a key was overwritten many times.
+    // We iterate too much: let's use Seek() to avoid too much key comparisons
    if (num_skipped >= max_skip_) {
      return FindValueForCurrentKeyUsingSeek();
    }
@@ -940,13 +892,10 @@ bool DBIter::FindValueForCurrentKey() {
    }

    PERF_COUNTER_ADD(internal_key_skipped_count, 1);
+    assert(user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey()));
    iter_->Prev();
    ++num_skipped;
-  }
-
-  if (!iter_->status().ok()) {
-    valid_ = false;
-    return false;
+    FindParseableKey(&ikey, kReverse);
  }

  Status s;
@@ -956,7 +905,7 @@ bool DBIter::FindValueForCurrentKey() {
    case kTypeSingleDeletion:
    case kTypeRangeDeletion:
      valid_ = false;
-      return true;
+      return false;
    case kTypeMerge:
      current_entry_is_merged_ = true;
      if (last_not_merge_type == kTypeDeletion ||
@@ -977,7 +926,7 @@ bool DBIter::FindValueForCurrentKey() {
              Status::NotSupported("Blob DB does not support merge operator.");
        }
        valid_ = false;
-        return false;
+        return true;
      } else {
        assert(last_not_merge_type == kTypeValue);
        s = MergeHelper::TimedFullMerge(
@@ -987,7 +936,7 @@ bool DBIter::FindValueForCurrentKey() {
      }
      break;
    case kTypeValue:
-      // do nothing - we've already has value in pinned_value_
+      // do nothing - we've already has value in saved_value_
      break;
    case kTypeBlobIndex:
      if (!allow_blob_) {
@@ -996,7 +945,7 @@ bool DBIter::FindValueForCurrentKey() {
            "Encounter unexpected blob index. Please open DB with "
            "rocksdb::blob_db::BlobDB instead.");
        valid_ = false;
-        return false;
+        return true;
      }
      is_blob_ = true;
      break;
@@ -1004,19 +953,17 @@ bool DBIter::FindValueForCurrentKey() {
      assert(false);
      break;
  }
-  if (!s.ok()) {
+  if (s.ok()) {
+    valid_ = true;
+  } else {
    valid_ = false;
    status_ = s;
-    return false;
  }
-  valid_ = true;
  return true;
 }

 // This function is used in FindValueForCurrentKey.
 // We use Seek() function instead of Prev() to find necessary value
-// TODO: This is very similar to FindNextUserEntry() and MergeValuesNewToOld().
-//       Would be nice to reuse some code.
 bool DBIter::FindValueForCurrentKeyUsingSeek() {
  // FindValueForCurrentKey will enable pinning before calling
  // FindValueForCurrentKeyUsingSeek()
@@ -1027,38 +974,26 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() {
  iter_->Seek(last_key);
  RecordTick(statistics_, NUMBER_OF_RESEEKS_IN_ITERATION);

-  // In case read_callback presents, the value we seek to may not be visible.
-  // Find the next value that's visible.
+  // assume there is at least one parseable key for this user key
  ParsedInternalKey ikey;
-  while (true) {
-    if (!iter_->Valid()) {
-      valid_ = false;
-      return iter_->status().ok();
-    }
-
-    if (!ParseKey(&ikey)) {
-      return false;
-    }
-    if (!user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) {
-      // No visible values for this key, even though FindValueForCurrentKey()
-      // has seen some. This is possible if we're using a tailing iterator, and
-      // the entries were discarded in a compaction.
-      valid_ = false;
-      return true;
-    }
-
-    if (IsVisible(ikey.sequence)) {
-      break;
-    }
+  FindParseableKey(&ikey, kForward);
+  assert(iter_->Valid());
+  assert(user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey()));

+  // In case read_callback presents, the value we seek to may not be visible.
+  // Seek for the next value that's visible.
+  while (!IsVisible(ikey.sequence)) {
    iter_->Next();
+    FindParseableKey(&ikey, kForward);
+    assert(iter_->Valid());
+    assert(user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey()));
  }

  if (ikey.type == kTypeDeletion || ikey.type == kTypeSingleDeletion ||
      range_del_agg_.ShouldDelete(
          ikey, RangeDelAggregator::RangePositioningMode::kBackwardTraversal)) {
    valid_ = false;
-    return true;
+    return false;
  }
  if (ikey.type == kTypeBlobIndex && !allow_blob_) {
    ROCKS_LOG_ERROR(logger_, "Encounter unexpected blob index.");
@@ -1066,7 +1001,7 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() {
        "Encounter unexpected blob index. Please open DB with "
        "rocksdb::blob_db::BlobDB instead.");
    valid_ = false;
-    return false;
+    return true;
  }
  if (ikey.type == kTypeValue || ikey.type == kTypeBlobIndex) {
    assert(iter_->IsValuePinned());
@@ -1077,147 +1012,115 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() {

  // kTypeMerge. We need to collect all kTypeMerge values and save them
  // in operands
-  assert(ikey.type == kTypeMerge);
  current_entry_is_merged_ = true;
  merge_context_.Clear();
-  merge_context_.PushOperand(iter_->value(),
-                             iter_->IsValuePinned() /* operand_pinned */);
-  while (true) {
+  while (
+      iter_->Valid() &&
+      user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey()) &&
+      ikey.type == kTypeMerge &&
+      !range_del_agg_.ShouldDelete(
+          ikey, RangeDelAggregator::RangePositioningMode::kBackwardTraversal)) {
+    merge_context_.PushOperand(iter_->value(),
+                               iter_->IsValuePinned() /* operand_pinned */);
+    PERF_COUNTER_ADD(internal_merge_count, 1);
    iter_->Next();
-
-    if (!iter_->Valid()) {
-      if (!iter_->status().ok()) {
-        valid_ = false;
-        return false;
-      }
-      break;
-    }
-    if (!ParseKey(&ikey)) {
-      return false;
-    }
-    if (!user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) {
-      break;
-    }
-
-    if (ikey.type == kTypeDeletion || ikey.type == kTypeSingleDeletion ||
-        range_del_agg_.ShouldDelete(
-            ikey,
-            RangeDelAggregator::RangePositioningMode::kBackwardTraversal)) {
-      break;
-    } else if (ikey.type == kTypeValue) {
-      const Slice val = iter_->value();
-      Status s = MergeHelper::TimedFullMerge(
-          merge_operator_, saved_key_.GetUserKey(), &val,
-          merge_context_.GetOperands(), &saved_value_, logger_, statistics_,
-          env_, &pinned_value_, true);
-      if (!s.ok()) {
-        valid_ = false;
-        status_ = s;
-        return false;
-      }
-      valid_ = true;
-      return true;
-    } else if (ikey.type == kTypeMerge) {
-      merge_context_.PushOperand(iter_->value(),
-                                 iter_->IsValuePinned() /* operand_pinned */);
-      PERF_COUNTER_ADD(internal_merge_count, 1);
-    } else if (ikey.type == kTypeBlobIndex) {
-      if (!allow_blob_) {
-        ROCKS_LOG_ERROR(logger_, "Encounter unexpected blob index.");
-        status_ = Status::NotSupported(
-            "Encounter unexpected blob index. Please open DB with "
-            "rocksdb::blob_db::BlobDB instead.");
-      } else {
-        status_ =
-            Status::NotSupported("Blob DB does not support merge operator.");
-      }
-      valid_ = false;
-      return false;
-    } else {
-      assert(false);
-    }
+    FindParseableKey(&ikey, kForward);
  }

-  Status s = MergeHelper::TimedFullMerge(
-      merge_operator_, saved_key_.GetUserKey(), nullptr,
-      merge_context_.GetOperands(), &saved_value_, logger_, statistics_, env_,
-      &pinned_value_, true);
-  if (!s.ok()) {
+  Status s;
+  if (!iter_->Valid() ||
+      !user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey()) ||
+      ikey.type == kTypeDeletion || ikey.type == kTypeSingleDeletion ||
+      range_del_agg_.ShouldDelete(
+          ikey, RangeDelAggregator::RangePositioningMode::kBackwardTraversal)) {
+    s = MergeHelper::TimedFullMerge(merge_operator_, saved_key_.GetUserKey(),
+                                    nullptr, merge_context_.GetOperands(),
+                                    &saved_value_, logger_, statistics_, env_,
+                                    &pinned_value_, true);
+    // Make iter_ valid and point to saved_key_
+    if (!iter_->Valid() ||
+        !user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) {
+      iter_->Seek(last_key);
+      RecordTick(statistics_, NUMBER_OF_RESEEKS_IN_ITERATION);
+    }
+    if (s.ok()) {
+      valid_ = true;
+    } else {
+      valid_ = false;
+      status_ = s;
+    }
+    return true;
+  }
+
+  const Slice& val = iter_->value();
+  s = MergeHelper::TimedFullMerge(merge_operator_, saved_key_.GetUserKey(),
+                                  &val, merge_context_.GetOperands(),
+                                  &saved_value_, logger_, statistics_, env_,
+                                  &pinned_value_, true);
+  if (s.ok()) {
+    valid_ = true;
+  } else {
    valid_ = false;
    status_ = s;
-    return false;
  }
-
-  // Make sure we leave iter_ in a good state. If it's valid and we don't care
-  // about prefixes, that's already good enough. Otherwise it needs to be
-  // seeked to the current key.
-  if ((prefix_extractor_ != nullptr && !total_order_seek_) || !iter_->Valid()) {
-    if (prefix_extractor_ != nullptr && !total_order_seek_) {
-      iter_->SeekForPrev(last_key);
-    } else {
-      iter_->Seek(last_key);
-      if (!iter_->Valid() && iter_->status().ok()) {
-        iter_->SeekToLast();
-      }
-    }
-    RecordTick(statistics_, NUMBER_OF_RESEEKS_IN_ITERATION);
-  }
-
-  valid_ = true;
  return true;
 }

-// Move backwards until the key smaller than saved_key_.
-// Changes valid_ only if return value is false.
-bool DBIter::FindUserKeyBeforeSavedKey() {
-  assert(status_.ok());
+// Used in Next to change directions
+// Go to next user key
+// Don't use Seek(),
+// because next user key will be very close
+void DBIter::FindNextUserKey() {
+  if (!iter_->Valid()) {
+    return;
+  }
+  ParsedInternalKey ikey;
+  FindParseableKey(&ikey, kForward);
+  while (iter_->Valid() &&
+         !user_comparator_->Equal(ikey.user_key, saved_key_.GetUserKey())) {
+    iter_->Next();
+    FindParseableKey(&ikey, kForward);
+  }
+}
+
+// Go to previous user_key
+void DBIter::FindPrevUserKey() {
+  if (!iter_->Valid()) {
+    return;
+  }
  size_t num_skipped = 0;
-  while (iter_->Valid()) {
-    ParsedInternalKey ikey;
-    if (!ParseKey(&ikey)) {
-      return false;
-    }
-
-    if (user_comparator_->Compare(ikey.user_key, saved_key_.GetUserKey()) < 0) {
-      return true;
-    }
-
+  ParsedInternalKey ikey;
+  FindParseableKey(&ikey, kReverse);
+  int cmp;
+  while (iter_->Valid() &&
+         ((cmp = user_comparator_->Compare(ikey.user_key,
+                                           saved_key_.GetUserKey())) == 0 ||
+          (cmp > 0 && !IsVisible(ikey.sequence)))) {
    if (TooManyInternalKeysSkipped()) {
-      return false;
+      return;
    }

+    if (cmp == 0) {
+      if (num_skipped >= max_skip_) {
+        num_skipped = 0;
+        IterKey last_key;
+        last_key.SetInternalKey(ParsedInternalKey(
+            saved_key_.GetUserKey(), kMaxSequenceNumber, kValueTypeForSeek));
+        iter_->Seek(last_key.GetInternalKey());
+        RecordTick(statistics_, NUMBER_OF_RESEEKS_IN_ITERATION);
+      } else {
+        ++num_skipped;
+      }
+    }
    assert(ikey.sequence != kMaxSequenceNumber);
    if (!IsVisible(ikey.sequence)) {
      PERF_COUNTER_ADD(internal_recent_skipped_count, 1);
    } else {
      PERF_COUNTER_ADD(internal_key_skipped_count, 1);
    }
-
-    if (num_skipped >= max_skip_) {
-      num_skipped = 0;
-      IterKey last_key;
-      last_key.SetInternalKey(ParsedInternalKey(
-          saved_key_.GetUserKey(), kMaxSequenceNumber, kValueTypeForSeek));
-      // It would be more efficient to use SeekForPrev() here, but some
-      // iterators may not support it.
-      iter_->Seek(last_key.GetInternalKey());
-      RecordTick(statistics_, NUMBER_OF_RESEEKS_IN_ITERATION);
-      if (!iter_->Valid()) {
-        break;
-      }
-    } else {
-      ++num_skipped;
-    }
-
    iter_->Prev();
+    FindParseableKey(&ikey, kReverse);
  }
-
-  if (!iter_->status().ok()) {
-    valid_ = false;
-    return false;
-  }
-
-  return true;
 }

 bool DBIter::TooManyInternalKeysSkipped(bool increment) {
@@ -1237,9 +1140,19 @@ bool DBIter::IsVisible(SequenceNumber sequence) {
         (read_callback_ == nullptr || read_callback_->IsCommitted(sequence));
 }

+// Skip all unparseable keys
+void DBIter::FindParseableKey(ParsedInternalKey* ikey, Direction direction) {
+  while (iter_->Valid() && !ParseKey(ikey)) {
+    if (direction == kReverse) {
+      iter_->Prev();
+    } else {
+      iter_->Next();
+    }
+  }
+}
+
 void DBIter::Seek(const Slice& target) {
  StopWatch sw(env_, statistics_, DB_SEEK);
-  status_ = Status::OK();
  ReleaseTempPinnedData();
  ResetInternalKeysSkippedCounter();
  saved_key_.Clear();
@@ -1288,7 +1201,6 @@ void DBIter::Seek(const Slice& target) {

 void DBIter::SeekForPrev(const Slice& target) {
  StopWatch sw(env_, statistics_, DB_SEEK);
-  status_ = Status::OK();
  ReleaseTempPinnedData();
  ResetInternalKeysSkippedCounter();
  saved_key_.Clear();
@@ -1337,16 +1249,15 @@ void DBIter::SeekForPrev(const Slice& target) {
 }

 void DBIter::SeekToFirst() {
+  // Don't use iter_::Seek() if we set a prefix extractor
+  // because prefix seek will be used.
+  if (prefix_extractor_ != nullptr) {
+    max_skip_ = std::numeric_limits<uint64_t>::max();
+  }
  if (iterate_lower_bound_ != nullptr) {
    Seek(*iterate_lower_bound_);
    return;
  }
-  // Don't use iter_::Seek() if we set a prefix extractor
-  // because prefix seek will be used.
-  if (prefix_extractor_ != nullptr && !total_order_seek_) {
-    max_skip_ = std::numeric_limits<uint64_t>::max();
-  }
-  status_ = Status::OK();
  direction_ = kForward;
  ReleaseTempPinnedData();
  ResetInternalKeysSkippedCounter();
@@ -1382,22 +1293,11 @@ void DBIter::SeekToFirst() {
 }

 void DBIter::SeekToLast() {
-  if (iterate_upper_bound_ != nullptr) {
-    // Seek to last key strictly less than ReadOptions.iterate_upper_bound.
-    SeekForPrev(*iterate_upper_bound_);
-    if (Valid() && user_comparator_->Equal(*iterate_upper_bound_, key())) {
-      ReleaseTempPinnedData();
-      PrevInternal();
-    }
-    return;
-  }
-
  // Don't use iter_::Seek() if we set a prefix extractor
  // because prefix seek will be used.
-  if (prefix_extractor_ != nullptr && !total_order_seek_) {
+  if (prefix_extractor_ != nullptr) {
    max_skip_ = std::numeric_limits<uint64_t>::max();
  }
-  status_ = Status::OK();
  direction_ = kReverse;
  ReleaseTempPinnedData();
  ResetInternalKeysSkippedCounter();
@@ -1408,7 +1308,21 @@ void DBIter::SeekToLast() {
    iter_->SeekToLast();
    range_del_agg_.InvalidateTombstoneMapPositions();
  }
-  PrevInternal();
+  // When the iterate_upper_bound is set to a value,
+  // it will seek to the last key before the
+  // ReadOptions.iterate_upper_bound
+  if (iter_->Valid() && iterate_upper_bound_ != nullptr) {
+    SeekForPrev(*iterate_upper_bound_);
+    range_del_agg_.InvalidateTombstoneMapPositions();
+    if (!Valid()) {
+      return;
+    } else if (user_comparator_->Equal(*iterate_upper_bound_, key())) {
+      ReleaseTempPinnedData();
+      PrevInternal();
+    }
+  } else {
+    PrevInternal();
+  }
  if (statistics_ != nullptr) {
    RecordTick(statistics_, NUMBER_DB_SEEK);
    if (valid_) {
@@ -1,652 +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).
-
-#include "db/db_iter.h"
-#include "db/dbformat.h"
-#include "rocksdb/comparator.h"
-#include "rocksdb/options.h"
-#include "rocksdb/slice.h"
-#include "util/random.h"
-#include "util/string_util.h"
-#include "util/testharness.h"
-#include "utilities/merge_operators.h"
-
-#ifdef GFLAGS
-
-#include "util/gflags_compat.h"
-
-using GFLAGS_NAMESPACE::ParseCommandLineFlags;
-
-DEFINE_bool(verbose, false,
-            "Print huge, detailed trace. Intended for debugging failures.");
-
-#else
-
-void ParseCommandLineFlags(int*, char***, bool) {}
-bool FLAGS_verbose = false;
-
-#endif
-
-namespace rocksdb {
-
-class DBIteratorStressTest : public testing::Test {
- public:
-  Env* env_;
-
-  DBIteratorStressTest() : env_(Env::Default()) {}
-};
-
-namespace {
-
-struct Entry {
-  std::string key;
-  ValueType type;  // kTypeValue, kTypeDeletion, kTypeMerge
-  uint64_t sequence;
-  std::string ikey;  // internal key, made from `key`, `sequence` and `type`
-  std::string value;
-  // If false, we'll pretend that this entry doesn't exist.
-  bool visible = true;
-
-  bool operator<(const Entry& e) const {
-    if (key != e.key) return key < e.key;
-    return std::tie(sequence, type) > std::tie(e.sequence, e.type);
-  }
-};
-
-struct Data {
-  std::vector<Entry> entries;
-
-  // Indices in `entries` with `visible` = false.
-  std::vector<size_t> hidden;
-  // Keys of entries whose `visible` changed since the last seek of iterators.
-  std::set<std::string> recently_touched_keys;
-};
-
-struct StressTestIterator : public InternalIterator {
-  Data* data;
-  Random64* rnd;
-  InternalKeyComparator cmp;
-
-  // Each operation will return error with this probability...
-  double error_probability = 0;
-  // ... and add/remove entries with this probability.
-  double mutation_probability = 0;
-  // The probability of adding vs removing entries will be chosen so that the
-  // amount of removed entries stays somewhat close to this number.
-  double target_hidden_fraction = 0;
-  // If true, print all mutations to stdout for debugging.
-  bool trace = false;
-
-  int iter = -1;
-  Status status_;
-
-  StressTestIterator(Data* _data, Random64* _rnd, const Comparator* _cmp)
-      : data(_data), rnd(_rnd), cmp(_cmp) {}
-
-  bool Valid() const override {
-    if (iter >= 0 && iter < (int)data->entries.size()) {
-      assert(status_.ok());
-      return true;
-    }
-    return false;
-  }
-
-  Status status() const override { return status_; }
-
-  bool MaybeFail() {
-    if (rnd->Next() >=
-        std::numeric_limits<uint64_t>::max() * error_probability) {
-      return false;
-    }
-    if (rnd->Next() % 2) {
-      status_ = Status::Incomplete("test");
-    } else {
-      status_ = Status::IOError("test");
-    }
-    if (trace) {
-      std::cout << "injecting " << status_.ToString() << std::endl;
-    }
-    iter = -1;
-    return true;
-  }
-
-  void MaybeMutate() {
-    if (rnd->Next() >=
-        std::numeric_limits<uint64_t>::max() * mutation_probability) {
-      return;
-    }
-    do {
-      // If too many entries are hidden, hide less, otherwise hide more.
-      double hide_probability =
-          data->hidden.size() > data->entries.size() * target_hidden_fraction
-              ? 1. / 3
-              : 2. / 3;
-      if (data->hidden.empty()) {
-        hide_probability = 1;
-      }
-      bool do_hide =
-          rnd->Next() < std::numeric_limits<uint64_t>::max() * hide_probability;
-      if (do_hide) {
-        // Hide a random entry.
-        size_t idx = rnd->Next() % data->entries.size();
-        Entry& e = data->entries[idx];
-        if (e.visible) {
-          if (trace) {
-            std::cout << "hiding idx " << idx << std::endl;
-          }
-          e.visible = false;
-          data->hidden.push_back(idx);
-          data->recently_touched_keys.insert(e.key);
-        } else {
-          // Already hidden. Let's go unhide something instead, just because
-          // it's easy and it doesn't really matter what we do.
-          do_hide = false;
-        }
-      }
-      if (!do_hide) {
-        // Unhide a random entry.
-        size_t hi = rnd->Next() % data->hidden.size();
-        size_t idx = data->hidden[hi];
-        if (trace) {
-          std::cout << "unhiding idx " << idx << std::endl;
-        }
-        Entry& e = data->entries[idx];
-        assert(!e.visible);
-        e.visible = true;
-        data->hidden[hi] = data->hidden.back();
-        data->hidden.pop_back();
-        data->recently_touched_keys.insert(e.key);
-      }
-    } while (rnd->Next() % 3 != 0);  // do 3 mutations on average
-  }
-
-  void SkipForward() {
-    while (iter < (int)data->entries.size() && !data->entries[iter].visible) {
-      ++iter;
-    }
-  }
-  void SkipBackward() {
-    while (iter >= 0 && !data->entries[iter].visible) {
-      --iter;
-    }
-  }
-
-  void SeekToFirst() {
-    if (MaybeFail()) return;
-    MaybeMutate();
-
-    status_ = Status::OK();
-    iter = 0;
-    SkipForward();
-  }
-  void SeekToLast() {
-    if (MaybeFail()) return;
-    MaybeMutate();
-
-    status_ = Status::OK();
-    iter = (int)data->entries.size() - 1;
-    SkipBackward();
-  }
-
-  void Seek(const Slice& target) {
-    if (MaybeFail()) return;
-    MaybeMutate();
-
-    status_ = Status::OK();
-    // Binary search.
-    auto it = std::partition_point(
-        data->entries.begin(), data->entries.end(),
-        [&](const Entry& e) { return cmp.Compare(e.ikey, target) < 0; });
-    iter = (int)(it - data->entries.begin());
-    SkipForward();
-  }
-  void SeekForPrev(const Slice& target) {
-    if (MaybeFail()) return;
-    MaybeMutate();
-
-    status_ = Status::OK();
-    // Binary search.
-    auto it = std::partition_point(
-        data->entries.begin(), data->entries.end(),
-        [&](const Entry& e) { return cmp.Compare(e.ikey, target) <= 0; });
-    iter = (int)(it - data->entries.begin());
-    --iter;
-    SkipBackward();
-  }
-
-  void Next() {
-    assert(Valid());
-    if (MaybeFail()) return;
-    MaybeMutate();
-    ++iter;
-    SkipForward();
-  }
-  void Prev() {
-    assert(Valid());
-    if (MaybeFail()) return;
-    MaybeMutate();
-    --iter;
-    SkipBackward();
-  }
-
-  Slice key() const {
-    assert(Valid());
-    return data->entries[iter].ikey;
-  }
-  Slice value() const {
-    assert(Valid());
-    return data->entries[iter].value;
-  }
-
-  bool IsKeyPinned() const override { return true; }
-  bool IsValuePinned() const override { return true; }
-};
-
-// A small reimplementation of DBIter, supporting only some of the features,
-// and doing everything in O(log n).
-// Skips all keys that are in recently_touched_keys.
-struct ReferenceIterator {
-  Data* data;
-  uint64_t sequence;  // ignore entries with sequence number below this
-
-  bool valid = false;
-  std::string key;
-  std::string value;
-
-  ReferenceIterator(Data* _data, uint64_t _sequence)
-      : data(_data), sequence(_sequence) {}
-
-  bool Valid() const { return valid; }
-
-  // Finds the first entry with key
-  // greater/less/greater-or-equal/less-or-equal than `key`, depending on
-  // arguments: if `skip`, inequality is strict; if `forward`, it's
-  // greater/greater-or-equal, otherwise less/less-or-equal.
-  // Sets `key` to the result.
-  // If no such key exists, returns false. Doesn't check `visible`.
-  bool FindNextKey(bool skip, bool forward) {
-    valid = false;
-    auto it = std::partition_point(data->entries.begin(), data->entries.end(),
-                                   [&](const Entry& e) {
-                                     if (forward != skip) {
-                                       return e.key < key;
-                                     } else {
-                                       return e.key <= key;
-                                     }
-                                   });
-    if (forward) {
-      if (it != data->entries.end()) {
-        key = it->key;
-        return true;
-      }
-    } else {
-      if (it != data->entries.begin()) {
-        --it;
-        key = it->key;
-        return true;
-      }
-    }
-    return false;
-  }
-
-  bool FindValueForCurrentKey() {
-    if (data->recently_touched_keys.count(key)) {
-      return false;
-    }
-
-    // Find the first entry for the key. The caller promises that it exists.
-    auto it = std::partition_point(data->entries.begin(), data->entries.end(),
-                                   [&](const Entry& e) {
-                                     if (e.key != key) {
-                                       return e.key < key;
-                                     }
-                                     return e.sequence > sequence;
-                                   });
-
-    // Find the first visible entry.
-    for (;; ++it) {
-      if (it == data->entries.end()) {
-        return false;
-      }
-      Entry& e = *it;
-      if (e.key != key) {
-        return false;
-      }
-      assert(e.sequence <= sequence);
-      if (!e.visible) continue;
-      if (e.type == kTypeDeletion) {
-        return false;
-      }
-      if (e.type == kTypeValue) {
-        value = e.value;
-        valid = true;
-        return true;
-      }
-      assert(e.type == kTypeMerge);
-      break;
-    }
-
-    // Collect merge operands.
-    std::vector<Slice> operands;
-    for (; it != data->entries.end(); ++it) {
-      Entry& e = *it;
-      if (e.key != key) {
-        break;
-      }
-      assert(e.sequence <= sequence);
-      if (!e.visible) continue;
-      if (e.type == kTypeDeletion) {
-        break;
-      }
-      operands.push_back(e.value);
-      if (e.type == kTypeValue) {
-        break;
-      }
-    }
-
-    // Do a merge.
-    value = operands.back().ToString();
-    for (int i = (int)operands.size() - 2; i >= 0; --i) {
-      value.append(",");
-      value.append(operands[i].data(), operands[i].size());
-    }
-
-    valid = true;
-    return true;
-  }
-
-  // Start at `key` and move until we encounter a valid value.
-  // `forward` defines the direction of movement.
-  // If `skip` is true, we're looking for key not equal to `key`.
-  void DoTheThing(bool skip, bool forward) {
-    while (FindNextKey(skip, forward) && !FindValueForCurrentKey()) {
-      skip = true;
-    }
-  }
-
-  void Seek(const Slice& target) {
-    key = target.ToString();
-    DoTheThing(false, true);
-  }
-  void SeekForPrev(const Slice& target) {
-    key = target.ToString();
-    DoTheThing(false, false);
-  }
-  void SeekToFirst() { Seek(""); }
-  void SeekToLast() {
-    key = data->entries.back().key;
-    DoTheThing(false, false);
-  }
-  void Next() {
-    assert(Valid());
-    DoTheThing(true, true);
-  }
-  void Prev() {
-    assert(Valid());
-    DoTheThing(true, false);
-  }
-};
-
-}  // namespace
-
-// Use an internal iterator that sometimes returns errors and sometimes
-// adds/removes entries on the fly. Do random operations on a DBIter and
-// check results.
-// TODO: can be improved for more coverage:
-//   * Override IsKeyPinned() and IsValuePinned() to actually use
-//     PinnedIteratorManager and check that there's no use-after free.
-//   * Try different combinations of prefix_extractor, total_order_seek,
-//     prefix_same_as_start, iterate_lower_bound, iterate_upper_bound.
-TEST_F(DBIteratorStressTest, StressTest) {
-  // We use a deterministic RNG, and everything happens in a single thread.
-  Random64 rnd(826909345792864532ll);
-
-  auto gen_key = [&](int max_key) {
-    int len = 0;
-    int a = max_key;
-    while (a) {
-      a /= 10;
-      ++len;
-    }
-    std::string s = ToString(rnd.Next() % (uint64_t)max_key);
-    s.insert(0, len - (int)s.size(), '0');
-    return s;
-  };
-
-  Options options;
-  options.merge_operator = MergeOperators::CreateFromStringId("stringappend");
-  ReadOptions ropt;
-
-  size_t num_matching = 0;
-  size_t num_at_end = 0;
-  size_t num_not_ok = 0;
-  size_t num_recently_removed = 0;
-
-  // Number of iterations for each combination of parameters
-  // (there are ~250 of those).
-  // Tweak this to change the test run time.
-  // As of the time of writing, the test takes ~4 seconds for value of 5000.
-  const int num_iterations = 5000;
-  // Enable this to print all the operations for debugging.
-  bool trace = FLAGS_verbose;
-
-  for (int num_entries : {5, 10, 100}) {
-    for (double key_space : {0.1, 1.0, 3.0}) {
-      for (ValueType prevalent_entry_type :
-           {kTypeValue, kTypeDeletion, kTypeMerge}) {
-        for (double error_probability : {0.01, 0.1}) {
-          for (double mutation_probability : {0.01, 0.5}) {
-            for (double target_hidden_fraction : {0.1, 0.5}) {
-              std::string trace_str =
-                  "entries: " + ToString(num_entries) +
-                  ", key_space: " + ToString(key_space) +
-                  ", error_probability: " + ToString(error_probability) +
-                  ", mutation_probability: " + ToString(mutation_probability) +
-                  ", target_hidden_fraction: " +
-                  ToString(target_hidden_fraction);
-              SCOPED_TRACE(trace_str);
-              if (trace) {
-                std::cout << trace_str << std::endl;
-              }
-
-              // Generate data.
-              Data data;
-              int max_key = (int)(num_entries * key_space) + 1;
-              for (int i = 0; i < num_entries; ++i) {
-                Entry e;
-                e.key = gen_key(max_key);
-                if (rnd.Next() % 10 != 0) {
-                  e.type = prevalent_entry_type;
-                } else {
-                  const ValueType types[] = {kTypeValue, kTypeDeletion,
-                                             kTypeMerge};
-                  e.type =
-                      types[rnd.Next() % (sizeof(types) / sizeof(types[0]))];
-                }
-                e.sequence = i;
-                e.value = "v" + ToString(i);
-                ParsedInternalKey internal_key(e.key, e.sequence, e.type);
-                AppendInternalKey(&e.ikey, internal_key);
-
-                data.entries.push_back(e);
-              }
-              std::sort(data.entries.begin(), data.entries.end());
-              if (trace) {
-                std::cout << "entries:";
-                for (size_t i = 0; i < data.entries.size(); ++i) {
-                  Entry& e = data.entries[i];
-                  std::cout
-                      << "\n  idx " << i << ": \"" << e.key << "\": \""
-                      << e.value << "\" seq: " << e.sequence << " type: "
-                      << (e.type == kTypeValue
-                              ? "val"
-                              : e.type == kTypeDeletion ? "del" : "merge");
-                }
-                std::cout << std::endl;
-              }
-
-              std::unique_ptr<Iterator> db_iter;
-              std::unique_ptr<ReferenceIterator> ref_iter;
-              for (int iteration = 0; iteration < num_iterations; ++iteration) {
-                SCOPED_TRACE(iteration);
-                // Create a new iterator every ~30 operations.
-                if (db_iter == nullptr || rnd.Next() % 30 == 0) {
-                  uint64_t sequence = rnd.Next() % (data.entries.size() + 2);
-                  ref_iter.reset(new ReferenceIterator(&data, sequence));
-                  if (trace) {
-                    std::cout << "new iterator, seq: " << sequence << std::endl;
-                  }
-
-                  auto internal_iter =
-                      new StressTestIterator(&data, &rnd, BytewiseComparator());
-                  internal_iter->error_probability = error_probability;
-                  internal_iter->mutation_probability = mutation_probability;
-                  internal_iter->target_hidden_fraction =
-                      target_hidden_fraction;
-                  internal_iter->trace = trace;
-                  db_iter.reset(NewDBIterator(
-                      env_, ropt, ImmutableCFOptions(options),
-                      BytewiseComparator(), internal_iter, sequence,
-                      options.max_sequential_skip_in_iterations,
-                      nullptr /*read_callback*/));
-                }
-
-                // Do a random operation. It's important to do it on ref_it
-                // later than on db_iter to make sure ref_it sees the correct
-                // recently_touched_keys.
-                std::string old_key;
-                bool forward = rnd.Next() % 2 > 0;
-                // Do Next()/Prev() ~90% of the time.
-                bool seek = !ref_iter->Valid() || rnd.Next() % 10 == 0;
-                if (trace) {
-                  std::cout << iteration << ": ";
-                }
-
-                if (!seek) {
-                  assert(db_iter->Valid());
-                  old_key = ref_iter->key;
-                  if (trace) {
-                    std::cout << (forward ? "Next" : "Prev") << std::endl;
-                  }
-
-                  if (forward) {
-                    db_iter->Next();
-                    ref_iter->Next();
-                  } else {
-                    db_iter->Prev();
-                    ref_iter->Prev();
-                  }
-                } else {
-                  data.recently_touched_keys.clear();
-                  // Do SeekToFirst less often than Seek.
-                  if (rnd.Next() % 4 == 0) {
-                    if (trace) {
-                      std::cout << (forward ? "SeekToFirst" : "SeekToLast")
-                                << std::endl;
-                    }
-
-                    if (forward) {
-                      old_key = "";
-                      db_iter->SeekToFirst();
-                      ref_iter->SeekToFirst();
-                    } else {
-                      old_key = data.entries.back().key;
-                      db_iter->SeekToLast();
-                      ref_iter->SeekToLast();
-                    }
-                  } else {
-                    old_key = gen_key(max_key);
-                    if (trace) {
-                      std::cout << (forward ? "Seek" : "SeekForPrev") << " \""
-                                << old_key << '"' << std::endl;
-                    }
-                    if (forward) {
-                      db_iter->Seek(old_key);
-                      ref_iter->Seek(old_key);
-                    } else {
-                      db_iter->SeekForPrev(old_key);
-                      ref_iter->SeekForPrev(old_key);
-                    }
-                  }
-                }
-
-                // Check the result.
-                if (db_iter->Valid()) {
-                  ASSERT_TRUE(db_iter->status().ok());
-                  if (data.recently_touched_keys.count(
-                          db_iter->key().ToString())) {
-                    // Ended on a key that may have been mutated during the
-                    // operation. Reference iterator skips such keys, so we
-                    // can't check the exact result.
-
-                    // Check that the key moved in the right direction.
-                    if (forward) {
-                      if (seek)
-                        ASSERT_GE(db_iter->key().ToString(), old_key);
-                      else
-                        ASSERT_GT(db_iter->key().ToString(), old_key);
-                    } else {
-                      if (seek)
-                        ASSERT_LE(db_iter->key().ToString(), old_key);
-                      else
-                        ASSERT_LT(db_iter->key().ToString(), old_key);
-                    }
-
-                    if (ref_iter->Valid()) {
-                      // Check that DBIter didn't miss any non-mutated key.
-                      if (forward) {
-                        ASSERT_LT(db_iter->key().ToString(), ref_iter->key);
-                      } else {
-                        ASSERT_GT(db_iter->key().ToString(), ref_iter->key);
-                      }
-                    }
-                    // Tell the next iteration of the loop to reseek the
-                    // iterators.
-                    ref_iter->valid = false;
-
-                    ++num_recently_removed;
-                  } else {
-                    ASSERT_TRUE(ref_iter->Valid());
-                    ASSERT_EQ(ref_iter->key, db_iter->key().ToString());
-                    ASSERT_EQ(ref_iter->value, db_iter->value());
-                    ++num_matching;
-                  }
-                } else if (db_iter->status().ok()) {
-                  ASSERT_FALSE(ref_iter->Valid());
-                  ++num_at_end;
-                } else {
-                  // Non-ok status. Nothing to check here.
-                  // Tell the next iteration of the loop to reseek the
-                  // iterators.
-                  ref_iter->valid = false;
-                  ++num_not_ok;
-                }
-              }
-            }
-          }
-        }
-      }
-    }
-  }
-
-  // Check that all cases were hit many times.
-  EXPECT_GT(num_matching, 10000);
-  EXPECT_GT(num_at_end, 10000);
-  EXPECT_GT(num_not_ok, 10000);
-  EXPECT_GT(num_recently_removed, 10000);
-
-  std::cout << "stats:\n  exact matches: " << num_matching
-            << "\n  end reached: " << num_at_end
-            << "\n  non-ok status: " << num_not_ok
-            << "\n  mutated on the fly: " << num_recently_removed << std::endl;
-}
-
-}  // namespace rocksdb
-
-int main(int argc, char** argv) {
-  ::testing::InitGoogleTest(&argc, argv);
-  ParseCommandLineFlags(&argc, &argv, true);
-  return RUN_ALL_TESTS();
-}
@@ -36,9 +36,7 @@ class TestIterator : public InternalIterator {
        valid_(false),
        sequence_number_(0),
        iter_(0),
-        cmp(comparator) {
-    data_.reserve(16);
-  }
+        cmp(comparator) {}

  void AddPut(std::string argkey, std::string argvalue) {
    Add(argkey, kTypeValue, argvalue);
@@ -86,36 +84,6 @@ class TestIterator : public InternalIterator {
    });
  }

-  // Removes the key from the set of keys over which this iterator iterates.
-  // Not to be confused with AddDeletion().
-  // If the iterator is currently positioned on this key, the deletion will
-  // apply next time the iterator moves.
-  // Used for simulating ForwardIterator updating to a new version that doesn't
-  // have some of the keys (e.g. after compaction with a filter).
-  void Vanish(std::string _key) {
-    if (valid_ && data_[iter_].first == _key) {
-      delete_current_ = true;
-      return;
-    }
-    for (auto it = data_.begin(); it != data_.end(); ++it) {
-      ParsedInternalKey ikey;
-      bool ok __attribute__((__unused__)) = ParseInternalKey(it->first, &ikey);
-      assert(ok);
-      if (ikey.user_key != _key) {
-        continue;
-      }
-      if (valid_ && data_.begin() + iter_ > it) {
-        --iter_;
-      }
-      data_.erase(it);
-      return;
-    }
-    assert(false);
-  }
-
-  // Number of operations done on this iterator since construction.
-  size_t steps() const { return steps_; }
-
  virtual bool Valid() const override {
    assert(initialized_);
    return valid_;
@@ -123,16 +91,12 @@ class TestIterator : public InternalIterator {

  virtual void SeekToFirst() override {
    assert(initialized_);
-    ++steps_;
-    DeleteCurrentIfNeeded();
    valid_ = (data_.size() > 0);
    iter_ = 0;
  }

  virtual void SeekToLast() override {
    assert(initialized_);
-    ++steps_;
-    DeleteCurrentIfNeeded();
    valid_ = (data_.size() > 0);
    iter_ = data_.size() - 1;
  }
@@ -140,7 +104,6 @@ class TestIterator : public InternalIterator {
  virtual void Seek(const Slice& target) override {
    assert(initialized_);
    SeekToFirst();
-    ++steps_;
    if (!valid_) {
      return;
    }
@@ -156,31 +119,20 @@ class TestIterator : public InternalIterator {

  virtual void SeekForPrev(const Slice& target) override {
    assert(initialized_);
-    DeleteCurrentIfNeeded();
    SeekForPrevImpl(target, &cmp);
  }

  virtual void Next() override {
    assert(initialized_);
-    assert(valid_);
-    assert(iter_ < data_.size());
-
-    ++steps_;
-    if (delete_current_) {
-      DeleteCurrentIfNeeded();
+    if (data_.empty() || (iter_ == data_.size() - 1)) {
+      valid_ = false;
    } else {
      ++iter_;
    }
-    valid_ = iter_ < data_.size();
  }

  virtual void Prev() override {
    assert(initialized_);
-    assert(valid_);
-    assert(iter_ < data_.size());
-
-    ++steps_;
-    DeleteCurrentIfNeeded();
    if (iter_ == 0) {
      valid_ = false;
    } else {
@@ -211,19 +163,9 @@ class TestIterator : public InternalIterator {
  bool valid_;
  size_t sequence_number_;
  size_t iter_;
-  size_t steps_ = 0;

  InternalKeyComparator cmp;
  std::vector<std::pair<std::string, std::string>> data_;
-  bool delete_current_ = false;
-
-  void DeleteCurrentIfNeeded() {
-    if (!delete_current_) {
-      return;
-    }
-    data_.erase(data_.begin() + iter_);
-    delete_current_ = false;
-  }
 };

 class DBIteratorTest : public testing::Test {
@@ -2607,7 +2549,7 @@ TEST_F(DBIterWithMergeIterTest, InnerMergeIteratorDataRace1) {
  // MergeIterator::Prev() realized the mem table iterator is at its end
  // and before an SeekToLast() is called.
  rocksdb::SyncPoint::GetInstance()->SetCallBack(
-      "MergeIterator::Prev:BeforePrev",
+      "MergeIterator::Prev:BeforeSeekToLast",
      [&](void* /*arg*/) { internal_iter2_->Add("z", kTypeValue, "7", 12u); });
  rocksdb::SyncPoint::GetInstance()->EnableProcessing();

@@ -2643,7 +2585,7 @@ TEST_F(DBIterWithMergeIterTest, InnerMergeIteratorDataRace2) {
  // mem table after MergeIterator::Prev() realized the mem tableiterator is at
  // its end and before an SeekToLast() is called.
  rocksdb::SyncPoint::GetInstance()->SetCallBack(
-      "MergeIterator::Prev:BeforePrev", [&](void* /*arg*/) {
+      "MergeIterator::Prev:BeforeSeekToLast", [&](void* /*arg*/) {
        internal_iter2_->Add("z", kTypeValue, "7", 12u);
        internal_iter2_->Add("z", kTypeValue, "7", 11u);
      });
@@ -2681,7 +2623,7 @@ TEST_F(DBIterWithMergeIterTest, InnerMergeIteratorDataRace3) {
  // mem table after MergeIterator::Prev() realized the mem table iterator is at
  // its end and before an SeekToLast() is called.
  rocksdb::SyncPoint::GetInstance()->SetCallBack(
-      "MergeIterator::Prev:BeforePrev", [&](void* /*arg*/) {
+      "MergeIterator::Prev:BeforeSeekToLast", [&](void* /*arg*/) {
        internal_iter2_->Add("z", kTypeValue, "7", 16u, true);
        internal_iter2_->Add("z", kTypeValue, "7", 15u, true);
        internal_iter2_->Add("z", kTypeValue, "7", 14u, true);
@@ -3076,41 +3018,6 @@ TEST_F(DBIteratorTest, SeekLessLowerBound) {
  ASSERT_EQ(lower_bound_str, db_iter->key().ToString());
 }

-TEST_F(DBIteratorTest, ReverseToForwardWithDisappearingKeys) {
-  Options options;
-  options.prefix_extractor.reset(NewCappedPrefixTransform(0));
-
-  TestIterator* internal_iter = new TestIterator(BytewiseComparator());
-  internal_iter->AddPut("a", "A");
-  internal_iter->AddPut("b", "B");
-  for (int i = 0; i < 100; ++i) {
-    internal_iter->AddPut("c" + ToString(i), "");
-  }
-  internal_iter->Finish();
-
-  std::unique_ptr<Iterator> db_iter(NewDBIterator(
-      env_, ReadOptions(), ImmutableCFOptions(options), BytewiseComparator(),
-      internal_iter, 10, options.max_sequential_skip_in_iterations,
-      nullptr /*read_callback*/));
-
-  db_iter->SeekForPrev("a");
-  ASSERT_TRUE(db_iter->Valid());
-  ASSERT_OK(db_iter->status());
-  ASSERT_EQ("a", db_iter->key().ToString());
-
-  internal_iter->Vanish("a");
-  db_iter->Next();
-  ASSERT_TRUE(db_iter->Valid());
-  ASSERT_OK(db_iter->status());
-  ASSERT_EQ("b", db_iter->key().ToString());
-
-  // A (sort of) bug used to cause DBIter to pointlessly drag the internal
-  // iterator all the way to the end. But this doesn't really matter at the time
-  // of writing because the only iterator that can see disappearing keys is
-  // ForwardIterator, which doesn't support SeekForPrev().
-  EXPECT_LT(internal_iter->steps(), 20);
-}
-
 }  // namespace rocksdb

 int main(int argc, char** argv) {
@@ -2043,43 +2043,6 @@ TEST_P(DBIteratorTest, CreationFailure) {
  delete iter;
 }

-TEST_P(DBIteratorTest, UpperBoundWithChangeDirection) {
-  Options options = CurrentOptions();
-  options.max_sequential_skip_in_iterations = 3;
-  DestroyAndReopen(options);
-
-  // write a bunch of kvs to the database.
-  ASSERT_OK(Put("a", "1"));
-  ASSERT_OK(Put("y", "1"));
-  ASSERT_OK(Put("y1", "1"));
-  ASSERT_OK(Put("y2", "1"));
-  ASSERT_OK(Put("y3", "1"));
-  ASSERT_OK(Put("z", "1"));
-  ASSERT_OK(Flush());
-  ASSERT_OK(Put("a", "1"));
-  ASSERT_OK(Put("z", "1"));
-  ASSERT_OK(Put("bar", "1"));
-  ASSERT_OK(Put("foo", "1"));
-
-  std::string upper_bound = "x";
-  Slice ub_slice(upper_bound);
-  ReadOptions ro;
-  ro.iterate_upper_bound = &ub_slice;
-  ro.max_skippable_internal_keys = 1000;
-
-  Iterator* iter = NewIterator(ro);
-  iter->Seek("foo");
-  ASSERT_TRUE(iter->Valid());
-  ASSERT_EQ("foo", iter->key().ToString());
-
-  iter->Prev();
-  ASSERT_TRUE(iter->Valid());
-  ASSERT_OK(iter->status());
-  ASSERT_EQ("bar", iter->key().ToString());
-
-  delete iter;
-}
-
 TEST_P(DBIteratorTest, TableFilter) {
  ASSERT_OK(Put("a", "1"));
  dbfull()->Flush(FlushOptions());
@@ -2146,47 +2109,6 @@ TEST_P(DBIteratorTest, TableFilter) {
  }
 }

-TEST_P(DBIteratorTest, UpperBoundWithPrevReseek) {
-  Options options = CurrentOptions();
-  options.max_sequential_skip_in_iterations = 3;
-  DestroyAndReopen(options);
-
-  // write a bunch of kvs to the database.
-  ASSERT_OK(Put("a", "1"));
-  ASSERT_OK(Put("y", "1"));
-  ASSERT_OK(Put("z", "1"));
-  ASSERT_OK(Flush());
-  ASSERT_OK(Put("a", "1"));
-  ASSERT_OK(Put("z", "1"));
-  ASSERT_OK(Put("bar", "1"));
-  ASSERT_OK(Put("foo", "1"));
-  ASSERT_OK(Put("foo", "2"));
-
-  ASSERT_OK(Put("foo", "3"));
-  ASSERT_OK(Put("foo", "4"));
-  ASSERT_OK(Put("foo", "5"));
-  const Snapshot* snapshot = db_->GetSnapshot();
-  ASSERT_OK(Put("foo", "6"));
-
-  std::string upper_bound = "x";
-  Slice ub_slice(upper_bound);
-  ReadOptions ro;
-  ro.snapshot = snapshot;
-  ro.iterate_upper_bound = &ub_slice;
-
-  Iterator* iter = NewIterator(ro);
-  iter->SeekForPrev("goo");
-  ASSERT_TRUE(iter->Valid());
-  ASSERT_EQ("foo", iter->key().ToString());
-  iter->Prev();
-
-  ASSERT_TRUE(iter->Valid());
-  ASSERT_EQ("bar", iter->key().ToString());
-
-  delete iter;
-  db_->ReleaseSnapshot(snapshot);
-}
-
 TEST_P(DBIteratorTest, SkipStatistics) {
  Options options = CurrentOptions();
  options.statistics = rocksdb::CreateDBStatistics();
@@ -2310,46 +2232,6 @@ TEST_P(DBIteratorTest, SeekAfterHittingManyInternalKeys) {
  ASSERT_EQ(iter2->value().ToString(), "val_6");
 }

-// Reproduces a former bug where iterator would skip some records when DBIter
-// re-seeks subiterator with Incomplete status.
-TEST_P(DBIteratorTest, NonBlockingIterationBugRepro) {
-  Options options = CurrentOptions();
-  BlockBasedTableOptions table_options;
-  // Make sure the sst file has more than one block.
-  table_options.flush_block_policy_factory =
-      std::make_shared<FlushBlockEveryKeyPolicyFactory>();
-  options.table_factory.reset(NewBlockBasedTableFactory(table_options));
-  DestroyAndReopen(options);
-
-  // Two records in sst file, each in its own block.
-  Put("b", "");
-  Put("d", "");
-  Flush();
-
-  // Create a nonblocking iterator before writing to memtable.
-  ReadOptions ropt;
-  ropt.read_tier = kBlockCacheTier;
-  unique_ptr<Iterator> iter(NewIterator(ropt));
-
-  // Overwrite a key in memtable many times to hit
-  // max_sequential_skip_in_iterations (which is 8 by default).
-  for (int i = 0; i < 20; ++i) {
-    Put("c", "");
-  }
-
-  // Load the second block in sst file into the block cache.
-  {
-    unique_ptr<Iterator> iter2(NewIterator(ReadOptions()));
-    iter2->Seek("d");
-  }
-
-  // Finally seek the nonblocking iterator.
-  iter->Seek("a");
-  // With the bug, the status used to be OK, and the iterator used to point to
-  // "d".
-  EXPECT_TRUE(iter->status().IsIncomplete());
-}
-
 INSTANTIATE_TEST_CASE_P(DBIteratorTestInstance, DBIteratorTest,
                        testing::Values(true, false));

@@ -1680,6 +1680,50 @@ TEST_P(DBTestUniversalCompaction, UniversalCompactionSecondPathRatio) {
  Destroy(options);
 }

+TEST_P(DBTestUniversalCompaction, FullCompactionInBottomPriThreadPool) {
+  const int kNumFilesTrigger = 3;
+  Env::Default()->SetBackgroundThreads(1, Env::Priority::BOTTOM);
+  for (bool allow_ingest_behind : {false, true}) {
+    Options options = CurrentOptions();
+    options.allow_ingest_behind = allow_ingest_behind;
+    options.compaction_style = kCompactionStyleUniversal;
+    options.num_levels = num_levels_;
+    options.write_buffer_size = 100 << 10;     // 100KB
+    options.target_file_size_base = 32 << 10;  // 32KB
+    options.level0_file_num_compaction_trigger = kNumFilesTrigger;
+    // Trigger compaction if size amplification exceeds 110%
+    options.compaction_options_universal.max_size_amplification_percent = 110;
+    DestroyAndReopen(options);
+
+    int num_bottom_pri_compactions = 0;
+    SyncPoint::GetInstance()->SetCallBack(
+        "DBImpl::BGWorkBottomCompaction",
+        [&](void* /*arg*/) { ++num_bottom_pri_compactions; });
+    SyncPoint::GetInstance()->EnableProcessing();
+
+    Random rnd(301);
+    for (int num = 0; num < kNumFilesTrigger; num++) {
+      ASSERT_EQ(NumSortedRuns(), num);
+      int key_idx = 0;
+      GenerateNewFile(&rnd, &key_idx);
+    }
+    dbfull()->TEST_WaitForCompact();
+
+    if (allow_ingest_behind || num_levels_ > 1) {
+      // allow_ingest_behind increases number of levels while sanitizing.
+      ASSERT_EQ(1, num_bottom_pri_compactions);
+    } else {
+      // for single-level universal, everything's bottom level so nothing should
+      // be executed in bottom-pri thread pool.
+      ASSERT_EQ(0, num_bottom_pri_compactions);
+    }
+    // Verify that size amplification did occur
+    ASSERT_EQ(NumSortedRuns(), 1);
+    rocksdb::SyncPoint::GetInstance()->DisableProcessing();
+  }
+  Env::Default()->SetBackgroundThreads(0, Env::Priority::BOTTOM);
+}
+
 TEST_P(DBTestUniversalCompaction, ConcurrentBottomPriLowPriCompactions) {
  if (num_levels_ == 1) {
    // for single-level universal, everything's bottom level so nothing should
@@ -50,7 +50,6 @@ TEST_P(DBWriteTest, IOErrorOnWALWritePropagateToWriteThreadFollower) {
  std::atomic<int> leader_count{0};
  std::vector<port::Thread> threads;
  mock_env->SetFilesystemActive(false);
-
  // Wait until all threads linked to write threads, to make sure
  // all threads join the same batch group.
  SyncPoint::GetInstance()->SetCallBack(
@@ -69,19 +68,7 @@ TEST_P(DBWriteTest, IOErrorOnWALWritePropagateToWriteThreadFollower) {
    threads.push_back(port::Thread(
        [&](int index) {
          // All threads should fail.
-          auto res = Put("key" + ToString(index), "value");
-          if (options.manual_wal_flush) {
-            ASSERT_TRUE(res.ok());
-            // we should see fs error when we do the flush
-
-            // TSAN reports a false alarm for lock-order-inversion but Open and
-            // FlushWAL are not run concurrently. Disabling this until TSAN is
-            // fixed.
-            // res = dbfull()->FlushWAL(false);
-            // ASSERT_FALSE(res.ok());
-          } else {
-            ASSERT_FALSE(res.ok());
-          }
+          ASSERT_FALSE(Put("key" + ToString(index), "value").ok());
        },
        i));
  }
@@ -93,22 +80,6 @@ TEST_P(DBWriteTest, IOErrorOnWALWritePropagateToWriteThreadFollower) {
  Close();
 }

-TEST_P(DBWriteTest, ManualWalFlushInEffect) {
-  Options options = GetOptions();
-  Reopen(options);
-  // try the 1st WAL created during open
-  ASSERT_TRUE(Put("key" + ToString(0), "value").ok());
-  ASSERT_TRUE(options.manual_wal_flush != dbfull()->TEST_WALBufferIsEmpty());
-  ASSERT_TRUE(dbfull()->FlushWAL(false).ok());
-  ASSERT_TRUE(dbfull()->TEST_WALBufferIsEmpty());
-  // try the 2nd wal created during SwitchWAL
-  dbfull()->TEST_SwitchWAL();
-  ASSERT_TRUE(Put("key" + ToString(0), "value").ok());
-  ASSERT_TRUE(options.manual_wal_flush != dbfull()->TEST_WALBufferIsEmpty());
-  ASSERT_TRUE(dbfull()->FlushWAL(false).ok());
-  ASSERT_TRUE(dbfull()->TEST_WALBufferIsEmpty());
-}
-
 TEST_P(DBWriteTest, IOErrorOnWALWriteTriggersReadOnlyMode) {
  std::unique_ptr<FaultInjectionTestEnv> mock_env(
      new FaultInjectionTestEnv(Env::Default()));
@@ -119,22 +90,7 @@ TEST_P(DBWriteTest, IOErrorOnWALWriteTriggersReadOnlyMode) {
    // Forcibly fail WAL write for the first Put only. Subsequent Puts should
    // fail due to read-only mode
    mock_env->SetFilesystemActive(i != 0);
-    auto res = Put("key" + ToString(i), "value");
-    // TSAN reports a false alarm for lock-order-inversion but Open and
-    // FlushWAL are not run concurrently. Disabling this until TSAN is
-    // fixed.
-    /*
-    if (options.manual_wal_flush && i == 0) {
-      // even with manual_wal_flush the 2nd Put should return error because of
-      // the read-only mode
-      ASSERT_TRUE(res.ok());
-      // we should see fs error when we do the flush
-      res = dbfull()->FlushWAL(false);
-    }
-    */
-    if (!options.manual_wal_flush) {
-      ASSERT_FALSE(res.ok());
-    }
+    ASSERT_FALSE(Put("key" + ToString(i), "value").ok());
  }
  // Close before mock_env destruct.
  Close();
@@ -27,7 +27,7 @@ namespace rocksdb {
 // Usage:
 //     ForwardLevelIterator iter;
 //     iter.SetFileIndex(file_index);
-//     iter.Seek(target); // or iter.SeekToFirst();
+//     iter.Seek(target);
 //     iter.Next()
 class ForwardLevelIterator : public InternalIterator {
 public:
@@ -53,11 +53,11 @@ class ForwardLevelIterator : public InternalIterator {

  void SetFileIndex(uint32_t file_index) {
    assert(file_index < files_.size());
-    status_ = Status::OK();
    if (file_index != file_index_) {
      file_index_ = file_index;
      Reset();
    }
+    valid_ = false;
  }
  void Reset() {
    assert(file_index_ < files_.size());
@@ -77,10 +77,10 @@ class ForwardLevelIterator : public InternalIterator {
        read_options_.ignore_range_deletions ? nullptr : &range_del_agg,
        nullptr /* table_reader_ptr */, nullptr, false);
    file_iter_->SetPinnedItersMgr(pinned_iters_mgr_);
-    valid_ = false;
    if (!range_del_agg.IsEmpty()) {
      status_ = Status::NotSupported(
          "Range tombstones unsupported with ForwardIterator");
+      valid_ = false;
    }
  }
  void SeekToLast() override {
@@ -95,27 +95,12 @@ class ForwardLevelIterator : public InternalIterator {
    return valid_;
  }
  void SeekToFirst() override {
-    assert(file_iter_ != nullptr);
-    if (!status_.ok()) {
-      assert(!valid_);
-      return;
-    }
+    SetFileIndex(0);
    file_iter_->SeekToFirst();
    valid_ = file_iter_->Valid();
  }
  void Seek(const Slice& internal_key) override {
    assert(file_iter_ != nullptr);
-
-    // This deviates from the usual convention for InternalIterator::Seek() in
-    // that it doesn't discard pre-existing error status. That's because this
-    // Seek() is only supposed to be called immediately after SetFileIndex()
-    // (which discards pre-existing error status), and SetFileIndex() may set
-    // an error status, which we shouldn't discard.
-    if (!status_.ok()) {
-      assert(!valid_);
-      return;
-    }
-
    file_iter_->Seek(internal_key);
    valid_ = file_iter_->Valid();
  }
@@ -127,12 +112,8 @@ class ForwardLevelIterator : public InternalIterator {
    assert(valid_);
    file_iter_->Next();
    for (;;) {
-      valid_ = file_iter_->Valid();
-      if (!file_iter_->status().ok()) {
-        assert(!valid_);
-        return;
-      }
-      if (valid_) {
+      if (file_iter_->status().IsIncomplete() || file_iter_->Valid()) {
+        valid_ = !file_iter_->status().IsIncomplete();
        return;
      }
      if (file_index_ + 1 >= files_.size()) {
@@ -140,10 +121,6 @@ class ForwardLevelIterator : public InternalIterator {
        return;
      }
      SetFileIndex(file_index_ + 1);
-      if (!status_.ok()) {
-        assert(!valid_);
-        return;
-      }
      file_iter_->SeekToFirst();
    }
  }
@@ -158,7 +135,7 @@ class ForwardLevelIterator : public InternalIterator {
  Status status() const override {
    if (!status_.ok()) {
      return status_;
-    } else if (file_iter_) {
+    } else if (file_iter_ && !file_iter_->status().ok()) {
      return file_iter_->status();
    }
    return Status::OK();
@@ -322,6 +299,9 @@ bool ForwardIterator::IsOverUpperBound(const Slice& internal_key) const {
 }

 void ForwardIterator::Seek(const Slice& internal_key) {
+  if (IsOverUpperBound(internal_key)) {
+    valid_ = false;
+  }
  if (sv_ == nullptr) {
    RebuildIterators(true);
  } else if (sv_->version_number != cfd_->GetSuperVersionNumber()) {
@@ -625,9 +605,7 @@ void ForwardIterator::RebuildIterators(bool refresh_sv) {
    if ((read_options_.iterate_upper_bound != nullptr) &&
        cfd_->internal_comparator().user_comparator()->Compare(
            l0->smallest.user_key(), *read_options_.iterate_upper_bound) > 0) {
-      // No need to set has_iter_trimmed_for_upper_bound_: this ForwardIterator
-      // will never be interested in files with smallest key above
-      // iterate_upper_bound, since iterate_upper_bound can't be changed.
+      has_iter_trimmed_for_upper_bound_ = true;
      l0_iters_.push_back(nullptr);
      continue;
    }
@@ -795,7 +773,7 @@ void ForwardIterator::UpdateCurrent() {
      current_ = mutable_iter_;
    }
  }
-  valid_ = current_ != nullptr && immutable_status_.ok();
+  valid_ = (current_ != nullptr);
  if (!status_.ok()) {
    status_ = Status::OK();
  }
@@ -92,8 +92,6 @@ Status Writer::AddRecord(const Slice& slice) {
  return s;
 }

-bool Writer::TEST_BufferIsEmpty() { return dest_->TEST_BufferIsEmpty(); }
-
 Status Writer::EmitPhysicalRecord(RecordType t, const char* ptr, size_t n) {
  assert(n <= 0xffff);  // Must fit in two bytes

@@ -85,8 +85,6 @@ class Writer {

  Status WriteBuffer();

-  bool TEST_BufferIsEmpty();
-
 private:
  unique_ptr<WritableFileWriter> dest_;
  size_t block_offset_;       // Current offset in block
@@ -101,7 +101,9 @@ void ManagedIterator::SeekToLast() {
  }
  assert(mutable_iter_ != nullptr);
  mutable_iter_->SeekToLast();
-  UpdateCurrent();
+  if (mutable_iter_->status().ok()) {
+    UpdateCurrent();
+  }
 }

 void ManagedIterator::SeekToFirst() {
@@ -144,13 +146,27 @@ void ManagedIterator::Prev() {
  }
  MILock l(&in_use_, this);
  if (NeedToRebuild()) {
-    RebuildIterator(true);
+    std::string current_key = key().ToString();
+    Slice old_key(current_key);
+    RebuildIterator();
+    SeekInternal(old_key, false);
+    UpdateCurrent();
    if (!valid_) {
      return;
    }
+    if (key().compare(old_key) != 0) {
+      valid_ = false;
+      status_ = Status::Incomplete("Cannot do Prev now");
+      return;
+    }
  }
  mutable_iter_->Prev();
-  UpdateCurrent();
+  if (mutable_iter_->status().ok()) {
+    UpdateCurrent();
+    status_ = Status::OK();
+  } else {
+    status_ = mutable_iter_->status();
+  }
 }

 void ManagedIterator::Next() {
@@ -160,10 +176,19 @@ void ManagedIterator::Next() {
  }
  MILock l(&in_use_, this);
  if (NeedToRebuild()) {
-    RebuildIterator(true);
+    std::string current_key = key().ToString();
+    Slice old_key(current_key.data(), cached_key_.Size());
+    RebuildIterator();
+    SeekInternal(old_key, false);
+    UpdateCurrent();
    if (!valid_) {
      return;
    }
+    if (key().compare(old_key) != 0) {
+      valid_ = false;
+      status_ = Status::Incomplete("Cannot do Next now");
+      return;
+    }
  }
  mutable_iter_->Next();
  UpdateCurrent();
@@ -181,38 +206,21 @@ Slice ManagedIterator::value() const {

 Status ManagedIterator::status() const { return status_; }

-void ManagedIterator::RebuildIterator(bool reseek) {
-  std::string current_key;
-  if (reseek) {
-    current_key = key().ToString();
-  }
-
+void ManagedIterator::RebuildIterator() {
  svnum_ = cfd_->GetSuperVersionNumber();
  mutable_iter_ = unique_ptr<Iterator>(db_->NewIterator(read_options_, &cfh_));
-
-  if (reseek) {
-    Slice old_key(current_key.data(), current_key.size());
-    SeekInternal(old_key, false);
-    UpdateCurrent();
-    if (!valid_ || key().compare(old_key) != 0) {
-      valid_ = false;
-      status_ = Status::Incomplete(
-          "Next/Prev failed because current key has "
-          "been removed");
-    }
-  }
 }

 void ManagedIterator::UpdateCurrent() {
  assert(mutable_iter_ != nullptr);

  valid_ = mutable_iter_->Valid();
-  status_ = mutable_iter_->status();
-
  if (!valid_) {
+    status_ = mutable_iter_->status();
    return;
  }

+  status_ = Status::OK();
  cached_key_.SetUserKey(mutable_iter_->key());
  cached_value_.SetUserKey(mutable_iter_->value());
 }
@@ -54,7 +54,7 @@ class ManagedIterator : public Iterator {
  }

 private:
-  void RebuildIterator(bool reseek = false);
+  void RebuildIterator();
  void UpdateCurrent();
  void SeekInternal(const Slice& user_key, bool seek_to_first);
  bool NeedToRebuild();
@@ -696,7 +696,7 @@ static bool SaveValue(void* arg, const char* entry) {
        *(s->merge_in_progress) = true;
        merge_context->PushOperand(
            v, s->inplace_update_support == false /* operand_pinned */);
-        if (merge_operator->ShouldMerge(merge_context->GetOperandsDirectionBackward())) {
+        if (merge_operator->ShouldMerge(merge_context->GetOperands())) {
          *(s->status) = MergeHelper::TimedFullMerge(
              merge_operator, s->key->user_key(), nullptr,
              merge_context->GetOperands(), s->value, s->logger, s->statistics,
@@ -74,13 +74,8 @@ class MergeContext {
    return (*operand_list_)[index];
  }

-  // Same as GetOperandsDirectionForward
+  // Return all the operands.
  const std::vector<Slice>& GetOperands() {
-    return GetOperandsDirectionForward();
-  }
-
-  // Return all the operands in the order as they were merged (passed to FullMerge or FullMergeV2)
-  const std::vector<Slice>& GetOperandsDirectionForward() {
    if (!operand_list_) {
      return empty_operand_list;
    }
@@ -89,16 +84,6 @@ class MergeContext {
    return *operand_list_;
  }

-  // Return all the operands in the reversed order relative to how they were merged (passed to FullMerge or FullMergeV2)
-  const std::vector<Slice>& GetOperandsDirectionBackward() {
-    if (!operand_list_) {
-      return empty_operand_list;
-    }
-
-    SetDirectionBackward();
-    return *operand_list_;
-  }
-
 private:
  void Initialize() {
    if (!operand_list_) {
@@ -205,7 +205,7 @@ class Repairer {
      ROCKS_LOG_WARN(db_options_.info_log,
                     "**** Repaired rocksdb %s; "
                     "recovered %" ROCKSDB_PRIszt " files; %" PRIu64
-                     " bytes. "
+                     "bytes. "
                     "Some data may have been lost. "
                     "****",
                     dbname_.c_str(), tables_.size(), bytes);
@@ -502,7 +502,13 @@ class LevelIterator final : public InternalIterator {
    return file_iter_.value();
  }
  virtual Status status() const override {
-    return file_iter_.iter() ? file_iter_.status() : Status::OK();
+    // It'd be nice if status() returned a const Status& instead of a Status
+    if (!status_.ok()) {
+      return status_;
+    } else if (file_iter_.iter() != nullptr) {
+      return file_iter_.status();
+    }
+    return Status::OK();
  }
  virtual void SetPinnedItersMgr(
      PinnedIteratorsManager* pinned_iters_mgr) override {
@@ -567,6 +573,7 @@ class LevelIterator final : public InternalIterator {
  RangeDelAggregator* range_del_agg_;
  IteratorWrapper file_iter_;  // May be nullptr
  PinnedIteratorsManager* pinned_iters_mgr_;
+  Status status_;
 };

 void LevelIterator::Seek(const Slice& target) {
@@ -621,9 +628,16 @@ void LevelIterator::Prev() {
 }

 void LevelIterator::SkipEmptyFileForward() {
+  // For an error (IO error, checksum mismatch, etc), we skip the file
+  // and move to the next one and continue reading data.
+  // TODO this behavior is from LevelDB. We should revisit it.
  while (file_iter_.iter() == nullptr ||
-         (!file_iter_.Valid() && file_iter_.status().ok() &&
-          !file_iter_.iter()->IsOutOfBound())) {
+         (!file_iter_.Valid() && !file_iter_.status().IsIncomplete())) {
+    if (file_iter_.iter() != nullptr && !file_iter_.Valid() &&
+        file_iter_.iter()->IsOutOfBound()) {
+      return;
+    }
+
    // Move to next file
    if (file_index_ >= flevel_->num_files - 1) {
      // Already at the last file
@@ -643,7 +657,7 @@ void LevelIterator::SkipEmptyFileForward() {

 void LevelIterator::SkipEmptyFileBackward() {
  while (file_iter_.iter() == nullptr ||
-         (!file_iter_.Valid() && file_iter_.status().ok())) {
+         (!file_iter_.Valid() && !file_iter_.status().IsIncomplete())) {
    // Move to previous file
    if (file_index_ == 0) {
      // Already the first file
@@ -658,6 +672,13 @@ void LevelIterator::SkipEmptyFileBackward() {
 }

 void LevelIterator::SetFileIterator(InternalIterator* iter) {
+  if (file_iter_.iter() != nullptr && status_.ok()) {
+    // TODO right now we don't invalidate the iterator even if the status is
+    // not OK. We should consider to do that so that it is harder for users to
+    // skip errors.
+    status_ = file_iter_.status();
+  }
+
  if (pinned_iters_mgr_ && iter) {
    iter->SetPinnedItersMgr(pinned_iters_mgr_);
  }
@@ -33,7 +33,6 @@ class Iterator : public Cleanable {

  // An iterator is either positioned at a key/value pair, or
  // not valid.  This method returns true iff the iterator is valid.
-  // Always returns false if !status().ok().
  virtual bool Valid() const = 0;

  // Position at the first key in the source.  The iterator is Valid()
@@ -47,9 +46,6 @@ class Iterator : public Cleanable {
  // Position at the first key in the source that at or past target.
  // The iterator is Valid() after this call iff the source contains
  // an entry that comes at or past target.
-  // All Seek*() methods clear any error status() that the iterator had prior to
-  // the call; after the seek, status() indicates only the error (if any) that
-  // happened during the seek, not any past errors.
  virtual void Seek(const Slice& target) = 0;

  // Position at the last key in the source that at or before target.
@@ -76,7 +72,7 @@ class Iterator : public Cleanable {
  // Return the value for the current entry.  The underlying storage for
  // the returned slice is valid only until the next modification of
  // the iterator.
-  // REQUIRES: Valid()
+  // REQUIRES: !AtEnd() && !AtStart()
  virtual Slice value() const = 0;

  // If an error has occurred, return it.  Else return an ok status.
@@ -195,11 +195,6 @@ class MergeOperator {
  // during a point lookup, thereby helping in limiting the number of levels to
  // read from.
  // Doesn't help with iterators.
-  //
-  // Note: the merge operands are passed to this function in the reversed order
-  // relative to how they were merged (passed to FullMerge or FullMergeV2)
-  // for performance reasons, see also:
-  // https://github.com/facebook/rocksdb/issues/3865
  virtual bool ShouldMerge(const std::vector<Slice>& /*operands*/) const {
    return false;
  }
@@ -98,13 +98,12 @@ class SstFileManager {
 //    DeleteScheduler immediately
 // @param bytes_max_delete_chunk: if a single file is larger than delete chunk,
 //    ftruncate the file by this size each time, rather than dropping the whole
-//    file. 0 means to always delete the whole file. NOTE this options may not
-//    work well with checkpoints, which relies on file system hard links.
+//    file. 0 means to always delete the whole file.
 extern SstFileManager* NewSstFileManager(
    Env* env, std::shared_ptr<Logger> info_log = nullptr,
    std::string trash_dir = "", int64_t rate_bytes_per_sec = 0,
    bool delete_existing_trash = true, Status* status = nullptr,
    double max_trash_db_ratio = 0.25,
-    uint64_t bytes_max_delete_chunk = 0);
+    uint64_t bytes_max_delete_chunk = 64 * 1024 * 1024);

 }  // namespace rocksdb
@@ -210,20 +210,12 @@ class LDBCommand {
  bool ParseStringOption(const std::map<std::string, std::string>& options,
                         const std::string& option, std::string* value);

-  /**
-   * Returns the value of the specified option as a boolean.
-   * default_val is used if the option is not found in options.
-   * Throws an exception if the value of the option is not
-   * "true" or "false" (case insensitive).
-   */
-  bool ParseBooleanOption(const std::map<std::string, std::string>& options,
-                          const std::string& option, bool default_val);
-
  Options options_;
  std::vector<ColumnFamilyDescriptor> column_families_;
  LDBOptions ldb_options_;

 private:
+  friend class WALDumperCommand;
  /**
   * Interpret command line options and flags to determine if the key
   * should be input/output in hex.
@@ -238,6 +230,15 @@ class LDBCommand {
  bool IsValueHex(const std::map<std::string, std::string>& options,
                  const std::vector<std::string>& flags);

+  /**
+   * Returns the value of the specified option as a boolean.
+   * default_val is used if the option is not found in options.
+   * Throws an exception if the value of the option is not
+   * "true" or "false" (case insensitive).
+   */
+  bool ParseBooleanOption(const std::map<std::string, std::string>& options,
+                          const std::string& option, bool default_val);
+
  /**
   * Converts val to a boolean.
   * val must be either true or false (case insensitive).
@@ -193,7 +193,6 @@ class TransactionDB : public StackableDB {
  }
  // Open a TransactionDB similar to DB::Open().
  // Internally call PrepareWrap() and WrapDB()
-  // If the return status is not ok, then dbptr is set to nullptr.
  static Status Open(const Options& options,
                     const TransactionDBOptions& txn_db_options,
                     const std::string& dbname, TransactionDB** dbptr);
@@ -204,29 +203,27 @@ class TransactionDB : public StackableDB {
                     const std::vector<ColumnFamilyDescriptor>& column_families,
                     std::vector<ColumnFamilyHandle*>* handles,
                     TransactionDB** dbptr);
+  // The following functions are used to open a TransactionDB internally using
+  // an opened DB or StackableDB.
+  // 1. Call prepareWrap(), passing an empty std::vector<size_t> to
+  // compaction_enabled_cf_indices.
+  // 2. Open DB or Stackable DB with db_options and column_families passed to
+  // prepareWrap()
  // Note: PrepareWrap() may change parameters, make copies before the
  // invocation if needed.
+  // 3. Call Wrap*DB() with compaction_enabled_cf_indices in step 1 and handles
+  // of the opened DB/StackableDB in step 2
  static void PrepareWrap(DBOptions* db_options,
                          std::vector<ColumnFamilyDescriptor>* column_families,
                          std::vector<size_t>* compaction_enabled_cf_indices);
-  // If the return status is not ok, then dbptr will bet set to nullptr. The
-  // input db parameter might or might not be deleted as a result of the
-  // failure. If it is properly deleted it will be set to nullptr. If the return
-  // status is ok, the ownership of db is transferred to dbptr.
  static Status WrapDB(DB* db, const TransactionDBOptions& txn_db_options,
                       const std::vector<size_t>& compaction_enabled_cf_indices,
                       const std::vector<ColumnFamilyHandle*>& handles,
                       TransactionDB** dbptr);
-  // If the return status is not ok, then dbptr will bet set to nullptr. The
-  // input db parameter might or might not be deleted as a result of the
-  // failure. If it is properly deleted it will be set to nullptr. If the return
-  // status is ok, the ownership of db is transferred to dbptr.
  static Status WrapStackableDB(
      StackableDB* db, const TransactionDBOptions& txn_db_options,
      const std::vector<size_t>& compaction_enabled_cf_indices,
      const std::vector<ColumnFamilyHandle*>& handles, TransactionDB** dbptr);
-  // Since the destructor in StackableDB is virtual, this destructor is virtual
-  // too. The root db will be deleted by the base's destructor.
  ~TransactionDB() override {}

  // Starts a new Transaction.
@@ -255,7 +252,6 @@ class TransactionDB : public StackableDB {

 protected:
  // To Create an TransactionDB, call Open()
-  // The ownership of db is transferred to the base StackableDB
  explicit TransactionDB(DB* db) : StackableDB(db) {}

 private:
@@ -5,8 +5,8 @@
 #pragma once

 #define ROCKSDB_MAJOR 5
-#define ROCKSDB_MINOR 14
-#define ROCKSDB_PATCH 3
+#define ROCKSDB_MINOR 13
+#define ROCKSDB_PATCH 0

 // 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
@@ -3230,7 +3230,7 @@ Java_org_rocksdb_ColumnFamilyOptions_setCompactionFilterFactoryHandle(
    JNIEnv* /*env*/, jobject /*jobj*/, jlong jopt_handle,
    jlong jcompactionfilterfactory_handle) {
  auto* cff_factory =
-      reinterpret_cast<std::shared_ptr<rocksdb::CompactionFilterFactoryJniCallback>*>(
+      reinterpret_cast<std::shared_ptr<rocksdb::CompactionFilterFactory>*>(
          jcompactionfilterfactory_handle);
  reinterpret_cast<rocksdb::ColumnFamilyOptions*>(jopt_handle)
      ->compaction_filter_factory = *cff_factory;
@@ -4292,12 +4292,25 @@ class JniUtil {
     * @param bytes The bytes to copy
     *
     * @return the Java byte[] or nullptr if an exception occurs
-     * 
-     * @throws RocksDBException thrown 
-     *   if memory size to copy exceeds general java specific array size limitation.
     */
    static jbyteArray copyBytes(JNIEnv* env, std::string bytes) {
-      return createJavaByteArrayWithSizeCheck(env, bytes.c_str(), bytes.size());
+      const jsize jlen = static_cast<jsize>(bytes.size());
+
+      jbyteArray jbytes = env->NewByteArray(jlen);
+      if(jbytes == nullptr) {
+        // exception thrown: OutOfMemoryError
+        return nullptr;
+      }
+
+      env->SetByteArrayRegion(jbytes, 0, jlen,
+        const_cast<jbyte*>(reinterpret_cast<const jbyte*>(bytes.c_str())));
+      if(env->ExceptionCheck()) {
+        // exception thrown: ArrayIndexOutOfBoundsException
+        env->DeleteLocalRef(jbytes);
+        return nullptr;
+      }
+
+      return jbytes;
    }

    /**
@@ -4460,47 +4473,6 @@ class JniUtil {

      return jbyte_strings;
    }
-    
-    /**
-      * Copies bytes to a new jByteArray with the check of java array size limitation.
-      *
-      * @param bytes pointer to memory to copy to a new jByteArray
-      * @param size number of bytes to copy
-      *
-      * @return the Java byte[] or nullptr if an exception occurs
-      * 
-      * @throws RocksDBException thrown 
-      *   if memory size to copy exceeds general java array size limitation to avoid overflow.
-      */
-    static jbyteArray createJavaByteArrayWithSizeCheck(JNIEnv* env, const char* bytes, const size_t size) {
-      // Limitation for java array size is vm specific
-      // In general it cannot exceed Integer.MAX_VALUE (2^31 - 1)
-      // Current HotSpot VM limitation for array size is Integer.MAX_VALUE - 5 (2^31 - 1 - 5)
-      // It means that the next call to env->NewByteArray can still end with 
-      // OutOfMemoryError("Requested array size exceeds VM limit") coming from VM
-      static const size_t MAX_JARRAY_SIZE = (static_cast<size_t>(1)) << 31;
-      if(size > MAX_JARRAY_SIZE) {
-        rocksdb::RocksDBExceptionJni::ThrowNew(env, "Requested array size exceeds VM limit");
-        return nullptr;
-      }
-      
-      const jsize jlen = static_cast<jsize>(size);
-      jbyteArray jbytes = env->NewByteArray(jlen);
-      if(jbytes == nullptr) {
-        // exception thrown: OutOfMemoryError	
-        return nullptr;
-      }
-      
-      env->SetByteArrayRegion(jbytes, 0, jlen,
-        const_cast<jbyte*>(reinterpret_cast<const jbyte*>(bytes)));
-      if(env->ExceptionCheck()) {
-        // exception thrown: ArrayIndexOutOfBoundsException
-        env->DeleteLocalRef(jbytes);
-        return nullptr;
-      }
-
-      return jbytes;
-    }

    /**
     * Copies bytes from a rocksdb::Slice to a jByteArray
@@ -4509,12 +4481,25 @@ class JniUtil {
     * @param bytes The bytes to copy
     *
     * @return the Java byte[] or nullptr if an exception occurs
-     * 
-     * @throws RocksDBException thrown 
-     *   if memory size to copy exceeds general java specific array size limitation.
     */
    static jbyteArray copyBytes(JNIEnv* env, const Slice& bytes) {
-      return createJavaByteArrayWithSizeCheck(env, bytes.data(), bytes.size());
+      const jsize jlen = static_cast<jsize>(bytes.size());
+
+      jbyteArray jbytes = env->NewByteArray(jlen);
+      if(jbytes == nullptr) {
+        // exception thrown: OutOfMemoryError
+        return nullptr;
+      }
+
+      env->SetByteArrayRegion(jbytes, 0, jlen,
+        const_cast<jbyte*>(reinterpret_cast<const jbyte*>(bytes.data())));
+      if(env->ExceptionCheck()) {
+        // exception thrown: ArrayIndexOutOfBoundsException
+        env->DeleteLocalRef(jbytes);
+        return nullptr;
+      }
+
+      return jbytes;
    }

    /*
@@ -138,7 +138,7 @@ jobject Java_org_rocksdb_SstFileManager_getTrackedFiles(JNIEnv* env,

  const rocksdb::HashMapJni::FnMapKV<const std::string, const uint64_t>
      fn_map_kv =
-          [env](
+          [env, &tracked_files](
              const std::pair<const std::string, const uint64_t>& pair) {
            const jstring jtracked_file_path =
                env->NewStringUTF(pair.first.c_str());
@@ -48,7 +48,7 @@ void Java_org_rocksdb_Transaction_setSnapshotOnNextOperation__JJ(
    jlong jtxn_notifier_handle) {
  auto* txn = reinterpret_cast<rocksdb::Transaction*>(jhandle);
  auto* txn_notifier =
-      reinterpret_cast<std::shared_ptr<rocksdb::TransactionNotifierJniCallback>*>(
+      reinterpret_cast<std::shared_ptr<rocksdb::TransactionNotifier>*>(
          jtxn_notifier_handle);
  txn->SetSnapshotOnNextOperation(*txn_notifier);
 }
@@ -309,7 +309,7 @@ jobject Java_org_rocksdb_TransactionDB_getLockStatusData(JNIEnv* env,

  const rocksdb::HashMapJni::FnMapKV<const int32_t, const rocksdb::KeyLockInfo>
      fn_map_kv =
-          [env](
+          [env, txn_db, &lock_status_data](
              const std::pair<const int32_t, const rocksdb::KeyLockInfo>&
                  pair) {
            const jobject jlong_column_family_id =
@@ -28,10 +28,8 @@ public class ColumnFamilyHandle extends RocksObject {
   * Gets the name of the Column Family.
   *
   * @return The name of the Column Family.
-   *
-   * @throws RocksDBException if an error occurs whilst retrieving the name.
   */
-  public byte[] getName() throws RocksDBException {
+  public byte[] getName() {
    return getName(nativeHandle_);
  }

@@ -73,22 +71,14 @@ public class ColumnFamilyHandle extends RocksObject {
    }

    final ColumnFamilyHandle that = (ColumnFamilyHandle) o;
-    try {
-      return rocksDB_.nativeHandle_ == that.rocksDB_.nativeHandle_ &&
-          getID() == that.getID() &&
-          Arrays.equals(getName(), that.getName());
-    } catch (RocksDBException e) {
-      throw new RuntimeException("Cannot compare column family handles", e);
-    }
+    return rocksDB_.nativeHandle_ == that.rocksDB_.nativeHandle_ &&
+        getID() == that.getID() &&
+        Arrays.equals(getName(), that.getName());
  }

  @Override
  public int hashCode() {
-    try {
-      return Objects.hash(getName(), getID(), rocksDB_.nativeHandle_);
-    } catch (RocksDBException e) {
-      throw new RuntimeException("Cannot calculate hash code of column family handle", e);
-    }
+    return Objects.hash(getName(), getID(), rocksDB_.nativeHandle_);
  }

  /**
@@ -106,7 +96,7 @@ public class ColumnFamilyHandle extends RocksObject {
    }
  }

-  private native byte[] getName(final long handle) throws RocksDBException;
+  private native byte[] getName(final long handle);
  private native int getID(final long handle);
  private native ColumnFamilyDescriptor getDescriptor(final long handle) throws RocksDBException;
  @Override protected final native void disposeInternal(final long handle);
@@ -65,11 +65,8 @@ public class WriteBatch extends AbstractWriteBatch {
   * Retrieve the serialized version of this batch.
   *
   * @return the serialized representation of this write batch.
-   *
-   * @throws RocksDBException if an error occurs whilst retrieving
-   *   the serialized batch data.
   */
-  public byte[] data() throws RocksDBException {
+  public byte[] data() {
    return data(nativeHandle_);
  }

@@ -256,7 +253,7 @@ public class WriteBatch extends AbstractWriteBatch {
      final int serializedLength);
  private native void iterate(final long handle, final long handlerHandle)
      throws RocksDBException;
-  private native byte[] data(final long nativeHandle) throws RocksDBException;
+  private native byte[] data(final long nativeHandle);
  private native long getDataSize(final long nativeHandle);
  private native boolean hasPut(final long nativeHandle);
  private native boolean hasDelete(final long nativeHandle);
@@ -4,11 +4,9 @@
 //  (found in the LICENSE.Apache file in the root directory).
 package org.rocksdb;

-import org.junit.Assume;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;

 import java.util.*;
@@ -145,39 +143,6 @@ public class RocksDBTest {
    }
  }

-  @Rule
-  public ExpectedException thrown = ExpectedException.none();
-
-  @Test
-  public void getOutOfArrayMaxSizeValue() throws RocksDBException {
-    final int numberOfValueSplits = 10;
-    final int splitSize = Integer.MAX_VALUE / numberOfValueSplits;
-
-    Runtime runtime = Runtime.getRuntime();
-    long neededMemory = ((long)(splitSize)) * (((long)numberOfValueSplits) + 3);
-    boolean isEnoughMemory = runtime.maxMemory() - runtime.totalMemory() > neededMemory;
-    Assume.assumeTrue(isEnoughMemory);
-
-    final byte[] valueSplit = new byte[splitSize];
-    final byte[] key = "key".getBytes();
-
-    thrown.expect(RocksDBException.class);
-    thrown.expectMessage("Requested array size exceeds VM limit");
-
-    // merge (numberOfValueSplits + 1) valueSplit's to get value size exceeding Integer.MAX_VALUE
-    try (final StringAppendOperator stringAppendOperator = new StringAppendOperator();
-         final Options opt = new Options()
-                 .setCreateIfMissing(true)
-                 .setMergeOperator(stringAppendOperator);
-         final RocksDB db = RocksDB.open(opt, dbFolder.getRoot().getAbsolutePath())) {
-      db.put(key, valueSplit);
-      for (int i = 0; i < numberOfValueSplits; i++) {
-        db.merge(key, valueSplit);
-      }
-      db.get(key);
-    }
-  }
-
  @Test
  public void multiGet() throws RocksDBException, InterruptedException {
    try (final RocksDB db = RocksDB.open(dbFolder.getRoot().getAbsolutePath());
@@ -10,21 +10,25 @@

 namespace rocksdb {
 namespace {
-Statistics* stats_for_report(Env* env, Statistics* stats) {
-  if (env != nullptr && stats != nullptr &&
-      stats->stats_level_ > kExceptTimeForMutex) {
-    return stats;
-  } else {
-    return nullptr;
-  }
+bool ShouldReportToStats(Env* env, Statistics* stats) {
+  return env != nullptr && stats != nullptr &&
+          stats->stats_level_ > kExceptTimeForMutex;
 }
 }  // namespace

 void InstrumentedMutex::Lock() {
-  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(
-      db_mutex_lock_nanos, stats_code_ == DB_MUTEX_WAIT_MICROS,
-      stats_for_report(env_, stats_), stats_code_);
-  LockInternal();
+  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(db_mutex_lock_nanos,
+                                         stats_code_ == DB_MUTEX_WAIT_MICROS);
+  uint64_t wait_time_micros = 0;
+  if (ShouldReportToStats(env_, stats_)) {
+    {
+      StopWatch sw(env_, nullptr, 0, &wait_time_micros);
+      LockInternal();
+    }
+    RecordTick(stats_, stats_code_, wait_time_micros);
+  } else {
+    LockInternal();
+  }
 }

 void InstrumentedMutex::LockInternal() {
@@ -35,10 +39,18 @@ void InstrumentedMutex::LockInternal() {
 }

 void InstrumentedCondVar::Wait() {
-  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(
-      db_condition_wait_nanos, stats_code_ == DB_MUTEX_WAIT_MICROS,
-      stats_for_report(env_, stats_), stats_code_);
-  WaitInternal();
+  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(db_condition_wait_nanos,
+                                         stats_code_ == DB_MUTEX_WAIT_MICROS);
+  uint64_t wait_time_micros = 0;
+  if (ShouldReportToStats(env_, stats_)) {
+    {
+      StopWatch sw(env_, nullptr, 0, &wait_time_micros);
+      WaitInternal();
+    }
+    RecordTick(stats_, stats_code_, wait_time_micros);
+  } else {
+    WaitInternal();
+  }
 }

 void InstrumentedCondVar::WaitInternal() {
@@ -49,10 +61,20 @@ void InstrumentedCondVar::WaitInternal() {
 }

 bool InstrumentedCondVar::TimedWait(uint64_t abs_time_us) {
-  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(
-      db_condition_wait_nanos, stats_code_ == DB_MUTEX_WAIT_MICROS,
-      stats_for_report(env_, stats_), stats_code_);
-  return TimedWaitInternal(abs_time_us);
+  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(db_condition_wait_nanos,
+                                         stats_code_ == DB_MUTEX_WAIT_MICROS);
+  uint64_t wait_time_micros = 0;
+  bool result = false;
+  if (ShouldReportToStats(env_, stats_)) {
+    {
+      StopWatch sw(env_, nullptr, 0, &wait_time_micros);
+      result = TimedWaitInternal(abs_time_us);
+    }
+    RecordTick(stats_, stats_code_, wait_time_micros);
+  } else {
+    result = TimedWaitInternal(abs_time_us);
+  }
+  return result;
 }

 bool InstrumentedCondVar::TimedWaitInternal(uint64_t abs_time_us) {
@@ -41,12 +41,10 @@ extern __thread PerfContext perf_context;
  PerfStepTimer perf_step_timer_##metric(&(perf_context.metric)); \
  perf_step_timer_##metric.Start();

-#define PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(metric, condition, stats,      \
-                                               ticker_type)                   \
-  PerfStepTimer perf_step_timer_##metric(&(perf_context.metric), true, stats, \
-                                         ticker_type);                        \
-  if (condition) {                                                            \
-    perf_step_timer_##metric.Start();                                         \
+#define PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(metric, condition)       \
+  PerfStepTimer perf_step_timer_##metric(&(perf_context.metric), true); \
+  if (condition) {                                                      \
+    perf_step_timer_##metric.Start();                                   \
  }

 // Update metric with time elapsed since last START. start time is reset
@@ -12,25 +12,19 @@ namespace rocksdb {

 class PerfStepTimer {
 public:
-  explicit PerfStepTimer(uint64_t* metric, bool for_mutex = false,
-                         Statistics* statistics = nullptr,
-                         uint32_t ticker_type = 0)
-      : perf_counter_enabled_(
-            perf_level >= PerfLevel::kEnableTime ||
-            (!for_mutex && perf_level >= kEnableTimeExceptForMutex)),
-        env_((perf_counter_enabled_ || statistics != nullptr) ? Env::Default()
-                                                              : nullptr),
+  explicit PerfStepTimer(uint64_t* metric, bool for_mutex = false)
+      : enabled_(perf_level >= PerfLevel::kEnableTime ||
+                 (!for_mutex && perf_level >= kEnableTimeExceptForMutex)),
+        env_(enabled_ ? Env::Default() : nullptr),
        start_(0),
-        metric_(metric),
-        statistics_(statistics),
-        ticker_type_(ticker_type) {}
+        metric_(metric) {}

  ~PerfStepTimer() {
    Stop();
  }

  void Start() {
-    if (perf_counter_enabled_ || statistics_ != nullptr) {
+    if (enabled_) {
      start_ = env_->NowNanos();
    }
  }
@@ -45,25 +39,16 @@ class PerfStepTimer {

  void Stop() {
    if (start_) {
-      uint64_t duration = env_->NowNanos() - start_;
-      if (perf_counter_enabled_) {
-        *metric_ += duration;
-      }
-
-      if (statistics_ != nullptr) {
-        RecordTick(statistics_, ticker_type_, duration);
-      }
+      *metric_ += env_->NowNanos() - start_;
      start_ = 0;
    }
  }

 private:
-  const bool perf_counter_enabled_;
+  const bool enabled_;
  Env* const env_;
  uint64_t start_;
  uint64_t* metric_;
-  Statistics* statistics_;
-  uint32_t ticker_type_;
 };

 }  // namespace rocksdb
@@ -124,8 +124,6 @@ DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
      immutable_db_options.allow_ingest_behind;
  options.preserve_deletes =
      immutable_db_options.preserve_deletes;
-  options.two_write_queues = immutable_db_options.two_write_queues;
-  options.manual_wal_flush = immutable_db_options.manual_wal_flush;

  return options;
 }
@@ -267,7 +267,6 @@ MAIN_SOURCES =                                                          \
  db/db_inplace_update_test.cc                                          \
  db/db_io_failure_test.cc                                              \
  db/db_iter_test.cc                                                    \
-  db/db_iter_stress_test.cc                                             \
  db/db_iterator_test.cc                                                \
  db/db_log_iter_test.cc                                                \
  db/db_memtable_test.cc                                                \
@@ -429,13 +429,12 @@ BlockIter* Block::NewIterator(const Comparator* cmp, BlockIter* iter,
    ret_iter = new BlockIter;
  }
  if (size_ < 2*sizeof(uint32_t)) {
-    ret_iter->Invalidate(Status::Corruption("bad block contents"));
+    ret_iter->SetStatus(Status::Corruption("bad block contents"));
    return ret_iter;
  }
  const uint32_t num_restarts = NumRestarts();
  if (num_restarts == 0) {
-    // Empty block.
-    ret_iter->Invalidate(Status::OK());
+    ret_iter->SetStatus(Status::OK());
    return ret_iter;
  } else {
    BlockPrefixIndex* prefix_index_ptr =
@@ -241,24 +241,8 @@ class BlockIter final : public InternalIterator {
    last_bitmap_offset_ = current_ + 1;
  }

-  // Makes Valid() return false, status() return `s`, and Seek()/Prev()/etc do
-  // nothing. Calls cleanup functions.
-  void Invalidate(Status s) {
-    // Assert that the BlockIter is never deleted while Pinning is Enabled.
-    assert(!pinned_iters_mgr_ ||
-           (pinned_iters_mgr_ && !pinned_iters_mgr_->PinningEnabled()));
-
-    data_ = nullptr;
-    current_ = restarts_;
+  void SetStatus(Status s) {
    status_ = s;
-
-    // Call cleanup callbacks.
-    Cleanable::Reset();
-
-    // Clear prev entries cache.
-    prev_entries_keys_buff_.clear();
-    prev_entries_.clear();
-    prev_entries_idx_ = -1;
  }

  virtual bool Valid() const override { return current_ < restarts_; }
@@ -1391,7 +1391,7 @@ InternalIterator* BlockBasedTable::NewIndexIterator(

  if (cache_handle == nullptr && no_io) {
    if (input_iter != nullptr) {
-      input_iter->Invalidate(Status::Incomplete("no blocking io"));
+      input_iter->SetStatus(Status::Incomplete("no blocking io"));
      return input_iter;
    } else {
      return NewErrorInternalIterator(Status::Incomplete("no blocking io"));
@@ -1438,7 +1438,7 @@ InternalIterator* BlockBasedTable::NewIndexIterator(
      RecordTick(statistics, BLOCK_CACHE_ADD_FAILURES);
      // make sure if something goes wrong, index_reader shall remain intact.
      if (input_iter != nullptr) {
-        input_iter->Invalidate(s);
+        input_iter->SetStatus(s);
        return input_iter;
      } else {
        return NewErrorInternalIterator(s);
@@ -1506,7 +1506,7 @@ BlockIter* BlockBasedTable::NewDataBlockIterator(
  if (s.ok() && block.value == nullptr) {
    if (no_io) {
      // Could not read from block_cache and can't do IO
-      iter->Invalidate(Status::Incomplete("no blocking io"));
+      iter->SetStatus(Status::Incomplete("no blocking io"));
      return iter;
    }
    std::unique_ptr<Block> block_value;
@@ -1566,7 +1566,7 @@ BlockIter* BlockBasedTable::NewDataBlockIterator(
    }
  } else {
    assert(block.value == nullptr);
-    iter->Invalidate(s);
+    iter->SetStatus(s);
  }
  return iter;
 }
@@ -1876,9 +1876,7 @@ void BlockBasedTableIterator::InitDataBlock() {
  BlockHandle data_block_handle;
  Slice handle_slice = index_iter_->value();
  if (!block_iter_points_to_real_block_ ||
-      handle_slice.compare(prev_index_value_) != 0 ||
-      // if previous attempt of reading the block missed cache, try again
-      data_block_iter_.status().IsIncomplete()) {
+      handle_slice.compare(prev_index_value_) != 0) {
    if (block_iter_points_to_real_block_) {
      ResetDataIter();
    }
@@ -420,7 +420,7 @@ struct BlockBasedTable::Rep {

  const ImmutableCFOptions& ioptions;
  const EnvOptions& env_options;
-  const BlockBasedTableOptions table_options;
+  const BlockBasedTableOptions& table_options;
  const FilterPolicy* const filter_policy;
  const InternalKeyComparator& internal_comparator;
  Status status;
@@ -528,9 +528,11 @@ class BlockBasedTableIterator : public InternalIterator {
    return data_block_iter_.value();
  }
  Status status() const override {
+    // It'd be nice if status() returned a const Status& instead of a Status
    if (!index_iter_->status().ok()) {
      return index_iter_->status();
-    } else if (block_iter_points_to_real_block_) {
+    } else if (block_iter_points_to_real_block_ &&
+               !data_block_iter_.status().ok()) {
      return data_block_iter_.status();
    } else {
      return Status::OK();
@@ -569,7 +571,8 @@ class BlockBasedTableIterator : public InternalIterator {
      if (pinned_iters_mgr_ != nullptr && pinned_iters_mgr_->PinningEnabled()) {
        data_block_iter_.DelegateCleanupsTo(pinned_iters_mgr_);
      }
-      data_block_iter_.Invalidate(Status::OK());
+      data_block_iter_.~BlockIter();
+      new (&data_block_iter_) BlockIter();
      block_iter_points_to_real_block_ = false;
    }
  }
@@ -206,7 +206,7 @@ class CuckooTableIterator : public InternalIterator {
  void Prev() override;
  Slice key() const override;
  Slice value() const override;
-  Status status() const override { return Status::OK(); }
+  Status status() const override { return status_; }
  void InitIfNeeded();

 private:
@@ -241,6 +241,7 @@ class CuckooTableIterator : public InternalIterator {
  void PrepareKVAtCurrIdx();
  CuckooTableReader* reader_;
  bool initialized_;
+  Status status_;
  // Contains a map of keys to bucket_id sorted in key order.
  std::vector<uint32_t> sorted_bucket_ids_;
  // We assume that the number of items can be stored in uint32 (4 Billion).
@@ -72,14 +72,8 @@ inline void FullFilterBlockBuilder::AddPrefix(const Slice& key) {
  }
 }

-void FullFilterBlockBuilder::Reset() {
-  last_whole_key_recorded_ = false;
-  last_prefix_recorded_ = false;
-}
-
 Slice FullFilterBlockBuilder::Finish(const BlockHandle& /*tmp*/,
                                     Status* status) {
-  Reset();
  // In this impl we ignore BlockHandle
  *status = Status::OK();
  if (num_added_ != 0) {
@@ -52,7 +52,6 @@ class FullFilterBlockBuilder : public FilterBlockBuilder {
 protected:
  virtual void AddKey(const Slice& key);
  std::unique_ptr<FilterBitsBuilder> filter_bits_builder_;
-  virtual void Reset();

 private:
  // important: all of these might point to invalid addresses
@@ -200,7 +200,7 @@ bool GetContext::SaveValue(const ParsedInternalKey& parsed_key,
          merge_context_->PushOperand(value, false);
        }
        if (merge_operator_ != nullptr &&
-            merge_operator_->ShouldMerge(merge_context_->GetOperandsDirectionBackward())) {
+            merge_operator_->ShouldMerge(merge_context_->GetOperands())) {
          state_ = kFound;
          if (LIKELY(pinnable_val_ != nullptr)) {
            Status merge_status = MergeHelper::TimedFullMerge(
@@ -22,7 +22,6 @@ class InternalIterator : public Cleanable {

  // An iterator is either positioned at a key/value pair, or
  // not valid.  This method returns true iff the iterator is valid.
-  // Always returns false if !status().ok().
  virtual bool Valid() const = 0;

  // Position at the first key in the source.  The iterator is Valid()
@@ -36,9 +35,6 @@ class InternalIterator : public Cleanable {
  // Position at the first key in the source that at or past target
  // The iterator is Valid() after this call iff the source contains
  // an entry that comes at or past target.
-  // All Seek*() methods clear any error status() that the iterator had prior to
-  // the call; after the seek, status() indicates only the error (if any) that
-  // happened during the seek, not any past errors.
  virtual void Seek(const Slice& target) = 0;

  // Position at the first key in the source that at or before target
@@ -65,7 +61,7 @@ class InternalIterator : public Cleanable {
  // Return the value for the current entry.  The underlying storage for
  // the returned slice is valid only until the next modification of
  // the iterator.
-  // REQUIRES: Valid()
+  // REQUIRES: !AtEnd() && !AtStart()
  virtual Slice value() const = 0;

  // If an error has occurred, return it.  Else return an ok status.
@@ -87,7 +87,6 @@ class IteratorWrapper {
    valid_ = iter_->Valid();
    if (valid_) {
      key_ = iter_->key();
-      assert(iter_->status().ok());
    }
  }

@@ -52,21 +52,12 @@ class MergingIterator : public InternalIterator {
    }
    for (auto& child : children_) {
      if (child.Valid()) {
-        assert(child.status().ok());
        minHeap_.push(&child);
-      } else {
-        considerStatus(child.status());
      }
    }
    current_ = CurrentForward();
  }

-  void considerStatus(Status s) {
-    if (!s.ok() && status_.ok()) {
-      status_ = s;
-    }
-  }
-
  virtual void AddIterator(InternalIterator* iter) {
    assert(direction_ == kForward);
    children_.emplace_back(iter);
@@ -75,11 +66,8 @@ class MergingIterator : public InternalIterator {
    }
    auto new_wrapper = children_.back();
    if (new_wrapper.Valid()) {
-      assert(new_wrapper.status().ok());
      minHeap_.push(&new_wrapper);
      current_ = CurrentForward();
-    } else {
-      considerStatus(new_wrapper.status());
    }
  }

@@ -89,22 +77,14 @@ class MergingIterator : public InternalIterator {
    }
  }

-  virtual bool Valid() const override {
-    return current_ != nullptr && status_.ok();
-  }
-
-  virtual Status status() const override { return status_; }
+  virtual bool Valid() const override { return (current_ != nullptr); }

  virtual void SeekToFirst() override {
    ClearHeaps();
-    status_ = Status::OK();
    for (auto& child : children_) {
      child.SeekToFirst();
      if (child.Valid()) {
-        assert(child.status().ok());
        minHeap_.push(&child);
-      } else {
-        considerStatus(child.status());
      }
    }
    direction_ = kForward;
@@ -114,14 +94,10 @@ class MergingIterator : public InternalIterator {
  virtual void SeekToLast() override {
    ClearHeaps();
    InitMaxHeap();
-    status_ = Status::OK();
    for (auto& child : children_) {
      child.SeekToLast();
      if (child.Valid()) {
-        assert(child.status().ok());
        maxHeap_->push(&child);
-      } else {
-        considerStatus(child.status());
      }
    }
    direction_ = kReverse;
@@ -130,7 +106,6 @@ class MergingIterator : public InternalIterator {

  virtual void Seek(const Slice& target) override {
    ClearHeaps();
-    status_ = Status::OK();
    for (auto& child : children_) {
      {
        PERF_TIMER_GUARD(seek_child_seek_time);
@@ -139,11 +114,8 @@ class MergingIterator : public InternalIterator {
      PERF_COUNTER_ADD(seek_child_seek_count, 1);

      if (child.Valid()) {
-        assert(child.status().ok());
        PERF_TIMER_GUARD(seek_min_heap_time);
        minHeap_.push(&child);
-      } else {
-        considerStatus(child.status());
      }
    }
    direction_ = kForward;
@@ -156,7 +128,6 @@ class MergingIterator : public InternalIterator {
  virtual void SeekForPrev(const Slice& target) override {
    ClearHeaps();
    InitMaxHeap();
-    status_ = Status::OK();

    for (auto& child : children_) {
      {
@@ -166,11 +137,8 @@ class MergingIterator : public InternalIterator {
      PERF_COUNTER_ADD(seek_child_seek_count, 1);

      if (child.Valid()) {
-        assert(child.status().ok());
        PERF_TIMER_GUARD(seek_max_heap_time);
        maxHeap_->push(&child);
-      } else {
-        considerStatus(child.status());
      }
    }
    direction_ = kReverse;
@@ -204,11 +172,9 @@ class MergingIterator : public InternalIterator {
      // current is still valid after the Next() call above.  Call
      // replace_top() to restore the heap property.  When the same child
      // iterator yields a sequence of keys, this is cheap.
-      assert(current_->status().ok());
      minHeap_.replace_top(current_);
    } else {
      // current stopped being valid, remove it from the heap.
-      considerStatus(current_->status());
      minHeap_.pop();
    }
    current_ = CurrentForward();
@@ -225,19 +191,28 @@ class MergingIterator : public InternalIterator {
      // just after the if-block.
      ClearHeaps();
      InitMaxHeap();
-      Slice target = key();
      for (auto& child : children_) {
        if (&child != current_) {
-          child.SeekForPrev(target);
-          TEST_SYNC_POINT_CALLBACK("MergeIterator::Prev:BeforePrev", &child);
-          considerStatus(child.status());
-          if (child.Valid() && comparator_->Equal(target, child.key())) {
-            child.Prev();
-            considerStatus(child.status());
+          if (!prefix_seek_mode_) {
+            child.Seek(key());
+            if (child.Valid()) {
+              // Child is at first entry >= key().  Step back one to be < key()
+              TEST_SYNC_POINT_CALLBACK("MergeIterator::Prev:BeforePrev",
+                                       &child);
+              child.Prev();
+            } else {
+              // Child has no entries >= key().  Position at last entry.
+              TEST_SYNC_POINT("MergeIterator::Prev:BeforeSeekToLast");
+              child.SeekToLast();
+            }
+          } else {
+            child.SeekForPrev(key());
+            if (child.Valid() && comparator_->Equal(key(), child.key())) {
+              child.Prev();
+            }
          }
        }
        if (child.Valid()) {
-          assert(child.status().ok());
          maxHeap_->push(&child);
        }
      }
@@ -263,11 +238,9 @@ class MergingIterator : public InternalIterator {
      // current is still valid after the Prev() call above.  Call
      // replace_top() to restore the heap property.  When the same child
      // iterator yields a sequence of keys, this is cheap.
-      assert(current_->status().ok());
      maxHeap_->replace_top(current_);
    } else {
      // current stopped being valid, remove it from the heap.
-      considerStatus(current_->status());
      maxHeap_->pop();
    }
    current_ = CurrentReverse();
@@ -283,6 +256,17 @@ class MergingIterator : public InternalIterator {
    return current_->value();
  }

+  virtual Status status() const override {
+    Status s;
+    for (auto& child : children_) {
+      s = child.status();
+      if (!s.ok()) {
+        break;
+      }
+    }
+    return s;
+  }
+
  virtual void SetPinnedItersMgr(
      PinnedIteratorsManager* pinned_iters_mgr) override {
    pinned_iters_mgr_ = pinned_iters_mgr;
@@ -318,8 +302,6 @@ class MergingIterator : public InternalIterator {
  // child iterators are valid.  This is the top of minHeap_ or maxHeap_
  // depending on the direction.
  IteratorWrapper* current_;
-  // If any of the children have non-ok status, this is one of them.
-  Status status_;
  // Which direction is the iterator moving?
  enum Direction {
    kForward,
@@ -352,14 +334,11 @@ void MergingIterator::SwitchToForward() {
  // Otherwise, advance the non-current children.  We advance current_
  // just after the if-block.
  ClearHeaps();
-  Slice target = key();
  for (auto& child : children_) {
    if (&child != current_) {
-      child.Seek(target);
-      considerStatus(child.status());
-      if (child.Valid() && comparator_->Equal(target, child.key())) {
+      child.Seek(key());
+      if (child.Valid() && comparator_->Equal(key(), child.key())) {
        child.Next();
-        considerStatus(child.status());
      }
    }
    if (child.Valid()) {
@@ -48,7 +48,6 @@ void PartitionedFilterBlockBuilder::MaybeCutAFilterBlock() {
  std::string& index_key = p_index_builder_->GetPartitionKey();
  filters.push_back({index_key, filter});
  filters_in_partition_ = 0;
-  Reset();
 }

 void PartitionedFilterBlockBuilder::AddKey(const Slice& key) {
@@ -625,7 +625,6 @@ bool PlainTableIterator::Valid() const {
 }

 void PlainTableIterator::SeekToFirst() {
-  status_ = Status::OK();
  next_offset_ = table_->data_start_offset_;
  if (next_offset_ >= table_->file_info_.data_end_offset) {
    next_offset_ = offset_ = table_->file_info_.data_end_offset;
@@ -637,7 +636,6 @@ void PlainTableIterator::SeekToFirst() {
 void PlainTableIterator::SeekToLast() {
  assert(false);
  status_ = Status::NotSupported("SeekToLast() is not supported in PlainTable");
-  next_offset_ = offset_ = table_->file_info_.data_end_offset;
 }

 void PlainTableIterator::Seek(const Slice& target) {
@@ -678,7 +676,6 @@ void PlainTableIterator::Seek(const Slice& target) {
  if (!table_->IsTotalOrderMode()) {
    prefix_hash = GetSliceHash(prefix_slice);
    if (!table_->MatchBloom(prefix_hash)) {
-      status_ = Status::OK();
      offset_ = next_offset_ = table_->file_info_.data_end_offset;
      return;
    }
@@ -714,7 +711,6 @@ void PlainTableIterator::SeekForPrev(const Slice& /*target*/) {
  assert(false);
  status_ =
      Status::NotSupported("SeekForPrev() is not supported in PlainTable");
-  offset_ = next_offset_ = table_->file_info_.data_end_offset;
 }

 void PlainTableIterator::Next() {
@@ -256,7 +256,7 @@ class KeyConvertingIterator : public InternalIterator {
      delete iter_;
    }
  }
-  virtual bool Valid() const override { return iter_->Valid() && status_.ok(); }
+  virtual bool Valid() const override { return iter_->Valid(); }
  virtual void Seek(const Slice& target) override {
    ParsedInternalKey ikey(target, kMaxSequenceNumber, kTypeValue);
    std::string encoded;
@@ -2368,7 +2368,6 @@ TEST_F(BlockBasedTableTest, BlockCacheLeak) {
    iter->Next();
  }
  ASSERT_OK(iter->status());
-  iter.reset();

  const ImmutableCFOptions ioptions1(opt);
  ASSERT_OK(c.Reopen(ioptions1));
@@ -47,8 +47,8 @@ class TwoLevelIterator : public InternalIterator {
    return second_level_iter_.value();
  }
  virtual Status status() const override {
+    // It'd be nice if status() returned a const Status& instead of a Status
    if (!first_level_iter_.status().ok()) {
-      assert(second_level_iter_.iter() == nullptr);
      return first_level_iter_.status();
    } else if (second_level_iter_.iter() != nullptr &&
               !second_level_iter_.status().ok()) {
@@ -101,7 +101,7 @@ void TwoLevelIterator::SeekForPrev(const Slice& target) {
    second_level_iter_.SeekForPrev(target);
  }
  if (!Valid()) {
-    if (!first_level_iter_.Valid() && first_level_iter_.status().ok()) {
+    if (!first_level_iter_.Valid()) {
      first_level_iter_.SeekToLast();
      InitDataBlock();
      if (second_level_iter_.iter() != nullptr) {
@@ -144,7 +144,8 @@ void TwoLevelIterator::Prev() {

 void TwoLevelIterator::SkipEmptyDataBlocksForward() {
  while (second_level_iter_.iter() == nullptr ||
-         (!second_level_iter_.Valid() && second_level_iter_.status().ok())) {
+         (!second_level_iter_.Valid() &&
+          !second_level_iter_.status().IsIncomplete())) {
    // Move to next block
    if (!first_level_iter_.Valid()) {
      SetSecondLevelIterator(nullptr);
@@ -160,7 +161,8 @@ void TwoLevelIterator::SkipEmptyDataBlocksForward() {

 void TwoLevelIterator::SkipEmptyDataBlocksBackward() {
  while (second_level_iter_.iter() == nullptr ||
-         (!second_level_iter_.Valid() && second_level_iter_.status().ok())) {
+         (!second_level_iter_.Valid() &&
+          !second_level_iter_.status().IsIncomplete())) {
    // Move to next block
    if (!first_level_iter_.Valid()) {
      SetSecondLevelIterator(nullptr);
@@ -175,6 +177,9 @@ void TwoLevelIterator::SkipEmptyDataBlocksBackward() {
 }

 void TwoLevelIterator::SetSecondLevelIterator(InternalIterator* iter) {
+  if (second_level_iter_.iter() != nullptr) {
+    SaveError(second_level_iter_.status());
+  }
  InternalIterator* old_iter = second_level_iter_.Set(iter);
  delete old_iter;
 }
@@ -857,7 +857,7 @@ class SharedState {
            "Cannot use --expected_values_path on when "
            "--clear_column_family_one_in is greater than zero.");
      }
-      uint64_t size = 0;
+      size_t size = 0;
      if (status.ok()) {
        status = FLAGS_env->GetFileSize(FLAGS_expected_values_path, &size);
      }
@@ -242,14 +242,6 @@ LDBCommand* LDBCommand::SelectCommand(const ParsedParams& parsed_params) {
  } else if (parsed_params.cmd == RestoreCommand::Name()) {
    return new RestoreCommand(parsed_params.cmd_params,
                              parsed_params.option_map, parsed_params.flags);
-  } else if (parsed_params.cmd == WriteExternalSstFilesCommand::Name()) {
-    return new WriteExternalSstFilesCommand(parsed_params.cmd_params,
-                                            parsed_params.option_map,
-                                            parsed_params.flags);
-  } else if (parsed_params.cmd == IngestExternalSstFilesCommand::Name()) {
-    return new IngestExternalSstFilesCommand(parsed_params.cmd_params,
-                                             parsed_params.option_map,
-                                             parsed_params.flags);
  }
  return nullptr;
 }
@@ -2944,180 +2936,5 @@ void DBFileDumperCommand::DoCommand() {
  }
 }

-void WriteExternalSstFilesCommand::Help(std::string& ret) {
-  ret.append("  ");
-  ret.append(WriteExternalSstFilesCommand::Name());
-  ret.append(" <output_sst_path>");
-  ret.append("\n");
-}
-
-WriteExternalSstFilesCommand::WriteExternalSstFilesCommand(
-    const std::vector<std::string>& params,
-    const std::map<std::string, std::string>& options,
-    const std::vector<std::string>& flags)
-    : LDBCommand(
-          options, flags, false /* is_read_only */,
-          BuildCmdLineOptions({ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_FROM,
-                               ARG_TO, ARG_CREATE_IF_MISSING})) {
-  create_if_missing_ =
-      IsFlagPresent(flags, ARG_CREATE_IF_MISSING) ||
-      ParseBooleanOption(options, ARG_CREATE_IF_MISSING, false);
-  if (params.size() != 1) {
-    exec_state_ = LDBCommandExecuteResult::Failed(
-        "output SST file path must be specified");
-  } else {
-    output_sst_path_ = params.at(0);
-  }
-}
-
-void WriteExternalSstFilesCommand::DoCommand() {
-  if (!db_) {
-    assert(GetExecuteState().IsFailed());
-    return;
-  }
-  ColumnFamilyHandle* cfh = GetCfHandle();
-  SstFileWriter sst_file_writer(EnvOptions(), db_->GetOptions(), cfh);
-  Status status = sst_file_writer.Open(output_sst_path_);
-  if (!status.ok()) {
-    exec_state_ = LDBCommandExecuteResult::Failed("failed to open SST file: " +
-                                                  status.ToString());
-    return;
-  }
-
-  int bad_lines = 0;
-  std::string line;
-  std::ifstream ifs_stdin("/dev/stdin");
-  std::istream* istream_p = ifs_stdin.is_open() ? &ifs_stdin : &std::cin;
-  while (getline(*istream_p, line, '\n')) {
-    std::string key;
-    std::string value;
-    if (ParseKeyValue(line, &key, &value, is_key_hex_, is_value_hex_)) {
-      status = sst_file_writer.Put(key, value);
-      if (!status.ok()) {
-        exec_state_ = LDBCommandExecuteResult::Failed(
-            "failed to write record to file: " + status.ToString());
-        return;
-      }
-    } else if (0 == line.find("Keys in range:")) {
-      // ignore this line
-    } else if (0 == line.find("Created bg thread 0x")) {
-      // ignore this line
-    } else {
-      bad_lines++;
-    }
-  }
-
-  status = sst_file_writer.Finish();
-  if (!status.ok()) {
-    exec_state_ = LDBCommandExecuteResult::Failed(
-        "Failed to finish writing to file: " + status.ToString());
-    return;
-  }
-
-  if (bad_lines > 0) {
-    fprintf(stderr, "Warning: %d bad lines ignored.\n", bad_lines);
-  }
-  exec_state_ = LDBCommandExecuteResult::Succeed(
-      "external SST file written to " + output_sst_path_);
-}
-
-Options WriteExternalSstFilesCommand::PrepareOptionsForOpenDB() {
-  Options opt = LDBCommand::PrepareOptionsForOpenDB();
-  opt.create_if_missing = create_if_missing_;
-  return opt;
-}
-
-const std::string IngestExternalSstFilesCommand::ARG_MOVE_FILES = "move_files";
-const std::string IngestExternalSstFilesCommand::ARG_SNAPSHOT_CONSISTENCY =
-    "snapshot_consistency";
-const std::string IngestExternalSstFilesCommand::ARG_ALLOW_GLOBAL_SEQNO =
-    "allow_global_seqno";
-const std::string IngestExternalSstFilesCommand::ARG_ALLOW_BLOCKING_FLUSH =
-    "allow_blocking_flush";
-const std::string IngestExternalSstFilesCommand::ARG_INGEST_BEHIND =
-    "ingest_behind";
-
-void IngestExternalSstFilesCommand::Help(std::string& ret) {
-  ret.append("  ");
-  ret.append(IngestExternalSstFilesCommand::Name());
-  ret.append(" <input_sst_path>");
-  ret.append(" [--" + ARG_MOVE_FILES + "] ");
-  ret.append(" [--" + ARG_SNAPSHOT_CONSISTENCY + "] ");
-  ret.append(" [--" + ARG_ALLOW_GLOBAL_SEQNO + "] ");
-  ret.append(" [--" + ARG_ALLOW_BLOCKING_FLUSH + "] ");
-  ret.append(" [--" + ARG_INGEST_BEHIND + "] ");
-  ret.append("\n");
-}
-
-IngestExternalSstFilesCommand::IngestExternalSstFilesCommand(
-    const std::vector<std::string>& params,
-    const std::map<std::string, std::string>& options,
-    const std::vector<std::string>& flags)
-    : LDBCommand(
-          options, flags, false /* is_read_only */,
-          BuildCmdLineOptions({ARG_MOVE_FILES, ARG_SNAPSHOT_CONSISTENCY,
-                               ARG_ALLOW_GLOBAL_SEQNO, ARG_CREATE_IF_MISSING,
-                               ARG_ALLOW_BLOCKING_FLUSH, ARG_INGEST_BEHIND})),
-      move_files_(false),
-      snapshot_consistency_(true),
-      allow_global_seqno_(true),
-      allow_blocking_flush_(true),
-      ingest_behind_(false) {
-  create_if_missing_ =
-      IsFlagPresent(flags, ARG_CREATE_IF_MISSING) ||
-      ParseBooleanOption(options, ARG_CREATE_IF_MISSING, false);
-  move_files_ = IsFlagPresent(flags, ARG_MOVE_FILES) ||
-                ParseBooleanOption(options, ARG_MOVE_FILES, false);
-  snapshot_consistency_ =
-      IsFlagPresent(flags, ARG_SNAPSHOT_CONSISTENCY) ||
-      ParseBooleanOption(options, ARG_SNAPSHOT_CONSISTENCY, true);
-  allow_global_seqno_ =
-      IsFlagPresent(flags, ARG_ALLOW_GLOBAL_SEQNO) ||
-      ParseBooleanOption(options, ARG_ALLOW_GLOBAL_SEQNO, true);
-  allow_blocking_flush_ =
-      IsFlagPresent(flags, ARG_ALLOW_BLOCKING_FLUSH) ||
-      ParseBooleanOption(options, ARG_ALLOW_BLOCKING_FLUSH, true);
-  ingest_behind_ = IsFlagPresent(flags, ARG_INGEST_BEHIND) ||
-                   ParseBooleanOption(options, ARG_INGEST_BEHIND, false);
-
-  if (params.size() != 1) {
-    exec_state_ =
-        LDBCommandExecuteResult::Failed("input SST path must be specified");
-  } else {
-    input_sst_path_ = params.at(0);
-  }
-}
-
-void IngestExternalSstFilesCommand::DoCommand() {
-  if (!db_) {
-    assert(GetExecuteState().IsFailed());
-    return;
-  }
-  if (GetExecuteState().IsFailed()) {
-    return;
-  }
-  ColumnFamilyHandle* cfh = GetCfHandle();
-  IngestExternalFileOptions ifo;
-  ifo.move_files = move_files_;
-  ifo.snapshot_consistency = snapshot_consistency_;
-  ifo.allow_global_seqno = allow_global_seqno_;
-  ifo.allow_blocking_flush = allow_blocking_flush_;
-  ifo.ingest_behind = ingest_behind_;
-  Status status = db_->IngestExternalFile(cfh, {input_sst_path_}, ifo);
-  if (!status.ok()) {
-    exec_state_ = LDBCommandExecuteResult::Failed(
-        "failed to ingest external SST: " + status.ToString());
-  } else {
-    exec_state_ =
-        LDBCommandExecuteResult::Succeed("external SST files ingested");
-  }
-}
-
-Options IngestExternalSstFilesCommand::PrepareOptionsForOpenDB() {
-  Options opt = LDBCommand::PrepareOptionsForOpenDB();
-  opt.create_if_missing = create_if_missing_;
-  return opt;
-}
-
 }   // namespace rocksdb
 #endif  // ROCKSDB_LITE
@@ -522,55 +522,4 @@ class RestoreCommand : public BackupableCommand {
  static void Help(std::string& ret);
 };

-class WriteExternalSstFilesCommand : public LDBCommand {
- public:
-  static std::string Name() { return "write_extern_sst"; }
-  WriteExternalSstFilesCommand(
-      const std::vector<std::string>& params,
-      const std::map<std::string, std::string>& options,
-      const std::vector<std::string>& flags);
-
-  virtual void DoCommand() override;
-
-  virtual bool NoDBOpen() override { return false; }
-
-  virtual Options PrepareOptionsForOpenDB() override;
-
-  static void Help(std::string& ret);
-
- private:
-  std::string output_sst_path_;
-};
-
-class IngestExternalSstFilesCommand : public LDBCommand {
- public:
-  static std::string Name() { return "ingest_extern_sst"; }
-  IngestExternalSstFilesCommand(
-      const std::vector<std::string>& params,
-      const std::map<std::string, std::string>& options,
-      const std::vector<std::string>& flags);
-
-  virtual void DoCommand() override;
-
-  virtual bool NoDBOpen() override { return false; }
-
-  virtual Options PrepareOptionsForOpenDB() override;
-
-  static void Help(std::string& ret);
-
- private:
-  std::string input_sst_path_;
-  bool move_files_;
-  bool snapshot_consistency_;
-  bool allow_global_seqno_;
-  bool allow_blocking_flush_;
-  bool ingest_behind_;
-
-  static const std::string ARG_MOVE_FILES;
-  static const std::string ARG_SNAPSHOT_CONSISTENCY;
-  static const std::string ARG_ALLOW_GLOBAL_SEQNO;
-  static const std::string ARG_ALLOW_BLOCKING_FLUSH;
-  static const std::string ARG_INGEST_BEHIND;
-};
-
 }  // namespace rocksdb
@@ -76,7 +76,7 @@ class LDBTestCase(unittest.TestCase):

            my_check_output("./ldb %s >/dev/null 2>&1 |grep -v \"Created bg \
                thread\"" % params, shell=True)
-        except Exception:
+        except Exception, e:
            return
        self.fail(
            "Exception should have been raised for command with params: %s" %
@@ -146,14 +146,6 @@ class LDBTestCase(unittest.TestCase):
    def loadDb(self, params, dumpFile):
        return 0 == run_err_null("cat %s | ./ldb load %s" % (dumpFile, params))

-    def writeExternSst(self, params, inputDumpFile, outputSst):
-        return 0 == run_err_null("cat %s | ./ldb write_extern_sst %s %s"
-                % (inputDumpFile, outputSst, params))
-
-    def ingestExternSst(self, params, inputSst):
-        return 0 == run_err_null("./ldb ingest_extern_sst %s %s"
-                                     % (inputSst, params))
-
    def testStringBatchPut(self):
        print "Running testStringBatchPut..."
        self.assertRunOK("batchput x1 y1 --create_if_missing", "OK")
@@ -555,38 +547,5 @@ class LDBTestCase(unittest.TestCase):
        # non-existing column family.
        self.assertRunFAIL("get cf3_1 --column_family=four")

-    def testIngestExternalSst(self):
-        print "Running testIngestExternalSst..."
-
-        # Dump, load, write external sst and ingest it in another db
-        dbPath = os.path.join(self.TMP_DIR, "db1")
-        self.assertRunOK(
-            "batchput --db=%s --create_if_missing x1 y1 x2 y2 x3 y3 x4 y4"
-            % dbPath,
-            "OK")
-        self.assertRunOK("scan --db=%s" % dbPath,
-                         "x1 : y1\nx2 : y2\nx3 : y3\nx4 : y4")
-        dumpFilePath = os.path.join(self.TMP_DIR, "dump1")
-        with open(dumpFilePath, 'w') as f:
-            f.write("x1 ==> y10\nx2 ==> y20\nx3 ==> y30\nx4 ==> y40")
-        externSstPath = os.path.join(self.TMP_DIR, "extern_data1.sst")
-        self.assertTrue(self.writeExternSst("--create_if_missing --db=%s"
-                            % dbPath,
-                        dumpFilePath,
-                        externSstPath))
-        # cannot ingest if allow_global_seqno is false
-        self.assertFalse(
-            self.ingestExternSst(
-                "--create_if_missing --allow_global_seqno=false --db=%s"
-                % dbPath,
-                externSstPath))
-        self.assertTrue(
-            self.ingestExternSst(
-                "--create_if_missing --allow_global_seqno --db=%s"
-                % dbPath,
-                externSstPath))
-        self.assertRunOKFull("scan --db=%s" % dbPath,
-                             "x1 : y10\nx2 : y20\nx3 : y30\nx4 : y40")
-
 if __name__ == "__main__":
    unittest.main()
@@ -88,8 +88,6 @@ void LDBCommandRunner::PrintHelp(const LDBOptions& ldb_options,
  BackupCommand::Help(ret);
  RestoreCommand::Help(ret);
  CheckPointCommand::Help(ret);
-  WriteExternalSstFilesCommand::Help(ret);
-  IngestExternalSstFilesCommand::Help(ret);

  fprintf(stderr, "%s\n", ret.c_str());
 }
@@ -189,8 +189,6 @@ class WritableFileWriter {

  bool use_direct_io() { return writable_file_->use_direct_io(); }

-  bool TEST_BufferIsEmpty() { return buf_.CurrentSize() == 0; }
-
 private:
  // Used when os buffering is OFF and we are writing
  // DMA such as in Direct I/O mode
@@ -9,27 +9,25 @@

 #include "rocksdb/status.h"
 #include <stdio.h>
-#ifdef OS_WIN
-#include <string.h>
-#endif
 #include <cstring>
 #include "port/port.h"

 namespace rocksdb {

 const char* Status::CopyState(const char* state) {
+  const size_t cch =
+      std::strlen(state) + 1; // +1 for the null terminator
+  char* const result =
+      new char[cch];
+  result[cch - 1] = '\0';
 #ifdef OS_WIN
-  const size_t cch = std::strlen(state) + 1;  // +1 for the null terminator
-  char* result = new char[cch];
  errno_t ret;
  ret = strncpy_s(result, cch, state, cch - 1);
-  result[cch - 1] = '\0';
  assert(ret == 0);
-  return result;
 #else
-  const size_t cch = std::strlen(state) + 1;  // +1 for the null terminator
-  return std::strncpy(new char[cch], state, cch);
+  std::strncpy(result, state, cch - 1);
 #endif
+  return result;
 }

 Status::Status(Code _code, SubCode _subcode, const Slice& msg, const Slice& msg2)
@@ -1068,7 +1068,7 @@ Status BlobDBImpl::GetBlobValue(const Slice& key, const Slice& index_entry,
                    "Failed to read blob from blob file %" PRIu64
                    ", blob_offset: %" PRIu64 ", blob_size: %" PRIu64
                    ", key_size: " PRIu64 ", read " PRIu64
-                    " bytes, status: '%s'",
+                    "bytes, status: '%s'",
                    bfile->BlobFileNumber(), blob_index.offset(),
                    blob_index.size(), key.size(), s.ToString().c_str());
    return s;
@@ -1078,7 +1078,7 @@ Status BlobDBImpl::GetBlobValue(const Slice& key, const Slice& index_entry,
                    "Failed to read blob from blob file %" PRIu64
                    ", blob_offset: %" PRIu64 ", blob_size: %" PRIu64
                    ", key_size: " PRIu64 ", read " PRIu64
-                    " bytes, status: '%s'",
+                    "bytes, status: '%s'",
                    bfile->BlobFileNumber(), blob_index.offset(),
                    blob_index.size(), key.size(), s.ToString().c_str());

@@ -119,7 +119,6 @@ class BlobDBIterator : public Iterator {
    TEST_SYNC_POINT("BlobDBIterator::UpdateBlobValue:Start:1");
    TEST_SYNC_POINT("BlobDBIterator::UpdateBlobValue:Start:2");
    value_.Reset();
-    status_ = Status::OK();
    if (iter_->Valid() && iter_->status().ok() && iter_->IsBlob()) {
      Status s = blob_db_->GetBlobValue(iter_->key(), iter_->value(), &value_);
      if (s.IsNotFound()) {
@@ -247,7 +247,7 @@ GeoIterator* GeoDBImpl::SearchRadial(const GeoPosition& pos,
      auto res = std::mismatch(qid.begin(), qid.end(), quadkey->begin());
      if (res.first == qid.end()) {
        GeoPosition obj_pos(atof(parts[3].c_str()), atof(parts[4].c_str()));
-        GeoObject obj(obj_pos, parts[2], iter->value().ToString());
+        GeoObject obj(obj_pos, parts[4], iter->value().ToString());
        values.push_back(obj);
        number_of_values--;
      } else {
@@ -284,11 +284,10 @@ std::string GeoDBImpl::MakeKey2(Slice id) {
 std::string GeoDBImpl::MakeKey1Prefix(std::string quadkey,
                                      Slice id) {
  std::string key = "p:";
-  key.reserve(4 + quadkey.size() + id.size());
+  key.reserve(3 + quadkey.size() + id.size());
  key.append(quadkey);
  key.append(":");
  key.append(id.ToString());
-  key.append(":");
  return key;
 }

@@ -108,17 +108,8 @@ TEST_F(GeoDBTest, Search) {
  ASSERT_EQ(iter1->geo_object().value, "midvalue1");
  uint32_t size = 0;
  while (iter1->Valid()) {
-    GeoObject obj;
-    status = getdb()->GetById(Slice(id1), &obj);
-    ASSERT_TRUE(status.ok());
-    ASSERT_EQ(iter1->geo_object().position.latitude, pos1.latitude);
-    ASSERT_EQ(iter1->geo_object().position.longitude, pos1.longitude);
-    ASSERT_EQ(iter1->geo_object().id.compare(id1), 0);
-    ASSERT_EQ(iter1->geo_object().value, value1);
-
    size++;
    iter1->Next();
-    ASSERT_TRUE(!iter1->Valid());
  }
  ASSERT_EQ(size, 1U);
  delete iter1;
@@ -131,57 +122,6 @@ TEST_F(GeoDBTest, Search) {
  delete iter2;
 }

-TEST_F(GeoDBTest, DifferentPosInSameQuadkey) {
-  // insert obj1 into database
-  GeoPosition pos1(40.00001, 116.00001);
-  std::string id1("12");
-  std::string value1("value1");
-
-  GeoObject obj1(pos1, id1, value1);
-  Status status = getdb()->Insert(obj1);
-  ASSERT_TRUE(status.ok());
-
-  // insert obj2 into database
-  GeoPosition pos2(40.00002, 116.00002);
-  std::string id2("123");
-  std::string value2 = "value2";
-
-  GeoObject obj2(pos2, id2, value2);
-  status = getdb()->Insert(obj2);
-  ASSERT_TRUE(status.ok());
-
-  // get obj1's quadkey
-  ReadOptions opt;
-  PinnableSlice quadkey1;
-  status = getdb()->Get(opt, getdb()->DefaultColumnFamily(), "k:" + id1, &quadkey1);
-  ASSERT_TRUE(status.ok());
-
-  // get obj2's quadkey
-  PinnableSlice quadkey2;
-  status = getdb()->Get(opt, getdb()->DefaultColumnFamily(), "k:" + id2, &quadkey2);
-  ASSERT_TRUE(status.ok());
-
-  // obj1 and obj2 have the same quadkey
-  ASSERT_EQ(quadkey1, quadkey2);
-
-  // get obj1 by id, and check value
-  GeoObject obj;
-  status = getdb()->GetById(Slice(id1), &obj);
-  ASSERT_TRUE(status.ok());
-  ASSERT_EQ(obj.position.latitude, pos1.latitude);
-  ASSERT_EQ(obj.position.longitude, pos1.longitude);
-  ASSERT_EQ(obj.id.compare(id1), 0);
-  ASSERT_EQ(obj.value, value1);
-
-  // get obj2 by id, and check value
-  status = getdb()->GetById(Slice(id2), &obj);
-  ASSERT_TRUE(status.ok());
-  ASSERT_EQ(obj.position.latitude, pos2.latitude);
-  ASSERT_EQ(obj.position.longitude, pos2.longitude);
-  ASSERT_EQ(obj.id.compare(id2), 0);
-  ASSERT_EQ(obj.value, value2);
-}
-
 }  // namespace rocksdb

 int main(int argc, char* argv[]) {
@@ -207,7 +207,7 @@ Status TransactionDB::Open(
    const std::vector<ColumnFamilyDescriptor>& column_families,
    std::vector<ColumnFamilyHandle*>* handles, TransactionDB** dbptr) {
  Status s;
-  DB* db = nullptr;
+  DB* db;

  ROCKS_LOG_WARN(db_options.info_log, "Transaction write_policy is %" PRId32,
                 static_cast<int>(txn_db_options.write_policy));
@@ -223,10 +223,6 @@ Status TransactionDB::Open(
    s = WrapDB(db, txn_db_options, compaction_enabled_cf_indices, *handles,
               dbptr);
  }
-  if (!s.ok()) {
-    // just in case it was not deleted (and not set to nullptr).
-    delete db;
-  }
  return s;
 }

@@ -258,62 +254,48 @@ Status TransactionDB::WrapDB(
    DB* db, const TransactionDBOptions& txn_db_options,
    const std::vector<size_t>& compaction_enabled_cf_indices,
    const std::vector<ColumnFamilyHandle*>& handles, TransactionDB** dbptr) {
-  assert(db != nullptr);
-  assert(dbptr != nullptr);
-  *dbptr = nullptr;
-  std::unique_ptr<PessimisticTransactionDB> txn_db;
+  PessimisticTransactionDB* txn_db;
  switch (txn_db_options.write_policy) {
    case WRITE_UNPREPARED:
      return Status::NotSupported("WRITE_UNPREPARED is not implemented yet");
    case WRITE_PREPARED:
-      txn_db.reset(new WritePreparedTxnDB(
-          db, PessimisticTransactionDB::ValidateTxnDBOptions(txn_db_options)));
+      txn_db = new WritePreparedTxnDB(
+          db, PessimisticTransactionDB::ValidateTxnDBOptions(txn_db_options));
      break;
    case WRITE_COMMITTED:
    default:
-      txn_db.reset(new WriteCommittedTxnDB(
-          db, PessimisticTransactionDB::ValidateTxnDBOptions(txn_db_options)));
+      txn_db = new WriteCommittedTxnDB(
+          db, PessimisticTransactionDB::ValidateTxnDBOptions(txn_db_options));
  }
  txn_db->UpdateCFComparatorMap(handles);
+  *dbptr = txn_db;
  Status s = txn_db->Initialize(compaction_enabled_cf_indices, handles);
-  // In case of a failure at this point, db is deleted via the txn_db destructor
-  // and set to nullptr.
-  if (s.ok()) {
-    *dbptr = txn_db.release();
-  }
  return s;
 }

 Status TransactionDB::WrapStackableDB(
    // make sure this stackable_db is already opened with memtable history
-    // enabled, auto compaction distabled and 2 phase commit enabled
+    // enabled,
+    // auto compaction distabled and 2 phase commit enabled
    StackableDB* db, const TransactionDBOptions& txn_db_options,
    const std::vector<size_t>& compaction_enabled_cf_indices,
    const std::vector<ColumnFamilyHandle*>& handles, TransactionDB** dbptr) {
-  assert(db != nullptr);
-  assert(dbptr != nullptr);
-  *dbptr = nullptr;
-  std::unique_ptr<PessimisticTransactionDB> txn_db;
-
+  PessimisticTransactionDB* txn_db;
  switch (txn_db_options.write_policy) {
    case WRITE_UNPREPARED:
      return Status::NotSupported("WRITE_UNPREPARED is not implemented yet");
    case WRITE_PREPARED:
-      txn_db.reset(new WritePreparedTxnDB(
-          db, PessimisticTransactionDB::ValidateTxnDBOptions(txn_db_options)));
+      txn_db = new WritePreparedTxnDB(
+          db, PessimisticTransactionDB::ValidateTxnDBOptions(txn_db_options));
      break;
    case WRITE_COMMITTED:
    default:
-      txn_db.reset(new WriteCommittedTxnDB(
-          db, PessimisticTransactionDB::ValidateTxnDBOptions(txn_db_options)));
+      txn_db = new WriteCommittedTxnDB(
+          db, PessimisticTransactionDB::ValidateTxnDBOptions(txn_db_options));
  }
  txn_db->UpdateCFComparatorMap(handles);
+  *dbptr = txn_db;
  Status s = txn_db->Initialize(compaction_enabled_cf_indices, handles);
-  // In case of a failure at this point, db is deleted via the txn_db destructor
-  // and set to nullptr.
-  if (s.ok()) {
-    *dbptr = txn_db.release();
-  }
  return s;
 }

@@ -97,10 +97,11 @@ TEST_P(TransactionTest, SuccessTest) {

  WriteOptions write_options;
  ReadOptions read_options;
-  std::string value;
+  string value;
+  Status s;

-  ASSERT_OK(db->Put(write_options, Slice("foo"), Slice("bar")));
-  ASSERT_OK(db->Put(write_options, Slice("foo2"), Slice("bar")));
+  db->Put(write_options, Slice("foo"), Slice("bar"));
+  db->Put(write_options, Slice("foo2"), Slice("bar"));

  Transaction* txn = db->BeginTransaction(write_options, TransactionOptions());
  ASSERT_TRUE(txn);
@@ -108,19 +109,24 @@ TEST_P(TransactionTest, SuccessTest) {
  ASSERT_EQ(0, txn->GetNumPuts());
  ASSERT_LE(0, txn->GetID());

-  ASSERT_OK(txn->GetForUpdate(read_options, "foo", &value));
+  s = txn->GetForUpdate(read_options, "foo", &value);
+  ASSERT_OK(s);
  ASSERT_EQ(value, "bar");

-  ASSERT_OK(txn->Put(Slice("foo"), Slice("bar2")));
+  s = txn->Put(Slice("foo"), Slice("bar2"));
+  ASSERT_OK(s);

  ASSERT_EQ(1, txn->GetNumPuts());

-  ASSERT_OK(txn->GetForUpdate(read_options, "foo", &value));
+  s = txn->GetForUpdate(read_options, "foo", &value);
+  ASSERT_OK(s);
  ASSERT_EQ(value, "bar2");

-  ASSERT_OK(txn->Commit());
+  s = txn->Commit();
+  ASSERT_OK(s);

-  ASSERT_OK(db->Get(read_options, "foo", &value));
+  s = db->Get(read_options, "foo", &value);
+  ASSERT_OK(s);
  ASSERT_EQ(value, "bar2");

  delete txn;
@@ -129,18 +135,22 @@ TEST_P(TransactionTest, SuccessTest) {
 // This test clarifies the contract of ValidateSnapshot
 TEST_P(TransactionTest, ValidateSnapshotTest) {
  for (bool with_2pc : {true, false}) {
-    ASSERT_OK(ReOpen());
+    ReOpen();
    WriteOptions write_options;
    ReadOptions read_options;
-    std::string value;
+    string value;
+    Status s;

    Transaction* txn1 =
        db->BeginTransaction(write_options, TransactionOptions());
    ASSERT_TRUE(txn1);
-    ASSERT_OK(txn1->Put(Slice("foo"), Slice("bar1")));
+    s = txn1->Put(Slice("foo"), Slice("bar1"));
+    ASSERT_OK(s);
    if (with_2pc) {
-      ASSERT_OK(txn1->SetName("xid1"));
-      ASSERT_OK(txn1->Prepare());
+      s = txn1->SetName("xid1");
+      ASSERT_OK(s);
+      s = txn1->Prepare();
+      ASSERT_OK(s);
    }

    Transaction* txn2 =
@@ -148,14 +158,15 @@ TEST_P(TransactionTest, ValidateSnapshotTest) {
    ASSERT_TRUE(txn2);
    txn2->SetSnapshot();

-    ASSERT_OK(txn1->Commit());
+    s = txn1->Commit();
+    ASSERT_OK(s);
    delete txn1;

    auto pes_txn2 = dynamic_cast<PessimisticTransaction*>(txn2);
    // Test the simple case where the key is not tracked yet
    auto trakced_seq = kMaxSequenceNumber;
-    auto s = pes_txn2->ValidateSnapshot(db->DefaultColumnFamily(), "foo",
-                                        &trakced_seq);
+    s = pes_txn2->ValidateSnapshot(db->DefaultColumnFamily(), "foo",
+                                   &trakced_seq);
    ASSERT_TRUE(s.IsBusy());
    delete txn2;
  }
@@ -340,8 +351,8 @@ TEST_P(TransactionTest, SharedLocks) {
  s = txn2->GetForUpdate(read_options, "foo", nullptr);
  ASSERT_OK(s);

-  ASSERT_OK(txn1->Rollback());
-  ASSERT_OK(txn2->Rollback());
+  txn1->Rollback();
+  txn2->Rollback();

  // Test txn1 trying to downgrade its lock.
  s = txn1->GetForUpdate(read_options, "foo", nullptr, true /* exclusive */);
@@ -741,13 +752,13 @@ TEST_P(TransactionTest, CommitTimeBatchFailTest) {
  WriteOptions write_options;
  TransactionOptions txn_options;

-  std::string value;
+  string value;
  Status s;

  Transaction* txn1 = db->BeginTransaction(write_options, txn_options);
  ASSERT_TRUE(txn1);

-  ASSERT_OK(txn1->GetCommitTimeWriteBatch()->Put("cat", "dog"));
+  txn1->GetCommitTimeWriteBatch()->Put("cat", "dog");

  s = txn1->Put("foo", "bar");
  ASSERT_OK(s);
@@ -763,7 +774,7 @@ TEST_P(TransactionTest, LogMarkLeakTest) {
  TransactionOptions txn_options;
  WriteOptions write_options;
  options.write_buffer_size = 1024;
-  ASSERT_OK(ReOpenNoDelete());
+  ReOpenNoDelete();
  Random rnd(47);
  std::vector<Transaction*> txns;
  DBImpl* db_impl = reinterpret_cast<DBImpl*>(db->GetRootDB());
@@ -797,7 +808,7 @@ TEST_P(TransactionTest, LogMarkLeakTest) {

 TEST_P(TransactionTest, SimpleTwoPhaseTransactionTest) {
  for (bool cwb4recovery : {true, false}) {
-    ASSERT_OK(ReOpen());
+    ReOpen();
    WriteOptions write_options;
    ReadOptions read_options;

@@ -1000,7 +1011,7 @@ TEST_P(TransactionTest, TwoPhaseEmptyWriteTest) {
      if (!cwb4recovery && test_with_empty_wal) {
        continue;
      }
-      ASSERT_OK(ReOpen());
+      ReOpen();
      Status s;
      std::string value;

@@ -1100,7 +1111,7 @@ TEST_P(TransactionTest, TwoPhaseRollbackTest) {

  TransactionOptions txn_options;

-  std::string value;
+  string value;
  Status s;

  DBImpl* db_impl = reinterpret_cast<DBImpl*>(db->GetRootDB());
@@ -1171,7 +1182,7 @@ TEST_P(TransactionTest, PersistentTwoPhaseTransactionTest) {

  TransactionOptions txn_options;

-  std::string value;
+  string value;
  Status s;

  DBImpl* db_impl = reinterpret_cast<DBImpl*>(db->GetRootDB());
@@ -1564,7 +1575,7 @@ TEST_P(TransactionTest, TwoPhaseLogRollingTest) {
  DBImpl* db_impl = reinterpret_cast<DBImpl*>(db->GetRootDB());

  Status s;
-  std::string v;
+  string v;
  ColumnFamilyHandle *cfa, *cfb;

  // Create 2 new column families
@@ -2006,7 +2017,7 @@ TEST_P(TransactionTest, WriteConflictTest2) {
  WriteOptions write_options;
  ReadOptions read_options;
  TransactionOptions txn_options;
-  std::string value;
+  string value;
  Status s;

  db->Put(write_options, "foo", "bar");
@@ -2054,7 +2065,7 @@ TEST_P(TransactionTest, ReadConflictTest) {
  WriteOptions write_options;
  ReadOptions read_options, snapshot_read_options;
  TransactionOptions txn_options;
-  std::string value;
+  string value;
  Status s;

  db->Put(write_options, "foo", "bar");
@@ -2092,7 +2103,7 @@ TEST_P(TransactionTest, TxnOnlyTest) {

  WriteOptions write_options;
  ReadOptions read_options;
-  std::string value;
+  string value;
  Status s;

  Transaction* txn = db->BeginTransaction(write_options);
@@ -2110,7 +2121,7 @@ TEST_P(TransactionTest, TxnOnlyTest) {
 TEST_P(TransactionTest, FlushTest) {
  WriteOptions write_options;
  ReadOptions read_options, snapshot_read_options;
-  std::string value;
+  string value;
  Status s;

  db->Put(write_options, Slice("foo"), Slice("bar"));
@@ -2302,7 +2313,7 @@ TEST_P(TransactionTest, FlushTest2) {
 TEST_P(TransactionTest, NoSnapshotTest) {
  WriteOptions write_options;
  ReadOptions read_options;
-  std::string value;
+  string value;
  Status s;

  db->Put(write_options, "AAA", "bar");
@@ -2332,12 +2343,12 @@ TEST_P(TransactionTest, NoSnapshotTest) {
 TEST_P(TransactionTest, MultipleSnapshotTest) {
  WriteOptions write_options;
  ReadOptions read_options, snapshot_read_options;
-  std::string value;
+  string value;
  Status s;

-  ASSERT_OK(db->Put(write_options, "AAA", "bar"));
-  ASSERT_OK(db->Put(write_options, "BBB", "bar"));
-  ASSERT_OK(db->Put(write_options, "CCC", "bar"));
+  db->Put(write_options, "AAA", "bar");
+  db->Put(write_options, "BBB", "bar");
+  db->Put(write_options, "CCC", "bar");

  Transaction* txn = db->BeginTransaction(write_options);
  ASSERT_TRUE(txn);
@@ -2345,24 +2356,24 @@ TEST_P(TransactionTest, MultipleSnapshotTest) {
  db->Put(write_options, "AAA", "bar1");

  // Read and write without a snapshot
-  ASSERT_OK(txn->GetForUpdate(read_options, "AAA", &value));
+  txn->GetForUpdate(read_options, "AAA", &value);
  ASSERT_EQ(value, "bar1");
  s = txn->Put("AAA", "bar2");
  ASSERT_OK(s);

  // Modify BBB before snapshot is taken
-  ASSERT_OK(db->Put(write_options, "BBB", "bar1"));
+  db->Put(write_options, "BBB", "bar1");

  txn->SetSnapshot();
  snapshot_read_options.snapshot = txn->GetSnapshot();

  // Read and write with snapshot
-  ASSERT_OK(txn->GetForUpdate(snapshot_read_options, "BBB", &value));
+  txn->GetForUpdate(snapshot_read_options, "BBB", &value);
  ASSERT_EQ(value, "bar1");
  s = txn->Put("BBB", "bar2");
  ASSERT_OK(s);

-  ASSERT_OK(db->Put(write_options, "CCC", "bar1"));
+  db->Put(write_options, "CCC", "bar1");

  // Set a new snapshot
  txn->SetSnapshot();
@@ -2801,7 +2812,7 @@ TEST_P(TransactionTest, LostUpdate) {
  WriteOptions write_options;
  ReadOptions read_options, read_options1, read_options2;
  TransactionOptions txn_options;
-  std::string value;
+  string value;
  Status s;

  // Test 2 transactions writing to the same key in multiple orders and
@@ -2926,7 +2937,7 @@ TEST_P(TransactionTest, LostUpdate) {
 TEST_P(TransactionTest, UntrackedWrites) {
  WriteOptions write_options;
  ReadOptions read_options;
-  std::string value;
+  string value;
  Status s;

  // Verify transaction rollback works for untracked keys.
@@ -3017,7 +3028,7 @@ TEST_P(TransactionTest, ReinitializeTest) {
  WriteOptions write_options;
  ReadOptions read_options;
  TransactionOptions txn_options;
-  std::string value;
+  string value;
  Status s;

  // Set txn expiration timeout to 0 microseconds (expires instantly)
@@ -3123,7 +3134,7 @@ TEST_P(TransactionTest, Rollback) {
  WriteOptions write_options;
  ReadOptions read_options;
  TransactionOptions txn_options;
-  std::string value;
+  string value;
  Status s;

  Transaction* txn1 = db->BeginTransaction(write_options, txn_options);
@@ -3266,7 +3277,7 @@ TEST_P(TransactionTest, IteratorTest) {
  WriteOptions write_options;
  ReadOptions read_options, snapshot_read_options;
  TransactionOptions txn_options;
-  std::string value;
+  string value;
  Status s;

  // Write some keys to the db
@@ -3384,7 +3395,7 @@ TEST_P(TransactionTest, IteratorTest) {
 TEST_P(TransactionTest, DisableIndexingTest) {
  WriteOptions write_options;
  ReadOptions read_options;
-  std::string value;
+  string value;
  Status s;

  Transaction* txn = db->BeginTransaction(write_options);
@@ -3447,7 +3458,7 @@ TEST_P(TransactionTest, SavepointTest) {
  WriteOptions write_options;
  ReadOptions read_options, snapshot_read_options;
  TransactionOptions txn_options;
-  std::string value;
+  string value;
  Status s;

  Transaction* txn = db->BeginTransaction(write_options);
@@ -3739,7 +3750,7 @@ TEST_P(TransactionTest, UndoGetForUpdateTest) {
  WriteOptions write_options;
  ReadOptions read_options;
  TransactionOptions txn_options;
-  std::string value;
+  string value;
  Status s;

  txn_options.lock_timeout = 1;  // 1 ms
@@ -3883,7 +3894,7 @@ TEST_P(TransactionTest, UndoGetForUpdateTest2) {
  WriteOptions write_options;
  ReadOptions read_options;
  TransactionOptions txn_options;
-  std::string value;
+  string value;
  Status s;

  s = db->Put(write_options, "A", "");
@@ -4088,7 +4099,7 @@ TEST_P(TransactionTest, UndoGetForUpdateTest2) {
 TEST_P(TransactionTest, TimeoutTest) {
  WriteOptions write_options;
  ReadOptions read_options;
-  std::string value;
+  string value;
  Status s;

  delete db;
@@ -4225,7 +4236,7 @@ TEST_P(TransactionTest, TimeoutTest) {
 TEST_P(TransactionTest, SingleDeleteTest) {
  WriteOptions write_options;
  ReadOptions read_options;
-  std::string value;
+  string value;
  Status s;

  Transaction* txn = db->BeginTransaction(write_options);
@@ -4323,7 +4334,7 @@ TEST_P(TransactionTest, SingleDeleteTest) {
 TEST_P(TransactionTest, MergeTest) {
  WriteOptions write_options;
  ReadOptions read_options;
-  std::string value;
+  string value;
  Status s;

  Transaction* txn = db->BeginTransaction(write_options, TransactionOptions());
@@ -4379,7 +4390,7 @@ TEST_P(TransactionTest, MergeTest) {
 TEST_P(TransactionTest, DeferSnapshotTest) {
  WriteOptions write_options;
  ReadOptions read_options;
-  std::string value;
+  string value;
  Status s;

  s = db->Put(write_options, "A", "a0");
@@ -4430,7 +4441,7 @@ TEST_P(TransactionTest, DeferSnapshotTest) {
 TEST_P(TransactionTest, DeferSnapshotTest2) {
  WriteOptions write_options;
  ReadOptions read_options, snapshot_read_options;
-  std::string value;
+  string value;
  Status s;

  Transaction* txn1 = db->BeginTransaction(write_options);
@@ -4487,7 +4498,7 @@ TEST_P(TransactionTest, DeferSnapshotTest2) {
 TEST_P(TransactionTest, DeferSnapshotSavePointTest) {
  WriteOptions write_options;
  ReadOptions read_options, snapshot_read_options;
-  std::string value;
+  string value;
  Status s;

  Transaction* txn1 = db->BeginTransaction(write_options);
@@ -4595,7 +4606,7 @@ TEST_P(TransactionTest, DeferSnapshotSavePointTest) {
 TEST_P(TransactionTest, SetSnapshotOnNextOperationWithNotification) {
  WriteOptions write_options;
  ReadOptions read_options;
-  std::string value;
+  string value;

  class Notifier : public TransactionNotifier {
   private:
@@ -4654,7 +4665,7 @@ TEST_P(TransactionTest, SetSnapshotOnNextOperationWithNotification) {
 TEST_P(TransactionTest, ClearSnapshotTest) {
  WriteOptions write_options;
  ReadOptions read_options, snapshot_read_options;
-  std::string value;
+  string value;
  Status s;

  s = db->Put(write_options, "foo", "0");
@@ -4895,7 +4906,7 @@ TEST_P(TransactionTest, MemoryLimitTest) {
  TransactionOptions txn_options;
  // Header (12 bytes) + NOOP (1 byte) + 2 * 8 bytes for data.
  txn_options.max_write_batch_size = 29;
-  std::string value;
+  string value;
  Status s;

  Transaction* txn = db->BeginTransaction(WriteOptions(), txn_options);
@@ -4931,7 +4942,7 @@ TEST_P(TransactionTest, SeqAdvanceTest) {
  FlushOptions fopt;

  options.disable_auto_compactions = true;
-  ASSERT_OK(ReOpen());
+  ReOpen();

  // Do the test with NUM_BRANCHES branches in it. Each run of a test takes some
  // of the branches. This is the same as counting a binary number where i-th
@@ -4945,7 +4956,7 @@ TEST_P(TransactionTest, SeqAdvanceTest) {
    return n & filter;
  };
  const size_t max_n = static_cast<size_t>(1) << NUM_BRANCHES;
-  for (size_t n = 0; n < max_n; n++) {
+  for (size_t n = 0; n < max_n; n++, ReOpen()) {
    DBImpl* db_impl = reinterpret_cast<DBImpl*>(db->GetRootDB());
    size_t branch = 0;
    auto seq = db_impl->GetLatestSequenceNumber();
@@ -4955,13 +4966,13 @@ TEST_P(TransactionTest, SeqAdvanceTest) {
    ASSERT_EQ(exp_seq, seq);

    if (branch_do(n, &branch)) {
-      ASSERT_OK(db_impl->Flush(fopt));
+      db_impl->Flush(fopt);
      seq = db_impl->TEST_GetLastVisibleSequence();
      ASSERT_EQ(exp_seq, seq);
    }
    if (!short_test && branch_do(n, &branch)) {
-      ASSERT_OK(db_impl->FlushWAL(true));
-      ASSERT_OK(ReOpenNoDelete());
+      db_impl->FlushWAL(true);
+      ReOpenNoDelete();
      db_impl = reinterpret_cast<DBImpl*>(db->GetRootDB());
      seq = db_impl->GetLatestSequenceNumber();
      ASSERT_EQ(exp_seq, seq);
@@ -4977,13 +4988,13 @@ TEST_P(TransactionTest, SeqAdvanceTest) {
    ASSERT_EQ(exp_seq, seq);

    if (branch_do(n, &branch)) {
-      ASSERT_OK(db_impl->Flush(fopt));
+      db_impl->Flush(fopt);
      seq = db_impl->TEST_GetLastVisibleSequence();
      ASSERT_EQ(exp_seq, seq);
    }
    if (!short_test && branch_do(n, &branch)) {
-      ASSERT_OK(db_impl->FlushWAL(true));
-      ASSERT_OK(ReOpenNoDelete());
+      db_impl->FlushWAL(true);
+      ReOpenNoDelete();
      db_impl = reinterpret_cast<DBImpl*>(db->GetRootDB());
      seq = db_impl->GetLatestSequenceNumber();
      ASSERT_EQ(exp_seq, seq);
@@ -4994,13 +5005,13 @@ TEST_P(TransactionTest, SeqAdvanceTest) {
    ASSERT_EQ(exp_seq, seq);

    if (branch_do(n, &branch)) {
-      ASSERT_OK(db_impl->Flush(fopt));
+      db_impl->Flush(fopt);
      seq = db_impl->TEST_GetLastVisibleSequence();
      ASSERT_EQ(exp_seq, seq);
    }
    if (!short_test && branch_do(n, &branch)) {
-      ASSERT_OK(db_impl->FlushWAL(true));
-      ASSERT_OK(ReOpenNoDelete());
+      db_impl->FlushWAL(true);
+      ReOpenNoDelete();
      db_impl = reinterpret_cast<DBImpl*>(db->GetRootDB());
      seq = db_impl->GetLatestSequenceNumber();
      ASSERT_EQ(exp_seq, seq);
@@ -5012,13 +5023,13 @@ TEST_P(TransactionTest, SeqAdvanceTest) {
    ASSERT_EQ(exp_seq, seq);

    if (branch_do(n, &branch)) {
-      ASSERT_OK(db_impl->Flush(fopt));
+      db_impl->Flush(fopt);
      seq = db_impl->TEST_GetLastVisibleSequence();
      ASSERT_EQ(exp_seq, seq);
    }
    if (!short_test && branch_do(n, &branch)) {
-      ASSERT_OK(db_impl->FlushWAL(true));
-      ASSERT_OK(ReOpenNoDelete());
+      db_impl->FlushWAL(true);
+      ReOpenNoDelete();
      db_impl = reinterpret_cast<DBImpl*>(db->GetRootDB());
      seq = db_impl->GetLatestSequenceNumber();
      ASSERT_EQ(exp_seq, seq);
@@ -5029,18 +5040,17 @@ TEST_P(TransactionTest, SeqAdvanceTest) {
    ASSERT_EQ(exp_seq, seq);

    if (branch_do(n, &branch)) {
-      ASSERT_OK(db_impl->Flush(fopt));
+      db_impl->Flush(fopt);
      seq = db_impl->TEST_GetLastVisibleSequence();
      ASSERT_EQ(exp_seq, seq);
    }
    if (!short_test && branch_do(n, &branch)) {
-      ASSERT_OK(db_impl->FlushWAL(true));
-      ASSERT_OK(ReOpenNoDelete());
+      db_impl->FlushWAL(true);
+      ReOpenNoDelete();
      db_impl = reinterpret_cast<DBImpl*>(db->GetRootDB());
      seq = db_impl->GetLatestSequenceNumber();
      ASSERT_EQ(exp_seq, seq);
    }
-    ASSERT_OK(ReOpen());
  }
 }

@@ -5052,7 +5062,7 @@ TEST_P(TransactionTest, Optimizations) {
    optimizations.skip_concurrency_control = IsInCombination(0, new_comb);
    optimizations.skip_duplicate_key_check = IsInCombination(1, new_comb);

-    ASSERT_OK(ReOpen());
+    ReOpen();
    WriteOptions write_options;
    WriteBatch batch;
    batch.Put(Slice("k"), Slice("v1"));
@@ -5101,7 +5111,7 @@ TEST_P(TransactionTest, DuplicateKeys) {
  std::string cf_name = "two";
  ColumnFamilyHandle* cf_handle = nullptr;
  {
-    ASSERT_OK(db->CreateColumnFamily(cf_options, cf_name, &cf_handle));
+    db->CreateColumnFamily(cf_options, cf_name, &cf_handle);
    WriteOptions write_options;
    WriteBatch batch;
    batch.Put(Slice("key"), Slice("value"));
@@ -5134,7 +5144,7 @@ TEST_P(TransactionTest, DuplicateKeys) {

  // Test with non-bytewise comparator
  {
-    ASSERT_OK(ReOpen());
+    ReOpen();
    std::unique_ptr<const Comparator> comp_gc(new ThreeBytewiseComparator());
    cf_options.comparator = comp_gc.get();
    ASSERT_OK(db->CreateColumnFamily(cf_options, cf_name, &cf_handle));
@@ -5178,8 +5188,8 @@ TEST_P(TransactionTest, DuplicateKeys) {
        if (with_commit_batch && do_rollback) {
          continue;
        }
-        ASSERT_OK(ReOpen());
-        ASSERT_OK(db->CreateColumnFamily(cf_options, cf_name, &cf_handle));
+        ReOpen();
+        db->CreateColumnFamily(cf_options, cf_name, &cf_handle);
        TransactionOptions txn_options;
        txn_options.use_only_the_last_commit_time_batch_for_recovery = false;
        WriteOptions write_options;
@@ -5306,11 +5316,11 @@ TEST_P(TransactionTest, DuplicateKeys) {
    // verify that.
    cf_options.max_successive_merges = 2;
    cf_options.merge_operator = MergeOperators::CreateStringAppendOperator();
-    ASSERT_OK(ReOpen());
+    ReOpen();
    db->CreateColumnFamily(cf_options, cf_name, &cf_handle);
    WriteOptions write_options;
    // Ensure one value for the key
-    ASSERT_OK(db->Put(write_options, cf_handle, Slice("key"), Slice("value")));
+    db->Put(write_options, cf_handle, Slice("key"), Slice("value"));
    WriteBatch batch;
    // Merge more than max_successive_merges times
    batch.Merge(cf_handle, Slice("key"), Slice("1"));
@@ -5334,14 +5344,14 @@ TEST_P(TransactionTest, DuplicateKeys) {
    ASSERT_OK(txn0->Put(Slice("foo0"), Slice("bar0a")));
    ASSERT_OK(txn0->Put(Slice("foo0"), Slice("bar0b")));
    txn0->SetSavePoint();
-    ASSERT_OK(txn0->RollbackToSavePoint());
+    txn0->RollbackToSavePoint();
    ASSERT_OK(txn0->Commit());
    delete txn0;
  }

  // Test sucessfull recovery after a crash
  {
-    ASSERT_OK(ReOpen());
+    ReOpen();
    TransactionOptions txn_options;
    WriteOptions write_options;
    ReadOptions ropt;
@@ -5373,7 +5383,7 @@ TEST_P(TransactionTest, DuplicateKeys) {
    ASSERT_OK(txn0->Prepare());
    delete txn0;
    // This will check the asserts inside recovery code
-    ASSERT_OK(db->FlushWAL(true));
+    db->FlushWAL(true);
    reinterpret_cast<PessimisticTransactionDB*>(db)->TEST_Crash();
    ASSERT_OK(ReOpenNoDelete(cfds, &handles));
    txn0 = db->GetTransactionByName("xid");
@@ -5432,7 +5442,7 @@ TEST_P(TransactionTest, DuplicateKeys) {
    ASSERT_OK(txn0->Prepare());
    delete txn0;
    // This will check the asserts inside recovery code
-    ASSERT_OK(db->FlushWAL(true));
+    db->FlushWAL(true);
    // Flush only cf 1
    reinterpret_cast<DBImpl*>(db->GetRootDB())
        ->TEST_FlushMemTable(true, handles[1]);
@@ -5465,7 +5475,7 @@ TEST_P(TransactionTest, DuplicateKeys) {
    ASSERT_OK(txn0->Prepare());
    delete txn0;
    // This will check the asserts inside recovery code
-    ASSERT_OK(db->FlushWAL(true));
+    db->FlushWAL(true);
    // Flush only cf 1
    reinterpret_cast<DBImpl*>(db->GetRootDB())
        ->TEST_FlushMemTable(true, handles[1]);
@@ -5492,7 +5502,7 @@ TEST_P(TransactionTest, DuplicateKeys) {
    ASSERT_OK(txn0->Prepare());
    delete txn0;
    // This will check the asserts inside recovery code
-    ASSERT_OK(db->FlushWAL(true));
+    db->FlushWAL(true);
    // Flush only cf 1
    reinterpret_cast<DBImpl*>(db->GetRootDB())
        ->TEST_FlushMemTable(true, handles[1]);
@@ -5519,7 +5529,7 @@ TEST_P(TransactionTest, DuplicateKeys) {
    ASSERT_OK(txn0->Prepare());
    delete txn0;
    // This will check the asserts inside recovery code
-    ASSERT_OK(db->FlushWAL(true));
+    db->FlushWAL(true);
    // Flush only cf 1
    reinterpret_cast<DBImpl*>(db->GetRootDB())
        ->TEST_FlushMemTable(true, handles[1]);
@@ -51,7 +51,7 @@ class TransactionTestBase : public ::testing::Test {

  TransactionTestBase(bool use_stackable_db, bool two_write_queue,
                      TxnDBWritePolicy write_policy)
-      : db(nullptr), env(nullptr), use_stackable_db_(use_stackable_db) {
+      : use_stackable_db_(use_stackable_db) {
    options.create_if_missing = true;
    options.max_write_buffer_number = 2;
    options.write_buffer_size = 4 * 1024;
@@ -78,7 +78,6 @@ class TransactionTestBase : public ::testing::Test {

  ~TransactionTestBase() {
    delete db;
-    db = nullptr;
    // This is to skip the assert statement in FaultInjectionTestEnv. There
    // seems to be a bug in btrfs that the makes readdir return recently
    // unlink-ed files. By using the default fs we simply ignore errors resulted
@@ -126,7 +125,6 @@ class TransactionTestBase : public ::testing::Test {

  Status ReOpen() {
    delete db;
-    db = nullptr;
    DestroyDB(dbname, options);
    Status s;
    if (use_stackable_db_ == false) {
@@ -141,23 +139,16 @@ class TransactionTestBase : public ::testing::Test {
                             std::vector<ColumnFamilyHandle*>* handles) {
    std::vector<size_t> compaction_enabled_cf_indices;
    TransactionDB::PrepareWrap(&options, &cfs, &compaction_enabled_cf_indices);
-    DB* root_db = nullptr;
+    DB* root_db;
    Options options_copy(options);
    const bool use_seq_per_batch =
        txn_db_options.write_policy == WRITE_PREPARED;
    Status s = DBImpl::Open(options_copy, dbname, cfs, handles, &root_db,
                            use_seq_per_batch);
-    StackableDB* stackable_db = new StackableDB(root_db);
    if (s.ok()) {
-      assert(root_db != nullptr);
-      s = TransactionDB::WrapStackableDB(stackable_db, txn_db_options,
-                                         compaction_enabled_cf_indices,
-                                         *handles, &db);
-    }
-    if (!s.ok()) {
-      delete stackable_db;
-      // just in case it was not deleted (and not set to nullptr).
-      delete root_db;
+      s = TransactionDB::WrapStackableDB(
+          new StackableDB(root_db), txn_db_options,
+          compaction_enabled_cf_indices, *handles, &db);
    }
    return s;
  }
@@ -170,26 +161,19 @@ class TransactionTestBase : public ::testing::Test {
    TransactionDB::PrepareWrap(&options, &column_families,
                               &compaction_enabled_cf_indices);
    std::vector<ColumnFamilyHandle*> handles;
-    DB* root_db = nullptr;
+    DB* root_db;
    Options options_copy(options);
    const bool use_seq_per_batch =
        txn_db_options.write_policy == WRITE_PREPARED;
    Status s = DBImpl::Open(options_copy, dbname, column_families, &handles,
                            &root_db, use_seq_per_batch);
-    StackableDB* stackable_db = new StackableDB(root_db);
    if (s.ok()) {
-      assert(root_db != nullptr);
      assert(handles.size() == 1);
-      s = TransactionDB::WrapStackableDB(stackable_db, txn_db_options,
-                                         compaction_enabled_cf_indices, handles,
-                                         &db);
+      s = TransactionDB::WrapStackableDB(
+          new StackableDB(root_db), txn_db_options,
+          compaction_enabled_cf_indices, handles, &db);
      delete handles[0];
    }
-    if (!s.ok()) {
-      delete stackable_db;
-      // just in case it was not deleted (and not set to nullptr).
-      delete root_db;
-    }
    return s;
  }

@@ -223,9 +207,9 @@ class TransactionTestBase : public ::testing::Test {
    // equivalent to commit without prepare.
    WriteBatch wb;
    auto istr = std::to_string(index);
-    ASSERT_OK(wb.Put("k1" + istr, "v1"));
-    ASSERT_OK(wb.Put("k2" + istr, "v2"));
-    ASSERT_OK(wb.Put("k3" + istr, "v3"));
+    wb.Put("k1" + istr, "v1");
+    wb.Put("k2" + istr, "v2");
+    wb.Put("k3" + istr, "v3");
    WriteOptions wopts;
    auto s = db->Write(wopts, &wb);
    if (txn_db_options.write_policy == TxnDBWritePolicy::WRITE_COMMITTED) {
@@ -247,12 +231,15 @@ class TransactionTestBase : public ::testing::Test {
    WriteOptions write_options;
    Transaction* txn = db->BeginTransaction(write_options, txn_options);
    auto istr = std::to_string(index);
-    ASSERT_OK(txn->SetName("xid" + istr));
-    ASSERT_OK(txn->Put(Slice("foo" + istr), Slice("bar")));
-    ASSERT_OK(txn->Put(Slice("foo2" + istr), Slice("bar2")));
-    ASSERT_OK(txn->Put(Slice("foo3" + istr), Slice("bar3")));
-    ASSERT_OK(txn->Put(Slice("foo4" + istr), Slice("bar4")));
-    ASSERT_OK(txn->Commit());
+    auto s = txn->SetName("xid" + istr);
+    ASSERT_OK(s);
+    s = txn->Put(Slice("foo" + istr), Slice("bar"));
+    s = txn->Put(Slice("foo2" + istr), Slice("bar2"));
+    s = txn->Put(Slice("foo3" + istr), Slice("bar3"));
+    s = txn->Put(Slice("foo4" + istr), Slice("bar4"));
+    ASSERT_OK(s);
+    s = txn->Commit();
+    ASSERT_OK(s);
    if (txn_db_options.write_policy == TxnDBWritePolicy::WRITE_COMMITTED) {
      // Consume one seq per key
      exp_seq += 4;
@@ -274,16 +261,20 @@ class TransactionTestBase : public ::testing::Test {
    WriteOptions write_options;
    Transaction* txn = db->BeginTransaction(write_options, txn_options);
    auto istr = std::to_string(index);
-    ASSERT_OK(txn->SetName("xid" + istr));
-    ASSERT_OK(txn->Put(Slice("foo" + istr), Slice("bar")));
-    ASSERT_OK(txn->Put(Slice("foo2" + istr), Slice("bar2")));
-    ASSERT_OK(txn->Put(Slice("foo3" + istr), Slice("bar3")));
-    ASSERT_OK(txn->Put(Slice("foo4" + istr), Slice("bar4")));
-    ASSERT_OK(txn->Put(Slice("foo5" + istr), Slice("bar5")));
+    auto s = txn->SetName("xid" + istr);
+    ASSERT_OK(s);
+    s = txn->Put(Slice("foo" + istr), Slice("bar"));
+    s = txn->Put(Slice("foo2" + istr), Slice("bar2"));
+    s = txn->Put(Slice("foo3" + istr), Slice("bar3"));
+    s = txn->Put(Slice("foo4" + istr), Slice("bar4"));
+    s = txn->Put(Slice("foo5" + istr), Slice("bar5"));
+    ASSERT_OK(s);
    expected_commits++;
-    ASSERT_OK(txn->Prepare());
+    s = txn->Prepare();
+    ASSERT_OK(s);
    commit_writes++;
-    ASSERT_OK(txn->Commit());
+    s = txn->Commit();
+    ASSERT_OK(s);
    if (txn_db_options.write_policy == TxnDBWritePolicy::WRITE_COMMITTED) {
      // Consume one seq per key
      exp_seq += 5;
@@ -301,16 +292,20 @@ class TransactionTestBase : public ::testing::Test {
    WriteOptions write_options;
    Transaction* txn = db->BeginTransaction(write_options, txn_options);
    auto istr = std::to_string(index);
-    ASSERT_OK(txn->SetName("xid" + istr));
-    ASSERT_OK(txn->Put(Slice("foo" + istr), Slice("bar")));
-    ASSERT_OK(txn->Put(Slice("foo2" + istr), Slice("bar2")));
-    ASSERT_OK(txn->Put(Slice("foo3" + istr), Slice("bar3")));
-    ASSERT_OK(txn->Put(Slice("foo4" + istr), Slice("bar4")));
-    ASSERT_OK(txn->Put(Slice("foo5" + istr), Slice("bar5")));
+    auto s = txn->SetName("xid" + istr);
+    ASSERT_OK(s);
+    s = txn->Put(Slice("foo" + istr), Slice("bar"));
+    s = txn->Put(Slice("foo2" + istr), Slice("bar2"));
+    s = txn->Put(Slice("foo3" + istr), Slice("bar3"));
+    s = txn->Put(Slice("foo4" + istr), Slice("bar4"));
+    s = txn->Put(Slice("foo5" + istr), Slice("bar5"));
+    ASSERT_OK(s);
    expected_commits++;
-    ASSERT_OK(txn->Prepare());
+    s = txn->Prepare();
+    ASSERT_OK(s);
    commit_writes++;
-    ASSERT_OK(txn->Rollback());
+    s = txn->Rollback();
+    ASSERT_OK(s);
    if (txn_db_options.write_policy == TxnDBWritePolicy::WRITE_COMMITTED) {
      // No seq is consumed for deleting the txn buffer
      exp_seq += 0;
@@ -350,12 +345,15 @@ class TransactionTestBase : public ::testing::Test {
      // For test the duplicate keys
      auto v2 = Slice("bar2-" + istr).ToString();
      auto type = rnd.Uniform(4);
+      Status s;
      switch (type) {
        case 0:
          committed_kvs[k] = v;
-          ASSERT_OK(db->Put(write_options, k, v));
+          s = db->Put(write_options, k, v);
+          ASSERT_OK(s);
          committed_kvs[k] = v2;
-          ASSERT_OK(db->Put(write_options, k, v2));
+          s = db->Put(write_options, k, v2);
+          ASSERT_OK(s);
          break;
        case 1: {
          WriteBatch wb;
@@ -363,22 +361,26 @@ class TransactionTestBase : public ::testing::Test {
          wb.Put(k, v);
          committed_kvs[k] = v2;
          wb.Put(k, v2);
-          ASSERT_OK(db->Write(write_options, &wb));
-
+          s = db->Write(write_options, &wb);
+          ASSERT_OK(s);
        } break;
        case 2:
        case 3:
          txn = db->BeginTransaction(write_options, txn_options);
-          ASSERT_OK(txn->SetName("xid" + istr));
+          s = txn->SetName("xid" + istr);
+          ASSERT_OK(s);
          committed_kvs[k] = v;
-          ASSERT_OK(txn->Put(k, v));
+          s = txn->Put(k, v);
+          ASSERT_OK(s);
          committed_kvs[k] = v2;
-          ASSERT_OK(txn->Put(k, v2));
-
+          s = txn->Put(k, v2);
+          ASSERT_OK(s);
          if (type == 3) {
-            ASSERT_OK(txn->Prepare());
+            s = txn->Prepare();
+            ASSERT_OK(s);
          }
-          ASSERT_OK(txn->Commit());
+          s = txn->Commit();
+          ASSERT_OK(s);
          if (type == 2) {
            auto pdb = reinterpret_cast<PessimisticTransactionDB*>(db);
            // TODO(myabandeh): this is counter-intuitive. The destructor should
@@ -79,7 +79,6 @@ class BaseDeltaIterator : public Iterator {
  void Next() override {
    if (!Valid()) {
      status_ = Status::NotSupported("Next() on invalid iterator");
-      return;
    }

    if (!forward_) {
@@ -115,7 +114,6 @@ class BaseDeltaIterator : public Iterator {
  void Prev() override {
    if (!Valid()) {
      status_ = Status::NotSupported("Prev() on invalid iterator");
-      return;
    }

    if (forward_) {
@@ -172,21 +170,6 @@ class BaseDeltaIterator : public Iterator {
 private:
  void AssertInvariants() {
 #ifndef NDEBUG
-    bool not_ok = false;
-    if (!base_iterator_->status().ok()) {
-      assert(!base_iterator_->Valid());
-      not_ok = true;
-    }
-    if (!delta_iterator_->status().ok()) {
-      assert(!delta_iterator_->Valid());
-      not_ok = true;
-    }
-    if (not_ok) {
-      assert(!Valid());
-      assert(!status().ok());
-      return;
-    }
-
    if (!Valid()) {
      return;
    }
@@ -255,25 +238,13 @@ class BaseDeltaIterator : public Iterator {
  void UpdateCurrent() {
 // Suppress false positive clang analyzer warnings.
 #ifndef __clang_analyzer__
-    status_ = Status::OK();
    while (true) {
      WriteEntry delta_entry;
      if (DeltaValid()) {
-        assert(delta_iterator_->status().ok());
        delta_entry = delta_iterator_->Entry();
-      } else if (!delta_iterator_->status().ok()) {
-        // Expose the error status and stop.
-        current_at_base_ = false;
-        return;
      }
      equal_keys_ = false;
      if (!BaseValid()) {
-        if (!base_iterator_->status().ok()) {
-          // Expose the error status and stop.
-          current_at_base_ = true;
-          return;
-        }
-
        // Base has finished.
        if (!DeltaValid()) {
          // Finished