TESTING ppc build based on known good base revision

This commit is actually a cherry-pick of #6643 but for testing purposes
only (see Issue #6653)

.travis.yml

@@ -71,89 +71,79 @@ matrix:
   - os: linux
     compiler: clang
   # Exclude most osx, arm64 and ppc64le tests for pull requests, but build in branches
-  # NB: the cmake build is a partial java test
-  - if: type = pull_request
+  - if: type != pull_request
     os: osx
     env: TEST_GROUP=1
-  - if: type = pull_request
+  - if: type != pull_request
     os : linux
     arch: arm64
     env: TEST_GROUP=1
-  - if: type = pull_request
+  - if: type != pull_request
     os: linux
     arch: ppc64le
     env: TEST_GROUP=1
-  - if: type = pull_request
+  - if: type != pull_request
     os: osx
     env: TEST_GROUP=2
-  - if: type = pull_request
+  - if: type != pull_request
     os : linux
     arch: arm64
     env: TEST_GROUP=2
-  - if: type = pull_request
+  - if: type != pull_request
     os: linux
     arch: ppc64le
     env: TEST_GROUP=2
-  - if: type = pull_request
+  - if: type != pull_request
     os: osx
     env: TEST_GROUP=3
-  - if: type = pull_request
+  - if: type != pull_request
     os : linux
     arch: arm64
     env: TEST_GROUP=3
-  - if: type = pull_request
+  - if: type != pull_request
     os: linux
     arch: ppc64le
     env: TEST_GROUP=3
-  - if: type = pull_request
+  - if: type != pull_request
     os: osx
     env: TEST_GROUP=4
-  - if: type = pull_request
+  - if: type != pull_request
     os : linux
     arch: arm64
     env: TEST_GROUP=4
-  - if: type = pull_request
+  - if: type != pull_request
     os: linux
     arch: ppc64le
     env: TEST_GROUP=4
-  - if: type = pull_request
-    os : osx
-    env: JOB_NAME=java_test
-  - if: type = pull_request
+  - if: type != pull_request
     os : linux
     arch: arm64
-    env: JOB_NAME=java_test
-  - if: type = pull_request
+    env: JOB_NAME=java-test
+  - if: type != pull_request
     os: linux
     arch: ppc64le
-    env: JOB_NAME=java_test
-  - if: type = pull_request
-    os : osx
-    env: JOB_NAME=lite_build
-  - if: type = pull_request
+    env: JOB_NAME=java-test
+  - if: type != pull_request
     os : linux
     arch: arm64
-    env: JOB_NAME=lite_build
-  - if: type = pull_request
+    env: JOB_NAME=lite-build
+  - if: type != pull_request
     os: linux
     arch: ppc64le
-    env: JOB_NAME=lite_build
-  - if: type = pull_request
-    os : osx
-    env: JOB_NAME=examples
-  - if: type = pull_request
+    env: JOB_NAME=lite-build
+  - if: type != pull_request
     os : linux
     arch: arm64
     env: JOB_NAME=examples
-  - if: type = pull_request
+  - if: type != pull_request
     os: linux
     arch: ppc64le
     env: JOB_NAME=examples
-  - if: type = pull_request
+  - if: type != pull_request
     os : linux
     arch: arm64
     env: JOB_NAME=cmake-gcc8
-  - if: type = pull_request
+  - if: type != pull_request
     os: linux
     arch: ppc64le
     env: JOB_NAME=cmake-gcc8
@@ -201,7 +191,7 @@ before_script:
   - ulimit -n 8192
 
 script:
-  - ${CXX} --version
+  - date; ${CXX} --version
   - if [ `command -v ccache` ]; then ccache -C; fi
   - case $TEST_GROUP in
     platform_dependent)

CMakeLists.txt

@@ -565,7 +565,6 @@ set(SOURCES
         db/trim_history_scheduler.cc
         db/version_builder.cc
         db/version_edit.cc
-        db/version_edit_handler.cc
         db/version_set.cc
         db/wal_manager.cc
         db/write_batch.cc
@@ -1023,7 +1022,6 @@ if(WITH_TESTS)
         env/io_posix_test.cc
         env/mock_env_test.cc
         file/delete_scheduler_test.cc
-        file/random_access_file_reader_test.cc
         logging/auto_roll_logger_test.cc
         logging/env_logger_test.cc
         logging/event_logger_test.cc

HISTORY.md

@@ -2,12 +2,10 @@
 ## Unreleased
 ### Public API Change
 * Fix spelling so that API now has correctly spelled transaction state name `COMMITTED`, while the old misspelled `COMMITED` is still available as an alias.
-* Updated default format_version in BlockBasedTableOptions from 2 to 4. SST files generated with the new default can be read by RocksDB versions 5.16 and newer, and use more efficient encoding of keys in index blocks.
 
 ### Bug Fixes
 * Fix a bug where range tombstone blocks in ingested files were cached incorrectly during ingestion. If range tombstones were read from those incorrectly cached blocks, the keys they covered would be exposed.
 * Fix a data race that might cause crash when calling DB::GetCreationTimeOfOldestFile() by a small chance. The bug was introduced in 6.6 Release.
-* Fix a bug where a boolean value optimize_filters_for_hits was for max threads when calling load table handles after a flush or compaction. The value is correct to 1. The bug should not cause user visible problems.
 
 ### Performance Improvements
 * In CompactRange, for levels starting from 0, if the level does not have any file with any key falling in the specified range, the level is skipped. So instead of always compacting from level 0, the compaction starts from the first level with keys in the specified range until the last such level.
@@ -15,7 +13,6 @@
 ### New Features
 * Basic support for user timestamp in iterator. Seek/SeekToFirst/Next and lower/upper bounds are supported. Reverse iteration is not supported. Merge is not considered.
 * When file lock failure when the lock is held by the current process, return acquiring time and thread ID in the error message.
-* Added a new option, best_efforts_recovery (default: false), to allow database to open in a db dir with missing table files. During best efforts recovery, missing table files are ignored, and database recovers to the most recent state without missing table file. Cross-column-family consistency is not guaranteed even if WAL is enabled.
 
 ## 6.8.0 (02/24/2020)
 ### Java API Changes

Makefile

@@ -8,7 +8,6 @@
 
 BASH_EXISTS := $(shell which bash)
 SHELL := $(shell which bash)
-PYTHON?=$(shell which python)
 
 CLEAN_FILES = # deliberately empty, so we can append below.
 CFLAGS += ${EXTRA_CFLAGS}
@@ -514,7 +513,6 @@ TESTS = \
 	fault_injection_test \
 	filelock_test \
 	filename_test \
-	random_access_file_reader_test \
 	file_reader_writer_test \
 	block_based_filter_block_test \
 	full_filter_block_test \
@@ -945,24 +943,24 @@ check: all
 	    $(MAKE) T="$$t" TMPD=$(TMPD) check_0;                       \
 	else                                                            \
 	    for t in $(TESTS); do                                       \
-	      echo "===== Running $$t"; ./$$t || exit 1; done;          \
+	      echo "===== Running $$t (`date`)"; ./$$t || exit 1; done;          \
 	fi
 	rm -rf $(TMPD)
 ifneq ($(PLATFORM), OS_AIX)
-	$(PYTHON) tools/check_all_python.py
+	python tools/check_all_python.py
 ifeq ($(filter -DROCKSDB_LITE,$(OPT)),)
-	$(PYTHON) tools/ldb_test.py
+	python tools/ldb_test.py
 	sh tools/rocksdb_dump_test.sh
 endif
 endif
 
 # TODO add ldb_tests
 check_some: $(SUBSET)
-	for t in $(SUBSET); do echo "===== Running $$t"; ./$$t || exit 1; done
+	for t in $(SUBSET); do echo "===== Running $$t (`date`)"; ./$$t || exit 1; done
 
 .PHONY: ldb_tests
 ldb_tests: ldb
-	$(PYTHON) tools/ldb_test.py
+	python tools/ldb_test.py
 
 crash_test: whitebox_crash_test blackbox_crash_test
 
@@ -971,31 +969,31 @@ crash_test_with_atomic_flush: whitebox_crash_test_with_atomic_flush blackbox_cra
 crash_test_with_txn: whitebox_crash_test_with_txn blackbox_crash_test_with_txn
 
 blackbox_crash_test: db_stress
-	$(PYTHON) -u tools/db_crashtest.py --simple blackbox $(CRASH_TEST_EXT_ARGS)
-	$(PYTHON) -u tools/db_crashtest.py blackbox $(CRASH_TEST_EXT_ARGS)
+	python -u tools/db_crashtest.py --simple blackbox $(CRASH_TEST_EXT_ARGS)
+	python -u tools/db_crashtest.py blackbox $(CRASH_TEST_EXT_ARGS)
 
 blackbox_crash_test_with_atomic_flush: db_stress
-	$(PYTHON) -u tools/db_crashtest.py --cf_consistency blackbox $(CRASH_TEST_EXT_ARGS)
+	python -u tools/db_crashtest.py --cf_consistency blackbox $(CRASH_TEST_EXT_ARGS)
 
 blackbox_crash_test_with_txn: db_stress
-	$(PYTHON) -u tools/db_crashtest.py --txn blackbox $(CRASH_TEST_EXT_ARGS)
+	python -u tools/db_crashtest.py --txn blackbox $(CRASH_TEST_EXT_ARGS)
 
 ifeq ($(CRASH_TEST_KILL_ODD),)
   CRASH_TEST_KILL_ODD=888887
 endif
 
 whitebox_crash_test: db_stress
-	$(PYTHON) -u tools/db_crashtest.py --simple whitebox --random_kill_odd \
+	python -u tools/db_crashtest.py --simple whitebox --random_kill_odd \
       $(CRASH_TEST_KILL_ODD) $(CRASH_TEST_EXT_ARGS)
-	$(PYTHON) -u tools/db_crashtest.py whitebox  --random_kill_odd \
+	python -u tools/db_crashtest.py whitebox  --random_kill_odd \
       $(CRASH_TEST_KILL_ODD) $(CRASH_TEST_EXT_ARGS)
 
 whitebox_crash_test_with_atomic_flush: db_stress
-	$(PYTHON) -u tools/db_crashtest.py --cf_consistency whitebox  --random_kill_odd \
+	python -u tools/db_crashtest.py --cf_consistency whitebox  --random_kill_odd \
       $(CRASH_TEST_KILL_ODD) $(CRASH_TEST_EXT_ARGS)
 
 whitebox_crash_test_with_txn: db_stress
-	$(PYTHON) -u tools/db_crashtest.py --txn whitebox --random_kill_odd \
+	python -u tools/db_crashtest.py --txn whitebox --random_kill_odd \
       $(CRASH_TEST_KILL_ODD) $(CRASH_TEST_EXT_ARGS)
 
 asan_check:
@@ -1064,7 +1062,7 @@ ifneq ($(PAR_TEST),)
 parloop:
 	ret_bad=0;							\
 	for t in $(PAR_TEST); do		\
-		echo "===== Running $$t in parallel $(NUM_PAR)";\
+		echo "===== Running $$t in parallel $(NUM_PAR) (`date`)";\
 		if [ $(db_test) -eq 1 ]; then \
 			seq $(J) | v="$$t" build_tools/gnu_parallel --gnu --plain 's=$(TMPD)/rdb-{};  export TEST_TMPDIR=$$s;' \
 				'timeout 2m ./db_test --gtest_filter=$$v >> $$s/log-{} 2>1'; \
@@ -1507,9 +1505,6 @@ delete_scheduler_test: file/delete_scheduler_test.o $(LIBOBJECTS) $(TESTHARNESS)
 filename_test: db/filename_test.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)
 
-random_access_file_reader_test: file/random_access_file_reader_test.o $(LIBOBJECTS) $(TESTHARNESS) $(TESTUTIL)
-	$(AM_LINK)
-
 file_reader_writer_test: util/file_reader_writer_test.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)
 
@@ -2094,7 +2089,7 @@ jtest_run:
 
 jtest: rocksdbjava
 	cd java;$(MAKE) sample;$(MAKE) test;
-	$(PYTHON) tools/check_all_python.py # TODO peterd: find a better place for this check in CI targets
+	python tools/check_all_python.py # TODO peterd: find a better place for this check in CI targets
 
 jdb_bench:
 	cd java;$(MAKE) db_bench;

TARGETS

@@ -171,7 +171,6 @@ cpp_library(
         "db/trim_history_scheduler.cc",
         "db/version_builder.cc",
         "db/version_edit.cc",
-        "db/version_edit_handler.cc",
         "db/version_set.cc",
         "db/wal_manager.cc",
         "db/write_batch.cc",
@@ -1301,13 +1300,6 @@ ROCKS_TESTS = [
         [],
         [],
     ],
-    [
-        "random_access_file_reader_test",
-        "file/random_access_file_reader_test.cc",
-        "serial",
-        [],
-        [],
-    ],
     [
         "random_test",
         "util/random_test.cc",

coverage/coverage_test.sh

@@ -17,19 +17,16 @@ if [ -d /mnt/gvfs/third-party -a -z "$CXX" ]; then
 else
   GCOV=$(which gcov)
 fi
-echo -e "Using $GCOV"
 
 COVERAGE_DIR="$PWD/COVERAGE_REPORT"
 mkdir -p $COVERAGE_DIR
 
 # Find all gcno files to generate the coverage report
 
-PYTHON=${1:-`which python`}
-echo -e "Using $PYTHON"
 GCNO_FILES=`find $ROOT -name "*.gcno"`
 $GCOV --preserve-paths --relative-only --no-output $GCNO_FILES 2>/dev/null |
   # Parse the raw gcov report to more human readable form.
-  $PYTHON $ROOT/coverage/parse_gcov_output.py |
+  python $ROOT/coverage/parse_gcov_output.py |
   # Write the output to both stdout and report file.
   tee $COVERAGE_DIR/coverage_report_all.txt &&
 echo -e "Generated coverage report for all files: $COVERAGE_DIR/coverage_report_all.txt\n"
@@ -44,7 +41,7 @@ RECENT_REPORT=$COVERAGE_DIR/coverage_report_recent.txt
 
 echo -e "Recently updated files: $LATEST_FILES\n" > $RECENT_REPORT
 $GCOV --preserve-paths --relative-only --no-output $GCNO_FILES 2>/dev/null |
-  $PYTHON $ROOT/coverage/parse_gcov_output.py -interested-files $LATEST_FILES |
+  python $ROOT/coverage/parse_gcov_output.py -interested-files $LATEST_FILES |
   tee -a $RECENT_REPORT &&
 echo -e "Generated coverage report for recently updated files: $RECENT_REPORT\n"
 

coverage/parse_gcov_output.py

@@ -1,12 +1,10 @@
-#!/usr/bin/env python
+#!/usr/bin/env python2
 # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
-
-from __future__ import print_function
-
-import optparse
 import re
 import sys
 
+from optparse import OptionParser
+
 # the gcov report follows certain pattern. Each file will have two lines
 # of report, from which we can extract the file name, total lines and coverage
 # percentage.
@@ -50,7 +48,7 @@ def parse_gcov_report(gcov_input):
 def get_option_parser():
     usage = "Parse the gcov output and generate more human-readable code " +\
             "coverage report."
-    parser = optparse.OptionParser(usage)
+    parser = OptionParser(usage)
 
     parser.add_option(
         "--interested-files", "-i",
@@ -75,8 +73,8 @@ def display_file_coverage(per_file_coverage, total_coverage):
     header_template = \
         "%" + str(max_file_name_length) + "s\t%s\t%s"
     separator = "-" * (max_file_name_length + 10 + 20)
-    print(header_template % ("Filename", "Coverage", "Lines"))  # noqa: E999 T25377293 Grandfathered in
-    print(separator)
+    print header_template % ("Filename", "Coverage", "Lines")  # noqa: E999 T25377293 Grandfathered in
+    print separator
 
     # -- Print body
     # template for printing coverage report for each file.
@@ -84,12 +82,12 @@ def display_file_coverage(per_file_coverage, total_coverage):
 
     for fname, coverage_info in per_file_coverage.items():
         coverage, lines = coverage_info
-        print(record_template % (fname, coverage, lines))
+        print record_template % (fname, coverage, lines)
 
     # -- Print footer
     if total_coverage:
-        print(separator)
-        print(record_template % ("Total", total_coverage[0], total_coverage[1]))
+        print separator
+        print record_template % ("Total", total_coverage[0], total_coverage[1])
 
 def report_coverage():
     parser = get_option_parser()
@@ -113,7 +111,7 @@ def report_coverage():
         total_coverage = None
 
     if not len(per_file_coverage):
-        print("Cannot find coverage info for the given files.", file=sys.stderr)
+        print >> sys.stderr, "Cannot find coverage info for the given files."
         return
     display_file_coverage(per_file_coverage, total_coverage)
 

db/column_family.cc

@@ -1397,8 +1397,8 @@ ColumnFamilySet::ColumnFamilySet(const std::string& dbname,
                                  const ImmutableDBOptions* db_options,
                                  const FileOptions& file_options,
                                  Cache* table_cache,
-                                 WriteBufferManager* _write_buffer_manager,
-                                 WriteController* _write_controller,
+                                 WriteBufferManager* write_buffer_manager,
+                                 WriteController* write_controller,
                                  BlockCacheTracer* const block_cache_tracer)
     : max_column_family_(0),
       dummy_cfd_(new ColumnFamilyData(
@@ -1410,8 +1410,8 @@ ColumnFamilySet::ColumnFamilySet(const std::string& dbname,
       db_options_(db_options),
       file_options_(file_options),
       table_cache_(table_cache),
-      write_buffer_manager_(_write_buffer_manager),
-      write_controller_(_write_controller),
+      write_buffer_manager_(write_buffer_manager),
+      write_controller_(write_controller),
       block_cache_tracer_(block_cache_tracer) {
   // initialize linked list
   dummy_cfd_->prev_ = dummy_cfd_;

db/column_family.h

@@ -647,8 +647,8 @@ class ColumnFamilySet {
   ColumnFamilySet(const std::string& dbname,
                   const ImmutableDBOptions* db_options,
                   const FileOptions& file_options, Cache* table_cache,
-                  WriteBufferManager* _write_buffer_manager,
-                  WriteController* _write_controller,
+                  WriteBufferManager* write_buffer_manager,
+                  WriteController* write_controller,
                   BlockCacheTracer* const block_cache_tracer);
   ~ColumnFamilySet();
 
@@ -678,10 +678,6 @@ class ColumnFamilySet {
 
   Cache* get_table_cache() { return table_cache_; }
 
-  WriteBufferManager* write_buffer_manager() { return write_buffer_manager_; }
-
-  WriteController* write_controller() { return write_controller_; }
-
  private:
   friend class ColumnFamilyData;
   // helper function that gets called from cfd destructor

db/corruption_test.cc

@@ -190,6 +190,7 @@ class CorruptionTest : public testing::Test {
     ASSERT_TRUE(s.ok()) << s.ToString();
     Options options;
     EnvOptions env_options;
+    options.file_system.reset(new LegacyFileSystemWrapper(options.env));
     ASSERT_NOK(VerifySstFileChecksum(options, env_options, fname));
   }
 

db/db_basic_test.cc

@@ -6,7 +6,6 @@
 // Copyright (c) 2011 The LevelDB Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
-
 #include "db/db_test_util.h"
 #include "port/stack_trace.h"
 #include "rocksdb/perf_context.h"
@@ -925,44 +924,44 @@ TEST_F(DBBasicTest, MmapAndBufferOptions) {
 #endif
 
 class TestEnv : public EnvWrapper {
- public:
-  explicit TestEnv(Env* base_env) : EnvWrapper(base_env), close_count(0) {}
+  public:
+   explicit TestEnv(Env* base_env) : EnvWrapper(base_env), close_count(0) {}
 
-  class TestLogger : public Logger {
-   public:
-    using Logger::Logv;
-    explicit TestLogger(TestEnv* env_ptr) : Logger() { env = env_ptr; }
-    ~TestLogger() override {
-      if (!closed_) {
-        CloseHelper();
-      }
+   class TestLogger : public Logger {
+    public:
+     using Logger::Logv;
+     explicit TestLogger(TestEnv* env_ptr) : Logger() { env = env_ptr; }
+     ~TestLogger() override {
+       if (!closed_) {
+         CloseHelper();
+       }
+     }
+     void Logv(const char* /*format*/, va_list /*ap*/) override {}
+
+    protected:
+     Status CloseImpl() override { return CloseHelper(); }
+
+    private:
+     Status CloseHelper() {
+       env->CloseCountInc();
+       ;
+       return Status::IOError();
+     }
+     TestEnv* env;
+   };
+
+    void CloseCountInc() { close_count++; }
+
+    int GetCloseCount() { return close_count; }
+
+    Status NewLogger(const std::string& /*fname*/,
+                     std::shared_ptr<Logger>* result) override {
+      result->reset(new TestLogger(this));
+      return Status::OK();
     }
-    void Logv(const char* /*format*/, va_list /*ap*/) override {}
-
-   protected:
-    Status CloseImpl() override { return CloseHelper(); }
 
    private:
-    Status CloseHelper() {
-      env->CloseCountInc();
-      ;
-      return Status::IOError();
-    }
-    TestEnv* env;
-  };
-
-  void CloseCountInc() { close_count++; }
-
-  int GetCloseCount() { return close_count; }
-
-  Status NewLogger(const std::string& /*fname*/,
-                   std::shared_ptr<Logger>* result) override {
-    result->reset(new TestLogger(this));
-    return Status::OK();
-  }
-
- private:
-  int close_count;
+    int close_count;
 };
 
 TEST_F(DBBasicTest, DBClose) {
@@ -1014,7 +1013,7 @@ TEST_F(DBBasicTest, DBCloseFlushError) {
   Options options = GetDefaultOptions();
   options.create_if_missing = true;
   options.manual_wal_flush = true;
-  options.write_buffer_size = 100;
+  options.write_buffer_size=100;
   options.env = fault_injection_env.get();
 
   Reopen(options);
@@ -1464,8 +1463,7 @@ TEST_F(DBBasicTest, MultiGetBatchedMultiLevelMerge) {
   ASSERT_EQ(0, num_keys);
 
   for (int i = 0; i < 128; i += 9) {
-    ASSERT_OK(
-        Merge("key_" + std::to_string(i), "val_mem_" + std::to_string(i)));
+    ASSERT_OK(Merge("key_" + std::to_string(i), "val_mem_" + std::to_string(i)));
   }
 
   std::vector<std::string> keys;
@@ -1703,8 +1701,8 @@ TEST_F(DBBasicTest, MultiGetIOBufferOverrun) {
   BlockBasedTableOptions table_options;
   table_options.pin_l0_filter_and_index_blocks_in_cache = true;
   table_options.block_size = 16 * 1024;
-  ASSERT_TRUE(table_options.block_size >
-            BlockBasedTable::kMultiGetReadStackBufSize);
+  assert(table_options.block_size >
+          BlockBasedTable::kMultiGetReadStackBufSize);
   options.table_factory.reset(new BlockBasedTableFactory(table_options));
   Reopen(options);
 
@@ -1736,160 +1734,6 @@ TEST_F(DBBasicTest, MultiGetIOBufferOverrun) {
                      keys.data(), values.data(), statuses.data(), true);
 }
 
-TEST_F(DBBasicTest, IncrementalRecoveryNoCorrupt) {
-  Options options = CurrentOptions();
-  DestroyAndReopen(options);
-  CreateAndReopenWithCF({"pikachu", "eevee"}, options);
-  size_t num_cfs = handles_.size();
-  ASSERT_EQ(3, num_cfs);
-  WriteOptions write_opts;
-  write_opts.disableWAL = true;
-  for (size_t cf = 0; cf != num_cfs; ++cf) {
-    for (size_t i = 0; i != 10000; ++i) {
-      std::string key_str = Key(static_cast<int>(i));
-      std::string value_str = std::to_string(cf) + "_" + std::to_string(i);
-
-      ASSERT_OK(Put(static_cast<int>(cf), key_str, value_str));
-      if (0 == (i % 1000)) {
-        ASSERT_OK(Flush(static_cast<int>(cf)));
-      }
-    }
-  }
-  for (size_t cf = 0; cf != num_cfs; ++cf) {
-    ASSERT_OK(Flush(static_cast<int>(cf)));
-  }
-  Close();
-  options.best_efforts_recovery = true;
-  ReopenWithColumnFamilies({kDefaultColumnFamilyName, "pikachu", "eevee"},
-                           options);
-  num_cfs = handles_.size();
-  ASSERT_EQ(3, num_cfs);
-  for (size_t cf = 0; cf != num_cfs; ++cf) {
-    for (int i = 0; i != 10000; ++i) {
-      std::string key_str = Key(static_cast<int>(i));
-      std::string expected_value_str =
-          std::to_string(cf) + "_" + std::to_string(i);
-      ASSERT_EQ(expected_value_str, Get(static_cast<int>(cf), key_str));
-    }
-  }
-}
-
-#ifndef ROCKSDB_LITE
-namespace {
-class TableFileListener : public EventListener {
- public:
-  void OnTableFileCreated(const TableFileCreationInfo& info) override {
-    InstrumentedMutexLock lock(&mutex_);
-    cf_to_paths_[info.cf_name].push_back(info.file_path);
-  }
-  std::vector<std::string>& GetFiles(const std::string& cf_name) {
-    InstrumentedMutexLock lock(&mutex_);
-    return cf_to_paths_[cf_name];
-  }
-
- private:
-  InstrumentedMutex mutex_;
-  std::unordered_map<std::string, std::vector<std::string>> cf_to_paths_;
-};
-}  // namespace
-
-TEST_F(DBBasicTest, RecoverWithMissingFiles) {
-  Options options = CurrentOptions();
-  DestroyAndReopen(options);
-  TableFileListener* listener = new TableFileListener();
-  // Disable auto compaction to simplify SST file name tracking.
-  options.disable_auto_compactions = true;
-  options.listeners.emplace_back(listener);
-  CreateAndReopenWithCF({"pikachu", "eevee"}, options);
-  std::vector<std::string> all_cf_names = {kDefaultColumnFamilyName, "pikachu",
-                                           "eevee"};
-  size_t num_cfs = handles_.size();
-  ASSERT_EQ(3, num_cfs);
-  for (size_t cf = 0; cf != num_cfs; ++cf) {
-    ASSERT_OK(Put(static_cast<int>(cf), "a", "0_value"));
-    ASSERT_OK(Flush(static_cast<int>(cf)));
-    ASSERT_OK(Put(static_cast<int>(cf), "b", "0_value"));
-    ASSERT_OK(Flush(static_cast<int>(cf)));
-    ASSERT_OK(Put(static_cast<int>(cf), "c", "0_value"));
-    ASSERT_OK(Flush(static_cast<int>(cf)));
-  }
-
-  // Delete files
-  for (size_t i = 0; i < all_cf_names.size(); ++i) {
-    std::vector<std::string>& files = listener->GetFiles(all_cf_names[i]);
-    ASSERT_EQ(3, files.size());
-    for (int j = static_cast<int>(files.size() - 1); j >= static_cast<int>(i);
-         --j) {
-      ASSERT_OK(env_->DeleteFile(files[j]));
-    }
-  }
-  options.best_efforts_recovery = true;
-  ReopenWithColumnFamilies(all_cf_names, options);
-  // Verify data
-  ReadOptions read_opts;
-  read_opts.total_order_seek = true;
-  {
-    std::unique_ptr<Iterator> iter(db_->NewIterator(read_opts, handles_[0]));
-    iter->SeekToFirst();
-    ASSERT_FALSE(iter->Valid());
-    iter.reset(db_->NewIterator(read_opts, handles_[1]));
-    iter->SeekToFirst();
-    ASSERT_TRUE(iter->Valid());
-    ASSERT_EQ("a", iter->key());
-    iter->Next();
-    ASSERT_FALSE(iter->Valid());
-    iter.reset(db_->NewIterator(read_opts, handles_[2]));
-    iter->SeekToFirst();
-    ASSERT_TRUE(iter->Valid());
-    ASSERT_EQ("a", iter->key());
-    iter->Next();
-    ASSERT_TRUE(iter->Valid());
-    ASSERT_EQ("b", iter->key());
-    iter->Next();
-    ASSERT_FALSE(iter->Valid());
-  }
-}
-
-TEST_F(DBBasicTest, SkipWALIfMissingTableFiles) {
-  Options options = CurrentOptions();
-  DestroyAndReopen(options);
-  TableFileListener* listener = new TableFileListener();
-  options.listeners.emplace_back(listener);
-  CreateAndReopenWithCF({"pikachu"}, options);
-  std::vector<std::string> kAllCfNames = {kDefaultColumnFamilyName, "pikachu"};
-  size_t num_cfs = handles_.size();
-  ASSERT_EQ(2, num_cfs);
-  for (int cf = 0; cf < static_cast<int>(kAllCfNames.size()); ++cf) {
-    ASSERT_OK(Put(cf, "a", "0_value"));
-    ASSERT_OK(Flush(cf));
-    ASSERT_OK(Put(cf, "b", "0_value"));
-  }
-  // Delete files
-  for (size_t i = 0; i < kAllCfNames.size(); ++i) {
-    std::vector<std::string>& files = listener->GetFiles(kAllCfNames[i]);
-    ASSERT_EQ(1, files.size());
-    for (int j = static_cast<int>(files.size() - 1); j >= static_cast<int>(i);
-         --j) {
-      ASSERT_OK(env_->DeleteFile(files[j]));
-    }
-  }
-  options.best_efforts_recovery = true;
-  ReopenWithColumnFamilies(kAllCfNames, options);
-  // Verify WAL is not applied
-  ReadOptions read_opts;
-  read_opts.total_order_seek = true;
-  std::unique_ptr<Iterator> iter(db_->NewIterator(read_opts, handles_[0]));
-  iter->SeekToFirst();
-  ASSERT_FALSE(iter->Valid());
-  iter.reset(db_->NewIterator(read_opts, handles_[1]));
-  iter->SeekToFirst();
-  ASSERT_TRUE(iter->Valid());
-  ASSERT_EQ("a", iter->key());
-  iter->Next();
-  ASSERT_FALSE(iter->Valid());
-}
-#endif  // !ROCKSDB_LITE
-
 class DBBasicTestWithParallelIO
     : public DBTestBase,
       public testing::WithParamInterface<std::tuple<bool, bool, bool, bool>> {
@@ -1921,17 +1765,10 @@ class DBBasicTestWithParallelIO
       compression_types = GetSupportedCompressions();
       // Not every platform may have compression libraries available, so
       // dynamically pick based on what's available
-      CompressionType tmp_type = kNoCompression;
-      for (auto c_type : compression_types) {
-        if (c_type != kNoCompression) {
-          tmp_type = c_type;
-          break;
-        }
-      }
-      if (tmp_type != kNoCompression) {
-        options.compression = tmp_type;
-      } else {
+      if (compression_types.size() == 0) {
         compression_enabled_ = false;
+      } else {
+        options.compression = compression_types[0];
       }
     }
 #else
@@ -1939,7 +1776,7 @@ class DBBasicTestWithParallelIO
     if (!Snappy_Supported()) {
       compression_enabled_ = false;
     }
-#endif  // ROCKSDB_LITE
+#endif //ROCKSDB_LITE
 
     table_options.block_cache = uncompressed_cache_;
     if (table_options.block_cache == nullptr) {
@@ -2141,6 +1978,8 @@ class DBBasicTestWithParallelIO
   bool fill_cache_;
 };
 
+// TODO: fails on CircleCI's Windows env
+#ifndef OS_WIN
 TEST_P(DBBasicTestWithParallelIO, MultiGet) {
   std::vector<std::string> key_data(10);
   std::vector<Slice> keys;
@@ -2263,6 +2102,7 @@ TEST_P(DBBasicTestWithParallelIO, MultiGet) {
     }
   }
 }
+#endif // OS_WIN
 
 TEST_P(DBBasicTestWithParallelIO, MultiGetWithChecksumMismatch) {
   std::vector<std::string> key_data(10);
@@ -2276,13 +2116,13 @@ TEST_P(DBBasicTestWithParallelIO, MultiGetWithChecksumMismatch) {
   ro.fill_cache = fill_cache();
 
   SyncPoint::GetInstance()->SetCallBack(
-      "RetrieveMultipleBlocks:VerifyChecksum", [&](void* status) {
-        Status* s = static_cast<Status*>(status);
-        read_count++;
-        if (read_count == 2) {
-          *s = Status::Corruption();
-        }
-      });
+      "RetrieveMultipleBlocks:VerifyChecksum", [&](void *status) {
+      Status* s = static_cast<Status*>(status);
+      read_count++;
+      if (read_count == 2) {
+        *s = Status::Corruption();
+      }
+    });
   SyncPoint::GetInstance()->EnableProcessing();
 
   // Warm up the cache first
@@ -2295,7 +2135,7 @@ TEST_P(DBBasicTestWithParallelIO, MultiGetWithChecksumMismatch) {
   dbfull()->MultiGet(ro, dbfull()->DefaultColumnFamily(), keys.size(),
                      keys.data(), values.data(), statuses.data(), true);
   ASSERT_TRUE(CheckValue(0, values[0].ToString()));
-  // ASSERT_TRUE(CheckValue(50, values[1].ToString()));
+  //ASSERT_TRUE(CheckValue(50, values[1].ToString()));
   ASSERT_EQ(statuses[0], Status::OK());
   ASSERT_EQ(statuses[1], Status::Corruption());
 
@@ -2313,10 +2153,10 @@ TEST_P(DBBasicTestWithParallelIO, MultiGetWithMissingFile) {
   ro.fill_cache = fill_cache();
 
   SyncPoint::GetInstance()->SetCallBack(
-      "TableCache::MultiGet:FindTable", [&](void* status) {
-        Status* s = static_cast<Status*>(status);
-        *s = Status::IOError();
-      });
+      "TableCache::MultiGet:FindTable", [&](void *status) {
+      Status* s = static_cast<Status*>(status);
+      *s = Status::IOError();
+    });
   // DB open will create table readers unless we reduce the table cache
   // capacity.
   // SanitizeOptions will set max_open_files to minimum of 20. Table cache
@@ -2325,10 +2165,10 @@ TEST_P(DBBasicTestWithParallelIO, MultiGetWithMissingFile) {
   // prevent file open during DB open and force the file to be opened
   // during MultiGet
   SyncPoint::GetInstance()->SetCallBack(
-      "SanitizeOptions::AfterChangeMaxOpenFiles", [&](void* arg) {
-        int* max_open_files = (int*)arg;
-        *max_open_files = 11;
-      });
+      "SanitizeOptions::AfterChangeMaxOpenFiles", [&](void *arg) {
+      int* max_open_files = (int*)arg;
+      *max_open_files = 11;
+    });
   SyncPoint::GetInstance()->EnableProcessing();
 
   Reopen(CurrentOptions());
@@ -2348,15 +2188,15 @@ TEST_P(DBBasicTestWithParallelIO, MultiGetWithMissingFile) {
   SyncPoint::GetInstance()->DisableProcessing();
 }
 
-INSTANTIATE_TEST_CASE_P(ParallelIO, DBBasicTestWithParallelIO,
-                        // Params are as follows -
-                        // Param 0 - Compressed cache enabled
-                        // Param 1 - Uncompressed cache enabled
-                        // Param 2 - Data compression enabled
-                        // Param 3 - ReadOptions::fill_cache
-                        ::testing::Combine(::testing::Bool(), ::testing::Bool(),
-                                           ::testing::Bool(),
-                                           ::testing::Bool()));
+INSTANTIATE_TEST_CASE_P(
+    ParallelIO, DBBasicTestWithParallelIO,
+    // Params are as follows -
+    // Param 0 - Compressed cache enabled
+    // Param 1 - Uncompressed cache enabled
+    // Param 2 - Data compression enabled
+    // Param 3 - ReadOptions::fill_cache
+    ::testing::Combine(::testing::Bool(), ::testing::Bool(),
+                       ::testing::Bool(), ::testing::Bool()));
 
 
 }  // namespace ROCKSDB_NAMESPACE

db/db_compaction_test.cc

@@ -593,72 +593,6 @@ TEST_P(DBCompactionTestWithParam, CompactionDeletionTriggerReopen) {
   }
 }
 
-TEST_F(DBCompactionTest, CompactRangeBottomPri) {
-  ASSERT_OK(Put(Key(50), ""));
-  ASSERT_OK(Flush());
-  ASSERT_OK(Put(Key(100), ""));
-  ASSERT_OK(Flush());
-  ASSERT_OK(Put(Key(200), ""));
-  ASSERT_OK(Flush());
-
-  {
-    CompactRangeOptions cro;
-    cro.change_level = true;
-    cro.target_level = 2;
-    dbfull()->CompactRange(cro, nullptr, nullptr);
-  }
-  ASSERT_EQ("0,0,3", FilesPerLevel(0));
-
-  ASSERT_OK(Put(Key(1), ""));
-  ASSERT_OK(Put(Key(199), ""));
-  ASSERT_OK(Flush());
-  ASSERT_OK(Put(Key(2), ""));
-  ASSERT_OK(Put(Key(199), ""));
-  ASSERT_OK(Flush());
-  ASSERT_EQ("2,0,3", FilesPerLevel(0));
-
-  // Now we have 2 L0 files, and 3 L2 files, and a manual compaction will
-  // be triggered.
-  // Two compaction jobs will run. One compacts 2 L0 files in Low Pri Pool
-  // and one compact to L2 in bottom pri pool.
-  int low_pri_count = 0;
-  int bottom_pri_count = 0;
-  SyncPoint::GetInstance()->SetCallBack(
-      "ThreadPoolImpl::Impl::BGThread:BeforeRun", [&](void* arg) {
-        Env::Priority* pri = reinterpret_cast<Env::Priority*>(arg);
-        // First time is low pri pool in the test case.
-        if (low_pri_count == 0 && bottom_pri_count == 0) {
-          ASSERT_EQ(Env::Priority::LOW, *pri);
-        }
-        if (*pri == Env::Priority::LOW) {
-          low_pri_count++;
-        } else {
-          bottom_pri_count++;
-        }
-      });
-  SyncPoint::GetInstance()->EnableProcessing();
-  env_->SetBackgroundThreads(1, Env::Priority::BOTTOM);
-  dbfull()->CompactRange(CompactRangeOptions(), nullptr, nullptr);
-  ASSERT_EQ(1, low_pri_count);
-  ASSERT_EQ(1, bottom_pri_count);
-  ASSERT_EQ("0,0,2", FilesPerLevel(0));
-
-  // Recompact bottom most level uses bottom pool
-  CompactRangeOptions cro;
-  cro.bottommost_level_compaction = BottommostLevelCompaction::kForce;
-  dbfull()->CompactRange(cro, nullptr, nullptr);
-  ASSERT_EQ(1, low_pri_count);
-  ASSERT_EQ(2, bottom_pri_count);
-
-  env_->SetBackgroundThreads(0, Env::Priority::BOTTOM);
-  dbfull()->CompactRange(cro, nullptr, nullptr);
-  // Low pri pool is used if bottom pool has size 0.
-  ASSERT_EQ(2, low_pri_count);
-  ASSERT_EQ(2, bottom_pri_count);
-
-  SyncPoint::GetInstance()->DisableProcessing();
-}
-
 TEST_F(DBCompactionTest, DisableStatsUpdateReopen) {
   uint64_t db_size[3];
   for (int test = 0; test < 2; ++test) {

db/db_impl/db_impl.cc

@@ -150,7 +150,7 @@ DBImpl::DBImpl(const DBOptions& options, const std::string& dbname,
       own_info_log_(options.info_log == nullptr),
       initial_db_options_(SanitizeOptions(dbname, options)),
       env_(initial_db_options_.env),
-      fs_(initial_db_options_.env->GetFileSystem()),
+      fs_(initial_db_options_.file_system),
       immutable_db_options_(initial_db_options_),
       mutable_db_options_(initial_db_options_),
       stats_(immutable_db_options_.statistics.get()),
@@ -1687,20 +1687,12 @@ std::vector<Status> DBImpl::MultiGet(
     const ReadOptions& read_options,
     const std::vector<ColumnFamilyHandle*>& column_family,
     const std::vector<Slice>& keys, std::vector<std::string>* values) {
-  return MultiGet(read_options, column_family, keys, values,
-                  /*timestamps*/ nullptr);
-}
-
-std::vector<Status> DBImpl::MultiGet(
-    const ReadOptions& read_options,
-    const std::vector<ColumnFamilyHandle*>& column_family,
-    const std::vector<Slice>& keys, std::vector<std::string>* values,
-    std::vector<std::string>* timestamps) {
   PERF_CPU_TIMER_GUARD(get_cpu_nanos, env_);
   StopWatch sw(env_, stats_, DB_MULTIGET);
   PERF_TIMER_GUARD(get_snapshot_time);
 
   SequenceNumber consistent_seqnum;
+  ;
 
   std::unordered_map<uint32_t, MultiGetColumnFamilyData> multiget_cf_data(
       column_family.size());
@@ -1731,9 +1723,6 @@ std::vector<Status> DBImpl::MultiGet(
   size_t num_keys = keys.size();
   std::vector<Status> stat_list(num_keys);
   values->resize(num_keys);
-  if (timestamps) {
-    timestamps->resize(num_keys);
-  }
 
   // Keep track of bytes that we read for statistics-recording later
   uint64_t bytes_read = 0;
@@ -1748,9 +1737,8 @@ std::vector<Status> DBImpl::MultiGet(
     merge_context.Clear();
     Status& s = stat_list[i];
     std::string* value = &(*values)[i];
-    std::string* timestamp = timestamps ? &(*timestamps)[i] : nullptr;
 
-    LookupKey lkey(keys[i], consistent_seqnum, read_options.timestamp);
+    LookupKey lkey(keys[i], consistent_seqnum);
     auto cfh = reinterpret_cast<ColumnFamilyHandleImpl*>(column_family[i]);
     SequenceNumber max_covering_tombstone_seq = 0;
     auto mgd_iter = multiget_cf_data.find(cfh->cfd()->GetID());
@@ -1762,12 +1750,13 @@ std::vector<Status> DBImpl::MultiGet(
          has_unpersisted_data_.load(std::memory_order_relaxed));
     bool done = false;
     if (!skip_memtable) {
-      if (super_version->mem->Get(lkey, value, timestamp, &s, &merge_context,
-                                  &max_covering_tombstone_seq, read_options)) {
+      if (super_version->mem->Get(lkey, value, /*timestamp=*/nullptr, &s,
+                                  &merge_context, &max_covering_tombstone_seq,
+                                  read_options)) {
         done = true;
         RecordTick(stats_, MEMTABLE_HIT);
       } else if (super_version->imm->Get(
-                     lkey, value, timestamp, &s, &merge_context,
+                     lkey, value, nullptr, &s, &merge_context,
                      &max_covering_tombstone_seq, read_options)) {
         done = true;
         RecordTick(stats_, MEMTABLE_HIT);
@@ -1776,8 +1765,8 @@ std::vector<Status> DBImpl::MultiGet(
     if (!done) {
       PinnableSlice pinnable_val;
       PERF_TIMER_GUARD(get_from_output_files_time);
-      super_version->current->Get(read_options, lkey, &pinnable_val, timestamp,
-                                  &s, &merge_context,
+      super_version->current->Get(read_options, lkey, &pinnable_val,
+                                  /*timestamp=*/nullptr, &s, &merge_context,
                                   &max_covering_tombstone_seq);
       value->assign(pinnable_val.data(), pinnable_val.size());
       RecordTick(stats_, MEMTABLE_MISS);
@@ -1940,14 +1929,6 @@ void DBImpl::MultiGet(const ReadOptions& read_options, const size_t num_keys,
                       ColumnFamilyHandle** column_families, const Slice* keys,
                       PinnableSlice* values, Status* statuses,
                       const bool sorted_input) {
-  return MultiGet(read_options, num_keys, column_families, keys, values,
-                  /*timestamps*/ nullptr, statuses, sorted_input);
-}
-
-void DBImpl::MultiGet(const ReadOptions& read_options, const size_t num_keys,
-                      ColumnFamilyHandle** column_families, const Slice* keys,
-                      PinnableSlice* values, std::string* timestamps,
-                      Status* statuses, const bool sorted_input) {
   if (num_keys == 0) {
     return;
   }
@@ -1956,7 +1937,7 @@ void DBImpl::MultiGet(const ReadOptions& read_options, const size_t num_keys,
   sorted_keys.resize(num_keys);
   for (size_t i = 0; i < num_keys; ++i) {
     key_context.emplace_back(column_families[i], keys[i], &values[i],
-                             &timestamps[i], &statuses[i]);
+                             &statuses[i]);
   }
   for (size_t i = 0; i < num_keys; ++i) {
     sorted_keys[i] = &key_context[i];
@@ -2076,22 +2057,11 @@ void DBImpl::MultiGet(const ReadOptions& read_options,
                       ColumnFamilyHandle* column_family, const size_t num_keys,
                       const Slice* keys, PinnableSlice* values,
                       Status* statuses, const bool sorted_input) {
-  return MultiGet(read_options, column_family, num_keys, keys, values,
-                  /*timestamp=*/nullptr, statuses, sorted_input);
-}
-
-void DBImpl::MultiGet(const ReadOptions& read_options,
-                      ColumnFamilyHandle* column_family, const size_t num_keys,
-                      const Slice* keys, PinnableSlice* values,
-                      std::string* timestamps, Status* statuses,
-                      const bool sorted_input) {
   autovector<KeyContext, MultiGetContext::MAX_BATCH_SIZE> key_context;
   autovector<KeyContext*, MultiGetContext::MAX_BATCH_SIZE> sorted_keys;
   sorted_keys.resize(num_keys);
   for (size_t i = 0; i < num_keys; ++i) {
-    key_context.emplace_back(column_family, keys[i], &values[i],
-                             timestamps ? &timestamps[i] : nullptr,
-                             &statuses[i]);
+    key_context.emplace_back(column_family, keys[i], &values[i], &statuses[i]);
   }
   for (size_t i = 0; i < num_keys; ++i) {
     sorted_keys[i] = &key_context[i];
@@ -3508,8 +3478,12 @@ Status DBImpl::Close() {
 Status DB::ListColumnFamilies(const DBOptions& db_options,
                               const std::string& name,
                               std::vector<std::string>* column_families) {
-  const std::shared_ptr<FileSystem>& fs = db_options.env->GetFileSystem();
-  return VersionSet::ListColumnFamilies(column_families, name, fs.get());
+  FileSystem* fs = db_options.file_system.get();
+  LegacyFileSystemWrapper legacy_fs(db_options.env);
+  if (!fs) {
+    fs = &legacy_fs;
+  }
+  return VersionSet::ListColumnFamilies(column_families, name, fs);
 }
 
 Snapshot::~Snapshot() {}

db/db_impl/db_impl.h

@@ -188,11 +188,6 @@ class DBImpl : public DB {
       const std::vector<ColumnFamilyHandle*>& column_family,
       const std::vector<Slice>& keys,
       std::vector<std::string>* values) override;
-  virtual std::vector<Status> MultiGet(
-      const ReadOptions& options,
-      const std::vector<ColumnFamilyHandle*>& column_family,
-      const std::vector<Slice>& keys, std::vector<std::string>* values,
-      std::vector<std::string>* timestamps) override;
 
   // This MultiGet is a batched version, which may be faster than calling Get
   // multiple times, especially if the keys have some spatial locality that
@@ -206,22 +201,11 @@ class DBImpl : public DB {
                         const size_t num_keys, const Slice* keys,
                         PinnableSlice* values, Status* statuses,
                         const bool sorted_input = false) override;
-  virtual void MultiGet(const ReadOptions& options,
-                        ColumnFamilyHandle* column_family,
-                        const size_t num_keys, const Slice* keys,
-                        PinnableSlice* values, std::string* timestamps,
-                        Status* statuses,
-                        const bool sorted_input = false) override;
 
   virtual void MultiGet(const ReadOptions& options, const size_t num_keys,
                         ColumnFamilyHandle** column_families, const Slice* keys,
                         PinnableSlice* values, Status* statuses,
                         const bool sorted_input = false) override;
-  virtual void MultiGet(const ReadOptions& options, const size_t num_keys,
-                        ColumnFamilyHandle** column_families, const Slice* keys,
-                        PinnableSlice* values, std::string* timestamps,
-                        Status* statuses,
-                        const bool sorted_input = false) override;
 
   virtual void MultiGetWithCallback(
       const ReadOptions& options, ColumnFamilyHandle* column_family,
@@ -1140,17 +1124,6 @@ class DBImpl : public DB {
 
   virtual bool OwnTablesAndLogs() const { return true; }
 
-  // REQUIRES: db mutex held when calling this function, but the db mutex can
-  // be released and re-acquired. Db mutex will be held when the function
-  // returns.
-  // Currently, this function should be called only in best-efforts recovery
-  // mode.
-  // After best-efforts recovery, there may be SST files in db/cf paths that are
-  // not referenced in the MANIFEST. We delete these SST files. In the
-  // meantime, we find out the largest file number present in the paths, and
-  // bump up the version set's next_file_number_ to be 1 + largest_file_number.
-  Status CleanupFilesAfterRecovery();
-
  private:
   friend class DB;
   friend class ErrorHandler;

db/db_impl/db_impl_compaction_flush.cc

@@ -1510,12 +1510,7 @@ Status DBImpl::RunManualCompaction(
       }
       manual.incomplete = false;
       bg_compaction_scheduled_++;
-      Env::Priority thread_pool_pri = Env::Priority::LOW;
-      if (compaction->bottommost_level() &&
-          env_->GetBackgroundThreads(Env::Priority::BOTTOM) > 0) {
-        thread_pool_pri = Env::Priority::BOTTOM;
-      }
-      env_->Schedule(&DBImpl::BGWorkCompaction, ca, thread_pool_pri, this,
+      env_->Schedule(&DBImpl::BGWorkCompaction, ca, Env::Priority::LOW, this,
                      &DBImpl::UnscheduleCompactionCallback);
       scheduled = true;
     }

db/db_impl/db_impl_files.cc

@@ -14,7 +14,6 @@
 #include "db/event_helpers.h"
 #include "db/memtable_list.h"
 #include "file/file_util.h"
-#include "file/filename.h"
 #include "file/sst_file_manager_impl.h"
 #include "util/autovector.h"
 
@@ -665,55 +664,4 @@ uint64_t PrecomputeMinLogNumberToKeep(
   return min_log_number_to_keep;
 }
 
-Status DBImpl::CleanupFilesAfterRecovery() {
-  mutex_.AssertHeld();
-  std::vector<std::string> paths;
-  paths.push_back(dbname_);
-  for (const auto& db_path : immutable_db_options_.db_paths) {
-    paths.push_back(db_path.path);
-  }
-  for (const auto* cfd : *versions_->GetColumnFamilySet()) {
-    for (const auto& cf_path : cfd->ioptions()->cf_paths) {
-      paths.push_back(cf_path.path);
-    }
-  }
-  // Dedup paths
-  std::sort(paths.begin(), paths.end());
-  paths.erase(std::unique(paths.begin(), paths.end()), paths.end());
-
-  uint64_t next_file_number = versions_->current_next_file_number();
-  uint64_t largest_file_number = next_file_number;
-  std::set<std::string> files_to_delete;
-  for (const auto& path : paths) {
-    std::vector<std::string> files;
-    env_->GetChildren(path, &files);
-    for (const auto& fname : files) {
-      uint64_t number = 0;
-      FileType type;
-      if (!ParseFileName(fname, &number, &type)) {
-        continue;
-      }
-      const std::string normalized_fpath = NormalizePath(path + fname);
-      largest_file_number = std::max(largest_file_number, number);
-      if (type == kTableFile && number >= next_file_number &&
-          files_to_delete.find(normalized_fpath) == files_to_delete.end()) {
-        files_to_delete.insert(normalized_fpath);
-      }
-    }
-  }
-  if (largest_file_number > next_file_number) {
-    versions_->next_file_number_.store(largest_file_number + 1);
-  }
-  mutex_.Unlock();
-  Status s;
-  for (const auto& fname : files_to_delete) {
-    s = env_->DeleteFile(fname);
-    if (!s.ok()) {
-      break;
-    }
-  }
-  mutex_.Lock();
-  return s;
-}
-
 }  // namespace ROCKSDB_NAMESPACE

db/db_impl/db_impl_open.cc

@@ -35,8 +35,16 @@ Options SanitizeOptions(const std::string& dbname, const Options& src) {
 DBOptions SanitizeOptions(const std::string& dbname, const DBOptions& src) {
   DBOptions result(src);
 
-  if (result.env == nullptr) {
-    result.env = Env::Default();
+  if (result.file_system == nullptr) {
+    if (result.env == Env::Default()) {
+      result.file_system = FileSystem::Default();
+    } else {
+      result.file_system.reset(new LegacyFileSystemWrapper(result.env));
+    }
+  } else {
+    if (result.env == nullptr) {
+      result.env = Env::Default();
+    }
   }
 
   // result.max_open_files means an "infinite" open files.
@@ -244,12 +252,6 @@ Status DBImpl::ValidateOptions(const DBOptions& db_options) {
         "atomic_flush is incompatible with enable_pipelined_write");
   }
 
-  // TODO remove this restriction
-  if (db_options.atomic_flush && db_options.best_efforts_recovery) {
-    return Status::InvalidArgument(
-        "atomic_flush is currently incompatible with best-efforts recovery");
-  }
-
   return Status::OK();
 }
 
@@ -417,17 +419,7 @@ Status DBImpl::Recover(
     }
   }
   assert(db_id_.empty());
-  Status s;
-  bool missing_table_file = false;
-  if (!immutable_db_options_.best_efforts_recovery) {
-    s = versions_->Recover(column_families, read_only, &db_id_);
-  } else {
-    s = versions_->TryRecover(column_families, read_only, &db_id_,
-                              &missing_table_file);
-    if (s.ok()) {
-      s = CleanupFilesAfterRecovery();
-    }
-  }
+  Status s = versions_->Recover(column_families, read_only, &db_id_);
   if (!s.ok()) {
     return s;
   }
@@ -507,9 +499,7 @@ Status DBImpl::Recover(
     // attention to it in case we are recovering a database
     // produced by an older version of rocksdb.
     std::vector<std::string> filenames;
-    if (!immutable_db_options_.best_efforts_recovery) {
-      s = env_->GetChildren(immutable_db_options_.wal_dir, &filenames);
-    }
+    s = env_->GetChildren(immutable_db_options_.wal_dir, &filenames);
     if (s.IsNotFound()) {
       return Status::InvalidArgument("wal_dir not found",
                                      immutable_db_options_.wal_dir);

db/db_logical_block_size_cache_test.cc

@@ -37,15 +37,17 @@ class DBLogicalBlockSizeCacheTest : public testing::Test {
         data_path_1_(dbname_ + "/data_path_1"),
         cf_path_0_(dbname_ + "/cf_path_0"),
         cf_path_1_(dbname_ + "/cf_path_1") {
-    auto get_fd_block_size = [&](int fd) { return fd; };
+    auto get_fd_block_size = [&](int fd) {
+      return fd;
+    };
     auto get_dir_block_size = [&](const std::string& /*dir*/, size_t* size) {
       *size = 1024;
       return Status::OK();
     };
-    cache_.reset(
-        new LogicalBlockSizeCache(get_fd_block_size, get_dir_block_size));
-    env_.reset(
-        new EnvWithCustomLogicalBlockSizeCache(Env::Default(), cache_.get()));
+    cache_.reset(new LogicalBlockSizeCache(
+        get_fd_block_size, get_dir_block_size));
+    env_.reset(new EnvWithCustomLogicalBlockSizeCache(
+        Env::Default(), cache_.get()));
   }
 
  protected:
@@ -505,7 +507,7 @@ TEST_F(DBLogicalBlockSizeCacheTest, MultiDBWithSamePaths) {
 }
 
 }  // namespace ROCKSDB_NAMESPACE
-#endif  // OS_LINUX
+#endif // OS_LINUX
 
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);

db/db_with_timestamp_basic_test.cc

@@ -580,7 +580,7 @@ TEST_P(DBBasicTestWithTimestampCompressionSettings, PutAndGetWithCompaction) {
   std::vector<std::string> write_ts_list;
   std::vector<std::string> read_ts_list;
 
-  const auto& verify_records_func = [&](size_t i, size_t begin, size_t end,
+  const auto& verify_record_func = [&](size_t i, size_t k,
                                        ColumnFamilyHandle* cfh) {
     std::string value;
     std::string timestamp;
@@ -591,11 +591,9 @@ TEST_P(DBBasicTestWithTimestampCompressionSettings, PutAndGetWithCompaction) {
     std::string expected_timestamp =
         std::string(write_ts_list[i].data(), write_ts_list[i].size());
 
-    for (size_t j = begin; j <= end; ++j) {
-      ASSERT_OK(db_->Get(ropts, cfh, Key1(j), &value, &timestamp));
-      ASSERT_EQ("value_" + std::to_string(j) + "_" + std::to_string(i), value);
-      ASSERT_EQ(expected_timestamp, timestamp);
-    }
+    ASSERT_OK(db_->Get(ropts, cfh, Key1(k), &value, &timestamp));
+    ASSERT_EQ("value_" + std::to_string(k) + "_" + std::to_string(i), value);
+    ASSERT_EQ(expected_timestamp, timestamp);
   };
 
   for (size_t i = 0; i != kNumTimestamps; ++i) {
@@ -611,7 +609,9 @@ TEST_P(DBBasicTestWithTimestampCompressionSettings, PutAndGetWithCompaction) {
                       "value_" + std::to_string(j) + "_" + std::to_string(i),
                       wopts));
         if (j == kSplitPosBase + i || j == kNumKeysPerTimestamp - 1) {
-          verify_records_func(i, memtable_get_start, j, handles_[cf]);
+          for (size_t k = memtable_get_start; k <= j; ++k) {
+            verify_record_func(i, k, handles_[cf]);
+          }
           memtable_get_start = j + 1;
 
           // flush all keys with the same timestamp to two sst files, split at
@@ -641,104 +641,15 @@ TEST_P(DBBasicTestWithTimestampCompressionSettings, PutAndGetWithCompaction) {
                                      write_ts_list[i].size());
       for (int cf = 0; cf != static_cast<int>(num_cfs); ++cf) {
         ColumnFamilyHandle* cfh = handles_[cf];
-        verify_records_func(i, 0, kNumKeysPerTimestamp - 1, cfh);
+        for (size_t j = 0; j != kNumKeysPerTimestamp; ++j) {
+          verify_record_func(i, j, cfh);
+        }
       }
     }
   };
   verify_db_func();
   Close();
 }
-
-TEST_F(DBBasicTestWithTimestamp, BatchWriteAndMultiGet) {
-  const int kNumKeysPerFile = 8192;
-  const size_t kNumTimestamps = 2;
-  const size_t kNumKeysPerTimestamp = (kNumKeysPerFile - 1) / kNumTimestamps;
-  Options options = CurrentOptions();
-  options.create_if_missing = true;
-  options.env = env_;
-  options.memtable_factory.reset(new SpecialSkipListFactory(kNumKeysPerFile));
-
-  size_t ts_sz = Timestamp(0, 0).size();
-  TestComparator test_cmp(ts_sz);
-  options.comparator = &test_cmp;
-  BlockBasedTableOptions bbto;
-  bbto.filter_policy.reset(NewBloomFilterPolicy(
-      10 /*bits_per_key*/, false /*use_block_based_builder*/));
-  bbto.whole_key_filtering = true;
-  options.table_factory.reset(NewBlockBasedTableFactory(bbto));
-  DestroyAndReopen(options);
-  CreateAndReopenWithCF({"pikachu"}, options);
-  size_t num_cfs = handles_.size();
-  ASSERT_EQ(2, num_cfs);
-  std::vector<std::string> write_ts_list;
-  std::vector<std::string> read_ts_list;
-
-  const auto& verify_records_func = [&](size_t i, ColumnFamilyHandle* cfh) {
-    std::vector<Slice> keys;
-    std::vector<std::string> key_vals;
-    std::vector<std::string> values;
-    std::vector<std::string> timestamps;
-
-    for (size_t j = 0; j != kNumKeysPerTimestamp; ++j) {
-      key_vals.push_back(Key1(j));
-    }
-    for (size_t j = 0; j != kNumKeysPerTimestamp; ++j) {
-      keys.push_back(key_vals[j]);
-    }
-
-    ReadOptions ropts;
-    const Slice read_ts = read_ts_list[i];
-    ropts.timestamp = &read_ts;
-    std::string expected_timestamp(write_ts_list[i].data(),
-                                   write_ts_list[i].size());
-
-    std::vector<ColumnFamilyHandle*> cfhs(keys.size(), cfh);
-    std::vector<Status> statuses =
-        db_->MultiGet(ropts, cfhs, keys, &values, &timestamps);
-    for (size_t j = 0; j != kNumKeysPerTimestamp; ++j) {
-      ASSERT_OK(statuses[j]);
-      ASSERT_EQ("value_" + std::to_string(j) + "_" + std::to_string(i),
-                values[j]);
-      ASSERT_EQ(expected_timestamp, timestamps[j]);
-    }
-  };
-
-  for (size_t i = 0; i != kNumTimestamps; ++i) {
-    write_ts_list.push_back(Timestamp(i * 2, 0));
-    read_ts_list.push_back(Timestamp(1 + i * 2, 0));
-    const Slice& write_ts = write_ts_list.back();
-    for (int cf = 0; cf != static_cast<int>(num_cfs); ++cf) {
-      WriteOptions wopts;
-      WriteBatch batch(0, 0, ts_sz);
-      for (size_t j = 0; j != kNumKeysPerTimestamp; ++j) {
-        ASSERT_OK(
-            batch.Put(handles_[cf], Key1(j),
-                      "value_" + std::to_string(j) + "_" + std::to_string(i)));
-      }
-      batch.AssignTimestamp(write_ts);
-      ASSERT_OK(db_->Write(wopts, &batch));
-
-      verify_records_func(i, handles_[cf]);
-
-      ASSERT_OK(Flush(cf));
-    }
-  }
-
-  const auto& verify_db_func = [&]() {
-    for (size_t i = 0; i != kNumTimestamps; ++i) {
-      ReadOptions ropts;
-      const Slice read_ts = read_ts_list[i];
-      ropts.timestamp = &read_ts;
-      for (int cf = 0; cf != static_cast<int>(num_cfs); ++cf) {
-        ColumnFamilyHandle* cfh = handles_[cf];
-        verify_records_func(i, cfh);
-      }
-    }
-  };
-  verify_db_func();
-  Close();
-}
-
 #endif  // !ROCKSDB_LITE
 
 INSTANTIATE_TEST_CASE_P(

db/error_handler_fs_test.cc

@@ -44,7 +44,7 @@ class DBErrorHandlingFSTest : public DBTestBase {
 class DBErrorHandlingFS : public FileSystemWrapper {
  public:
   DBErrorHandlingFS()
-      : FileSystemWrapper(FileSystem::Default()),
+      : FileSystemWrapper(FileSystem::Default().get()),
         trig_no_space(false),
         trig_io_error(false) {}
 
@@ -150,13 +150,12 @@ class ErrorHandlerFSListener : public EventListener {
 };
 
 TEST_F(DBErrorHandlingFSTest, FLushWriteError) {
-  std::shared_ptr<FaultInjectionTestFS> fault_fs(
-      new FaultInjectionTestFS(FileSystem::Default()));
-  std::unique_ptr<Env> fault_fs_env(NewCompositeEnv(fault_fs));
+  FaultInjectionTestFS* fault_fs =
+      new FaultInjectionTestFS(FileSystem::Default().get());
   std::shared_ptr<ErrorHandlerFSListener> listener(
       new ErrorHandlerFSListener());
   Options options = GetDefaultOptions();
-  options.env = fault_fs_env.get();
+  options.file_system.reset(fault_fs);
   options.create_if_missing = true;
   options.listeners.emplace_back(listener);
   Status s;
@@ -182,13 +181,12 @@ TEST_F(DBErrorHandlingFSTest, FLushWriteError) {
 }
 
 TEST_F(DBErrorHandlingFSTest, ManifestWriteError) {
-  std::shared_ptr<FaultInjectionTestFS> fault_fs(
-      new FaultInjectionTestFS(FileSystem::Default()));
-  std::unique_ptr<Env> fault_fs_env(NewCompositeEnv(fault_fs));
+  FaultInjectionTestFS* fault_fs =
+      new FaultInjectionTestFS(FileSystem::Default().get());
   std::shared_ptr<ErrorHandlerFSListener> listener(
       new ErrorHandlerFSListener());
   Options options = GetDefaultOptions();
-  options.env = fault_fs_env.get();
+  options.file_system.reset(fault_fs);
   options.create_if_missing = true;
   options.listeners.emplace_back(listener);
   Status s;
@@ -225,13 +223,12 @@ TEST_F(DBErrorHandlingFSTest, ManifestWriteError) {
 }
 
 TEST_F(DBErrorHandlingFSTest, DoubleManifestWriteError) {
-  std::shared_ptr<FaultInjectionTestFS> fault_fs(
-      new FaultInjectionTestFS(FileSystem::Default()));
-  std::unique_ptr<Env> fault_fs_env(NewCompositeEnv(fault_fs));
+  FaultInjectionTestFS* fault_fs =
+      new FaultInjectionTestFS(FileSystem::Default().get());
   std::shared_ptr<ErrorHandlerFSListener> listener(
       new ErrorHandlerFSListener());
   Options options = GetDefaultOptions();
-  options.env = fault_fs_env.get();
+  options.file_system.reset(fault_fs);
   options.create_if_missing = true;
   options.listeners.emplace_back(listener);
   Status s;
@@ -275,13 +272,12 @@ TEST_F(DBErrorHandlingFSTest, DoubleManifestWriteError) {
 }
 
 TEST_F(DBErrorHandlingFSTest, CompactionManifestWriteError) {
-  std::shared_ptr<FaultInjectionTestFS> fault_fs(
-      new FaultInjectionTestFS(FileSystem::Default()));
-  std::unique_ptr<Env> fault_fs_env(NewCompositeEnv(fault_fs));
+  FaultInjectionTestFS* fault_fs =
+      new FaultInjectionTestFS(FileSystem::Default().get());
   std::shared_ptr<ErrorHandlerFSListener> listener(
       new ErrorHandlerFSListener());
   Options options = GetDefaultOptions();
-  options.env = fault_fs_env.get();
+  options.file_system.reset(fault_fs);
   options.create_if_missing = true;
   options.level0_file_num_compaction_trigger = 2;
   options.listeners.emplace_back(listener);
@@ -348,13 +344,12 @@ TEST_F(DBErrorHandlingFSTest, CompactionManifestWriteError) {
 }
 
 TEST_F(DBErrorHandlingFSTest, CompactionWriteError) {
-  std::shared_ptr<FaultInjectionTestFS> fault_fs(
-      new FaultInjectionTestFS(FileSystem::Default()));
-  std::unique_ptr<Env> fault_fs_env(NewCompositeEnv(fault_fs));
+  FaultInjectionTestFS* fault_fs =
+      new FaultInjectionTestFS(FileSystem::Default().get());
   std::shared_ptr<ErrorHandlerFSListener> listener(
       new ErrorHandlerFSListener());
   Options options = GetDefaultOptions();
-  options.env = fault_fs_env.get();
+  options.file_system.reset(fault_fs);
   options.create_if_missing = true;
   options.level0_file_num_compaction_trigger = 2;
   options.listeners.emplace_back(listener);
@@ -392,11 +387,10 @@ TEST_F(DBErrorHandlingFSTest, CompactionWriteError) {
 }
 
 TEST_F(DBErrorHandlingFSTest, CorruptionError) {
-  std::shared_ptr<FaultInjectionTestFS> fault_fs(
-      new FaultInjectionTestFS(FileSystem::Default()));
-  std::unique_ptr<Env> fault_fs_env(NewCompositeEnv(fault_fs));
+  FaultInjectionTestFS* fault_fs =
+      new FaultInjectionTestFS(FileSystem::Default().get());
   Options options = GetDefaultOptions();
-  options.env = fault_fs_env.get();
+  options.file_system.reset(fault_fs);
   options.create_if_missing = true;
   options.level0_file_num_compaction_trigger = 2;
   Status s;
@@ -432,13 +426,12 @@ TEST_F(DBErrorHandlingFSTest, CorruptionError) {
 }
 
 TEST_F(DBErrorHandlingFSTest, AutoRecoverFlushError) {
-  std::shared_ptr<FaultInjectionTestFS> fault_fs(
-      new FaultInjectionTestFS(FileSystem::Default()));
-  std::unique_ptr<Env> fault_fs_env(NewCompositeEnv(fault_fs));
+  FaultInjectionTestFS* fault_fs =
+      new FaultInjectionTestFS(FileSystem::Default().get());
   std::shared_ptr<ErrorHandlerFSListener> listener(
       new ErrorHandlerFSListener());
   Options options = GetDefaultOptions();
-  options.env = fault_fs_env.get();
+  options.file_system.reset(fault_fs);
   options.create_if_missing = true;
   options.listeners.emplace_back(listener);
   Status s;
@@ -467,13 +460,12 @@ TEST_F(DBErrorHandlingFSTest, AutoRecoverFlushError) {
 }
 
 TEST_F(DBErrorHandlingFSTest, FailRecoverFlushError) {
-  std::shared_ptr<FaultInjectionTestFS> fault_fs(
-      new FaultInjectionTestFS(FileSystem::Default()));
-  std::unique_ptr<Env> fault_fs_env(NewCompositeEnv(fault_fs));
+  FaultInjectionTestFS* fault_fs =
+      new FaultInjectionTestFS(FileSystem::Default().get());
   std::shared_ptr<ErrorHandlerFSListener> listener(
       new ErrorHandlerFSListener());
   Options options = GetDefaultOptions();
-  options.env = fault_fs_env.get();
+  options.file_system.reset(fault_fs);
   options.create_if_missing = true;
   options.listeners.emplace_back(listener);
   Status s;
@@ -495,13 +487,12 @@ TEST_F(DBErrorHandlingFSTest, FailRecoverFlushError) {
 }
 
 TEST_F(DBErrorHandlingFSTest, WALWriteError) {
-  std::shared_ptr<FaultInjectionTestFS> fault_fs(
-      new FaultInjectionTestFS(FileSystem::Default()));
-  std::unique_ptr<Env> fault_fs_env(NewCompositeEnv(fault_fs));
+  FaultInjectionTestFS* fault_fs =
+      new FaultInjectionTestFS(FileSystem::Default().get());
   std::shared_ptr<ErrorHandlerFSListener> listener(
       new ErrorHandlerFSListener());
   Options options = GetDefaultOptions();
-  options.env = fault_fs_env.get();
+  options.file_system.reset(fault_fs);
   options.create_if_missing = true;
   options.writable_file_max_buffer_size = 32768;
   options.listeners.emplace_back(listener);
@@ -567,13 +558,12 @@ TEST_F(DBErrorHandlingFSTest, WALWriteError) {
 }
 
 TEST_F(DBErrorHandlingFSTest, MultiCFWALWriteError) {
-  std::shared_ptr<FaultInjectionTestFS> fault_fs(
-      new FaultInjectionTestFS(FileSystem::Default()));
-  std::unique_ptr<Env> fault_fs_env(NewCompositeEnv(fault_fs));
+  FaultInjectionTestFS* fault_fs =
+      new FaultInjectionTestFS(FileSystem::Default().get());
   std::shared_ptr<ErrorHandlerFSListener> listener(
       new ErrorHandlerFSListener());
   Options options = GetDefaultOptions();
-  options.env = fault_fs_env.get();
+  options.file_system.reset(fault_fs);
   options.create_if_missing = true;
   options.writable_file_max_buffer_size = 32768;
   options.listeners.emplace_back(listener);
@@ -653,7 +643,6 @@ TEST_F(DBErrorHandlingFSTest, MultiCFWALWriteError) {
 
 TEST_F(DBErrorHandlingFSTest, MultiDBCompactionError) {
   FaultInjectionTestEnv* def_env = new FaultInjectionTestEnv(Env::Default());
-  std::vector<std::unique_ptr<Env>> fault_envs;
   std::vector<FaultInjectionTestFS*> fault_fs;
   std::vector<Options> options;
   std::vector<std::shared_ptr<ErrorHandlerFSListener>> listener;
@@ -665,13 +654,12 @@ TEST_F(DBErrorHandlingFSTest, MultiDBCompactionError) {
   for (auto i = 0; i < kNumDbInstances; ++i) {
     listener.emplace_back(new ErrorHandlerFSListener());
     options.emplace_back(GetDefaultOptions());
-    fault_fs.emplace_back(new FaultInjectionTestFS(FileSystem::Default()));
-    std::shared_ptr<FileSystem> fs(fault_fs.back());
-    fault_envs.emplace_back(new CompositeEnvWrapper(def_env, fs));
-    options[i].env = fault_envs.back().get();
+    fault_fs.emplace_back(
+        new FaultInjectionTestFS(FileSystem::Default().get()));
     options[i].create_if_missing = true;
     options[i].level0_file_num_compaction_trigger = 2;
     options[i].writable_file_max_buffer_size = 32768;
+    options[i].file_system.reset(fault_fs[i]);
     options[i].listeners.emplace_back(listener[i]);
     options[i].sst_file_manager = sfm;
     DB* dbptr;
@@ -754,7 +742,6 @@ TEST_F(DBErrorHandlingFSTest, MultiDBCompactionError) {
 
 TEST_F(DBErrorHandlingFSTest, MultiDBVariousErrors) {
   FaultInjectionTestEnv* def_env = new FaultInjectionTestEnv(Env::Default());
-  std::vector<std::unique_ptr<Env>> fault_envs;
   std::vector<FaultInjectionTestFS*> fault_fs;
   std::vector<Options> options;
   std::vector<std::shared_ptr<ErrorHandlerFSListener>> listener;
@@ -766,13 +753,12 @@ TEST_F(DBErrorHandlingFSTest, MultiDBVariousErrors) {
   for (auto i = 0; i < kNumDbInstances; ++i) {
     listener.emplace_back(new ErrorHandlerFSListener());
     options.emplace_back(GetDefaultOptions());
-    fault_fs.emplace_back(new FaultInjectionTestFS(FileSystem::Default()));
-    std::shared_ptr<FileSystem> fs(fault_fs.back());
-    fault_envs.emplace_back(new CompositeEnvWrapper(def_env, fs));
-    options[i].env = fault_envs.back().get();
+    fault_fs.emplace_back(
+        new FaultInjectionTestFS(FileSystem::Default().get()));
     options[i].create_if_missing = true;
     options[i].level0_file_num_compaction_trigger = 2;
     options[i].writable_file_max_buffer_size = 32768;
+    options[i].file_system.reset(fault_fs[i]);
     options[i].listeners.emplace_back(listener[i]);
     options[i].sst_file_manager = sfm;
     DB* dbptr;

db/memtable.cc

@@ -919,9 +919,9 @@ void MemTable::MultiGet(const ReadOptions& read_options, MultiGetRange* range,
           range_del_iter->MaxCoveringTombstoneSeqnum(iter->lkey->user_key()));
     }
     GetFromTable(*(iter->lkey), iter->max_covering_tombstone_seq, true,
-                 callback, is_blob, iter->value->GetSelf(), iter->timestamp,
-                 iter->s, &(iter->merge_context), &seq, &found_final_value,
-                 &merge_in_progress);
+                 callback, is_blob, iter->value->GetSelf(),
+                 /*timestamp=*/nullptr, iter->s, &(iter->merge_context), &seq,
+                 &found_final_value, &merge_in_progress);
 
     if (!found_final_value && merge_in_progress) {
       *(iter->s) = Status::MergeInProgress();

db/memtable_list_test.cc

@@ -92,6 +92,7 @@ class MemTableListTest : public testing::Test {
     CreateDB();
     // Create a mock VersionSet
     DBOptions db_options;
+    db_options.file_system = FileSystem::Default();
     ImmutableDBOptions immutable_db_options(db_options);
     EnvOptions env_options;
     std::shared_ptr<Cache> table_cache(NewLRUCache(50000, 16));
@@ -138,6 +139,7 @@ class MemTableListTest : public testing::Test {
     CreateDB();
     // Create a mock VersionSet
     DBOptions db_options;
+    db_options.file_system.reset(new LegacyFileSystemWrapper(db_options.env));
 
     ImmutableDBOptions immutable_db_options(db_options);
     EnvOptions env_options;

db/repair.cc

@@ -672,6 +672,10 @@ Status RepairDB(const std::string& dbname, const DBOptions& db_options,
 
 Status RepairDB(const std::string& dbname, const Options& options) {
   Options opts(options);
+  if (opts.file_system == nullptr) {
+    opts.file_system.reset(new LegacyFileSystemWrapper(opts.env));
+    ;
+  }
 
   DBOptions db_options(opts);
   ColumnFamilyOptions cf_options(opts);

db/version_builder.cc

@@ -528,26 +528,4 @@ Status VersionBuilder::LoadTableHandlers(
                                  is_initial_load, prefix_extractor);
 }
 
-BaseReferencedVersionBuilder::BaseReferencedVersionBuilder(
-    ColumnFamilyData* cfd)
-    : version_builder_(new VersionBuilder(
-          cfd->current()->version_set()->file_options(), cfd->table_cache(),
-          cfd->current()->storage_info(), cfd->ioptions()->info_log)),
-      version_(cfd->current()) {
-  version_->Ref();
-}
-
-BaseReferencedVersionBuilder::BaseReferencedVersionBuilder(
-    ColumnFamilyData* cfd, Version* v)
-    : version_builder_(new VersionBuilder(
-          cfd->current()->version_set()->file_options(), cfd->table_cache(),
-          v->storage_info(), cfd->ioptions()->info_log)),
-      version_(v) {
-  assert(version_ != cfd->current());
-}
-
-BaseReferencedVersionBuilder::~BaseReferencedVersionBuilder() {
-  version_->Unref();
-}
-
 }  // namespace ROCKSDB_NAMESPACE

db/version_builder.h

@@ -21,8 +21,6 @@ class VersionStorageInfo;
 class VersionEdit;
 struct FileMetaData;
 class InternalStats;
-class Version;
-class ColumnFamilyData;
 
 // A helper class so we can efficiently apply a whole sequence
 // of edits to a particular state without creating intermediate
@@ -46,20 +44,5 @@ class VersionBuilder {
   std::unique_ptr<Rep> rep_;
 };
 
-// A wrapper of version builder which references the current version in
-// constructor and unref it in the destructor.
-// Both of the constructor and destructor need to be called inside DB Mutex.
-class BaseReferencedVersionBuilder {
- public:
-  explicit BaseReferencedVersionBuilder(ColumnFamilyData* cfd);
-  BaseReferencedVersionBuilder(ColumnFamilyData* cfd, Version* v);
-  ~BaseReferencedVersionBuilder();
-  VersionBuilder* version_builder() const { return version_builder_.get(); }
-
- private:
-  std::unique_ptr<VersionBuilder> version_builder_;
-  Version* version_;
-};
-
 extern bool NewestFirstBySeqNo(FileMetaData* a, FileMetaData* b);
 }  // namespace ROCKSDB_NAMESPACE

db/version_edit.cc

@@ -59,6 +59,8 @@ enum Tag : uint32_t {
   kDbId,
   kBlobFileAddition,
   kBlobFileGarbage,
+  kStateUponManifestSwitch,
+  kManifestSwitched,
 };
 
 enum NewFileCustomTag : uint32_t {
@@ -160,6 +162,8 @@ void VersionEdit::Clear() {
   column_family_name_.clear();
   is_in_atomic_group_ = false;
   remaining_entries_ = 0;
+  state_upon_manifest_switch_ = false;
+  manifest_switched_ = false;
 }
 
 bool VersionEdit::EncodeTo(std::string* dst) const {
@@ -293,6 +297,14 @@ bool VersionEdit::EncodeTo(std::string* dst) const {
     PutVarint32Varint32(dst, kColumnFamily, column_family_);
   }
 
+  if (state_upon_manifest_switch_) {
+    PutVarint32(dst, kStateUponManifestSwitch);
+  }
+
+  if (manifest_switched_) {
+    PutVarint32(dst, kManifestSwitched);
+  }
+
   if (is_column_family_add_) {
     PutVarint32(dst, kColumnFamilyAdd);
     PutLengthPrefixedSlice(dst, Slice(column_family_name_));
@@ -635,6 +647,14 @@ Status VersionEdit::DecodeFrom(const Slice& src) {
         is_column_family_drop_ = true;
         break;
 
+      case kStateUponManifestSwitch:
+        state_upon_manifest_switch_ = true;
+        break;
+
+      case kManifestSwitched:
+        manifest_switched_ = true;
+        break;
+
       case kInAtomicGroup:
         is_in_atomic_group_ = true;
         if (!GetVarint32(&input, &remaining_entries_)) {

db/version_edit.h

@@ -423,10 +423,21 @@ class VersionEdit {
   std::string DebugString(bool hex_key = false) const;
   std::string DebugJSON(int edit_num, bool hex_key = false) const;
 
+  void SetStateUponManifestSwitch(bool tag) {
+    state_upon_manifest_switch_ = tag;
+  }
+  bool GetStateUponManifestSwitch() const {
+    return state_upon_manifest_switch_;
+  }
+  void SetManifestSwitched(bool tag) {
+    manifest_switched_ = tag;
+  }
+  bool GetManifestSwitched() const {
+    return manifest_switched_;
+  }
+
  private:
   friend class ReactiveVersionSet;
-  friend class VersionEditHandler;
-  friend class VersionEditHandlerPointInTime;
   friend class VersionSet;
   friend class Version;
   friend class AtomicGroupReadBuffer;
@@ -472,6 +483,13 @@ class VersionEdit {
 
   bool is_in_atomic_group_ = false;
   uint32_t remaining_entries_ = 0;
+  // To distinguish the version edit written by WriteCurrentStateToManifest
+  // and other regular writes. Default is false.
+  bool state_upon_manifest_switch_ = false;
+  // To indicate when WriteCurrentStateToManifest is successful. When both
+  // state_upon_manifest_switch and manifest_switch_finished are true, it can
+  // ensure the manifest switch is finished.
+  bool manifest_switched_ = false;
 };
 
 }  // namespace ROCKSDB_NAMESPACE

db/version_edit_handler.cc

@@ -1,576 +0,0 @@
-//  Copyright (c) 2011-present, Facebook, Inc.  All rights reserved.
-//  This source code is licensed under both the GPLv2 (found in the
-//  COPYING file in the root directory) and Apache 2.0 License
-//  (found in the LICENSE.Apache file in the root directory).
-//
-// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file. See the AUTHORS file for names of contributors.
-
-#include "db/version_edit_handler.h"
-
-#include "monitoring/persistent_stats_history.h"
-
-namespace ROCKSDB_NAMESPACE {
-
-VersionEditHandler::VersionEditHandler(
-    bool read_only, const std::vector<ColumnFamilyDescriptor>& column_families,
-    VersionSet* version_set, bool track_missing_files,
-    bool no_error_if_table_files_missing)
-    : read_only_(read_only),
-      column_families_(column_families),
-      status_(),
-      version_set_(version_set),
-      track_missing_files_(track_missing_files),
-      no_error_if_table_files_missing_(no_error_if_table_files_missing),
-      initialized_(false) {
-  assert(version_set_ != nullptr);
-}
-
-Status VersionEditHandler::Iterate(log::Reader& reader, std::string* db_id) {
-  Slice record;
-  std::string scratch;
-  size_t recovered_edits = 0;
-  Status s = Initialize();
-  while (reader.ReadRecord(&record, &scratch) && s.ok()) {
-    VersionEdit edit;
-    s = edit.DecodeFrom(record);
-    if (!s.ok()) {
-      break;
-    }
-    if (edit.has_db_id_) {
-      version_set_->db_id_ = edit.GetDbId();
-      if (db_id != nullptr) {
-        *db_id = version_set_->db_id_;
-      }
-    }
-    s = read_buffer_.AddEdit(&edit);
-    if (!s.ok()) {
-      break;
-    }
-    ColumnFamilyData* cfd = nullptr;
-    if (edit.is_in_atomic_group_) {
-      if (read_buffer_.IsFull()) {
-        for (auto& e : read_buffer_.replay_buffer()) {
-          s = ApplyVersionEdit(e, &cfd);
-          if (!s.ok()) {
-            break;
-          }
-          ++recovered_edits;
-        }
-        if (!s.ok()) {
-          break;
-        }
-        read_buffer_.Clear();
-      }
-    } else {
-      s = ApplyVersionEdit(edit, &cfd);
-      if (s.ok()) {
-        ++recovered_edits;
-      }
-    }
-  }
-
-  CheckIterationResult(reader, &s);
-
-  if (!s.ok()) {
-    status_ = s;
-  }
-  return s;
-}
-
-Status VersionEditHandler::Initialize() {
-  Status s;
-  if (!initialized_) {
-    for (const auto& cf_desc : column_families_) {
-      name_to_options_.emplace(cf_desc.name, cf_desc.options);
-    }
-    auto default_cf_iter = name_to_options_.find(kDefaultColumnFamilyName);
-    if (default_cf_iter == name_to_options_.end()) {
-      s = Status::InvalidArgument("Default column family not specified");
-    }
-    if (s.ok()) {
-      VersionEdit default_cf_edit;
-      default_cf_edit.AddColumnFamily(kDefaultColumnFamilyName);
-      default_cf_edit.SetColumnFamily(0);
-      ColumnFamilyData* cfd =
-          CreateCfAndInit(default_cf_iter->second, default_cf_edit);
-      assert(cfd != nullptr);
-#ifdef NDEBUG
-      (void)cfd;
-#endif
-      initialized_ = true;
-    }
-  }
-  return s;
-}
-
-Status VersionEditHandler::ApplyVersionEdit(VersionEdit& edit,
-                                            ColumnFamilyData** cfd) {
-  Status s;
-  if (edit.is_column_family_add_) {
-    s = OnColumnFamilyAdd(edit, cfd);
-  } else if (edit.is_column_family_drop_) {
-    s = OnColumnFamilyDrop(edit, cfd);
-  } else {
-    s = OnNonCfOperation(edit, cfd);
-  }
-  if (s.ok()) {
-    assert(cfd != nullptr);
-    s = ExtractInfoFromVersionEdit(*cfd, edit);
-  }
-  return s;
-}
-
-Status VersionEditHandler::OnColumnFamilyAdd(VersionEdit& edit,
-                                             ColumnFamilyData** cfd) {
-  bool cf_in_not_found = false;
-  bool cf_in_builders = false;
-  CheckColumnFamilyId(edit, &cf_in_not_found, &cf_in_builders);
-
-  assert(cfd != nullptr);
-  *cfd = nullptr;
-  Status s;
-  if (cf_in_builders || cf_in_not_found) {
-    s = Status::Corruption("MANIFEST adding the same column family twice: " +
-                           edit.column_family_name_);
-  }
-  if (s.ok()) {
-    auto cf_options = name_to_options_.find(edit.column_family_name_);
-    // implicitly add persistent_stats column family without requiring user
-    // to specify
-    ColumnFamilyData* tmp_cfd = nullptr;
-    bool is_persistent_stats_column_family =
-        edit.column_family_name_.compare(kPersistentStatsColumnFamilyName) == 0;
-    if (cf_options == name_to_options_.end() &&
-        !is_persistent_stats_column_family) {
-      column_families_not_found_.emplace(edit.column_family_,
-                                         edit.column_family_name_);
-    } else {
-      if (is_persistent_stats_column_family) {
-        ColumnFamilyOptions cfo;
-        OptimizeForPersistentStats(&cfo);
-        tmp_cfd = CreateCfAndInit(cfo, edit);
-      } else {
-        tmp_cfd = CreateCfAndInit(cf_options->second, edit);
-      }
-      *cfd = tmp_cfd;
-    }
-  }
-  return s;
-}
-
-Status VersionEditHandler::OnColumnFamilyDrop(VersionEdit& edit,
-                                              ColumnFamilyData** cfd) {
-  bool cf_in_not_found = false;
-  bool cf_in_builders = false;
-  CheckColumnFamilyId(edit, &cf_in_not_found, &cf_in_builders);
-
-  assert(cfd != nullptr);
-  *cfd = nullptr;
-  ColumnFamilyData* tmp_cfd = nullptr;
-  Status s;
-  if (cf_in_builders) {
-    tmp_cfd = DestroyCfAndCleanup(edit);
-  } else if (cf_in_not_found) {
-    column_families_not_found_.erase(edit.column_family_);
-  } else {
-    s = Status::Corruption("MANIFEST - dropping non-existing column family");
-  }
-  *cfd = tmp_cfd;
-  return s;
-}
-
-Status VersionEditHandler::OnNonCfOperation(VersionEdit& edit,
-                                            ColumnFamilyData** cfd) {
-  bool cf_in_not_found = false;
-  bool cf_in_builders = false;
-  CheckColumnFamilyId(edit, &cf_in_not_found, &cf_in_builders);
-
-  assert(cfd != nullptr);
-  *cfd = nullptr;
-  Status s;
-  if (!cf_in_not_found) {
-    if (!cf_in_builders) {
-      s = Status::Corruption(
-          "MANIFEST record referencing unknown column family");
-    }
-    ColumnFamilyData* tmp_cfd = nullptr;
-    if (s.ok()) {
-      auto builder_iter = builders_.find(edit.column_family_);
-      assert(builder_iter != builders_.end());
-      tmp_cfd = version_set_->GetColumnFamilySet()->GetColumnFamily(
-          edit.column_family_);
-      assert(tmp_cfd != nullptr);
-      s = MaybeCreateVersion(edit, tmp_cfd, /*force_create_version=*/false);
-      if (s.ok()) {
-        s = builder_iter->second->version_builder()->Apply(&edit);
-      }
-    }
-    *cfd = tmp_cfd;
-  }
-  return s;
-}
-
-// TODO maybe cache the computation result
-bool VersionEditHandler::HasMissingFiles() const {
-  bool ret = false;
-  for (const auto& elem : cf_to_missing_files_) {
-    const auto& missing_files = elem.second;
-    if (!missing_files.empty()) {
-      ret = true;
-      break;
-    }
-  }
-  return ret;
-}
-
-void VersionEditHandler::CheckColumnFamilyId(const VersionEdit& edit,
-                                             bool* cf_in_not_found,
-                                             bool* cf_in_builders) const {
-  assert(cf_in_not_found != nullptr);
-  assert(cf_in_builders != nullptr);
-  // Not found means that user didn't supply that column
-  // family option AND we encountered column family add
-  // record. Once we encounter column family drop record,
-  // we will delete the column family from
-  // column_families_not_found.
-  bool in_not_found = column_families_not_found_.find(edit.column_family_) !=
-                      column_families_not_found_.end();
-  // in builders means that user supplied that column family
-  // option AND that we encountered column family add record
-  bool in_builders = builders_.find(edit.column_family_) != builders_.end();
-  // They cannot both be true
-  assert(!(in_not_found && in_builders));
-  *cf_in_not_found = in_not_found;
-  *cf_in_builders = in_builders;
-}
-
-void VersionEditHandler::CheckIterationResult(const log::Reader& reader,
-                                              Status* s) {
-  assert(s != nullptr);
-  if (!s->ok()) {
-    read_buffer_.Clear();
-  } else if (!version_edit_params_.has_log_number_ ||
-             !version_edit_params_.has_next_file_number_ ||
-             !version_edit_params_.has_last_sequence_) {
-    std::string msg("no ");
-    if (!version_edit_params_.has_log_number_) {
-      msg.append("log_file_number, ");
-    }
-    if (!version_edit_params_.has_next_file_number_) {
-      msg.append("next_file_number, ");
-    }
-    if (!version_edit_params_.has_last_sequence_) {
-      msg.append("last_sequence, ");
-    }
-    msg = msg.substr(0, msg.size() - 2);
-    msg.append(" entry in MANIFEST");
-    *s = Status::Corruption(msg);
-  }
-  if (s->ok() && !read_only_ && !column_families_not_found_.empty()) {
-    std::string msg;
-    for (const auto& cf : column_families_not_found_) {
-      msg.append(", ");
-      msg.append(cf.second);
-    }
-    msg = msg.substr(2);
-    *s = Status::InvalidArgument("Column families not opened: " + msg);
-  }
-  if (s->ok()) {
-    version_set_->GetColumnFamilySet()->UpdateMaxColumnFamily(
-        version_edit_params_.max_column_family_);
-    version_set_->MarkMinLogNumberToKeep2PC(
-        version_edit_params_.min_log_number_to_keep_);
-    version_set_->MarkFileNumberUsed(version_edit_params_.prev_log_number_);
-    version_set_->MarkFileNumberUsed(version_edit_params_.log_number_);
-    for (auto* cfd : *(version_set_->GetColumnFamilySet())) {
-      auto builder_iter = builders_.find(cfd->GetID());
-      assert(builder_iter != builders_.end());
-      auto* builder = builder_iter->second->version_builder();
-      if (!builder->CheckConsistencyForNumLevels()) {
-        *s = Status::InvalidArgument(
-            "db has more levels than options.num_levels");
-        break;
-      }
-    }
-  }
-  if (s->ok()) {
-    for (auto* cfd : *(version_set_->GetColumnFamilySet())) {
-      if (cfd->IsDropped()) {
-        continue;
-      }
-      if (read_only_) {
-        cfd->table_cache()->SetTablesAreImmortal();
-      }
-      *s = LoadTables(cfd, /*prefetch_index_and_filter_in_cache=*/false,
-                      /*is_initial_load=*/true);
-      if (!s->ok()) {
-        break;
-      }
-    }
-  }
-  if (s->ok()) {
-    for (auto* cfd : *(version_set_->column_family_set_)) {
-      if (cfd->IsDropped()) {
-        continue;
-      }
-      assert(cfd->initialized());
-      VersionEdit edit;
-      *s = MaybeCreateVersion(edit, cfd, /*force_create_version=*/true);
-      if (!s->ok()) {
-        break;
-      }
-    }
-  }
-  if (s->ok()) {
-    version_set_->manifest_file_size_ = reader.GetReadOffset();
-    assert(version_set_->manifest_file_size_ > 0);
-    version_set_->next_file_number_.store(
-        version_edit_params_.next_file_number_ + 1);
-    version_set_->last_allocated_sequence_ =
-        version_edit_params_.last_sequence_;
-    version_set_->last_published_sequence_ =
-        version_edit_params_.last_sequence_;
-    version_set_->last_sequence_ = version_edit_params_.last_sequence_;
-    version_set_->prev_log_number_ = version_edit_params_.prev_log_number_;
-  }
-}
-
-ColumnFamilyData* VersionEditHandler::CreateCfAndInit(
-    const ColumnFamilyOptions& cf_options, const VersionEdit& edit) {
-  ColumnFamilyData* cfd = version_set_->CreateColumnFamily(cf_options, &edit);
-  assert(cfd != nullptr);
-  cfd->set_initialized();
-  assert(builders_.find(edit.column_family_) == builders_.end());
-  builders_.emplace(edit.column_family_,
-                    VersionBuilderUPtr(new BaseReferencedVersionBuilder(cfd)));
-  if (track_missing_files_) {
-    cf_to_missing_files_.emplace(edit.column_family_,
-                                 std::unordered_set<uint64_t>());
-  }
-  return cfd;
-}
-
-ColumnFamilyData* VersionEditHandler::DestroyCfAndCleanup(
-    const VersionEdit& edit) {
-  auto builder_iter = builders_.find(edit.column_family_);
-  assert(builder_iter != builders_.end());
-  builders_.erase(builder_iter);
-  if (track_missing_files_) {
-    auto missing_files_iter = cf_to_missing_files_.find(edit.column_family_);
-    assert(missing_files_iter != cf_to_missing_files_.end());
-    cf_to_missing_files_.erase(missing_files_iter);
-  }
-  ColumnFamilyData* ret =
-      version_set_->GetColumnFamilySet()->GetColumnFamily(edit.column_family_);
-  assert(ret != nullptr);
-  if (ret->UnrefAndTryDelete()) {
-    ret = nullptr;
-  } else {
-    assert(false);
-  }
-  return ret;
-}
-
-Status VersionEditHandler::MaybeCreateVersion(const VersionEdit& /*edit*/,
-                                              ColumnFamilyData* cfd,
-                                              bool force_create_version) {
-  assert(cfd->initialized());
-  if (force_create_version) {
-    auto builder_iter = builders_.find(cfd->GetID());
-    assert(builder_iter != builders_.end());
-    auto* builder = builder_iter->second->version_builder();
-    auto* v = new Version(cfd, version_set_, version_set_->file_options_,
-                          *cfd->GetLatestMutableCFOptions(),
-                          version_set_->current_version_number_++);
-    builder->SaveTo(v->storage_info());
-    // Install new version
-    v->PrepareApply(*cfd->GetLatestMutableCFOptions(),
-                    !(version_set_->db_options_->skip_stats_update_on_db_open));
-    version_set_->AppendVersion(cfd, v);
-  }
-  return Status::OK();
-}
-
-Status VersionEditHandler::LoadTables(ColumnFamilyData* cfd,
-                                      bool prefetch_index_and_filter_in_cache,
-                                      bool is_initial_load) {
-  assert(cfd != nullptr);
-  assert(!cfd->IsDropped());
-  Status s;
-  auto builder_iter = builders_.find(cfd->GetID());
-  assert(builder_iter != builders_.end());
-  assert(builder_iter->second != nullptr);
-  VersionBuilder* builder = builder_iter->second->version_builder();
-  assert(builder);
-  s = builder->LoadTableHandlers(
-      cfd->internal_stats(),
-      version_set_->db_options_->max_file_opening_threads,
-      prefetch_index_and_filter_in_cache, is_initial_load,
-      cfd->GetLatestMutableCFOptions()->prefix_extractor.get());
-  if (s.IsPathNotFound() && no_error_if_table_files_missing_) {
-    s = Status::OK();
-  }
-  if (!s.ok() && !version_set_->db_options_->paranoid_checks) {
-    s = Status::OK();
-  }
-  return s;
-}
-
-Status VersionEditHandler::ExtractInfoFromVersionEdit(ColumnFamilyData* cfd,
-                                                      const VersionEdit& edit) {
-  Status s;
-  if (cfd != nullptr) {
-    if (edit.has_db_id_) {
-      version_edit_params_.SetDBId(edit.db_id_);
-    }
-    if (edit.has_log_number_) {
-      if (cfd->GetLogNumber() > edit.log_number_) {
-        ROCKS_LOG_WARN(
-            version_set_->db_options()->info_log,
-            "MANIFEST corruption detected, but ignored - Log numbers in "
-            "records NOT monotonically increasing");
-      } else {
-        cfd->SetLogNumber(edit.log_number_);
-        version_edit_params_.SetLogNumber(edit.log_number_);
-      }
-    }
-    if (edit.has_comparator_ &&
-        edit.comparator_ != cfd->user_comparator()->Name()) {
-      s = Status::InvalidArgument(
-          cfd->user_comparator()->Name(),
-          "does not match existing comparator " + edit.comparator_);
-    }
-  }
-
-  if (s.ok()) {
-    if (edit.has_prev_log_number_) {
-      version_edit_params_.SetPrevLogNumber(edit.prev_log_number_);
-    }
-    if (edit.has_next_file_number_) {
-      version_edit_params_.SetNextFile(edit.next_file_number_);
-    }
-    if (edit.has_max_column_family_) {
-      version_edit_params_.SetMaxColumnFamily(edit.max_column_family_);
-    }
-    if (edit.has_min_log_number_to_keep_) {
-      version_edit_params_.min_log_number_to_keep_ =
-          std::max(version_edit_params_.min_log_number_to_keep_,
-                   edit.min_log_number_to_keep_);
-    }
-    if (edit.has_last_sequence_) {
-      version_edit_params_.SetLastSequence(edit.last_sequence_);
-    }
-    if (!version_edit_params_.has_prev_log_number_) {
-      version_edit_params_.SetPrevLogNumber(0);
-    }
-  }
-  return s;
-}
-
-VersionEditHandlerPointInTime::VersionEditHandlerPointInTime(
-    bool read_only, const std::vector<ColumnFamilyDescriptor>& column_families,
-    VersionSet* version_set)
-    : VersionEditHandler(read_only, column_families, version_set,
-                         /*track_missing_files=*/true,
-                         /*no_error_if_table_files_missing=*/true) {}
-
-VersionEditHandlerPointInTime::~VersionEditHandlerPointInTime() {
-  for (const auto& elem : versions_) {
-    delete elem.second;
-  }
-  versions_.clear();
-}
-
-void VersionEditHandlerPointInTime::CheckIterationResult(
-    const log::Reader& reader, Status* s) {
-  VersionEditHandler::CheckIterationResult(reader, s);
-  assert(s != nullptr);
-  if (s->ok()) {
-    for (auto* cfd : *(version_set_->column_family_set_)) {
-      if (cfd->IsDropped()) {
-        continue;
-      }
-      assert(cfd->initialized());
-      auto v_iter = versions_.find(cfd->GetID());
-      if (v_iter != versions_.end()) {
-        assert(v_iter->second != nullptr);
-
-        version_set_->AppendVersion(cfd, v_iter->second);
-        versions_.erase(v_iter);
-      }
-    }
-  }
-}
-
-ColumnFamilyData* VersionEditHandlerPointInTime::DestroyCfAndCleanup(
-    const VersionEdit& edit) {
-  ColumnFamilyData* cfd = VersionEditHandler::DestroyCfAndCleanup(edit);
-  auto v_iter = versions_.find(edit.column_family_);
-  if (v_iter != versions_.end()) {
-    delete v_iter->second;
-    versions_.erase(v_iter);
-  }
-  return cfd;
-}
-
-Status VersionEditHandlerPointInTime::MaybeCreateVersion(
-    const VersionEdit& edit, ColumnFamilyData* cfd, bool force_create_version) {
-  assert(cfd != nullptr);
-  if (!force_create_version) {
-    assert(edit.column_family_ == cfd->GetID());
-  }
-  auto missing_files_iter = cf_to_missing_files_.find(cfd->GetID());
-  assert(missing_files_iter != cf_to_missing_files_.end());
-  std::unordered_set<uint64_t>& missing_files = missing_files_iter->second;
-  const bool prev_has_missing_files = !missing_files.empty();
-  for (const auto& file : edit.GetDeletedFiles()) {
-    uint64_t file_num = file.second;
-    auto fiter = missing_files.find(file_num);
-    if (fiter != missing_files.end()) {
-      missing_files.erase(fiter);
-    }
-  }
-  Status s;
-  for (const auto& elem : edit.GetNewFiles()) {
-    const FileMetaData& meta = elem.second;
-    const FileDescriptor& fd = meta.fd;
-    uint64_t file_num = fd.GetNumber();
-    const std::string fpath =
-        MakeTableFileName(cfd->ioptions()->cf_paths[0].path, file_num);
-    s = version_set_->VerifyFileMetadata(fpath, meta);
-    if (s.IsPathNotFound() || s.IsNotFound()) {
-      missing_files.insert(file_num);
-      s = Status::OK();
-    }
-  }
-  bool missing_info = !version_edit_params_.has_log_number_ ||
-                      !version_edit_params_.has_next_file_number_ ||
-                      !version_edit_params_.has_last_sequence_;
-
-  // Create version before apply edit
-  if (!missing_info && ((!missing_files.empty() && !prev_has_missing_files) ||
-                        (missing_files.empty() && force_create_version))) {
-    auto builder_iter = builders_.find(cfd->GetID());
-    assert(builder_iter != builders_.end());
-    auto* builder = builder_iter->second->version_builder();
-    auto* version = new Version(cfd, version_set_, version_set_->file_options_,
-                                *cfd->GetLatestMutableCFOptions(),
-                                version_set_->current_version_number_++);
-    builder->SaveTo(version->storage_info());
-    version->PrepareApply(
-        *cfd->GetLatestMutableCFOptions(),
-        !version_set_->db_options_->skip_stats_update_on_db_open);
-    auto v_iter = versions_.find(cfd->GetID());
-    if (v_iter != versions_.end()) {
-      delete v_iter->second;
-      v_iter->second = version;
-    } else {
-      versions_.emplace(cfd->GetID(), version);
-    }
-  }
-  return s;
-}
-
-}  // namespace ROCKSDB_NAMESPACE

db/version_edit_handler.h

@@ -1,123 +0,0 @@
-//  Copyright (c) 2011-present, Facebook, Inc.  All rights reserved.
-//  This source code is licensed under both the GPLv2 (found in the
-//  COPYING file in the root directory) and Apache 2.0 License
-//  (found in the LICENSE.Apache file in the root directory).
-//
-// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file. See the AUTHORS file for names of contributors.
-
-#pragma once
-
-#include "db/version_builder.h"
-#include "db/version_edit.h"
-#include "db/version_set.h"
-
-namespace ROCKSDB_NAMESPACE {
-
-typedef std::unique_ptr<BaseReferencedVersionBuilder> VersionBuilderUPtr;
-
-// A class used for scanning MANIFEST file.
-// VersionEditHandler reads a MANIFEST file, parses the version edits, and
-// builds the version set's in-memory state, e.g. the version storage info for
-// the versions of column families.
-// To use this class and its subclasses,
-// 1. Create an object of VersionEditHandler or its subclasses.
-//    VersionEditHandler handler(read_only, column_families, version_set,
-//                               track_missing_files, ignore_missing_files);
-// 2. Status s = handler.Iterate(reader, &db_id);
-// 3. Check s and handle possible errors.
-//
-// Not thread-safe, external synchronization is necessary if an object of
-// VersionEditHandler is shared by multiple threads.
-class VersionEditHandler {
- public:
-  explicit VersionEditHandler(
-      bool read_only,
-      const std::vector<ColumnFamilyDescriptor>& column_families,
-      VersionSet* version_set, bool track_missing_files,
-      bool ignore_missing_files);
-
-  virtual ~VersionEditHandler() {}
-
-  Status Iterate(log::Reader& reader, std::string* db_id);
-
-  const Status& status() const { return status_; }
-
-  bool HasMissingFiles() const;
-
- protected:
-  Status ApplyVersionEdit(VersionEdit& edit, ColumnFamilyData** cfd);
-
-  Status OnColumnFamilyAdd(VersionEdit& edit, ColumnFamilyData** cfd);
-
-  Status OnColumnFamilyDrop(VersionEdit& edit, ColumnFamilyData** cfd);
-
-  Status OnNonCfOperation(VersionEdit& edit, ColumnFamilyData** cfd);
-
-  Status Initialize();
-
-  void CheckColumnFamilyId(const VersionEdit& edit, bool* cf_in_not_found,
-                           bool* cf_in_builders) const;
-
-  virtual void CheckIterationResult(const log::Reader& reader, Status* s);
-
-  ColumnFamilyData* CreateCfAndInit(const ColumnFamilyOptions& cf_options,
-                                    const VersionEdit& edit);
-
-  virtual ColumnFamilyData* DestroyCfAndCleanup(const VersionEdit& edit);
-
-  virtual Status MaybeCreateVersion(const VersionEdit& edit,
-                                    ColumnFamilyData* cfd,
-                                    bool force_create_version);
-
-  Status LoadTables(ColumnFamilyData* cfd,
-                    bool prefetch_index_and_filter_in_cache,
-                    bool is_initial_load);
-
-  const bool read_only_;
-  const std::vector<ColumnFamilyDescriptor>& column_families_;
-  Status status_;
-  VersionSet* version_set_;
-  AtomicGroupReadBuffer read_buffer_;
-  std::unordered_map<uint32_t, VersionBuilderUPtr> builders_;
-  std::unordered_map<std::string, ColumnFamilyOptions> name_to_options_;
-  std::unordered_map<uint32_t, std::string> column_families_not_found_;
-  VersionEditParams version_edit_params_;
-  const bool track_missing_files_;
-  std::unordered_map<uint32_t, std::unordered_set<uint64_t>>
-      cf_to_missing_files_;
-  bool no_error_if_table_files_missing_;
-
- private:
-  Status ExtractInfoFromVersionEdit(ColumnFamilyData* cfd,
-                                    const VersionEdit& edit);
-
-  bool initialized_;
-};
-
-// A class similar to its base class, i.e. VersionEditHandler.
-// VersionEditHandlerPointInTime restores the versions to the most recent point
-// in time such that at this point, the version does not have missing files.
-//
-// Not thread-safe, external synchronization is necessary if an object of
-// VersionEditHandlerPointInTime is shared by multiple threads.
-class VersionEditHandlerPointInTime : public VersionEditHandler {
- public:
-  VersionEditHandlerPointInTime(
-      bool read_only,
-      const std::vector<ColumnFamilyDescriptor>& column_families,
-      VersionSet* version_set);
-  ~VersionEditHandlerPointInTime() override;
-
- protected:
-  void CheckIterationResult(const log::Reader& reader, Status* s) override;
-  ColumnFamilyData* DestroyCfAndCleanup(const VersionEdit& edit) override;
-  Status MaybeCreateVersion(const VersionEdit& edit, ColumnFamilyData* cfd,
-                            bool force_create_version) override;
-
- private:
-  std::unordered_map<uint32_t, Version*> versions_;
-};
-
-}  // namespace ROCKSDB_NAMESPACE

db/version_edit_test.cc

@@ -46,6 +46,7 @@ TEST_F(VersionEditTest, EncodeDecode) {
   edit.SetLogNumber(kBig + 100);
   edit.SetNextFile(kBig + 200);
   edit.SetLastSequence(kBig + 1000);
+  edit.SetStateUponManifestSwitch(true);
   TestEncodeDecode(edit);
 }
 
@@ -80,6 +81,7 @@ TEST_F(VersionEditTest, EncodeDecodeNewFile4) {
   edit.SetLogNumber(kBig + 100);
   edit.SetNextFile(kBig + 200);
   edit.SetLastSequence(kBig + 1000);
+  edit.SetStateUponManifestSwitch(true);
   TestEncodeDecode(edit);
 
   std::string encoded, encoded2;
@@ -103,6 +105,7 @@ TEST_F(VersionEditTest, EncodeDecodeNewFile4) {
   ASSERT_EQ(kInvalidBlobFileNumber,
             new_files[2].second.oldest_blob_file_number);
   ASSERT_EQ(1001, new_files[3].second.oldest_blob_file_number);
+  ASSERT_TRUE(parsed.GetStateUponManifestSwitch());
 }
 
 TEST_F(VersionEditTest, ForwardCompatibleNewFile4) {
@@ -279,6 +282,38 @@ TEST_F(VersionEditTest, DbId) {
   TestEncodeDecode(edit);
 }
 
+TEST_F(VersionEditTest, ManifestSwitchTag) {
+  VersionEdit edit1, decode1;
+  edit1.SetStateUponManifestSwitch(true);
+  TestEncodeDecode(edit1);
+  std::string encoded1;
+  edit1.EncodeTo(&encoded1);
+  ASSERT_OK(decode1.DecodeFrom(encoded1));
+  ASSERT_TRUE(decode1.GetStateUponManifestSwitch());
+  ASSERT_TRUE(!decode1.GetManifestSwitched());
+
+  VersionEdit edit2, decode2;
+  edit2.SetManifestSwitched(true);
+  TestEncodeDecode(edit2);
+  std::string encoded2;
+  edit2.EncodeTo(&encoded2);
+  ASSERT_OK(decode2.DecodeFrom(encoded2));
+  ASSERT_TRUE(!decode2.GetStateUponManifestSwitch());
+  ASSERT_TRUE(decode2.GetManifestSwitched());
+
+  VersionEdit edit3, decode3;
+  edit3.SetStateUponManifestSwitch(true);
+  edit3.SetManifestSwitched(true);
+  TestEncodeDecode(edit3);
+  std::string encoded3;
+  edit3.EncodeTo(&encoded3);
+  ASSERT_OK(decode3.DecodeFrom(encoded3));
+  ASSERT_TRUE(decode3.GetStateUponManifestSwitch());
+  ASSERT_TRUE(decode3.GetManifestSwitched());
+
+
+}
+
 TEST_F(VersionEditTest, BlobFileAdditionAndGarbage) {
   VersionEdit edit;
 

db/version_set.cc

@@ -29,7 +29,6 @@
 #include "db/pinned_iterators_manager.h"
 #include "db/table_cache.h"
 #include "db/version_builder.h"
-#include "db/version_edit_handler.h"
 #include "file/filename.h"
 #include "file/random_access_file_reader.h"
 #include "file/read_write_util.h"
@@ -1203,6 +1202,28 @@ void LevelIterator::InitFileIterator(size_t new_file_index) {
 }
 }  // anonymous namespace
 
+// A wrapper of version builder which references the current version in
+// constructor and unref it in the destructor.
+// Both of the constructor and destructor need to be called inside DB Mutex.
+class BaseReferencedVersionBuilder {
+ public:
+  explicit BaseReferencedVersionBuilder(ColumnFamilyData* cfd)
+      : version_builder_(new VersionBuilder(
+            cfd->current()->version_set()->file_options(), cfd->table_cache(),
+            cfd->current()->storage_info(), cfd->ioptions()->info_log)),
+        version_(cfd->current()) {
+    version_->Ref();
+  }
+  ~BaseReferencedVersionBuilder() {
+    version_->Unref();
+  }
+  VersionBuilder* version_builder() { return version_builder_.get(); }
+
+ private:
+  std::unique_ptr<VersionBuilder> version_builder_;
+  Version* version_;
+};
+
 Status Version::GetTableProperties(std::shared_ptr<const TableProperties>* tp,
                                    const FileMetaData* file_meta,
                                    const std::string* fname) const {
@@ -1897,8 +1918,8 @@ void Version::MultiGet(const ReadOptions& read_options, MultiGetRange* range,
     get_ctx.emplace_back(
         user_comparator(), merge_operator_, info_log_, db_statistics_,
         iter->s->ok() ? GetContext::kNotFound : GetContext::kMerge, iter->ukey,
-        iter->value, iter->timestamp, nullptr, &(iter->merge_context), true,
-        &iter->max_covering_tombstone_seq, this->env_, nullptr,
+        iter->value, /*timestamp*/ nullptr, nullptr, &(iter->merge_context),
+        true, &iter->max_covering_tombstone_seq, this->env_, nullptr,
         merge_operator_ ? &pinned_iters_mgr : nullptr, callback, is_blob,
         tracing_mget_id);
     // MergeInProgress status, if set, has been transferred to the get_context
@@ -3549,33 +3570,6 @@ VersionSet::~VersionSet() {
   obsolete_files_.clear();
 }
 
-void VersionSet::Reset() {
-  if (column_family_set_) {
-    Cache* table_cache = column_family_set_->get_table_cache();
-    WriteBufferManager* wbm = column_family_set_->write_buffer_manager();
-    WriteController* wc = column_family_set_->write_controller();
-    column_family_set_.reset(new ColumnFamilySet(dbname_, db_options_,
-                                                 file_options_, table_cache,
-                                                 wbm, wc, block_cache_tracer_));
-  }
-  db_id_.clear();
-  next_file_number_.store(2);
-  min_log_number_to_keep_2pc_.store(0);
-  manifest_file_number_ = 0;
-  options_file_number_ = 0;
-  pending_manifest_file_number_ = 0;
-  last_sequence_.store(0);
-  last_allocated_sequence_.store(0);
-  last_published_sequence_.store(0);
-  prev_log_number_ = 0;
-  descriptor_log_.reset();
-  current_version_number_ = 0;
-  manifest_writers_.clear();
-  manifest_file_size_ = 0;
-  obsolete_files_.clear();
-  obsolete_manifests_.clear();
-}
-
 void VersionSet::AppendVersion(ColumnFamilyData* column_family_data,
                                Version* v) {
   // compute new compaction score
@@ -3811,7 +3805,7 @@ Status VersionSet::ProcessManifestWrites(
                builder_guards.size() == versions.size());
         ColumnFamilyData* cfd = versions[i]->cfd_;
         s = builder_guards[i]->version_builder()->LoadTableHandlers(
-            cfd->internal_stats(), 1 /* max_threads */,
+            cfd->internal_stats(), cfd->ioptions()->optimize_filters_for_hits,
             true /* prefetch_index_and_filter_in_cache */,
             false /* is_initial_load */,
             mutable_cf_options_ptrs[i]->prefix_extractor.get());
@@ -4458,6 +4452,8 @@ Status VersionSet::Recover(
     reporter.status = &s;
     log::Reader reader(nullptr, std::move(manifest_file_reader), &reporter,
                        true /* checksum */, 0 /* log_number */);
+    Slice record;
+    std::string scratch;
     AtomicGroupReadBuffer read_buffer;
     s = ReadAndRecover(&reader, &read_buffer, cf_name_to_options,
                        column_families_not_found, builders,
@@ -4585,159 +4581,6 @@ Status VersionSet::Recover(
   return s;
 }
 
-namespace {
-class ManifestPicker {
- public:
-  explicit ManifestPicker(const std::string& dbname, FileSystem* fs);
-  void SeekToFirstManifest();
-  // REQUIRES Valid() == true
-  std::string GetNextManifest(uint64_t* file_number, std::string* file_name);
-  bool Valid() const { return manifest_file_iter_ != manifest_files_.end(); }
-  const Status& status() const { return status_; }
-
- private:
-  const std::string& dbname_;
-  FileSystem* const fs_;
-  // MANIFEST file names(s)
-  std::vector<std::string> manifest_files_;
-  std::vector<std::string>::const_iterator manifest_file_iter_;
-  Status status_;
-};
-
-ManifestPicker::ManifestPicker(const std::string& dbname, FileSystem* fs)
-    : dbname_(dbname), fs_(fs) {}
-
-void ManifestPicker::SeekToFirstManifest() {
-  assert(fs_ != nullptr);
-  std::vector<std::string> children;
-  Status s = fs_->GetChildren(dbname_, IOOptions(), &children, /*dbg=*/nullptr);
-  if (!s.ok()) {
-    status_ = s;
-    return;
-  }
-  for (const auto& fname : children) {
-    uint64_t file_num = 0;
-    FileType file_type;
-    bool parse_ok = ParseFileName(fname, &file_num, &file_type);
-    if (parse_ok && file_type == kDescriptorFile) {
-      manifest_files_.push_back(fname);
-    }
-  }
-  std::sort(manifest_files_.begin(), manifest_files_.end(),
-            [](const std::string& lhs, const std::string& rhs) {
-              uint64_t num1 = 0;
-              uint64_t num2 = 0;
-              FileType type1;
-              FileType type2;
-              bool parse_ok1 = ParseFileName(lhs, &num1, &type1);
-              bool parse_ok2 = ParseFileName(rhs, &num2, &type2);
-#ifndef NDEBUG
-              assert(parse_ok1);
-              assert(parse_ok2);
-#else
-              (void)parse_ok1;
-              (void)parse_ok2;
-#endif
-              return num1 > num2;
-            });
-  manifest_file_iter_ = manifest_files_.begin();
-}
-
-std::string ManifestPicker::GetNextManifest(uint64_t* number,
-                                            std::string* file_name) {
-  assert(status_.ok());
-  assert(Valid());
-  std::string ret;
-  if (manifest_file_iter_ != manifest_files_.end()) {
-    ret.assign(dbname_);
-    if (ret.back() != kFilePathSeparator) {
-      ret.push_back(kFilePathSeparator);
-    }
-    ret.append(*manifest_file_iter_);
-    if (number) {
-      FileType type;
-      bool parse = ParseFileName(*manifest_file_iter_, number, &type);
-      assert(type == kDescriptorFile);
-#ifndef NDEBUG
-      assert(parse);
-#else
-      (void)parse;
-#endif
-    }
-    if (file_name) {
-      *file_name = *manifest_file_iter_;
-    }
-    ++manifest_file_iter_;
-  }
-  return ret;
-}
-}  // namespace
-
-Status VersionSet::TryRecover(
-    const std::vector<ColumnFamilyDescriptor>& column_families, bool read_only,
-    std::string* db_id, bool* has_missing_table_file) {
-  ManifestPicker manifest_picker(dbname_, fs_);
-  manifest_picker.SeekToFirstManifest();
-  Status s = manifest_picker.status();
-  if (!s.ok()) {
-    return s;
-  }
-  if (!manifest_picker.Valid()) {
-    return Status::Corruption("Cannot locate MANIFEST file in " + dbname_);
-  }
-  std::string manifest_path =
-      manifest_picker.GetNextManifest(&manifest_file_number_, nullptr);
-  while (!manifest_path.empty()) {
-    s = TryRecoverFromOneManifest(manifest_path, column_families, read_only,
-                                  db_id, has_missing_table_file);
-    if (s.ok() || !manifest_picker.Valid()) {
-      break;
-    }
-    Reset();
-    manifest_path =
-        manifest_picker.GetNextManifest(&manifest_file_number_, nullptr);
-  }
-  return s;
-}
-
-Status VersionSet::TryRecoverFromOneManifest(
-    const std::string& manifest_path,
-    const std::vector<ColumnFamilyDescriptor>& column_families, bool read_only,
-    std::string* db_id, bool* has_missing_table_file) {
-  ROCKS_LOG_INFO(db_options_->info_log, "Trying to recover from manifest: %s\n",
-                 manifest_path.c_str());
-  std::unique_ptr<SequentialFileReader> manifest_file_reader;
-  Status s;
-  {
-    std::unique_ptr<FSSequentialFile> manifest_file;
-    s = fs_->NewSequentialFile(manifest_path,
-                               fs_->OptimizeForManifestRead(file_options_),
-                               &manifest_file, nullptr);
-    if (!s.ok()) {
-      return s;
-    }
-    manifest_file_reader.reset(
-        new SequentialFileReader(std::move(manifest_file), manifest_path,
-                                 db_options_->log_readahead_size));
-  }
-
-  VersionSet::LogReporter reporter;
-  reporter.status = &s;
-  log::Reader reader(nullptr, std::move(manifest_file_reader), &reporter,
-                     /*checksum=*/true, /*log_num=*/0);
-  {
-    VersionEditHandlerPointInTime handler_pit(read_only, column_families,
-                                              const_cast<VersionSet*>(this));
-
-    s = handler_pit.Iterate(reader, db_id);
-
-    assert(nullptr != has_missing_table_file);
-    *has_missing_table_file = handler_pit.HasMissingFiles();
-  }
-
-  return s;
-}
-
 Status VersionSet::ListColumnFamilies(std::vector<std::string>* column_families,
                                       const std::string& dbname,
                                       FileSystem* fs) {
@@ -4928,10 +4771,9 @@ Status VersionSet::DumpManifest(Options& options, std::string& dscname,
   Status s;
   {
     std::unique_ptr<FSSequentialFile> file;
-    const std::shared_ptr<FileSystem>& fs = options.env->GetFileSystem();
-    s = fs->NewSequentialFile(
+    s = options.file_system->NewSequentialFile(
         dscname,
-        fs->OptimizeForManifestRead(file_options_), &file,
+        options.file_system->OptimizeForManifestRead(file_options_), &file,
         nullptr);
     if (!s.ok()) {
       return s;
@@ -5161,6 +5003,7 @@ Status VersionSet::WriteCurrentStateToManifest(
     VersionEdit edit_for_db_id;
     assert(!db_id_.empty());
     edit_for_db_id.SetDBId(db_id_);
+    edit_for_db_id.SetStateUponManifestSwitch(true);
     std::string db_id_record;
     if (!edit_for_db_id.EncodeTo(&db_id_record)) {
       return Status::Corruption("Unable to Encode VersionEdit:" +
@@ -5186,6 +5029,7 @@ Status VersionSet::WriteCurrentStateToManifest(
         edit.AddColumnFamily(cfd->GetName());
         edit.SetColumnFamily(cfd->GetID());
       }
+      edit.SetStateUponManifestSwitch(true);
       edit.SetComparatorName(
           cfd->internal_comparator().user_comparator()->Name());
       std::string record;
@@ -5203,6 +5047,7 @@ Status VersionSet::WriteCurrentStateToManifest(
       // Save files
       VersionEdit edit;
       edit.SetColumnFamily(cfd->GetID());
+      edit.SetStateUponManifestSwitch(true);
 
       for (int level = 0; level < cfd->NumberLevels(); level++) {
         for (const auto& f :
@@ -5230,7 +5075,16 @@ Status VersionSet::WriteCurrentStateToManifest(
       }
     }
   }
-  return Status::OK();
+  VersionEdit end_flag;
+  end_flag.SetStateUponManifestSwitch(true);
+  end_flag.SetManifestSwitched(true);
+  std::string end_record;
+  if (!end_flag.EncodeTo(&end_record)) {
+    return Status::Corruption("Unable to Encode VersionEdit:" +
+                              end_flag.DebugString(true));
+  }
+  Status s_end_record = log->AddRecord(end_record);
+  return  s_end_record;
 }
 
 // TODO(aekmekji): in CompactionJob::GenSubcompactionBoundaries(), this
@@ -5664,7 +5518,7 @@ void VersionSet::GetObsoleteFiles(std::vector<ObsoleteFileInfo>* files,
 }
 
 ColumnFamilyData* VersionSet::CreateColumnFamily(
-    const ColumnFamilyOptions& cf_options, const VersionEdit* edit) {
+    const ColumnFamilyOptions& cf_options, VersionEdit* edit) {
   assert(edit->is_column_family_add_);
 
   MutableCFOptions dummy_cf_options;
@@ -5719,18 +5573,6 @@ uint64_t VersionSet::GetTotalSstFilesSize(Version* dummy_versions) {
   return total_files_size;
 }
 
-Status VersionSet::VerifyFileMetadata(const std::string& fpath,
-                                      const FileMetaData& meta) const {
-  uint64_t fsize = 0;
-  Status status = fs_->GetFileSize(fpath, IOOptions(), &fsize, nullptr);
-  if (status.ok()) {
-    if (fsize != meta.fd.GetFileSize()) {
-      status = Status::Corruption("File size mismatch: " + fpath);
-    }
-  }
-  return status;
-}
-
 ReactiveVersionSet::ReactiveVersionSet(const std::string& dbname,
                                        const ImmutableDBOptions* _db_options,
                                        const FileOptions& _file_options,

db/version_set.h

@@ -688,8 +688,6 @@ class Version {
   FileSystem* fs_;
   friend class ReactiveVersionSet;
   friend class VersionSet;
-  friend class VersionEditHandler;
-  friend class VersionEditHandlerPointInTime;
 
   const InternalKeyComparator* internal_comparator() const {
     return storage_info_.internal_comparator_;
@@ -878,17 +876,6 @@ class VersionSet {
   Status Recover(const std::vector<ColumnFamilyDescriptor>& column_families,
                  bool read_only = false, std::string* db_id = nullptr);
 
-  Status TryRecover(const std::vector<ColumnFamilyDescriptor>& column_families,
-                    bool read_only, std::string* db_id,
-                    bool* has_missing_table_file);
-
-  // Try to recover the version set to the most recent consistent state
-  // recorded in the specified manifest.
-  Status TryRecoverFromOneManifest(
-      const std::string& manifest_path,
-      const std::vector<ColumnFamilyDescriptor>& column_families,
-      bool read_only, std::string* db_id, bool* has_missing_table_file);
-
   // Reads a manifest file and returns a list of column families in
   // column_families.
   static Status ListColumnFamilies(std::vector<std::string>* column_families,
@@ -1072,8 +1059,6 @@ class VersionSet {
   struct ManifestWriter;
 
   friend class Version;
-  friend class VersionEditHandler;
-  friend class VersionEditHandlerPointInTime;
   friend class DBImpl;
   friend class DBImplReadOnly;
 
@@ -1084,8 +1069,6 @@ class VersionSet {
     }
   };
 
-  void Reset();
-
   // Returns approximated offset of a key in a file for a given version.
   uint64_t ApproximateOffsetOf(Version* v, const FdWithKeyRange& f,
                                const Slice& key, TableReaderCaller caller);
@@ -1108,7 +1091,7 @@ class VersionSet {
   void AppendVersion(ColumnFamilyData* column_family_data, Version* v);
 
   ColumnFamilyData* CreateColumnFamily(const ColumnFamilyOptions& cf_options,
-                                       const VersionEdit* edit);
+                                       VersionEdit* edit);
 
   Status ReadAndRecover(
       log::Reader* reader, AtomicGroupReadBuffer* read_buffer,
@@ -1132,10 +1115,8 @@ class VersionSet {
                                     const VersionEdit& from_edit,
                                     VersionEditParams* version_edit_params);
 
-  Status VerifyFileMetadata(const std::string& fpath,
-                            const FileMetaData& meta) const;
-
   std::unique_ptr<ColumnFamilySet> column_family_set_;
+
   Env* const env_;
   FileSystem* const fs_;
   const std::string dbname_;

db/version_set_test.cc

@@ -10,7 +10,6 @@
 #include "db/version_set.h"
 #include "db/db_impl/db_impl.h"
 #include "db/log_writer.h"
-#include "env/mock_env.h"
 #include "logging/logging.h"
 #include "table/mock_table.h"
 #include "test_util/testharness.h"
@@ -612,37 +611,16 @@ class VersionSetTestBase {
   const static std::string kColumnFamilyName3;
   int num_initial_edits_;
 
-  explicit VersionSetTestBase(const std::string& name)
-      : mem_env_(nullptr),
-        env_(nullptr),
-        env_guard_(),
-        fs_(),
-        dbname_(test::PerThreadDBPath(name)),
-        options_(),
-        db_options_(options_),
-        cf_options_(options_),
-        immutable_cf_options_(db_options_, cf_options_),
+  VersionSetTestBase()
+      : env_(Env::Default()),
+        fs_(std::make_shared<LegacyFileSystemWrapper>(env_)),
+        dbname_(test::PerThreadDBPath("version_set_test")),
+        db_options_(),
         mutable_cf_options_(cf_options_),
         table_cache_(NewLRUCache(50000, 16)),
         write_buffer_manager_(db_options_.db_write_buffer_size),
         shutting_down_(false),
         mock_table_factory_(std::make_shared<mock::MockTableFactory>()) {
-    const char* test_env_uri = getenv("TEST_ENV_URI");
-    Env* base_env = nullptr;
-    if (test_env_uri) {
-      Status s = Env::LoadEnv(test_env_uri, &base_env, &env_guard_);
-      EXPECT_OK(s);
-      EXPECT_NE(Env::Default(), base_env);
-    } else {
-      base_env = Env::Default();
-    }
-    EXPECT_NE(nullptr, base_env);
-    if (getenv("MEM_ENV")) {
-      mem_env_ = new MockEnv(base_env);
-    }
-    env_ = mem_env_ ? mem_env_ : base_env;
-
-    fs_ = std::make_shared<LegacyFileSystemWrapper>(env_);
     EXPECT_OK(env_->CreateDirIfMissing(dbname_));
 
     db_options_.env = env_;
@@ -650,38 +628,23 @@ class VersionSetTestBase {
     versions_.reset(new VersionSet(dbname_, &db_options_, env_options_,
                                    table_cache_.get(), &write_buffer_manager_,
                                    &write_controller_,
-                                   /*block_cache_tracer=*/nullptr));
-    reactive_versions_ = std::make_shared<ReactiveVersionSet>(
-        dbname_, &db_options_, env_options_, table_cache_.get(),
-        &write_buffer_manager_, &write_controller_);
+                                   /*block_cache_tracer=*/nullptr)),
+        reactive_versions_ = std::make_shared<ReactiveVersionSet>(
+            dbname_, &db_options_, env_options_, table_cache_.get(),
+            &write_buffer_manager_, &write_controller_);
     db_options_.db_paths.emplace_back(dbname_,
                                       std::numeric_limits<uint64_t>::max());
   }
 
-  virtual ~VersionSetTestBase() {
-    if (getenv("KEEP_DB")) {
-      fprintf(stdout, "DB is still at %s\n", dbname_.c_str());
-    } else {
-      Options options;
-      options.env = env_;
-      EXPECT_OK(DestroyDB(dbname_, options));
-    }
-    if (mem_env_) {
-      delete mem_env_;
-      mem_env_ = nullptr;
-    }
-  }
-
- protected:
-  virtual void PrepareManifest(
-      std::vector<ColumnFamilyDescriptor>* column_families,
-      SequenceNumber* last_seqno, std::unique_ptr<log::Writer>* log_writer) {
+  void PrepareManifest(std::vector<ColumnFamilyDescriptor>* column_families,
+                       SequenceNumber* last_seqno,
+                       std::unique_ptr<log::Writer>* log_writer) {
     assert(column_families != nullptr);
     assert(last_seqno != nullptr);
     assert(log_writer != nullptr);
     VersionEdit new_db;
     if (db_options_.write_dbid_to_manifest) {
-      std::unique_ptr<DBImpl> impl(new DBImpl(DBOptions(), dbname_));
+      DBImpl* impl = new DBImpl(DBOptions(), dbname_);
       std::string db_id;
       impl->GetDbIdentityFromIdentityFile(&db_id);
       new_db.SetDBId(db_id);
@@ -752,25 +715,12 @@ class VersionSetTestBase {
               versions_->GetColumnFamilySet()->NumberOfColumnFamilies());
   }
 
-  void VerifyManifest(std::string* manifest_path) const {
-    assert(manifest_path != nullptr);
-    uint64_t manifest_file_number = 0;
-    Status s = versions_->GetCurrentManifestPath(
-        dbname_, fs_.get(), manifest_path, &manifest_file_number);
-    ASSERT_OK(s);
-    ASSERT_EQ(1, manifest_file_number);
-  }
-
-  MockEnv* mem_env_;
   Env* env_;
-  std::shared_ptr<Env> env_guard_;
   std::shared_ptr<FileSystem> fs_;
   const std::string dbname_;
   EnvOptions env_options_;
-  Options options_;
   ImmutableDBOptions db_options_;
   ColumnFamilyOptions cf_options_;
-  ImmutableCFOptions immutable_cf_options_;
   MutableCFOptions mutable_cf_options_;
   std::shared_ptr<Cache> table_cache_;
   WriteController write_controller_;
@@ -788,7 +738,7 @@ const std::string VersionSetTestBase::kColumnFamilyName3 = "charles";
 
 class VersionSetTest : public VersionSetTestBase, public testing::Test {
  public:
-  VersionSetTest() : VersionSetTestBase("version_set_test") {}
+  VersionSetTest() : VersionSetTestBase() {}
 };
 
 TEST_F(VersionSetTest, SameColumnFamilyGroupCommit) {
@@ -830,8 +780,7 @@ TEST_F(VersionSetTest, SameColumnFamilyGroupCommit) {
 class VersionSetAtomicGroupTest : public VersionSetTestBase,
                                   public testing::Test {
  public:
-  VersionSetAtomicGroupTest()
-      : VersionSetTestBase("version_set_atomic_group_test") {}
+  VersionSetAtomicGroupTest() : VersionSetTestBase() {}
 
   void SetUp() override {
     PrepareManifest(&column_families_, &last_seqno_, &log_writer_);
@@ -1215,8 +1164,7 @@ TEST_F(VersionSetAtomicGroupTest,
 class VersionSetTestDropOneCF : public VersionSetTestBase,
                                 public testing::TestWithParam<std::string> {
  public:
-  VersionSetTestDropOneCF()
-      : VersionSetTestBase("version_set_test_drop_one_cf") {}
+  VersionSetTestDropOneCF() : VersionSetTestBase() {}
 };
 
 // This test simulates the following execution sequence
@@ -1331,706 +1279,6 @@ INSTANTIATE_TEST_CASE_P(
     testing::Values(VersionSetTestBase::kColumnFamilyName1,
                     VersionSetTestBase::kColumnFamilyName2,
                     VersionSetTestBase::kColumnFamilyName3));
-
-class EmptyDefaultCfNewManifest : public VersionSetTestBase,
-                                  public testing::Test {
- public:
-  EmptyDefaultCfNewManifest() : VersionSetTestBase("version_set_new_db_test") {}
-  // Emulate DBImpl::NewDB()
-  void PrepareManifest(std::vector<ColumnFamilyDescriptor>* /*column_families*/,
-                       SequenceNumber* /*last_seqno*/,
-                       std::unique_ptr<log::Writer>* log_writer) override {
-    assert(log_writer != nullptr);
-    VersionEdit new_db;
-    new_db.SetLogNumber(0);
-    std::unique_ptr<WritableFile> file;
-    const std::string manifest_path = DescriptorFileName(dbname_, 1);
-    Status s = env_->NewWritableFile(
-        manifest_path, &file, env_->OptimizeForManifestWrite(env_options_));
-    ASSERT_OK(s);
-    std::unique_ptr<WritableFileWriter> file_writer(
-        new WritableFileWriter(NewLegacyWritableFileWrapper(std::move(file)),
-                               manifest_path, env_options_));
-    log_writer->reset(new log::Writer(std::move(file_writer), 0, true));
-    std::string record;
-    ASSERT_TRUE(new_db.EncodeTo(&record));
-    s = (*log_writer)->AddRecord(record);
-    ASSERT_OK(s);
-    // Create new column family
-    VersionEdit new_cf;
-    new_cf.AddColumnFamily(VersionSetTestBase::kColumnFamilyName1);
-    new_cf.SetColumnFamily(1);
-    new_cf.SetLastSequence(2);
-    new_cf.SetNextFile(2);
-    record.clear();
-    ASSERT_TRUE(new_cf.EncodeTo(&record));
-    s = (*log_writer)->AddRecord(record);
-    ASSERT_OK(s);
-  }
-
- protected:
-  bool write_dbid_to_manifest_ = false;
-  std::unique_ptr<log::Writer> log_writer_;
-};
-
-// Create db, create column family. Cf creation will switch to a new MANIFEST.
-// Then reopen db, trying to recover.
-TEST_F(EmptyDefaultCfNewManifest, Recover) {
-  PrepareManifest(nullptr, nullptr, &log_writer_);
-  log_writer_.reset();
-  Status s = SetCurrentFile(env_, dbname_, 1, /*directory_to_fsync=*/nullptr);
-  ASSERT_OK(s);
-  std::string manifest_path;
-  VerifyManifest(&manifest_path);
-  std::vector<ColumnFamilyDescriptor> column_families;
-  column_families.emplace_back(kDefaultColumnFamilyName, cf_options_);
-  column_families.emplace_back(VersionSetTestBase::kColumnFamilyName1,
-                               cf_options_);
-  std::string db_id;
-  bool has_missing_table_file = false;
-  s = versions_->TryRecoverFromOneManifest(
-      manifest_path, column_families, false, &db_id, &has_missing_table_file);
-  ASSERT_OK(s);
-  ASSERT_FALSE(has_missing_table_file);
-}
-
-class VersionSetTestEmptyDb
-    : public VersionSetTestBase,
-      public testing::TestWithParam<
-          std::tuple<bool, bool, std::vector<std::string>>> {
- public:
-  static const std::string kUnknownColumnFamilyName;
-  VersionSetTestEmptyDb() : VersionSetTestBase("version_set_test_empty_db") {}
-
- protected:
-  void PrepareManifest(std::vector<ColumnFamilyDescriptor>* /*column_families*/,
-                       SequenceNumber* /*last_seqno*/,
-                       std::unique_ptr<log::Writer>* log_writer) override {
-    assert(nullptr != log_writer);
-    VersionEdit new_db;
-    if (db_options_.write_dbid_to_manifest) {
-      std::unique_ptr<DBImpl> impl(new DBImpl(DBOptions(), dbname_));
-      std::string db_id;
-      impl->GetDbIdentityFromIdentityFile(&db_id);
-      new_db.SetDBId(db_id);
-    }
-    const std::string manifest_path = DescriptorFileName(dbname_, 1);
-    std::unique_ptr<WritableFile> file;
-    Status s = env_->NewWritableFile(
-        manifest_path, &file, env_->OptimizeForManifestWrite(env_options_));
-    ASSERT_OK(s);
-    std::unique_ptr<WritableFileWriter> file_writer(
-        new WritableFileWriter(NewLegacyWritableFileWrapper(std::move(file)),
-                               manifest_path, env_options_));
-    {
-      log_writer->reset(new log::Writer(std::move(file_writer), 0, false));
-      std::string record;
-      new_db.EncodeTo(&record);
-      s = (*log_writer)->AddRecord(record);
-      ASSERT_OK(s);
-    }
-  }
-
-  std::unique_ptr<log::Writer> log_writer_;
-};
-
-const std::string VersionSetTestEmptyDb::kUnknownColumnFamilyName = "unknown";
-
-TEST_P(VersionSetTestEmptyDb, OpenFromIncompleteManifest0) {
-  db_options_.write_dbid_to_manifest = std::get<0>(GetParam());
-  PrepareManifest(nullptr, nullptr, &log_writer_);
-  log_writer_.reset();
-  Status s = SetCurrentFile(env_, dbname_, 1, /*directory_to_fsync=*/nullptr);
-  ASSERT_OK(s);
-
-  std::string manifest_path;
-  VerifyManifest(&manifest_path);
-
-  bool read_only = std::get<1>(GetParam());
-  const std::vector<std::string> cf_names = std::get<2>(GetParam());
-
-  std::vector<ColumnFamilyDescriptor> column_families;
-  for (const auto& cf_name : cf_names) {
-    column_families.emplace_back(cf_name, cf_options_);
-  }
-
-  std::string db_id;
-  bool has_missing_table_file = false;
-  s = versions_->TryRecoverFromOneManifest(manifest_path, column_families,
-                                           read_only, &db_id,
-                                           &has_missing_table_file);
-  auto iter =
-      std::find(cf_names.begin(), cf_names.end(), kDefaultColumnFamilyName);
-  if (iter == cf_names.end()) {
-    ASSERT_TRUE(s.IsInvalidArgument());
-  } else {
-    ASSERT_TRUE(s.IsCorruption());
-  }
-}
-
-TEST_P(VersionSetTestEmptyDb, OpenFromIncompleteManifest1) {
-  db_options_.write_dbid_to_manifest = std::get<0>(GetParam());
-  PrepareManifest(nullptr, nullptr, &log_writer_);
-  // Only a subset of column families in the MANIFEST.
-  VersionEdit new_cf1;
-  new_cf1.AddColumnFamily(VersionSetTestBase::kColumnFamilyName1);
-  new_cf1.SetColumnFamily(1);
-  Status s;
-  {
-    std::string record;
-    new_cf1.EncodeTo(&record);
-    s = log_writer_->AddRecord(record);
-    ASSERT_OK(s);
-  }
-  log_writer_.reset();
-  s = SetCurrentFile(env_, dbname_, 1, /*directory_to_fsync=*/nullptr);
-  ASSERT_OK(s);
-
-  std::string manifest_path;
-  VerifyManifest(&manifest_path);
-
-  bool read_only = std::get<1>(GetParam());
-  const std::vector<std::string>& cf_names = std::get<2>(GetParam());
-  std::vector<ColumnFamilyDescriptor> column_families;
-  for (const auto& cf_name : cf_names) {
-    column_families.emplace_back(cf_name, cf_options_);
-  }
-  std::string db_id;
-  bool has_missing_table_file = false;
-  s = versions_->TryRecoverFromOneManifest(manifest_path, column_families,
-                                           read_only, &db_id,
-                                           &has_missing_table_file);
-  auto iter =
-      std::find(cf_names.begin(), cf_names.end(), kDefaultColumnFamilyName);
-  if (iter == cf_names.end()) {
-    ASSERT_TRUE(s.IsInvalidArgument());
-  } else {
-    ASSERT_TRUE(s.IsCorruption());
-  }
-}
-
-TEST_P(VersionSetTestEmptyDb, OpenFromInCompleteManifest2) {
-  db_options_.write_dbid_to_manifest = std::get<0>(GetParam());
-  PrepareManifest(nullptr, nullptr, &log_writer_);
-  // Write all column families but no log_number, next_file_number and
-  // last_sequence.
-  const std::vector<std::string> all_cf_names = {
-      kDefaultColumnFamilyName, kColumnFamilyName1, kColumnFamilyName2,
-      kColumnFamilyName3};
-  uint32_t cf_id = 1;
-  Status s;
-  for (size_t i = 1; i != all_cf_names.size(); ++i) {
-    VersionEdit new_cf;
-    new_cf.AddColumnFamily(all_cf_names[i]);
-    new_cf.SetColumnFamily(cf_id++);
-    std::string record;
-    ASSERT_TRUE(new_cf.EncodeTo(&record));
-    s = log_writer_->AddRecord(record);
-    ASSERT_OK(s);
-  }
-  log_writer_.reset();
-  s = SetCurrentFile(env_, dbname_, 1, /*directory_to_fsync=*/nullptr);
-  ASSERT_OK(s);
-
-  std::string manifest_path;
-  VerifyManifest(&manifest_path);
-
-  bool read_only = std::get<1>(GetParam());
-  const std::vector<std::string>& cf_names = std::get<2>(GetParam());
-  std::vector<ColumnFamilyDescriptor> column_families;
-  for (const auto& cf_name : cf_names) {
-    column_families.emplace_back(cf_name, cf_options_);
-  }
-  std::string db_id;
-  bool has_missing_table_file = false;
-  s = versions_->TryRecoverFromOneManifest(manifest_path, column_families,
-                                           read_only, &db_id,
-                                           &has_missing_table_file);
-  auto iter =
-      std::find(cf_names.begin(), cf_names.end(), kDefaultColumnFamilyName);
-  if (iter == cf_names.end()) {
-    ASSERT_TRUE(s.IsInvalidArgument());
-  } else {
-    ASSERT_TRUE(s.IsCorruption());
-  }
-}
-
-TEST_P(VersionSetTestEmptyDb, OpenManifestWithUnknownCF) {
-  db_options_.write_dbid_to_manifest = std::get<0>(GetParam());
-  PrepareManifest(nullptr, nullptr, &log_writer_);
-  // Write all column families but no log_number, next_file_number and
-  // last_sequence.
-  const std::vector<std::string> all_cf_names = {
-      kDefaultColumnFamilyName, kColumnFamilyName1, kColumnFamilyName2,
-      kColumnFamilyName3};
-  uint32_t cf_id = 1;
-  Status s;
-  for (size_t i = 1; i != all_cf_names.size(); ++i) {
-    VersionEdit new_cf;
-    new_cf.AddColumnFamily(all_cf_names[i]);
-    new_cf.SetColumnFamily(cf_id++);
-    std::string record;
-    ASSERT_TRUE(new_cf.EncodeTo(&record));
-    s = log_writer_->AddRecord(record);
-    ASSERT_OK(s);
-  }
-  {
-    VersionEdit tmp_edit;
-    tmp_edit.SetColumnFamily(4);
-    tmp_edit.SetLogNumber(0);
-    tmp_edit.SetNextFile(2);
-    tmp_edit.SetLastSequence(0);
-    std::string record;
-    ASSERT_TRUE(tmp_edit.EncodeTo(&record));
-    s = log_writer_->AddRecord(record);
-    ASSERT_OK(s);
-  }
-  log_writer_.reset();
-  s = SetCurrentFile(env_, dbname_, 1, /*directory_to_fsync=*/nullptr);
-  ASSERT_OK(s);
-
-  std::string manifest_path;
-  VerifyManifest(&manifest_path);
-
-  bool read_only = std::get<1>(GetParam());
-  const std::vector<std::string>& cf_names = std::get<2>(GetParam());
-  std::vector<ColumnFamilyDescriptor> column_families;
-  for (const auto& cf_name : cf_names) {
-    column_families.emplace_back(cf_name, cf_options_);
-  }
-  std::string db_id;
-  bool has_missing_table_file = false;
-  s = versions_->TryRecoverFromOneManifest(manifest_path, column_families,
-                                           read_only, &db_id,
-                                           &has_missing_table_file);
-  auto iter =
-      std::find(cf_names.begin(), cf_names.end(), kDefaultColumnFamilyName);
-  if (iter == cf_names.end()) {
-    ASSERT_TRUE(s.IsInvalidArgument());
-  } else {
-    ASSERT_TRUE(s.IsCorruption());
-  }
-}
-
-TEST_P(VersionSetTestEmptyDb, OpenCompleteManifest) {
-  db_options_.write_dbid_to_manifest = std::get<0>(GetParam());
-  PrepareManifest(nullptr, nullptr, &log_writer_);
-  // Write all column families but no log_number, next_file_number and
-  // last_sequence.
-  const std::vector<std::string> all_cf_names = {
-      kDefaultColumnFamilyName, kColumnFamilyName1, kColumnFamilyName2,
-      kColumnFamilyName3};
-  uint32_t cf_id = 1;
-  Status s;
-  for (size_t i = 1; i != all_cf_names.size(); ++i) {
-    VersionEdit new_cf;
-    new_cf.AddColumnFamily(all_cf_names[i]);
-    new_cf.SetColumnFamily(cf_id++);
-    std::string record;
-    ASSERT_TRUE(new_cf.EncodeTo(&record));
-    s = log_writer_->AddRecord(record);
-    ASSERT_OK(s);
-  }
-  {
-    VersionEdit tmp_edit;
-    tmp_edit.SetLogNumber(0);
-    tmp_edit.SetNextFile(2);
-    tmp_edit.SetLastSequence(0);
-    std::string record;
-    ASSERT_TRUE(tmp_edit.EncodeTo(&record));
-    s = log_writer_->AddRecord(record);
-    ASSERT_OK(s);
-  }
-  log_writer_.reset();
-  s = SetCurrentFile(env_, dbname_, 1, /*directory_to_fsync=*/nullptr);
-  ASSERT_OK(s);
-
-  std::string manifest_path;
-  VerifyManifest(&manifest_path);
-
-  bool read_only = std::get<1>(GetParam());
-  const std::vector<std::string>& cf_names = std::get<2>(GetParam());
-  std::vector<ColumnFamilyDescriptor> column_families;
-  for (const auto& cf_name : cf_names) {
-    column_families.emplace_back(cf_name, cf_options_);
-  }
-  std::string db_id;
-  bool has_missing_table_file = false;
-  s = versions_->TryRecoverFromOneManifest(manifest_path, column_families,
-                                           read_only, &db_id,
-                                           &has_missing_table_file);
-  auto iter =
-      std::find(cf_names.begin(), cf_names.end(), kDefaultColumnFamilyName);
-  if (iter == cf_names.end()) {
-    ASSERT_TRUE(s.IsInvalidArgument());
-  } else if (read_only) {
-    ASSERT_OK(s);
-    ASSERT_FALSE(has_missing_table_file);
-  } else if (cf_names.size() == all_cf_names.size()) {
-    ASSERT_OK(s);
-    ASSERT_FALSE(has_missing_table_file);
-  } else if (cf_names.size() < all_cf_names.size()) {
-    ASSERT_TRUE(s.IsInvalidArgument());
-  } else {
-    ASSERT_OK(s);
-    ASSERT_FALSE(has_missing_table_file);
-    ColumnFamilyData* cfd = versions_->GetColumnFamilySet()->GetColumnFamily(
-        kUnknownColumnFamilyName);
-    ASSERT_EQ(nullptr, cfd);
-  }
-}
-
-INSTANTIATE_TEST_CASE_P(
-    BestEffortRecovery, VersionSetTestEmptyDb,
-    testing::Combine(
-        /*write_dbid_to_manifest=*/testing::Bool(),
-        /*read_only=*/testing::Bool(),
-        /*cf_names=*/
-        testing::Values(
-            std::vector<std::string>(),
-            std::vector<std::string>({kDefaultColumnFamilyName}),
-            std::vector<std::string>({VersionSetTestBase::kColumnFamilyName1,
-                                      VersionSetTestBase::kColumnFamilyName2,
-                                      VersionSetTestBase::kColumnFamilyName3}),
-            std::vector<std::string>({kDefaultColumnFamilyName,
-                                      VersionSetTestBase::kColumnFamilyName1}),
-            std::vector<std::string>({kDefaultColumnFamilyName,
-                                      VersionSetTestBase::kColumnFamilyName1,
-                                      VersionSetTestBase::kColumnFamilyName2,
-                                      VersionSetTestBase::kColumnFamilyName3}),
-            std::vector<std::string>(
-                {kDefaultColumnFamilyName,
-                 VersionSetTestBase::kColumnFamilyName1,
-                 VersionSetTestBase::kColumnFamilyName2,
-                 VersionSetTestBase::kColumnFamilyName3,
-                 VersionSetTestEmptyDb::kUnknownColumnFamilyName}))));
-
-class VersionSetTestMissingFiles : public VersionSetTestBase,
-                                   public testing::Test {
- public:
-  VersionSetTestMissingFiles()
-      : VersionSetTestBase("version_set_test_missing_files"),
-        block_based_table_options_(),
-        table_factory_(std::make_shared<BlockBasedTableFactory>(
-            block_based_table_options_)),
-        internal_comparator_(
-            std::make_shared<InternalKeyComparator>(options_.comparator)) {}
-
- protected:
-  void PrepareManifest(std::vector<ColumnFamilyDescriptor>* column_families,
-                       SequenceNumber* last_seqno,
-                       std::unique_ptr<log::Writer>* log_writer) override {
-    assert(column_families != nullptr);
-    assert(last_seqno != nullptr);
-    assert(log_writer != nullptr);
-    const std::string manifest = DescriptorFileName(dbname_, 1);
-    std::unique_ptr<WritableFile> file;
-    Status s = env_->NewWritableFile(
-        manifest, &file, env_->OptimizeForManifestWrite(env_options_));
-    ASSERT_OK(s);
-    std::unique_ptr<WritableFileWriter> file_writer(new WritableFileWriter(
-        NewLegacyWritableFileWrapper(std::move(file)), manifest, env_options_));
-    log_writer->reset(new log::Writer(std::move(file_writer), 0, false));
-    VersionEdit new_db;
-    if (db_options_.write_dbid_to_manifest) {
-      std::unique_ptr<DBImpl> impl(new DBImpl(DBOptions(), dbname_));
-      std::string db_id;
-      impl->GetDbIdentityFromIdentityFile(&db_id);
-      new_db.SetDBId(db_id);
-    }
-    {
-      std::string record;
-      ASSERT_TRUE(new_db.EncodeTo(&record));
-      s = (*log_writer)->AddRecord(record);
-      ASSERT_OK(s);
-    }
-    const std::vector<std::string> cf_names = {
-        kDefaultColumnFamilyName, kColumnFamilyName1, kColumnFamilyName2,
-        kColumnFamilyName3};
-    uint32_t cf_id = 1;  // default cf id is 0
-    cf_options_.table_factory = table_factory_;
-    for (const auto& cf_name : cf_names) {
-      column_families->emplace_back(cf_name, cf_options_);
-      if (cf_name == kDefaultColumnFamilyName) {
-        continue;
-      }
-      VersionEdit new_cf;
-      new_cf.AddColumnFamily(cf_name);
-      new_cf.SetColumnFamily(cf_id);
-      std::string record;
-      ASSERT_TRUE(new_cf.EncodeTo(&record));
-      s = (*log_writer)->AddRecord(record);
-      ASSERT_OK(s);
-
-      VersionEdit cf_files;
-      cf_files.SetColumnFamily(cf_id);
-      cf_files.SetLogNumber(0);
-      record.clear();
-      ASSERT_TRUE(cf_files.EncodeTo(&record));
-      s = (*log_writer)->AddRecord(record);
-      ASSERT_OK(s);
-      ++cf_id;
-    }
-    SequenceNumber seq = 2;
-    {
-      VersionEdit edit;
-      edit.SetNextFile(7);
-      edit.SetLastSequence(seq);
-      std::string record;
-      ASSERT_TRUE(edit.EncodeTo(&record));
-      s = (*log_writer)->AddRecord(record);
-      ASSERT_OK(s);
-    }
-    *last_seqno = seq + 1;
-  }
-
-  struct SstInfo {
-    uint64_t file_number;
-    std::string column_family;
-    std::string key;  // the only key
-    int level = 0;
-    SstInfo(uint64_t file_num, const std::string& cf_name,
-            const std::string& _key)
-        : SstInfo(file_num, cf_name, _key, 0) {}
-    SstInfo(uint64_t file_num, const std::string& cf_name,
-            const std::string& _key, int lvl)
-        : file_number(file_num),
-          column_family(cf_name),
-          key(_key),
-          level(lvl) {}
-  };
-
-  // Create dummy sst, return their metadata. Note that only file name and size
-  // are used.
-  void CreateDummyTableFiles(const std::vector<SstInfo>& file_infos,
-                             std::vector<FileMetaData>* file_metas) {
-    assert(file_metas != nullptr);
-    for (const auto& info : file_infos) {
-      uint64_t file_num = info.file_number;
-      std::string fname = MakeTableFileName(dbname_, file_num);
-      std::unique_ptr<FSWritableFile> file;
-      Status s = fs_->NewWritableFile(fname, FileOptions(), &file, nullptr);
-      ASSERT_OK(s);
-      std::unique_ptr<WritableFileWriter> fwriter(
-          new WritableFileWriter(std::move(file), fname, FileOptions(), env_));
-      std::vector<std::unique_ptr<IntTblPropCollectorFactory>>
-          int_tbl_prop_collector_factories;
-
-      std::unique_ptr<TableBuilder> builder(table_factory_->NewTableBuilder(
-          TableBuilderOptions(
-              immutable_cf_options_, mutable_cf_options_, *internal_comparator_,
-              &int_tbl_prop_collector_factories, kNoCompression,
-              /*_sample_for_compression=*/0, CompressionOptions(),
-              /*_skip_filters=*/false, info.column_family, info.level),
-          TablePropertiesCollectorFactory::Context::kUnknownColumnFamily,
-          fwriter.get()));
-      InternalKey ikey(info.key, 0, ValueType::kTypeValue);
-      builder->Add(ikey.Encode(), "value");
-      ASSERT_OK(builder->Finish());
-      fwriter->Flush();
-      uint64_t file_size = 0;
-      s = fs_->GetFileSize(fname, IOOptions(), &file_size, nullptr);
-      ASSERT_OK(s);
-      ASSERT_NE(0, file_size);
-      FileMetaData meta;
-      meta = FileMetaData(file_num, /*file_path_id=*/0, file_size, ikey, ikey,
-                          0, 0, false, 0, 0, 0, kUnknownFileChecksum,
-                          kUnknownFileChecksumFuncName);
-      file_metas->emplace_back(meta);
-    }
-  }
-
-  // This method updates last_sequence_.
-  void WriteFileAdditionAndDeletionToManifest(
-      uint32_t cf, const std::vector<std::pair<int, FileMetaData>>& added_files,
-      const std::vector<std::pair<int, uint64_t>>& deleted_files) {
-    VersionEdit edit;
-    edit.SetColumnFamily(cf);
-    for (const auto& elem : added_files) {
-      int level = elem.first;
-      edit.AddFile(level, elem.second);
-    }
-    for (const auto& elem : deleted_files) {
-      int level = elem.first;
-      edit.DeleteFile(level, elem.second);
-    }
-    edit.SetLastSequence(last_seqno_);
-    ++last_seqno_;
-    assert(log_writer_.get() != nullptr);
-    std::string record;
-    ASSERT_TRUE(edit.EncodeTo(&record));
-    Status s = log_writer_->AddRecord(record);
-    ASSERT_OK(s);
-  }
-
-  BlockBasedTableOptions block_based_table_options_;
-  std::shared_ptr<TableFactory> table_factory_;
-  std::shared_ptr<InternalKeyComparator> internal_comparator_;
-  std::vector<ColumnFamilyDescriptor> column_families_;
-  SequenceNumber last_seqno_;
-  std::unique_ptr<log::Writer> log_writer_;
-};
-
-TEST_F(VersionSetTestMissingFiles, ManifestFarBehindSst) {
-  std::vector<SstInfo> existing_files = {
-      SstInfo(100, kDefaultColumnFamilyName, "a"),
-      SstInfo(102, kDefaultColumnFamilyName, "b"),
-      SstInfo(103, kDefaultColumnFamilyName, "c"),
-      SstInfo(107, kDefaultColumnFamilyName, "d"),
-      SstInfo(110, kDefaultColumnFamilyName, "e")};
-  std::vector<FileMetaData> file_metas;
-  CreateDummyTableFiles(existing_files, &file_metas);
-
-  PrepareManifest(&column_families_, &last_seqno_, &log_writer_);
-  std::vector<std::pair<int, FileMetaData>> added_files;
-  for (uint64_t file_num = 10; file_num < 15; ++file_num) {
-    std::string smallest_ukey = "a";
-    std::string largest_ukey = "b";
-    InternalKey smallest_ikey(smallest_ukey, 1, ValueType::kTypeValue);
-    InternalKey largest_ikey(largest_ukey, 1, ValueType::kTypeValue);
-    FileMetaData meta =
-        FileMetaData(file_num, /*file_path_id=*/0, /*file_size=*/12,
-                     smallest_ikey, largest_ikey, 0, 0, false, 0, 0, 0,
-                     kUnknownFileChecksum, kUnknownFileChecksumFuncName);
-    added_files.emplace_back(0, meta);
-  }
-  WriteFileAdditionAndDeletionToManifest(
-      /*cf=*/0, added_files, std::vector<std::pair<int, uint64_t>>());
-  std::vector<std::pair<int, uint64_t>> deleted_files;
-  deleted_files.emplace_back(0, 10);
-  WriteFileAdditionAndDeletionToManifest(
-      /*cf=*/0, std::vector<std::pair<int, FileMetaData>>(), deleted_files);
-  log_writer_.reset();
-  Status s = SetCurrentFile(env_, dbname_, 1, nullptr);
-  ASSERT_OK(s);
-  std::string manifest_path;
-  VerifyManifest(&manifest_path);
-  std::string db_id;
-  bool has_missing_table_file = false;
-  s = versions_->TryRecoverFromOneManifest(manifest_path, column_families_,
-                                           /*read_only=*/false, &db_id,
-                                           &has_missing_table_file);
-  ASSERT_OK(s);
-  ASSERT_TRUE(has_missing_table_file);
-  for (ColumnFamilyData* cfd : *(versions_->GetColumnFamilySet())) {
-    VersionStorageInfo* vstorage = cfd->current()->storage_info();
-    const std::vector<FileMetaData*>& files = vstorage->LevelFiles(0);
-    ASSERT_TRUE(files.empty());
-  }
-}
-
-TEST_F(VersionSetTestMissingFiles, ManifestAheadofSst) {
-  std::vector<SstInfo> existing_files = {
-      SstInfo(100, kDefaultColumnFamilyName, "a"),
-      SstInfo(102, kDefaultColumnFamilyName, "b"),
-      SstInfo(103, kDefaultColumnFamilyName, "c"),
-      SstInfo(107, kDefaultColumnFamilyName, "d"),
-      SstInfo(110, kDefaultColumnFamilyName, "e")};
-  std::vector<FileMetaData> file_metas;
-  CreateDummyTableFiles(existing_files, &file_metas);
-
-  PrepareManifest(&column_families_, &last_seqno_, &log_writer_);
-  std::vector<std::pair<int, FileMetaData>> added_files;
-  for (size_t i = 3; i != 5; ++i) {
-    added_files.emplace_back(0, file_metas[i]);
-  }
-  WriteFileAdditionAndDeletionToManifest(
-      /*cf=*/0, added_files, std::vector<std::pair<int, uint64_t>>());
-
-  added_files.clear();
-  for (uint64_t file_num = 120; file_num < 130; ++file_num) {
-    std::string smallest_ukey = "a";
-    std::string largest_ukey = "b";
-    InternalKey smallest_ikey(smallest_ukey, 1, ValueType::kTypeValue);
-    InternalKey largest_ikey(largest_ukey, 1, ValueType::kTypeValue);
-    FileMetaData meta =
-        FileMetaData(file_num, /*file_path_id=*/0, /*file_size=*/12,
-                     smallest_ikey, largest_ikey, 0, 0, false, 0, 0, 0,
-                     kUnknownFileChecksum, kUnknownFileChecksumFuncName);
-    added_files.emplace_back(0, meta);
-  }
-  WriteFileAdditionAndDeletionToManifest(
-      /*cf=*/0, added_files, std::vector<std::pair<int, uint64_t>>());
-  log_writer_.reset();
-  Status s = SetCurrentFile(env_, dbname_, 1, nullptr);
-  ASSERT_OK(s);
-  std::string manifest_path;
-  VerifyManifest(&manifest_path);
-  std::string db_id;
-  bool has_missing_table_file = false;
-  s = versions_->TryRecoverFromOneManifest(manifest_path, column_families_,
-                                           /*read_only=*/false, &db_id,
-                                           &has_missing_table_file);
-  ASSERT_OK(s);
-  ASSERT_TRUE(has_missing_table_file);
-  for (ColumnFamilyData* cfd : *(versions_->GetColumnFamilySet())) {
-    VersionStorageInfo* vstorage = cfd->current()->storage_info();
-    const std::vector<FileMetaData*>& files = vstorage->LevelFiles(0);
-    if (cfd->GetName() == kDefaultColumnFamilyName) {
-      ASSERT_EQ(2, files.size());
-      for (const auto* fmeta : files) {
-        if (fmeta->fd.GetNumber() != 107 && fmeta->fd.GetNumber() != 110) {
-          ASSERT_FALSE(true);
-        }
-      }
-    } else {
-      ASSERT_TRUE(files.empty());
-    }
-  }
-}
-
-TEST_F(VersionSetTestMissingFiles, NoFileMissing) {
-  std::vector<SstInfo> existing_files = {
-      SstInfo(100, kDefaultColumnFamilyName, "a"),
-      SstInfo(102, kDefaultColumnFamilyName, "b"),
-      SstInfo(103, kDefaultColumnFamilyName, "c"),
-      SstInfo(107, kDefaultColumnFamilyName, "d"),
-      SstInfo(110, kDefaultColumnFamilyName, "e")};
-  std::vector<FileMetaData> file_metas;
-  CreateDummyTableFiles(existing_files, &file_metas);
-
-  PrepareManifest(&column_families_, &last_seqno_, &log_writer_);
-  std::vector<std::pair<int, FileMetaData>> added_files;
-  for (const auto& meta : file_metas) {
-    added_files.emplace_back(0, meta);
-  }
-  WriteFileAdditionAndDeletionToManifest(
-      /*cf=*/0, added_files, std::vector<std::pair<int, uint64_t>>());
-  std::vector<std::pair<int, uint64_t>> deleted_files;
-  deleted_files.emplace_back(/*level=*/0, 100);
-  WriteFileAdditionAndDeletionToManifest(
-      /*cf=*/0, std::vector<std::pair<int, FileMetaData>>(), deleted_files);
-  log_writer_.reset();
-  Status s = SetCurrentFile(env_, dbname_, 1, nullptr);
-  ASSERT_OK(s);
-  std::string manifest_path;
-  VerifyManifest(&manifest_path);
-  std::string db_id;
-  bool has_missing_table_file = false;
-  s = versions_->TryRecoverFromOneManifest(manifest_path, column_families_,
-                                           /*read_only=*/false, &db_id,
-                                           &has_missing_table_file);
-  ASSERT_OK(s);
-  ASSERT_FALSE(has_missing_table_file);
-  for (ColumnFamilyData* cfd : *(versions_->GetColumnFamilySet())) {
-    VersionStorageInfo* vstorage = cfd->current()->storage_info();
-    const std::vector<FileMetaData*>& files = vstorage->LevelFiles(0);
-    if (cfd->GetName() == kDefaultColumnFamilyName) {
-      ASSERT_EQ(existing_files.size() - deleted_files.size(), files.size());
-      bool has_deleted_file = false;
-      for (const auto* fmeta : files) {
-        if (fmeta->fd.GetNumber() == 100) {
-          has_deleted_file = true;
-          break;
-        }
-      }
-      ASSERT_FALSE(has_deleted_file);
-    } else {
-      ASSERT_TRUE(files.empty());
-    }
-  }
-}
-
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

defs.bzl

@@ -3,7 +3,6 @@
 # defs.bzl - Definitions for Facebook-specific buck build integration
 # in TARGETS
 
-load("@fbcode_macros//build_defs:coverage.bzl", "coverage")
 load("@fbcode_macros//build_defs:cpp_binary.bzl", "cpp_binary")
 load("@fbcode_macros//build_defs:custom_unittest.bzl", "custom_unittest")
 
@@ -38,19 +37,9 @@ def test_binary(
 
     binary_path = "$(location :{})".format(test_bin)
 
-    base_path = native.package_name()
-    tags = []
-    if coverage.is_coverage_enabled(base_path):
-        # This tag instructs testpilot to use
-        # the lower-memory coverage runner
-        # (e.g. it tells testpilot that the binary
-        # is actually instrumented with coverage info)
-        tags = ["coverage"]
-
     custom_unittest(
         name = test_name,
         command = [TEST_RUNNER, binary_path],
         type = ttype,
         env = {"BUCK_BASE_BINARY": binary_path},
-        tags = tags,
     )

env/composite_env_wrapper.h

@@ -291,18 +291,20 @@ class CompositeEnvWrapper : public Env {
  public:
   // Initialize a CompositeEnvWrapper that delegates all thread/time related
   // calls to env, and all file operations to fs
-  explicit CompositeEnvWrapper(Env* env, std::shared_ptr<FileSystem> fs)
-      : Env(fs), env_target_(env) {}
+  explicit CompositeEnvWrapper(Env* env, FileSystem* fs)
+      : env_target_(env), fs_env_target_(fs) {}
   ~CompositeEnvWrapper() {}
 
   // Return the target to which this Env forwards all calls
   Env* env_target() const { return env_target_; }
 
+  FileSystem* fs_env_target() const { return fs_env_target_; }
+
   Status RegisterDbPaths(const std::vector<std::string>& paths) override {
-    return file_system_->RegisterDbPaths(paths);
+    return fs_env_target_->RegisterDbPaths(paths);
   }
   Status UnregisterDbPaths(const std::vector<std::string>& paths) override {
-    return file_system_->UnregisterDbPaths(paths);
+    return fs_env_target_->UnregisterDbPaths(paths);
   }
 
   // The following text is boilerplate that forwards all methods to target()
@@ -313,7 +315,7 @@ class CompositeEnvWrapper : public Env {
     std::unique_ptr<FSSequentialFile> file;
     Status status;
     status =
-        file_system_->NewSequentialFile(f, FileOptions(options), &file, &dbg);
+        fs_env_target_->NewSequentialFile(f, FileOptions(options), &file, &dbg);
     if (status.ok()) {
       r->reset(new CompositeSequentialFileWrapper(file));
     }
@@ -325,8 +327,8 @@ class CompositeEnvWrapper : public Env {
     IODebugContext dbg;
     std::unique_ptr<FSRandomAccessFile> file;
     Status status;
-    status =
-        file_system_->NewRandomAccessFile(f, FileOptions(options), &file, &dbg);
+    status = fs_env_target_->NewRandomAccessFile(f, FileOptions(options), &file,
+                                                 &dbg);
     if (status.ok()) {
       r->reset(new CompositeRandomAccessFileWrapper(file));
     }
@@ -338,7 +340,7 @@ class CompositeEnvWrapper : public Env {
     std::unique_ptr<FSWritableFile> file;
     Status status;
     status =
-        file_system_->NewWritableFile(f, FileOptions(options), &file, &dbg);
+        fs_env_target_->NewWritableFile(f, FileOptions(options), &file, &dbg);
     if (status.ok()) {
       r->reset(new CompositeWritableFileWrapper(file));
     }
@@ -350,8 +352,8 @@ class CompositeEnvWrapper : public Env {
     IODebugContext dbg;
     Status status;
     std::unique_ptr<FSWritableFile> file;
-    status = file_system_->ReopenWritableFile(fname, FileOptions(options),
-                                              &file, &dbg);
+    status = fs_env_target_->ReopenWritableFile(fname, FileOptions(options),
+                                                &file, &dbg);
     if (status.ok()) {
       result->reset(new CompositeWritableFileWrapper(file));
     }
@@ -364,8 +366,8 @@ class CompositeEnvWrapper : public Env {
     IODebugContext dbg;
     Status status;
     std::unique_ptr<FSWritableFile> file;
-    status = file_system_->ReuseWritableFile(fname, old_fname,
-                                             FileOptions(options), &file, &dbg);
+    status = fs_env_target_->ReuseWritableFile(
+        fname, old_fname, FileOptions(options), &file, &dbg);
     if (status.ok()) {
       r->reset(new CompositeWritableFileWrapper(file));
     }
@@ -377,8 +379,8 @@ class CompositeEnvWrapper : public Env {
     IODebugContext dbg;
     std::unique_ptr<FSRandomRWFile> file;
     Status status;
-    status =
-        file_system_->NewRandomRWFile(fname, FileOptions(options), &file, &dbg);
+    status = fs_env_target_->NewRandomRWFile(fname, FileOptions(options), &file,
+                                             &dbg);
     if (status.ok()) {
       result->reset(new CompositeRandomRWFileWrapper(file));
     }
@@ -387,7 +389,7 @@ class CompositeEnvWrapper : public Env {
   Status NewMemoryMappedFileBuffer(
       const std::string& fname,
       std::unique_ptr<MemoryMappedFileBuffer>* result) override {
-    return file_system_->NewMemoryMappedFileBuffer(fname, result);
+    return fs_env_target_->NewMemoryMappedFileBuffer(fname, result);
   }
   Status NewDirectory(const std::string& name,
                       std::unique_ptr<Directory>* result) override {
@@ -395,7 +397,7 @@ class CompositeEnvWrapper : public Env {
     IODebugContext dbg;
     std::unique_ptr<FSDirectory> dir;
     Status status;
-    status = file_system_->NewDirectory(name, io_opts, &dir, &dbg);
+    status = fs_env_target_->NewDirectory(name, io_opts, &dir, &dbg);
     if (status.ok()) {
       result->reset(new CompositeDirectoryWrapper(dir));
     }
@@ -404,108 +406,102 @@ class CompositeEnvWrapper : public Env {
   Status FileExists(const std::string& f) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->FileExists(f, io_opts, &dbg);
+    return fs_env_target_->FileExists(f, io_opts, &dbg);
   }
   Status GetChildren(const std::string& dir,
                      std::vector<std::string>* r) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->GetChildren(dir, io_opts, r, &dbg);
+    return fs_env_target_->GetChildren(dir, io_opts, r, &dbg);
   }
   Status GetChildrenFileAttributes(
       const std::string& dir, std::vector<FileAttributes>* result) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->GetChildrenFileAttributes(dir, io_opts, result, &dbg);
+    return fs_env_target_->GetChildrenFileAttributes(dir, io_opts, result,
+                                                     &dbg);
   }
   Status DeleteFile(const std::string& f) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->DeleteFile(f, io_opts, &dbg);
+    return fs_env_target_->DeleteFile(f, io_opts, &dbg);
   }
   Status Truncate(const std::string& fname, size_t size) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->Truncate(fname, size, io_opts, &dbg);
+    return fs_env_target_->Truncate(fname, size, io_opts, &dbg);
   }
   Status CreateDir(const std::string& d) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->CreateDir(d, io_opts, &dbg);
+    return fs_env_target_->CreateDir(d, io_opts, &dbg);
   }
   Status CreateDirIfMissing(const std::string& d) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->CreateDirIfMissing(d, io_opts, &dbg);
+    return fs_env_target_->CreateDirIfMissing(d, io_opts, &dbg);
   }
   Status DeleteDir(const std::string& d) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->DeleteDir(d, io_opts, &dbg);
+    return fs_env_target_->DeleteDir(d, io_opts, &dbg);
   }
   Status GetFileSize(const std::string& f, uint64_t* s) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->GetFileSize(f, io_opts, s, &dbg);
+    return fs_env_target_->GetFileSize(f, io_opts, s, &dbg);
   }
 
   Status GetFileModificationTime(const std::string& fname,
                                  uint64_t* file_mtime) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->GetFileModificationTime(fname, io_opts, file_mtime,
-                                                 &dbg);
+    return fs_env_target_->GetFileModificationTime(fname, io_opts, file_mtime,
+                                                   &dbg);
   }
 
   Status RenameFile(const std::string& s, const std::string& t) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->RenameFile(s, t, io_opts, &dbg);
+    return fs_env_target_->RenameFile(s, t, io_opts, &dbg);
   }
 
   Status LinkFile(const std::string& s, const std::string& t) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->LinkFile(s, t, io_opts, &dbg);
+    return fs_env_target_->LinkFile(s, t, io_opts, &dbg);
   }
 
   Status NumFileLinks(const std::string& fname, uint64_t* count) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->NumFileLinks(fname, io_opts, count, &dbg);
+    return fs_env_target_->NumFileLinks(fname, io_opts, count, &dbg);
   }
 
   Status AreFilesSame(const std::string& first, const std::string& second,
                       bool* res) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->AreFilesSame(first, second, io_opts, res, &dbg);
+    return fs_env_target_->AreFilesSame(first, second, io_opts, res, &dbg);
   }
 
   Status LockFile(const std::string& f, FileLock** l) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->LockFile(f, io_opts, l, &dbg);
+    return fs_env_target_->LockFile(f, io_opts, l, &dbg);
   }
 
   Status UnlockFile(FileLock* l) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->UnlockFile(l, io_opts, &dbg);
+    return fs_env_target_->UnlockFile(l, io_opts, &dbg);
   }
 
   Status GetAbsolutePath(const std::string& db_path,
                          std::string* output_path) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->GetAbsolutePath(db_path, io_opts, output_path, &dbg);
-  }
-
-  Status NewLogger(const std::string& fname,
-                   std::shared_ptr<Logger>* result) override {
-    IOOptions io_opts;
-    IODebugContext dbg;
-    return file_system_->NewLogger(fname, io_opts, result, &dbg);
+    return fs_env_target_->GetAbsolutePath(db_path, io_opts, output_path, &dbg);
   }
 
 #if !defined(OS_WIN) && !defined(ROCKSDB_NO_DYNAMIC_EXTENSION)
@@ -535,6 +531,10 @@ class CompositeEnvWrapper : public Env {
   Status GetTestDirectory(std::string* path) override {
     return env_target_->GetTestDirectory(path);
   }
+  Status NewLogger(const std::string& fname,
+                   std::shared_ptr<Logger>* result) override {
+    return env_target_->NewLogger(fname, result);
+  }
   uint64_t NowMicros() override { return env_target_->NowMicros(); }
   uint64_t NowNanos() override { return env_target_->NowNanos(); }
   uint64_t NowCPUNanos() override { return env_target_->NowCPUNanos(); }
@@ -590,41 +590,46 @@ class CompositeEnvWrapper : public Env {
   }
 
   EnvOptions OptimizeForLogRead(const EnvOptions& env_options) const override {
-    return file_system_->OptimizeForLogRead(FileOptions(env_options));
+    return fs_env_target_->OptimizeForLogRead(FileOptions(env_options));
   }
   EnvOptions OptimizeForManifestRead(
       const EnvOptions& env_options) const override {
-    return file_system_->OptimizeForManifestRead(FileOptions(env_options));
+    return fs_env_target_->OptimizeForManifestRead(
+                                FileOptions(env_options));
   }
   EnvOptions OptimizeForLogWrite(const EnvOptions& env_options,
                                  const DBOptions& db_options) const override {
-    return file_system_->OptimizeForLogWrite(FileOptions(env_options),
-                                             db_options);
+    return fs_env_target_->OptimizeForLogWrite(FileOptions(env_options),
+                                               db_options);
   }
   EnvOptions OptimizeForManifestWrite(
       const EnvOptions& env_options) const override {
-    return file_system_->OptimizeForManifestWrite(FileOptions(env_options));
+    return fs_env_target_->OptimizeForManifestWrite(
+                                FileOptions(env_options));
   }
   EnvOptions OptimizeForCompactionTableWrite(
       const EnvOptions& env_options,
       const ImmutableDBOptions& immutable_ops) const override {
-    return file_system_->OptimizeForCompactionTableWrite(
-        FileOptions(env_options), immutable_ops);
+    return fs_env_target_->OptimizeForCompactionTableWrite(
+                                FileOptions(env_options),
+                                immutable_ops);
   }
   EnvOptions OptimizeForCompactionTableRead(
       const EnvOptions& env_options,
       const ImmutableDBOptions& db_options) const override {
-    return file_system_->OptimizeForCompactionTableRead(
-        FileOptions(env_options), db_options);
+    return fs_env_target_->OptimizeForCompactionTableRead(
+                                FileOptions(env_options),
+                                db_options);
   }
   Status GetFreeSpace(const std::string& path, uint64_t* diskfree) override {
     IOOptions io_opts;
     IODebugContext dbg;
-    return file_system_->GetFreeSpace(path, io_opts, diskfree, &dbg);
+    return fs_env_target_->GetFreeSpace(path, io_opts, diskfree, &dbg);
   }
 
  private:
   Env* env_target_;
+  FileSystem* fs_env_target_;
 };
 
 class LegacySequentialFileWrapper : public FSSequentialFile {
@@ -1062,10 +1067,6 @@ class LegacyFileSystemWrapper : public FileSystem {
     return status_to_io_status(target_->NewLogger(fname, result));
   }
 
-  void SanitizeFileOptions(FileOptions* opts) const override {
-    target_->SanitizeEnvOptions(opts);
-  }
-
   FileOptions OptimizeForLogRead(
                   const FileOptions& file_options) const override {
     return target_->OptimizeForLogRead(file_options);

env/env.cc

@@ -22,14 +22,6 @@
 
 namespace ROCKSDB_NAMESPACE {
 
-Env::Env() : thread_status_updater_(nullptr) {
-  file_system_ = std::make_shared<LegacyFileSystemWrapper>(this);
-}
-
-Env::Env(std::shared_ptr<FileSystem> fs)
-  : thread_status_updater_(nullptr),
-    file_system_(fs) {}
-
 Env::~Env() {
 }
 
@@ -480,14 +472,4 @@ Status NewEnvLogger(const std::string& fname, Env* env,
   return Status::OK();
 }
 
-const std::shared_ptr<FileSystem>& Env::GetFileSystem() const {
-  return file_system_;
-}
-
-#ifdef OS_WIN
-std::unique_ptr<Env> NewCompositeEnv(std::shared_ptr<FileSystem> fs) {
-  return std::unique_ptr<Env>(new CompositeEnvWrapper(Env::Default(), fs));
-}
-#endif
-
 }  // namespace ROCKSDB_NAMESPACE

env/env_posix.cc

@@ -128,25 +128,21 @@ class PosixDynamicLibrary : public DynamicLibrary {
 
 class PosixEnv : public CompositeEnvWrapper {
  public:
-  // This constructor is for constructing non-default Envs, mainly by
-  // NewCompositeEnv(). It allows new instances to share the same
-  // threadpool and other resources as the default Env, while allowing
-  // a non-default FileSystem implementation
-  PosixEnv(const PosixEnv* default_env, std::shared_ptr<FileSystem> fs);
+  PosixEnv();
 
   ~PosixEnv() override {
-    if (this == Env::Default()) {
-      for (const auto tid : threads_to_join_) {
-        pthread_join(tid, nullptr);
-      }
-      for (int pool_id = 0; pool_id < Env::Priority::TOTAL; ++pool_id) {
-        thread_pools_[pool_id].JoinAllThreads();
-      }
-      // Do not delete the thread_status_updater_ in order to avoid the
-      // free after use when Env::Default() is destructed while some other
-      // child threads are still trying to update thread status. All
-      // PosixEnv instances use the same thread_status_updater_, so never
-      // explicitly delete it.
+    for (const auto tid : threads_to_join_) {
+      pthread_join(tid, nullptr);
+    }
+    for (int pool_id = 0; pool_id < Env::Priority::TOTAL; ++pool_id) {
+      thread_pools_[pool_id].JoinAllThreads();
+    }
+    // Delete the thread_status_updater_ only when the current Env is not
+    // Env::Default().  This is to avoid the free-after-use error when
+    // Env::Default() is destructed while some other child threads are
+    // still trying to update thread status.
+    if (this != Env::Default()) {
+      delete thread_status_updater_;
     }
   }
 
@@ -256,6 +252,34 @@ class PosixEnv : public CompositeEnvWrapper {
 
   uint64_t GetThreadID() const override { return gettid(pthread_self()); }
 
+  Status NewLogger(const std::string& fname,
+                   std::shared_ptr<Logger>* result) override {
+    FILE* f;
+    {
+      IOSTATS_TIMER_GUARD(open_nanos);
+      f = fopen(fname.c_str(),
+                "w"
+#ifdef __GLIBC_PREREQ
+#if __GLIBC_PREREQ(2, 7)
+                "e"  // glibc extension to enable O_CLOEXEC
+#endif
+#endif
+      );
+    }
+    if (f == nullptr) {
+      result->reset();
+      return IOError("when fopen a file for new logger", fname, errno);
+    } else {
+      int fd = fileno(f);
+#ifdef ROCKSDB_FALLOCATE_PRESENT
+      fallocate(fd, FALLOC_FL_KEEP_SIZE, 0, 4 * 1024);
+#endif
+      SetFD_CLOEXEC(fd, nullptr);
+      result->reset(new PosixLogger(f, &PosixEnv::gettid, this));
+      return Status::OK();
+    }
+  }
+
   uint64_t NowMicros() override {
     struct timeval tv;
     gettimeofday(&tv, nullptr);
@@ -382,34 +406,18 @@ class PosixEnv : public CompositeEnvWrapper {
   }
 
  private:
-  friend Env* Env::Default();
-  // Constructs the default Env, a singleton
-  PosixEnv();
-
-  // The below 4 members are only used by the default PosixEnv instance.
-  // Non-default instances simply maintain references to the backing
-  // members in te default instance
-  std::vector<ThreadPoolImpl> thread_pools_storage_;
-  pthread_mutex_t mu_storage_;
-  std::vector<pthread_t> threads_to_join_storage_;
-  bool allow_non_owner_access_storage_;
-
-  std::vector<ThreadPoolImpl>& thread_pools_;
-  pthread_mutex_t& mu_;
-  std::vector<pthread_t>& threads_to_join_;
+  std::vector<ThreadPoolImpl> thread_pools_;
+  pthread_mutex_t mu_;
+  std::vector<pthread_t> threads_to_join_;
   // If true, allow non owner read access for db files. Otherwise, non-owner
   //  has no access to db files.
-  bool& allow_non_owner_access_;
+  bool allow_non_owner_access_;
 };
 
 PosixEnv::PosixEnv()
-    : CompositeEnvWrapper(this, FileSystem::Default()),
-      thread_pools_storage_(Priority::TOTAL),
-      allow_non_owner_access_storage_(true),
-      thread_pools_(thread_pools_storage_),
-      mu_(mu_storage_),
-      threads_to_join_(threads_to_join_storage_),
-      allow_non_owner_access_(allow_non_owner_access_storage_) {
+    : CompositeEnvWrapper(this, FileSystem::Default().get()),
+      thread_pools_(Priority::TOTAL),
+      allow_non_owner_access_(true) {
   ThreadPoolImpl::PthreadCall("mutex_init", pthread_mutex_init(&mu_, nullptr));
   for (int pool_id = 0; pool_id < Env::Priority::TOTAL; ++pool_id) {
     thread_pools_[pool_id].SetThreadPriority(
@@ -420,15 +428,6 @@ PosixEnv::PosixEnv()
   thread_status_updater_ = CreateThreadStatusUpdater();
 }
 
-PosixEnv::PosixEnv(const PosixEnv* default_env, std::shared_ptr<FileSystem> fs)
-  : CompositeEnvWrapper(this, fs),
-    thread_pools_(default_env->thread_pools_),
-    mu_(default_env->mu_),
-    threads_to_join_(default_env->threads_to_join_),
-    allow_non_owner_access_(default_env->allow_non_owner_access_) {
-  thread_status_updater_ = default_env->thread_status_updater_;
-}
-
 void PosixEnv::Schedule(void (*function)(void* arg1), void* arg, Priority pri,
                         void* tag, void (*unschedFunction)(void* arg)) {
   assert(pri >= Priority::BOTTOM && pri <= Priority::HIGH);
@@ -520,12 +519,9 @@ Env* Env::Default() {
   CompressionContextCache::InitSingleton();
   INIT_SYNC_POINT_SINGLETONS();
   static PosixEnv default_env;
-  return &default_env;
-}
-
-std::unique_ptr<Env> NewCompositeEnv(std::shared_ptr<FileSystem> fs) {
-  PosixEnv* default_env = static_cast<PosixEnv*>(Env::Default());
-  return std::unique_ptr<Env>(new PosixEnv(default_env, fs));
+  static CompositeEnvWrapper composite_env(&default_env,
+                                           FileSystem::Default().get());
+  return &composite_env;
 }
 
 }  // namespace ROCKSDB_NAMESPACE

env/env_test.cc

@@ -35,8 +35,6 @@
 #include "port/malloc.h"
 #include "port/port.h"
 #include "rocksdb/env.h"
-#include "test_util/fault_injection_test_env.h"
-#include "test_util/fault_injection_test_fs.h"
 #include "test_util/sync_point.h"
 #include "test_util/testharness.h"
 #include "test_util/testutil.h"
@@ -1274,7 +1272,7 @@ TEST_F(EnvPosixTest, MultiReadNonAlignedLargeNum) {
     ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->DisableProcessing();
   }
 }
-
+  
 // Only works in linux platforms
 #ifdef OS_WIN
 TEST_P(EnvPosixTestWithParam, DISABLED_InvalidateCache) {
@@ -1982,114 +1980,6 @@ INSTANTIATE_TEST_CASE_P(
     ::testing::Values(std::pair<Env*, bool>(chroot_env.get(), true)));
 #endif  // !defined(ROCKSDB_LITE) && !defined(OS_WIN)
 
-class EnvFSTestWithParam
-    : public ::testing::Test,
-      public ::testing::WithParamInterface<std::tuple<bool, bool, bool>> {
- public:
-  EnvFSTestWithParam() {
-    bool env_non_null = std::get<0>(GetParam());
-    bool env_default = std::get<1>(GetParam());
-    bool fs_default = std::get<2>(GetParam());
-
-    env_ = env_non_null ? (env_default ? Env::Default() : nullptr) : nullptr;
-    fs_ = fs_default
-              ? FileSystem::Default()
-              : std::make_shared<FaultInjectionTestFS>(FileSystem::Default());
-    if (env_non_null && env_default && !fs_default) {
-      env_ptr_ = NewCompositeEnv(fs_);
-    }
-    if (env_non_null && !env_default && fs_default) {
-      env_ptr_ = std::unique_ptr<Env>(new FaultInjectionTestEnv(Env::Default()));
-      fs_.reset();
-    }
-    if (env_non_null && !env_default && !fs_default) {
-      env_ptr_.reset(new FaultInjectionTestEnv(Env::Default()));
-      composite_env_ptr_.reset(new CompositeEnvWrapper(env_ptr_.get(), fs_));
-      env_ = composite_env_ptr_.get();
-    } else {
-      env_ = env_ptr_.get();
-    }
-
-    dbname1_ = test::PerThreadDBPath("env_fs_test1");
-    dbname2_ = test::PerThreadDBPath("env_fs_test2");
-  }
-
-  ~EnvFSTestWithParam() = default;
-
-  Env* env_;
-  std::unique_ptr<Env> env_ptr_;
-  std::unique_ptr<Env> composite_env_ptr_;
-  std::shared_ptr<FileSystem> fs_;
-  std::string dbname1_;
-  std::string dbname2_;
-};
-
-TEST_P(EnvFSTestWithParam, OptionsTest) {
-  Options opts;
-  opts.env = env_;
-  opts.create_if_missing = true;
-  std::string dbname = dbname1_;
-
-  if (env_) {
-    if (fs_) {
-      ASSERT_EQ(fs_.get(), env_->GetFileSystem().get());
-    } else {
-      ASSERT_NE(FileSystem::Default().get(), env_->GetFileSystem().get());
-    }
-  }
-  for (int i = 0; i < 2; ++i) {
-    DB* db;
-    Status s = DB::Open(opts, dbname, &db);
-    ASSERT_OK(s);
-
-    WriteOptions wo;
-    db->Put(wo, "a", "a");
-    db->Flush(FlushOptions());
-    db->Put(wo, "b", "b");
-    db->Flush(FlushOptions());
-    db->CompactRange(CompactRangeOptions(), nullptr, nullptr);
-
-    std::string val;
-    ASSERT_OK(db->Get(ReadOptions(), "a", &val));
-    ASSERT_EQ("a", val);
-    ASSERT_OK(db->Get(ReadOptions(), "b", &val));
-    ASSERT_EQ("b", val);
-
-    db->Close();
-    delete db;
-    DestroyDB(dbname, opts);
-
-    dbname = dbname2_;
-  }
-}
-
-// The parameters are as follows -
-// 1. True means Options::env is non-null, false means null
-// 2. True means use Env::Default, false means custom
-// 3. True means use FileSystem::Default, false means custom
-INSTANTIATE_TEST_CASE_P(
-    EnvFSTest, EnvFSTestWithParam,
-    ::testing::Combine(::testing::Bool(), ::testing::Bool(),
-                       ::testing::Bool()));
-
-// This test ensures that default Env and those allocated by
-// NewCompositeEnv() all share the same threadpool
-TEST_F(EnvTest, MultipleCompositeEnv) {
-  std::shared_ptr<FaultInjectionTestFS> fs1 =
-    std::make_shared<FaultInjectionTestFS>(FileSystem::Default());
-  std::shared_ptr<FaultInjectionTestFS> fs2 =
-    std::make_shared<FaultInjectionTestFS>(FileSystem::Default());
-  std::unique_ptr<Env> env1 = NewCompositeEnv(fs1);
-  std::unique_ptr<Env> env2 = NewCompositeEnv(fs2);
-  Env::Default()->SetBackgroundThreads(8, Env::HIGH);
-  Env::Default()->SetBackgroundThreads(16, Env::LOW);
-
-  ASSERT_EQ(env1->GetBackgroundThreads(Env::LOW), 16);
-  ASSERT_EQ(env1->GetBackgroundThreads(Env::HIGH), 8);
-  ASSERT_EQ(env2->GetBackgroundThreads(Env::LOW), 16);
-  ASSERT_EQ(env2->GetBackgroundThreads(Env::HIGH), 8);
-}
-
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

env/file_system.cc

@@ -26,18 +26,6 @@ Status FileSystem::Load(const std::string& value,
   return s;
 }
 
-IOStatus FileSystem::ReuseWritableFile(const std::string& fname,
-                                       const std::string& old_fname,
-                                       const FileOptions& opts,
-                                       std::unique_ptr<FSWritableFile>* result,
-                                       IODebugContext* dbg) {
-  IOStatus s = RenameFile(old_fname, fname, opts.io_options, dbg);
-  if (!s.ok()) {
-    return s;
-  }
-  return NewWritableFile(fname, opts, result, dbg);
-}
-
 FileOptions FileSystem::OptimizeForLogRead(
               const FileOptions& file_options) const {
   FileOptions optimized_file_options(file_options);

env/fs_posix.cc

@@ -47,7 +47,6 @@
 #include <set>
 #include <vector>
 
-#include "env/composite_env_wrapper.h"
 #include "env/io_posix.h"
 #include "logging/logging.h"
 #include "logging/posix_logger.h"
@@ -82,10 +81,6 @@ inline mode_t GetDBFileMode(bool allow_non_owner_access) {
   return allow_non_owner_access ? 0644 : 0600;
 }
 
-static uint64_t gettid() {
-  return Env::Default()->GetThreadID();
-}
-
 // list of pathnames that are locked
 // Only used for error message.
 struct LockHoldingInfo {
@@ -189,8 +184,7 @@ class PosixFileSystem : public FileSystem {
       }
     }
     result->reset(new PosixSequentialFile(
-        fname, file, fd, GetLogicalBlockSizeForReadIfNeeded(options, fname, fd),
-        options));
+        fname, file, fd, GetLogicalBlockSize(fname, fd), options));
     return IOStatus::OK();
   }
 
@@ -250,8 +244,7 @@ class PosixFileSystem : public FileSystem {
 #endif
       }
       result->reset(new PosixRandomAccessFile(
-          fname, fd, GetLogicalBlockSizeForReadIfNeeded(options, fname, fd),
-          options
+          fname, fd, GetLogicalBlockSize(fname, fd), options
 #if defined(ROCKSDB_IOURING_PRESENT)
           ,
           thread_local_io_urings_.get()
@@ -338,17 +331,13 @@ class PosixFileSystem : public FileSystem {
       }
 #endif
       result->reset(new PosixWritableFile(
-          fname, fd, GetLogicalBlockSizeForWriteIfNeeded(options, fname, fd),
-          options));
+          fname, fd, GetLogicalBlockSize(fname, fd), options));
     } else {
       // disable mmap writes
       EnvOptions no_mmap_writes_options = options;
       no_mmap_writes_options.use_mmap_writes = false;
-      result->reset(
-          new PosixWritableFile(fname, fd,
-                                GetLogicalBlockSizeForWriteIfNeeded(
-                                    no_mmap_writes_options, fname, fd),
-                                no_mmap_writes_options));
+      result->reset(new PosixWritableFile(
+          fname, fd, GetLogicalBlockSize(fname, fd), no_mmap_writes_options));
     }
     return s;
   }
@@ -444,17 +433,13 @@ class PosixFileSystem : public FileSystem {
       }
 #endif
       result->reset(new PosixWritableFile(
-          fname, fd, GetLogicalBlockSizeForWriteIfNeeded(options, fname, fd),
-          options));
+          fname, fd, GetLogicalBlockSize(fname, fd), options));
     } else {
       // disable mmap writes
       FileOptions no_mmap_writes_options = options;
       no_mmap_writes_options.use_mmap_writes = false;
-      result->reset(
-          new PosixWritableFile(fname, fd,
-                                GetLogicalBlockSizeForWriteIfNeeded(
-                                    no_mmap_writes_options, fname, fd),
-                                no_mmap_writes_options));
+      result->reset(new PosixWritableFile(
+          fname, fd, GetLogicalBlockSize(fname, fd), no_mmap_writes_options));
     }
     return s;
   }
@@ -545,34 +530,10 @@ class PosixFileSystem : public FileSystem {
     return IOStatus::OK();
   }
 
-  IOStatus NewLogger(const std::string& fname, const IOOptions& /*opts*/,
-                   std::shared_ptr<Logger>* result,
-                   IODebugContext* /*dbg*/) override {
-    FILE* f;
-    {
-      IOSTATS_TIMER_GUARD(open_nanos);
-      f = fopen(fname.c_str(),
-                "w"
-#ifdef __GLIBC_PREREQ
-#if __GLIBC_PREREQ(2, 7)
-                "e"  // glibc extension to enable O_CLOEXEC
-#endif
-#endif
-      );
-    }
-    if (f == nullptr) {
-      result->reset();
-      return status_to_io_status(
-              IOError("when fopen a file for new logger", fname, errno));
-    } else {
-      int fd = fileno(f);
-#ifdef ROCKSDB_FALLOCATE_PRESENT
-      fallocate(fd, FALLOC_FL_KEEP_SIZE, 0, 4 * 1024);
-#endif
-      SetFD_CLOEXEC(fd, nullptr);
-      result->reset(new PosixLogger(f, &gettid, Env::Default()));
-      return IOStatus::OK();
-    }
+  IOStatus NewLogger(const std::string& /*fname*/, const IOOptions& /*opts*/,
+                     std::shared_ptr<ROCKSDB_NAMESPACE::Logger>* /*ptr*/,
+                     IODebugContext* /*dbg*/) override {
+    return IOStatus::NotSupported();
   }
 
   IOStatus FileExists(const std::string& fname, const IOOptions& /*opts*/,
@@ -957,14 +918,6 @@ class PosixFileSystem : public FileSystem {
   static LogicalBlockSizeCache logical_block_size_cache_;
 #endif
   static size_t GetLogicalBlockSize(const std::string& fname, int fd);
-  // In non-direct IO mode, this directly returns kDefaultPageSize.
-  // Otherwise call GetLogicalBlockSize.
-  static size_t GetLogicalBlockSizeForReadIfNeeded(const EnvOptions& options,
-                                                   const std::string& fname,
-                                                   int fd);
-  static size_t GetLogicalBlockSizeForWriteIfNeeded(const EnvOptions& options,
-                                                    const std::string& fname,
-                                                    int fd);
 };
 
 #ifdef OS_LINUX
@@ -975,25 +928,11 @@ size_t PosixFileSystem::GetLogicalBlockSize(const std::string& fname, int fd) {
 #ifdef OS_LINUX
   return logical_block_size_cache_.GetLogicalBlockSize(fname, fd);
 #else
-  (void)fname;
+  (void) fname;
   return PosixHelper::GetLogicalBlockSizeOfFd(fd);
 #endif
 }
 
-size_t PosixFileSystem::GetLogicalBlockSizeForReadIfNeeded(
-    const EnvOptions& options, const std::string& fname, int fd) {
-  return options.use_direct_reads
-             ? PosixFileSystem::GetLogicalBlockSize(fname, fd)
-             : kDefaultPageSize;
-}
-
-size_t PosixFileSystem::GetLogicalBlockSizeForWriteIfNeeded(
-    const EnvOptions& options, const std::string& fname, int fd) {
-  return options.use_direct_writes
-             ? PosixFileSystem::GetLogicalBlockSize(fname, fd)
-             : kDefaultPageSize;
-}
-
 PosixFileSystem::PosixFileSystem()
     : checkedDiskForMmap_(false),
       forceMmapOff_(false),

env/mock_env.cc

@@ -10,7 +10,6 @@
 #include "env/mock_env.h"
 #include <algorithm>
 #include <chrono>
-#include "file/filename.h"
 #include "port/sys_time.h"
 #include "util/cast_util.h"
 #include "util/murmurhash.h"
@@ -748,6 +747,17 @@ Status MockEnv::CorruptBuffer(const std::string& fname) {
   return Status::OK();
 }
 
+std::string MockEnv::NormalizePath(const std::string path) {
+  std::string dst;
+  for (auto c : path) {
+    if (!dst.empty() && c == '/' && dst.back() == '/') {
+      continue;
+    }
+    dst.push_back(c);
+  }
+  return dst;
+}
+
 void MockEnv::FakeSleepForMicroseconds(int64_t micros) {
   fake_sleep_micros_.fetch_add(micros);
 }

env/mock_env.h

@@ -106,6 +106,8 @@ class MockEnv : public EnvWrapper {
   void FakeSleepForMicroseconds(int64_t micros);
 
  private:
+  std::string NormalizePath(const std::string path);
+
   // Map from filenames to MemFile objects, representing a simple file system.
   typedef std::map<std::string, MemFile*> FileSystem;
   port::Mutex mutex_;

file/delete_scheduler.cc

@@ -32,13 +32,13 @@ DeleteScheduler::DeleteScheduler(Env* env, FileSystem* fs,
       bytes_max_delete_chunk_(bytes_max_delete_chunk),
       closing_(false),
       cv_(&mu_),
-      bg_thread_(nullptr),
       info_log_(info_log),
       sst_file_manager_(sst_file_manager),
       max_trash_db_ratio_(max_trash_db_ratio) {
   assert(sst_file_manager != nullptr);
   assert(max_trash_db_ratio >= 0);
-  MaybeCreateBackgroundThread();
+  bg_thread_.reset(
+      new port::Thread(&DeleteScheduler::BackgroundEmptyTrash, this));
 }
 
 DeleteScheduler::~DeleteScheduler() {
@@ -352,13 +352,6 @@ void DeleteScheduler::WaitForEmptyTrash() {
   }
 }
 
-void DeleteScheduler::MaybeCreateBackgroundThread() {
-  if(bg_thread_ == nullptr && rate_bytes_per_sec_.load() > 0) {
-    bg_thread_.reset(
-        new port::Thread(&DeleteScheduler::BackgroundEmptyTrash, this));
-  }
-}
-
 }  // namespace ROCKSDB_NAMESPACE
 
 #endif  // ROCKSDB_LITE

file/delete_scheduler.h

@@ -45,7 +45,6 @@ class DeleteScheduler {
   // Set delete rate limit in bytes per second
   void SetRateBytesPerSecond(int64_t bytes_per_sec) {
     rate_bytes_per_sec_.store(bytes_per_sec);
-    MaybeCreateBackgroundThread();
   }
 
   // Mark file as trash directory and schedule it's deletion. If force_bg is
@@ -92,8 +91,6 @@ class DeleteScheduler {
 
   void BackgroundEmptyTrash();
 
-  void MaybeCreateBackgroundThread();
-
   Env* env_;
   FileSystem* fs_;
 

file/filename.cc

@@ -453,16 +453,4 @@ Status GetInfoLogFiles(Env* env, const std::string& db_log_dir,
   return Status::OK();
 }
 
-std::string NormalizePath(const std::string& path) {
-  std::string dst;
-  for (auto c : path) {
-    if (!dst.empty() && c == kFilePathSeparator &&
-        dst.back() == kFilePathSeparator) {
-      continue;
-    }
-    dst.push_back(c);
-  }
-  return dst;
-}
-
 }  // namespace ROCKSDB_NAMESPACE

file/filename.h

@@ -29,12 +29,6 @@ class Env;
 class Directory;
 class WritableFileWriter;
 
-#ifdef OS_WIN
-const char kFilePathSeparator = '\\';
-#else
-const char kFilePathSeparator = '/';
-#endif
-
 enum FileType {
   kLogFile,
   kDBLockFile,
@@ -189,6 +183,4 @@ extern Status GetInfoLogFiles(Env* env, const std::string& db_log_dir,
                               const std::string& dbname,
                               std::string* parent_dir,
                               std::vector<std::string>* file_names);
-
-extern std::string NormalizePath(const std::string& path);
 }  // namespace ROCKSDB_NAMESPACE

file/random_access_file_reader.cc

@@ -20,11 +20,11 @@
 #include "util/rate_limiter.h"
 
 namespace ROCKSDB_NAMESPACE {
-
 Status RandomAccessFileReader::Read(uint64_t offset, size_t n, Slice* result,
-                                    char* scratch, AlignedBuf* aligned_buf,
+                                    char* scratch,
+                                    std::unique_ptr<const char[]>* internal_buf,
                                     bool for_compaction) const {
-  (void)aligned_buf;
+  (void) internal_buf;
   Status s;
   uint64_t elapsed = 0;
   {
@@ -81,11 +81,11 @@ Status RandomAccessFileReader::Read(uint64_t offset, size_t n, Slice* result,
       size_t res_len = 0;
       if (s.ok() && offset_advance < buf.CurrentSize()) {
         res_len = std::min(buf.CurrentSize() - offset_advance, n);
-        if (aligned_buf == nullptr) {
+        if (internal_buf == nullptr) {
           buf.Read(scratch, offset_advance, res_len);
         } else {
           scratch = buf.BufferStart();
-          aligned_buf->reset(buf.Release());
+          internal_buf->reset(buf.Release());
         }
       }
       *result = Slice(scratch, res_len);
@@ -154,45 +154,11 @@ Status RandomAccessFileReader::Read(uint64_t offset, size_t n, Slice* result,
   return s;
 }
 
-size_t End(const FSReadRequest& r) {
-  return static_cast<size_t>(r.offset) + r.len;
-}
-
-FSReadRequest Align(const FSReadRequest& r, size_t alignment) {
-  FSReadRequest req;
-  req.offset = static_cast<uint64_t>(
-    TruncateToPageBoundary(alignment, static_cast<size_t>(r.offset)));
-  req.len = Roundup(End(r), alignment) - req.offset;
-  req.scratch = nullptr;
-  return req;
-}
-
-// Try to merge src to dest if they have overlap.
-//
-// Each request represents an inclusive interval [offset, offset + len].
-// If the intervals have overlap, update offset and len to represent the
-// merged interval, and return true.
-// Otherwise, do nothing and return false.
-bool TryMerge(FSReadRequest* dest, const FSReadRequest& src) {
-  size_t dest_offset = static_cast<size_t>(dest->offset);
-  size_t src_offset = static_cast<size_t>(src.offset);
-  size_t dest_end = End(*dest);
-  size_t src_end = End(src);
-  if (std::max(dest_offset, dest_offset) > std::min(dest_end, src_end)) {
-    return false;
-  }
-  dest->offset = static_cast<uint64_t>(std::min(dest_offset, src_offset));
-  dest->len = std::max(dest_end, src_end) - dest->offset;
-  return true;
-}
-
 Status RandomAccessFileReader::MultiRead(FSReadRequest* read_reqs,
-                                         size_t num_reqs,
-                                         AlignedBuf* aligned_buf) const {
-  (void)aligned_buf;  // suppress warning of unused variable in LITE mode
-  assert(num_reqs > 0);
+                                         size_t num_reqs) const {
   Status s;
   uint64_t elapsed = 0;
+  assert(!use_direct_io());
   {
     StopWatch sw(env_, stats_, hist_type_,
                  (stats_ != nullptr) ? &elapsed : nullptr, true /*overwrite*/,
@@ -200,44 +166,6 @@ Status RandomAccessFileReader::MultiRead(FSReadRequest* read_reqs,
     auto prev_perf_level = GetPerfLevel();
     IOSTATS_TIMER_GUARD(read_nanos);
 
-    FSReadRequest* fs_reqs = read_reqs;
-    size_t num_fs_reqs = num_reqs;
-#ifndef ROCKSDB_LITE
-    std::vector<FSReadRequest> aligned_reqs;
-    if (use_direct_io()) {
-      // num_reqs is the max possible size,
-      // this can reduce std::vecector's internal resize operations.
-      aligned_reqs.reserve(num_reqs);
-      // Align and merge the read requests.
-      size_t alignment = file_->GetRequiredBufferAlignment();
-      aligned_reqs.push_back(Align(read_reqs[0], alignment));
-      for (size_t i = 1; i < num_reqs; i++) {
-        const auto& r = Align(read_reqs[i], alignment);
-        if (!TryMerge(&aligned_reqs.back(), r)) {
-          aligned_reqs.push_back(r);
-        }
-      }
-
-      // Allocate aligned buffer and let scratch buffers point to it.
-      size_t total_len = 0;
-      for (const auto& r : aligned_reqs) {
-        total_len += r.len;
-      }
-      AlignedBuffer buf;
-      buf.Alignment(alignment);
-      buf.AllocateNewBuffer(total_len);
-      char* scratch = buf.BufferStart();
-      for (auto& r : aligned_reqs) {
-        r.scratch = scratch;
-        scratch += r.len;
-      }
-
-      aligned_buf->reset(buf.Release());
-      fs_reqs = aligned_reqs.data();
-      num_fs_reqs = aligned_reqs.size();
-    }
-#endif  // ROCKSDB_LITE
-
 #ifndef ROCKSDB_LITE
     FileOperationInfo::TimePoint start_ts;
     if (ShouldNotifyListeners()) {
@@ -246,31 +174,8 @@ Status RandomAccessFileReader::MultiRead(FSReadRequest* read_reqs,
 #endif  // ROCKSDB_LITE
     {
       IOSTATS_CPU_TIMER_GUARD(cpu_read_nanos, env_);
-      s = file_->MultiRead(fs_reqs, num_fs_reqs, IOOptions(), nullptr);
+      s = file_->MultiRead(read_reqs, num_reqs, IOOptions(), nullptr);
     }
-
-#ifndef ROCKSDB_LITE
-    if (use_direct_io()) {
-      // Populate results in the unaligned read requests.
-      size_t aligned_i = 0;
-      for (size_t i = 0; i < num_reqs; i++) {
-        auto& r = read_reqs[i];
-        if (static_cast<size_t>(r.offset) > End(aligned_reqs[aligned_i])) {
-          aligned_i++;
-        }
-        const auto& fs_r = fs_reqs[aligned_i];
-        r.status = fs_r.status;
-        if (r.status.ok()) {
-          uint64_t offset = r.offset - fs_r.offset;
-          size_t len = std::min(r.len, static_cast<size_t>(fs_r.len - offset));
-          r.result = Slice(fs_r.scratch + offset, len);
-        } else {
-          r.result = Slice();
-        }
-      }
-    }
-#endif  // ROCKSDB_LITE
-
     for (size_t i = 0; i < num_reqs; ++i) {
 #ifndef ROCKSDB_LITE
       if (ShouldNotifyListeners()) {
@@ -289,5 +194,4 @@ Status RandomAccessFileReader::MultiRead(FSReadRequest* read_reqs,
 
   return s;
 }
-
 }  // namespace ROCKSDB_NAMESPACE

file/random_access_file_reader.h

@@ -22,8 +22,6 @@ namespace ROCKSDB_NAMESPACE {
 class Statistics;
 class HistogramImpl;
 
-using AlignedBuf = std::unique_ptr<const char[]>;
-
 // RandomAccessFileReader is a wrapper on top of Env::RnadomAccessFile. It is
 // responsible for:
 // - Handling Buffered and Direct reads appropriately.
@@ -61,7 +59,7 @@ class RandomAccessFileReader {
 
  public:
   explicit RandomAccessFileReader(
-      std::unique_ptr<FSRandomAccessFile>&& raf, const std::string& _file_name,
+      std::unique_ptr<FSRandomAccessFile>&& raf, std::string _file_name,
       Env* env = nullptr, Statistics* stats = nullptr, uint32_t hist_type = 0,
       HistogramImpl* file_read_hist = nullptr,
       RateLimiter* rate_limiter = nullptr,
@@ -108,22 +106,17 @@ class RandomAccessFileReader {
   // 1. if using mmap, result is stored in a buffer other than scratch;
   // 2. if not using mmap, result is stored in the buffer starting from scratch.
   //
-  // In direct IO mode, an aligned buffer is allocated internally.
-  // 1. If aligned_buf is null, then results are copied to the buffer
+  // In direct IO mode, an internal aligned buffer is allocated.
+  // 1. If internal_buf is null, then results are copied to the buffer
   // starting from scratch;
-  // 2. Otherwise, scratch is not used and can be null, the aligned_buf owns
+  // 2. Otherwise, scratch is not used and can be null, the internal_buf owns
   // the internally allocated buffer on return, and the result refers to a
-  // region in aligned_buf.
+  // region in internal_buf.
   Status Read(uint64_t offset, size_t n, Slice* result, char* scratch,
-              AlignedBuf* aligned_buf, bool for_compaction = false) const;
+              std::unique_ptr<const char[]>* internal_buf,
+              bool for_compaction = false) const;
 
-  // REQUIRES:
-  // num_reqs > 0, reqs do not overlap, and offsets in reqs are increasing.
-  // In non-direct IO mode, aligned_buf should be null;
-  // In direct IO mode, aligned_buf stores the aligned buffer allocated inside
-  // MultiRead, the result Slices in reqs refer to aligned_buf.
-  Status MultiRead(FSReadRequest* reqs, size_t num_reqs,
-                   AlignedBuf* aligned_buf) const;
+  Status MultiRead(FSReadRequest* reqs, size_t num_reqs) const;
 
   Status Prefetch(uint64_t offset, size_t n) const {
     return file_->Prefetch(offset, n, IOOptions(), nullptr);

file/random_access_file_reader_test.cc

@@ -1,255 +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 "port/port.h"
-#include "port/stack_trace.h"
-#include "rocksdb/file_system.h"
-#include "file/random_access_file_reader.h"
-#include "test_util/testharness.h"
-#include "test_util/testutil.h"
-
-namespace ROCKSDB_NAMESPACE {
-
-class RandomAccessFileReaderTest : public testing::Test {
- public:
-  void SetUp() override {
-#ifdef OS_LINUX
-    // TEST_TMPDIR may be set to /dev/shm in Makefile,
-    // but /dev/shm does not support direct IO.
-    // The default TEST_TMPDIR is under /tmp, but /tmp might also be a tmpfs
-    // which does not support direct IO neither.
-    unsetenv("TEST_TMPDIR");
-    char* tmpdir = getenv("DISK_TEMP_DIR");
-    if (tmpdir == nullptr) {
-      tmpdir = getenv("HOME");
-    }
-    if (tmpdir != nullptr) {
-      setenv("TEST_TMPDIR", tmpdir, 1);
-    }
-#endif
-    env_ = Env::Default();
-    fs_ = FileSystem::Default();
-    test_dir_ = test::PerThreadDBPath("random_access_file_reader_test");
-    ASSERT_OK(fs_->CreateDir(test_dir_, IOOptions(), nullptr));
-    alignment_ = GetAlignment();
-  }
-
-  void TearDown() override {
-    EXPECT_OK(test::DestroyDir(env_, test_dir_));
-  }
-
-  bool IsDirectIOSupported() {
-    Write(".direct", "");
-    FileOptions opt;
-    opt.use_direct_reads = true;
-    std::unique_ptr<FSRandomAccessFile> f;
-    auto s = fs_->NewRandomAccessFile(Path(".direct"), opt, &f, nullptr);
-    return s.ok();
-  }
-
-  void Write(const std::string& fname, const std::string& content) {
-    std::unique_ptr<FSWritableFile> f;
-    ASSERT_OK(fs_->NewWritableFile(Path(fname), FileOptions(), &f, nullptr));
-    ASSERT_OK(f->Append(content, IOOptions(), nullptr));
-    ASSERT_OK(f->Close(IOOptions(), nullptr));
-  }
-
-  void Read(const std::string& fname, const FileOptions& opts,
-                std::unique_ptr<RandomAccessFileReader>* reader) {
-    std::string fpath = Path(fname);
-    std::unique_ptr<FSRandomAccessFile> f;
-    ASSERT_OK(fs_->NewRandomAccessFile(fpath, opts, &f, nullptr));
-    (*reader).reset(new RandomAccessFileReader(std::move(f), fpath, env_));
-  }
-
-  void AssertResult(const std::string& content,
-                    const std::vector<FSReadRequest>& reqs) {
-    for (const auto& r : reqs) {
-      ASSERT_OK(r.status);
-      ASSERT_EQ(r.len, r.result.size());
-      ASSERT_EQ(content.substr(r.offset, r.len), r.result.ToString());
-    }
-  }
-
-  size_t alignment() const { return alignment_; }
-
- private:
-  Env* env_;
-  std::shared_ptr<FileSystem> fs_;
-  std::string test_dir_;
-  size_t alignment_;
-
-  std::string Path(const std::string& fname) {
-    return test_dir_ + "/" + fname;
-  }
-
-  size_t GetAlignment() {
-    std::string f = "get_alignment";
-    Write(f, "");
-    std::unique_ptr<RandomAccessFileReader> r;
-    Read(f, FileOptions(), &r);
-    size_t alignment = r->file()->GetRequiredBufferAlignment();
-    EXPECT_OK(fs_->DeleteFile(Path(f), IOOptions(), nullptr));
-    return alignment;
-  }
-};
-
-TEST_F(RandomAccessFileReaderTest, MultiReadDirectIO) {
-  if (!IsDirectIOSupported()) {
-    printf("Direct IO is not supported, skip this test\n");
-    return;
-  }
-
-  // Creates a file with 3 pages.
-  std::string fname = "multi-read-direct-io";
-  Random rand(0);
-  std::string content;
-  test::RandomString(&rand, 3 * static_cast<int>(alignment()), &content);
-  Write(fname, content);
-
-  FileOptions opts;
-  opts.use_direct_reads = true;
-  std::unique_ptr<RandomAccessFileReader> r;
-  Read(fname, opts, &r);
-  ASSERT_TRUE(r->use_direct_io());
-
-  {
-    // Reads 2 blocks in the 1st page.
-    // The results should be SharedSlices of the same underlying buffer.
-    //
-    // Illustration (each x is a 1/4 page)
-    // First page: xxxx
-    // 1st block:  x
-    // 2nd block:    xx
-    FSReadRequest r0;
-    r0.offset = 0;
-    r0.len = alignment() / 4;
-    r0.scratch = nullptr;
-
-    FSReadRequest r1;
-    r1.offset = alignment() / 2;
-    r1.len = alignment() / 2;
-    r1.scratch = nullptr;
-
-    std::vector<FSReadRequest> reqs;
-    reqs.push_back(std::move(r0));
-    reqs.push_back(std::move(r1));
-    AlignedBuf aligned_buf;
-    ASSERT_OK(r->MultiRead(reqs.data(), reqs.size(), &aligned_buf));
-
-    AssertResult(content, reqs);
-  }
-
-  {
-    // Reads 3 blocks:
-    // 1st block in the 1st page;
-    // 2nd block from the middle of the 1st page to the middle of the 2nd page;
-    // 3rd block in the 2nd page.
-    // The results should be SharedSlices of the same underlying buffer.
-    //
-    // Illustration (each x is a 1/4 page)
-    // 2 pages:   xxxxxxxx
-    // 1st block: x
-    // 2nd block:   xxxx
-    // 3rd block:        x
-    FSReadRequest r0;
-    r0.offset = 0;
-    r0.len = alignment() / 4;
-    r0.scratch = nullptr;
-
-    FSReadRequest r1;
-    r1.offset = alignment() / 2;
-    r1.len = alignment();
-    r1.scratch = nullptr;
-
-    FSReadRequest r2;
-    r2.offset = 2 * alignment() - alignment() / 4;
-    r2.len = alignment() / 4;
-    r2.scratch = nullptr;
-
-    std::vector<FSReadRequest> reqs;
-    reqs.push_back(std::move(r0));
-    reqs.push_back(std::move(r1));
-    reqs.push_back(std::move(r2));
-    AlignedBuf aligned_buf;
-    ASSERT_OK(r->MultiRead(reqs.data(), reqs.size(), &aligned_buf));
-
-    AssertResult(content, reqs);
-  }
-
-  {
-    // Reads 3 blocks:
-    // 1st block in the middle of the 1st page;
-    // 2nd block in the middle of the 2nd page;
-    // 3rd block in the middle of the 3rd page.
-    // The results should be SharedSlices of the same underlying buffer.
-    //
-    // Illustration (each x is a 1/4 page)
-    // 3 pages:   xxxxxxxxxxxx
-    // 1st block:  xx
-    // 2nd block:      xx
-    // 3rd block:          xx
-    FSReadRequest r0;
-    r0.offset = alignment() / 4;
-    r0.len = alignment() / 2;
-    r0.scratch = nullptr;
-
-    FSReadRequest r1;
-    r1.offset = alignment() + alignment() / 4;
-    r1.len = alignment() / 2;
-    r1.scratch = nullptr;
-
-    FSReadRequest r2;
-    r2.offset = 2 * alignment() + alignment() / 4;
-    r2.len = alignment() / 2;
-    r2.scratch = nullptr;
-
-    std::vector<FSReadRequest> reqs;
-    reqs.push_back(std::move(r0));
-    reqs.push_back(std::move(r1));
-    reqs.push_back(std::move(r2));
-    AlignedBuf aligned_buf;
-    ASSERT_OK(r->MultiRead(reqs.data(), reqs.size(), &aligned_buf));
-
-    AssertResult(content, reqs);
-  }
-
-  {
-    // Reads 2 blocks:
-    // 1st block in the middle of the 1st page;
-    // 2nd block in the middle of the 3rd page.
-    // The results are two different buffers.
-    //
-    // Illustration (each x is a 1/4 page)
-    // 3 pages:   xxxxxxxxxxxx
-    // 1st block:  xx
-    // 2nd block:          xx
-    FSReadRequest r0;
-    r0.offset = alignment() / 4;
-    r0.len = alignment() / 2;
-    r0.scratch = nullptr;
-
-    FSReadRequest r1;
-    r1.offset = 2 * alignment() + alignment() / 4;
-    r1.len = alignment() / 2;
-    r1.scratch = nullptr;
-
-    std::vector<FSReadRequest> reqs;
-    reqs.push_back(std::move(r0));
-    reqs.push_back(std::move(r1));
-    AlignedBuf aligned_buf;
-    ASSERT_OK(r->MultiRead(reqs.data(), reqs.size(), &aligned_buf));
-
-    AssertResult(content, reqs);
-  }
-}
-
-}  // namespace ROCKSDB_NAMESPACE
-
-int main(int argc, char** argv) {
-  ROCKSDB_NAMESPACE::port::InstallStackTraceHandler();
-  ::testing::InitGoogleTest(&argc, argv);
-  return RUN_ALL_TESTS();
-}

include/rocksdb/db.h

@@ -480,25 +480,6 @@ class DB {
         keys, values);
   }
 
-  virtual std::vector<Status> MultiGet(
-      const ReadOptions& /*options*/,
-      const std::vector<ColumnFamilyHandle*>& /*column_family*/,
-      const std::vector<Slice>& keys, std::vector<std::string>* /*values*/,
-      std::vector<std::string>* /*timestamps*/) {
-    return std::vector<Status>(
-        keys.size(), Status::NotSupported(
-                         "MultiGet() returning timestamps not implemented."));
-  }
-  virtual std::vector<Status> MultiGet(const ReadOptions& options,
-                                       const std::vector<Slice>& keys,
-                                       std::vector<std::string>* values,
-                                       std::vector<std::string>* timestamps) {
-    return MultiGet(
-        options,
-        std::vector<ColumnFamilyHandle*>(keys.size(), DefaultColumnFamily()),
-        keys, values, timestamps);
-  }
-
   // Overloaded MultiGet API that improves performance by batching operations
   // in the read path for greater efficiency. Currently, only the block based
   // table format with full filters are supported. Other table formats such
@@ -540,30 +521,6 @@ class DB {
     }
   }
 
-  virtual void MultiGet(const ReadOptions& options,
-                        ColumnFamilyHandle* column_family,
-                        const size_t num_keys, const Slice* keys,
-                        PinnableSlice* values, std::string* timestamps,
-                        Status* statuses, const bool /*sorted_input*/ = false) {
-    std::vector<ColumnFamilyHandle*> cf;
-    std::vector<Slice> user_keys;
-    std::vector<Status> status;
-    std::vector<std::string> vals;
-    std::vector<std::string> tss;
-
-    for (size_t i = 0; i < num_keys; ++i) {
-      cf.emplace_back(column_family);
-      user_keys.emplace_back(keys[i]);
-    }
-    status = MultiGet(options, cf, user_keys, &vals, &tss);
-    std::copy(status.begin(), status.end(), statuses);
-    std::copy(tss.begin(), tss.end(), timestamps);
-    for (auto& value : vals) {
-      values->PinSelf(value);
-      values++;
-    }
-  }
-
   // Overloaded MultiGet API that improves performance by batching operations
   // in the read path for greater efficiency. Currently, only the block based
   // table format with full filters are supported. Other table formats such
@@ -603,28 +560,6 @@ class DB {
       values++;
     }
   }
-  virtual void MultiGet(const ReadOptions& options, const size_t num_keys,
-                        ColumnFamilyHandle** column_families, const Slice* keys,
-                        PinnableSlice* values, std::string* timestamps,
-                        Status* statuses, const bool /*sorted_input*/ = false) {
-    std::vector<ColumnFamilyHandle*> cf;
-    std::vector<Slice> user_keys;
-    std::vector<Status> status;
-    std::vector<std::string> vals;
-    std::vector<std::string> tss;
-
-    for (size_t i = 0; i < num_keys; ++i) {
-      cf.emplace_back(column_families[i]);
-      user_keys.emplace_back(keys[i]);
-    }
-    status = MultiGet(options, cf, user_keys, &vals, &tss);
-    std::copy(status.begin(), status.end(), statuses);
-    std::copy(tss.begin(), tss.end(), timestamps);
-    for (auto& value : vals) {
-      values->PinSelf(value);
-      values++;
-    }
-  }
 
   // If the key definitely does not exist in the database, then this method
   // returns false, else true. If the caller wants to obtain value when the key

include/rocksdb/env.h

@@ -57,7 +57,6 @@ struct MutableDBOptions;
 class RateLimiter;
 class ThreadStatusUpdater;
 struct ThreadStatus;
-class FileSystem;
 
 const size_t kDefaultPageSize = 4 * 1024;
 
@@ -141,9 +140,7 @@ class Env {
     uint64_t size_bytes;
   };
 
-  Env();
-  // Construct an Env with a separate FileSystem implementation
-  Env(std::shared_ptr<FileSystem> fs);
+  Env() : thread_status_updater_(nullptr) {}
   // No copying allowed
   Env(const Env&) = delete;
   void operator=(const Env&) = delete;
@@ -542,19 +539,12 @@ class Env {
 
   virtual void SanitizeEnvOptions(EnvOptions* /*env_opts*/) const {}
 
-  // Get the FileSystem implementation this Env was constructed with. It
-  // could be a fully implemented one, or a wrapper class around the Env
-  const std::shared_ptr<FileSystem>& GetFileSystem() const;
-
   // If you're adding methods here, remember to add them to EnvWrapper too.
 
  protected:
   // The pointer to an internal structure that will update the
   // status of each thread.
   ThreadStatusUpdater* thread_status_updater_;
-
-  // Pointer to the underlying FileSystem implementation
-  std::shared_ptr<FileSystem> file_system_;
 };
 
 // The factory function to construct a ThreadStatusUpdater.  Any Env
@@ -1613,6 +1603,4 @@ Env* NewTimedEnv(Env* base_env);
 Status NewEnvLogger(const std::string& fname, Env* env,
                     std::shared_ptr<Logger>* result);
 
-std::unique_ptr<Env> NewCompositeEnv(std::shared_ptr<FileSystem> fs);
-
 }  // namespace ROCKSDB_NAMESPACE

include/rocksdb/file_system.h

@@ -102,9 +102,6 @@ struct FileOptions : EnvOptions {
 
   FileOptions(const EnvOptions& opts)
     : EnvOptions(opts) {}
-
-  FileOptions(const FileOptions& opts)
-    : EnvOptions(opts), io_options(opts.io_options) {}
 };
 
 // A structure to pass back some debugging information from the FileSystem
@@ -266,7 +263,7 @@ class FileSystem {
                                      const std::string& old_fname,
                                      const FileOptions& file_opts,
                                      std::unique_ptr<FSWritableFile>* result,
-                                     IODebugContext* dbg);
+                                     IODebugContext* dbg) = 0;
 
   // Open `fname` for random read and write, if file doesn't exist the file
   // will be created.  On success, stores a pointer to the new file in
@@ -468,10 +465,6 @@ class FileSystem {
                                    std::string* output_path,
                                    IODebugContext* dbg) = 0;
 
-  // Sanitize the FileOptions. Typically called by a FileOptions/EnvOptions
-  // copy constructor
-  virtual void SanitizeFileOptions(FileOptions* /*opts*/) const {}
-
   // OptimizeForLogRead will create a new FileOptions object that is a copy of
   // the FileOptions in the parameters, but is optimized for reading log files.
   virtual FileOptions OptimizeForLogRead(const FileOptions& file_options) const;
@@ -1008,13 +1001,11 @@ class FSDirectory {
 class FileSystemWrapper : public FileSystem {
  public:
   // Initialize an EnvWrapper that delegates all calls to *t
-  explicit FileSystemWrapper(std::shared_ptr<FileSystem> t) : target_(t) {}
+  explicit FileSystemWrapper(FileSystem* t) : target_(t) {}
   ~FileSystemWrapper() override {}
 
-  const char* Name() const override { return target_->Name(); }
-
   // Return the target to which this Env forwards all calls
-  FileSystem* target() const { return target_.get(); }
+  FileSystem* target() const { return target_; }
 
   // The following text is boilerplate that forwards all methods to target()
   IOStatus NewSequentialFile(const std::string& f,
@@ -1158,10 +1149,6 @@ class FileSystemWrapper : public FileSystem {
     return target_->NewLogger(fname, options, result, dbg);
   }
 
-  void SanitizeFileOptions(FileOptions* opts) const override {
-    target_->SanitizeFileOptions(opts);
-  }
-
   FileOptions OptimizeForLogRead(
                   const FileOptions& file_options) const override {
     return target_->OptimizeForLogRead(file_options);
@@ -1195,7 +1182,7 @@ class FileSystemWrapper : public FileSystem {
   }
 
  private:
-  std::shared_ptr<FileSystem> target_;
+  FileSystem* target_;
 };
 
 class FSSequentialFileWrapper : public FSSequentialFile {

include/rocksdb/options.h

@@ -396,6 +396,11 @@ struct DBOptions {
   // Default: Env::Default()
   Env* env = Env::Default();
 
+  // Use the specified object to interact with the storage to
+  // read/write files. This is in addition to env. This option should be used
+  // if the desired storage subsystem provides a FileSystem implementation.
+  std::shared_ptr<FileSystem> file_system = nullptr;
+
   // Use to control write rate of flush and compaction. Flush has higher
   // priority than compaction. Rate limiting is disabled if nullptr.
   // If rate limiter is enabled, bytes_per_sync is set to 1MB by default.
@@ -1125,16 +1130,6 @@ struct DBOptions {
   //
   // Default: nullptr
   std::shared_ptr<FileChecksumFunc> sst_file_checksum_func = nullptr;
-
-  // By default, RocksDB recovery fails if any table file referenced in
-  // MANIFEST are missing after scanning the MANIFEST.
-  // Best-efforts recovery is another recovery mode that
-  // tries to restore the database to the most recent point in time without
-  // missing file.
-  // Currently not compatible with atomic flush. Furthermore, WAL files will
-  // not be used for recovery if best_efforts_recovery is true.
-  // Default: false
-  bool best_efforts_recovery = false;
 };
 
 // Options to control the behavior of a database (passed to DB::Open)

include/rocksdb/table.h

@@ -272,7 +272,7 @@ struct BlockBasedTableOptions {
   // 5 -- Can be read by RocksDB's versions since 6.6.0. Full and partitioned
   // filters use a generally faster and more accurate Bloom filter
   // implementation, with a different schema.
-  uint32_t format_version = 4;
+  uint32_t format_version = 2;
 
   // Store index blocks on disk in compressed format. Changing this option to
   // false  will avoid the overhead of decompression if index blocks are evicted

options/db_options.cc

@@ -26,7 +26,7 @@ ImmutableDBOptions::ImmutableDBOptions(const DBOptions& options)
       error_if_exists(options.error_if_exists),
       paranoid_checks(options.paranoid_checks),
       env(options.env),
-      fs(options.env->GetFileSystem()),
+      fs(options.file_system),
       rate_limiter(options.rate_limiter),
       sst_file_manager(options.sst_file_manager),
       info_log(options.info_log),
@@ -95,8 +95,7 @@ ImmutableDBOptions::ImmutableDBOptions(const DBOptions& options)
       persist_stats_to_disk(options.persist_stats_to_disk),
       write_dbid_to_manifest(options.write_dbid_to_manifest),
       log_readahead_size(options.log_readahead_size),
-      sst_file_checksum_func(options.sst_file_checksum_func),
-      best_efforts_recovery(options.best_efforts_recovery) {
+      sst_file_checksum_func(options.sst_file_checksum_func) {
 }
 
 void ImmutableDBOptions::Dump(Logger* log) const {
@@ -251,8 +250,6 @@ void ImmutableDBOptions::Dump(Logger* log) const {
                    sst_file_checksum_func
                        ? sst_file_checksum_func->Name()
                        : kUnknownFileChecksumFuncName.c_str());
-  ROCKS_LOG_HEADER(log, "                Options.best_efforts_recovery: %d",
-                   static_cast<int>(best_efforts_recovery));
 }
 
 MutableDBOptions::MutableDBOptions()

options/db_options.h

@@ -88,7 +88,6 @@ struct ImmutableDBOptions {
   bool write_dbid_to_manifest;
   size_t log_readahead_size;
   std::shared_ptr<FileChecksumFunc> sst_file_checksum_func;
-  bool best_efforts_recovery;
 };
 
 struct MutableDBOptions {

options/options_helper.cc

@@ -38,6 +38,7 @@ DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
   options.error_if_exists = immutable_db_options.error_if_exists;
   options.paranoid_checks = immutable_db_options.paranoid_checks;
   options.env = immutable_db_options.env;
+  options.file_system = immutable_db_options.fs;
   options.rate_limiter = immutable_db_options.rate_limiter;
   options.sst_file_manager = immutable_db_options.sst_file_manager;
   options.info_log = immutable_db_options.info_log;
@@ -144,7 +145,6 @@ DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options,
       immutable_db_options.avoid_unnecessary_blocking_io;
   options.log_readahead_size = immutable_db_options.log_readahead_size;
   options.sst_file_checksum_func = immutable_db_options.sst_file_checksum_func;
-  options.best_efforts_recovery = immutable_db_options.best_efforts_recovery;
   return options;
 }
 
@@ -1681,9 +1681,6 @@ std::unordered_map<std::string, OptionTypeInfo>
         {"log_readahead_size",
          {offsetof(struct DBOptions, log_readahead_size), OptionType::kSizeT,
           OptionVerificationType::kNormal, false, 0}},
-        {"best_efforts_recovery",
-         {offsetof(struct DBOptions, best_efforts_recovery),
-          OptionType::kBoolean, OptionVerificationType::kNormal, false, 0}},
 };
 
 std::unordered_map<std::string, BlockBasedTableOptions::IndexType>

options/options_settable_test.cc

@@ -181,6 +181,8 @@ TEST_F(OptionsSettableTest, BlockBasedTableOptionsAllFieldsSettable) {
 TEST_F(OptionsSettableTest, DBOptionsAllFieldsSettable) {
   const OffsetGap kDBOptionsBlacklist = {
       {offsetof(struct DBOptions, env), sizeof(Env*)},
+      {offsetof(struct DBOptions, file_system),
+       sizeof(std::shared_ptr<FileSystem>)},
       {offsetof(struct DBOptions, rate_limiter),
        sizeof(std::shared_ptr<RateLimiter>)},
       {offsetof(struct DBOptions, sst_file_manager),
@@ -301,8 +303,7 @@ TEST_F(OptionsSettableTest, DBOptionsAllFieldsSettable) {
                              "atomic_flush=false;"
                              "avoid_unnecessary_blocking_io=false;"
                              "log_readahead_size=0;"
-                             "write_dbid_to_manifest=false;"
-                             "best_efforts_recovery=false",
+                             "write_dbid_to_manifest=false",
                              new_options));
 
   ASSERT_EQ(unset_bytes_base, NumUnsetBytes(new_options_ptr, sizeof(DBOptions),

port/win/env_win.cc

@@ -1424,7 +1424,7 @@ Status WinEnv::UnlockFile(FileLock* lock) {
   return winenv_io_.UnlockFile(lock);
 }
 
-Status WinEnv::GetTestDirectory(std::string* result) {
+Status  WinEnv::GetTestDirectory(std::string* result) {
   return winenv_io_.GetTestDirectory(result);
 }
 

src.mk

@@ -59,7 +59,6 @@ LIB_SOURCES =                                                   \
   db/trim_history_scheduler.cc                                  \
   db/version_builder.cc                                         \
   db/version_edit.cc                                            \
-  db/version_edit_handler.cc                                    \
   db/version_set.cc                                             \
   db/wal_manager.cc                                             \
   db/write_batch.cc                                             \
@@ -396,7 +395,6 @@ MAIN_SOURCES =                                                          \
   env/env_test.cc                                                       \
   env/io_posix_test.cc                                                  \
   env/mock_env_test.cc                                                  \
-  file/random_access_file_reader_test.cc                                \
   logging/auto_roll_logger_test.cc                                      \
   logging/env_logger_test.cc                                            \
   logging/event_logger_test.cc                                          \

table/block_based/block_based_table_reader.cc

@@ -1607,6 +1607,7 @@ void BlockBasedTable::RetrieveMultipleBlocks(
           req.scratch = scratch + buf_offset;
           buf_offset += req.len;
         }
+        req.status = IOStatus::OK();
         read_reqs.emplace_back(req);
       }
 
@@ -1627,10 +1628,11 @@ void BlockBasedTable::RetrieveMultipleBlocks(
     } else {
       req.scratch = scratch + buf_offset;
     }
+    req.status = IOStatus::OK();
     read_reqs.emplace_back(req);
   }
 
-  file->MultiRead(&read_reqs[0], read_reqs.size(), nullptr);
+  file->MultiRead(&read_reqs[0], read_reqs.size());
 
   idx_in_batch = 0;
   size_t valid_batch_idx = 0;
@@ -1697,7 +1699,7 @@ void BlockBasedTable::RetrieveMultipleBlocks(
         // in each read request. Checksum is stored in the block trailer,
         // which is handle.size() + 1.
         s = ROCKSDB_NAMESPACE::VerifyChecksum(footer.checksum(),
-                                              data + req_offset,
+                                              req.result.data() + req_offset,
                                               handle.size() + 1, expected);
         TEST_SYNC_POINT_CALLBACK("RetrieveMultipleBlocks:VerifyChecksum", &s);
       }

table/multiget_context.h

@@ -29,11 +29,10 @@ struct KeyContext {
   bool key_exists;
   void* cb_arg;
   PinnableSlice* value;
-  std::string* timestamp;
   GetContext* get_context;
 
   KeyContext(ColumnFamilyHandle* col_family, const Slice& user_key,
-             PinnableSlice* val, std::string* ts, Status* stat)
+             PinnableSlice* val, Status* stat)
       : key(&user_key),
         lkey(nullptr),
         column_family(col_family),
@@ -42,7 +41,6 @@ struct KeyContext {
         key_exists(false),
         cb_arg(nullptr),
         value(val),
-        timestamp(ts),
         get_context(nullptr) {}
 
   KeyContext() = default;

test_util/fault_injection_test_fs.h

@@ -137,7 +137,7 @@ class TestFSDirectory : public FSDirectory {
 
 class FaultInjectionTestFS : public FileSystemWrapper {
  public:
-  explicit FaultInjectionTestFS(std::shared_ptr<FileSystem> base)
+  explicit FaultInjectionTestFS(FileSystem* base)
       : FileSystemWrapper(base), filesystem_active_(true) {}
   virtual ~FaultInjectionTestFS() {}
 

tools/check_format_compatible.sh

@@ -55,9 +55,9 @@ EOF
 
 declare -a backward_compatible_checkout_objs=("2.2.fb.branch" "2.3.fb.branch" "2.4.fb.branch" "2.5.fb.branch" "2.6.fb.branch" "2.7.fb.branch" "2.8.1.fb" "3.0.fb.branch" "3.1.fb" "3.2.fb" "3.3.fb" "3.4.fb" "3.5.fb" "3.6.fb" "3.7.fb" "3.8.fb" "3.9.fb")
 declare -a forward_compatible_checkout_objs=("4.2.fb" "4.3.fb" "4.4.fb" "4.5.fb" "4.6.fb" "4.7.fb" "4.8.fb" "4.9.fb" "4.10.fb" "4.11.fb" "4.12.fb" "4.13.fb" "5.0.fb" "5.1.fb" "5.2.fb" "5.3.fb" "5.4.fb" "5.5.fb" "5.6.fb" "5.7.fb" "5.8.fb" "5.9.fb" "5.10.fb")
-declare -a forward_compatible_with_options_checkout_objs=("5.11.fb" "5.12.fb" "5.13.fb" "5.14.fb" "5.15.fb" "5.16.fb" "5.17.fb" "5.18.fb" "6.0.fb" "6.1.fb" "6.2.fb" "6.3.fb" "6.4.fb" "6.5.fb" "6.6.fb" "6.7.fb" "6.8.fb")
+declare -a forward_compatible_with_options_checkout_objs=("5.11.fb" "5.12.fb" "5.13.fb" "5.14.fb" "5.15.fb" "5.16.fb" "5.17.fb" "5.18.fb" "6.0.fb" "6.1.fb" "6.2.fb" "6.3.fb" "6.4.fb" "6.5.fb")
 declare -a checkout_objs=(${backward_compatible_checkout_objs[@]} ${forward_compatible_checkout_objs[@]} ${forward_compatible_with_options_checkout_objs[@]})
-declare -a extern_sst_ingestion_compatible_checkout_objs=("5.16.fb" "5.17.fb" "5.18.fb" "6.0.fb" "6.1.fb" "6.2.fb" "6.3.fb" "6.4.fb" "6.5.fb" "6.6.fb" "6.7.fb" "6.8.fb")
+declare -a extern_sst_ingestion_compatible_checkout_objs=("5.14.fb" "5.15.fb" "5.16.fb" "5.17.fb" "5.18.fb" "6.0.fb" "6.1.fb" "6.2.fb" "6.3.fb" "6.4.fb" "6.5.fb")
 
 generate_db()
 {

tools/db_crashtest.py

@@ -1,7 +1,5 @@
-#!/usr/bin/env python
+#!/usr/bin/env python2
 # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
-from __future__ import absolute_import, division, print_function, unicode_literals
-
 import os
 import sys
 import time
@@ -134,7 +132,7 @@ def is_direct_io_supported(dbname):
     with tempfile.NamedTemporaryFile(dir=dbname) as f:
         try:
             os.open(f.name, os.O_DIRECT)
-        except BaseException:
+        except:
             return False
         return True
 
@@ -299,8 +297,8 @@ def blackbox_crash_main(args, unknown_args):
         killtime = time.time() + cmd_params['interval']
 
         cmd = gen_cmd(dict(
-            list(cmd_params.items())
-            + list({'db': dbname}.items())), unknown_args)
+            cmd_params.items() +
+            {'db': dbname}.items()), unknown_args)
 
         child = subprocess.Popen(cmd, stderr=subprocess.PIPE)
         print("Running db_stress with pid=%d: %s\n\n"
@@ -325,7 +323,7 @@ def blackbox_crash_main(args, unknown_args):
                 time.sleep(1)  # time to stabilize after a kill
 
         while True:
-            line = child.stderr.readline().strip().decode('utf-8')
+            line = child.stderr.readline().strip()
             if line == '':
                 break
             elif not line.startswith('WARNING'):
@@ -350,7 +348,7 @@ def whitebox_crash_main(args, unknown_args):
 
     cur_time = time.time()
     exit_time = cur_time + cmd_params['duration']
-    half_time = cur_time + cmd_params['duration'] // 2
+    half_time = cur_time + cmd_params['duration'] / 2
 
     print("Running whitebox-crash-test with \n"
           + "total-duration=" + str(cmd_params['duration']) + "\n")
@@ -376,9 +374,9 @@ def whitebox_crash_main(args, unknown_args):
                 })
             elif kill_mode == 1:
                 if cmd_params.get('disable_wal', 0) == 1:
-                    my_kill_odd = kill_random_test // 50 + 1
+                    my_kill_odd = kill_random_test / 50 + 1
                 else:
-                    my_kill_odd = kill_random_test // 10 + 1
+                    my_kill_odd = kill_random_test / 10 + 1
                 additional_opts.update({
                     "kill_random_test": my_kill_odd,
                     "kill_prefix_blacklist": "WritableFileWriter::Append,"
@@ -388,7 +386,7 @@ def whitebox_crash_main(args, unknown_args):
                 # TODO: May need to adjust random odds if kill_random_test
                 # is too small.
                 additional_opts.update({
-                    "kill_random_test": (kill_random_test // 5000 + 1),
+                    "kill_random_test": (kill_random_test / 5000 + 1),
                     "kill_prefix_blacklist": "WritableFileWriter::Append,"
                     "WritableFileWriter::WriteBuffered,"
                     "PosixMmapFile::Allocate,WritableFileWriter::Flush",
@@ -408,7 +406,7 @@ def whitebox_crash_main(args, unknown_args):
             # style is quite a bit slower on reads with lot of files
             additional_opts = {
                 "kill_random_test": None,
-                "ops_per_thread": cmd_params['ops_per_thread'] // 5,
+                "ops_per_thread": cmd_params['ops_per_thread'] / 5,
                 "compaction_style": 2,
             }
         else:
@@ -418,24 +416,19 @@ def whitebox_crash_main(args, unknown_args):
                 "ops_per_thread": cmd_params['ops_per_thread'],
             }
 
-        cmd = gen_cmd(dict(list(cmd_params.items())
-            + list(additional_opts.items())
-            + list({'db': dbname}.items())), unknown_args)
+        cmd = gen_cmd(dict(cmd_params.items() + additional_opts.items()
+                           + {'db': dbname}.items()), unknown_args)
 
-        print("Running:" + ' '.join(cmd) + "\n")  # noqa: E999 T25377293 Grandfathered in
+        print "Running:" + ' '.join(cmd) + "\n"  # noqa: E999 T25377293 Grandfathered in
 
         popen = subprocess.Popen(cmd, stdout=subprocess.PIPE,
                                  stderr=subprocess.STDOUT)
         stdoutdata, stderrdata = popen.communicate()
-        if stdoutdata:
-            stdoutdata = stdoutdata.decode('utf-8')
-        if stderrdata:
-            stderrdata = stderrdata.decode('utf-8')
         retncode = popen.returncode
         msg = ("check_mode={0}, kill option={1}, exitcode={2}\n".format(
                check_mode, additional_opts['kill_random_test'], retncode))
-        print(msg)
-        print(stdoutdata)
+        print msg
+        print stdoutdata
 
         expected = False
         if additional_opts['kill_random_test'] is None and (retncode == 0):
@@ -447,19 +440,19 @@ def whitebox_crash_main(args, unknown_args):
             expected = True
 
         if not expected:
-            print("TEST FAILED. See kill option and exit code above!!!\n")
+            print "TEST FAILED. See kill option and exit code above!!!\n"
             sys.exit(1)
 
         stdoutdata = stdoutdata.lower()
         errorcount = (stdoutdata.count('error') -
                       stdoutdata.count('got errors 0 times'))
-        print("#times error occurred in output is " + str(errorcount) + "\n")
+        print "#times error occurred in output is " + str(errorcount) + "\n"
 
         if (errorcount > 0):
-            print("TEST FAILED. Output has 'error'!!!\n")
+            print "TEST FAILED. Output has 'error'!!!\n"
             sys.exit(2)
         if (stdoutdata.find('fail') >= 0):
-            print("TEST FAILED. Output has 'fail'!!!\n")
+            print "TEST FAILED. Output has 'fail'!!!\n"
             sys.exit(2)
 
         # First half of the duration, keep doing kill test. For the next half,
@@ -483,12 +476,12 @@ def main():
     parser.add_argument("--cf_consistency", action='store_true')
     parser.add_argument("--txn", action='store_true')
 
-    all_params = dict(list(default_params.items())
-                      + list(blackbox_default_params.items())
-                      + list(whitebox_default_params.items())
-                      + list(simple_default_params.items())
-                      + list(blackbox_simple_default_params.items())
-                      + list(whitebox_simple_default_params.items()))
+    all_params = dict(default_params.items()
+                      + blackbox_default_params.items()
+                      + whitebox_default_params.items()
+                      + simple_default_params.items()
+                      + blackbox_simple_default_params.items()
+                      + whitebox_simple_default_params.items())
 
     for k, v in all_params.items():
         parser.add_argument("--" + k, type=type(v() if callable(v) else v))

tools/ldb_cmd.cc

@@ -295,6 +295,8 @@ void LDBCommand::Run() {
     options_.env = env;
   }
 
+  options_.file_system.reset(new LegacyFileSystemWrapper(options_.env));
+
   if (db_ == nullptr && !NoDBOpen()) {
     OpenDB();
     if (exec_state_.IsFailed() && try_load_options_) {
@@ -1168,7 +1170,7 @@ void GetLiveFilesChecksumInfoFromVersionSet(Options options,
                       /*block_cache_tracer=*/nullptr);
   std::vector<std::string> cf_name_list;
   s = versions.ListColumnFamilies(&cf_name_list, db_path,
-                                  immutable_db_options.fs.get());
+                                  options.file_system.get());
   if (s.ok()) {
     std::vector<ColumnFamilyDescriptor> cf_list;
     for (const auto& name : cf_name_list) {
@@ -1911,6 +1913,8 @@ void ReduceDBLevelsCommand::DoCommand() {
   Status st;
   Options opt = PrepareOptionsForOpenDB();
   int old_level_num = -1;
+  opt.file_system.reset(new LegacyFileSystemWrapper(opt.env));
+  ;
   st = GetOldNumOfLevels(opt, &old_level_num);
   if (!st.ok()) {
     exec_state_ = LDBCommandExecuteResult::Failed(st.ToString());

tools/ldb_cmd_test.cc

@@ -81,6 +81,8 @@ TEST_F(LdbCmdTest, MemEnv) {
   opts.env = env.get();
   opts.create_if_missing = true;
 
+  opts.file_system.reset(new LegacyFileSystemWrapper(opts.env));
+
   DB* db = nullptr;
   std::string dbname = test::TmpDir();
   ASSERT_OK(DB::Open(opts, dbname, &db));
@@ -197,7 +199,7 @@ class FileChecksumTestHelper {
     std::vector<std::string> cf_name_list;
     Status s;
     s = versions.ListColumnFamilies(&cf_name_list, dbname_,
-                                    immutable_db_options.fs.get());
+                                    options_.file_system.get());
     if (s.ok()) {
       std::vector<ColumnFamilyDescriptor> cf_list;
       for (const auto& name : cf_name_list) {
@@ -262,6 +264,7 @@ TEST_F(LdbCmdTest, DumpFileChecksumNoChecksum) {
   Options opts;
   opts.env = env.get();
   opts.create_if_missing = true;
+  opts.file_system.reset(new LegacyFileSystemWrapper(opts.env));
 
   DB* db = nullptr;
   std::string dbname = test::TmpDir();
@@ -348,6 +351,7 @@ TEST_F(LdbCmdTest, DumpFileChecksumCRC32) {
   opts.create_if_missing = true;
   opts.sst_file_checksum_func =
       std::shared_ptr<FileChecksumFunc>(CreateFileChecksumFuncCrc32c());
+  opts.file_system.reset(new LegacyFileSystemWrapper(opts.env));
 
   DB* db = nullptr;
   std::string dbname = test::TmpDir();

tools/ldb_test.py

@@ -1,7 +1,5 @@
-#!/usr/bin/env python
+#!/usr/bin/env python2
 # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
-from __future__ import absolute_import, division, print_function, unicode_literals
-
 import os
 import glob
 import os.path
@@ -29,7 +27,7 @@ def my_check_output(*popenargs, **kwargs):
             cmd = popenargs[0]
         raise Exception("Exit code is not 0.  It is %d.  Command: %s" %
                 (retcode, cmd))
-    return output.decode('utf-8')
+    return output
 
 def run_err_null(cmd):
     return os.system(cmd + " 2>/dev/null ")
@@ -54,6 +52,7 @@ class LDBTestCase(unittest.TestCase):
         """
         All command-line params must be specified.
         Allows full flexibility in testing; for example: missing db param.
+
         """
         output = my_check_output("./ldb %s |grep -v \"Created bg thread\"" %
                             params, shell=True)
@@ -73,6 +72,7 @@ class LDBTestCase(unittest.TestCase):
         """
         All command-line params must be specified.
         Allows full flexibility in testing; for example: missing db param.
+
         """
         try:
 
@@ -87,6 +87,7 @@ class LDBTestCase(unittest.TestCase):
     def assertRunOK(self, params, expectedOutput, unexpected=False):
         """
         Uses the default test db.
+
         """
         self.assertRunOKFull("%s %s" % (self.dbParam(self.DB_NAME), params),
                              expectedOutput, unexpected)
@@ -98,7 +99,7 @@ class LDBTestCase(unittest.TestCase):
         self.assertRunFAILFull("%s %s" % (self.dbParam(self.DB_NAME), params))
 
     def testSimpleStringPutGet(self):
-        print("Running testSimpleStringPutGet...")
+        print "Running testSimpleStringPutGet..."
         self.assertRunFAIL("put x1 y1")
         self.assertRunOK("put --create_if_missing x1 y1", "OK")
         self.assertRunOK("get x1", "y1")
@@ -156,7 +157,7 @@ class LDBTestCase(unittest.TestCase):
                                      % (inputSst, params))
 
     def testStringBatchPut(self):
-        print("Running testStringBatchPut...")
+        print "Running testStringBatchPut..."
         self.assertRunOK("batchput x1 y1 --create_if_missing", "OK")
         self.assertRunOK("scan", "x1 : y1")
         self.assertRunOK("batchput x2 y2 x3 y3 \"x4 abc\" \"y4 xyz\"", "OK")
@@ -166,7 +167,7 @@ class LDBTestCase(unittest.TestCase):
         self.assertRunFAIL("batchput k1 v1 k2")
 
     def testCountDelimDump(self):
-        print("Running testCountDelimDump...")
+        print "Running testCountDelimDump..."
         self.assertRunOK("batchput x.1 x1 --create_if_missing", "OK")
         self.assertRunOK("batchput y.abc abc y.2 2 z.13c pqr", "OK")
         self.assertRunOK("dump --count_delim", "x => count:1\tsize:5\ny => count:2\tsize:12\nz => count:1\tsize:8")
@@ -175,7 +176,7 @@ class LDBTestCase(unittest.TestCase):
         self.assertRunOK("dump --count_delim=\",\"", "x => count:2\tsize:14\nx.1 => count:1\tsize:5\ny.2 => count:1\tsize:4\ny.abc => count:1\tsize:8\nz.13c => count:1\tsize:8")
 
     def testCountDelimIDump(self):
-        print("Running testCountDelimIDump...")
+        print "Running testCountDelimIDump..."
         self.assertRunOK("batchput x.1 x1 --create_if_missing", "OK")
         self.assertRunOK("batchput y.abc abc y.2 2 z.13c pqr", "OK")
         self.assertRunOK("idump --count_delim", "x => count:1\tsize:5\ny => count:2\tsize:12\nz => count:1\tsize:8")
@@ -184,7 +185,7 @@ class LDBTestCase(unittest.TestCase):
         self.assertRunOK("idump --count_delim=\",\"", "x => count:2\tsize:14\nx.1 => count:1\tsize:5\ny.2 => count:1\tsize:4\ny.abc => count:1\tsize:8\nz.13c => count:1\tsize:8")
 
     def testInvalidCmdLines(self):
-        print("Running testInvalidCmdLines...")
+        print "Running testInvalidCmdLines..."
         # db not specified
         self.assertRunFAILFull("put 0x6133 0x6233 --hex --create_if_missing")
         # No param called he
@@ -194,7 +195,7 @@ class LDBTestCase(unittest.TestCase):
         # hex has invalid boolean value
 
     def testHexPutGet(self):
-        print("Running testHexPutGet...")
+        print "Running testHexPutGet..."
         self.assertRunOK("put a1 b1 --create_if_missing", "OK")
         self.assertRunOK("scan", "a1 : b1")
         self.assertRunOK("scan --hex", "0x6131 : 0x6231")
@@ -224,7 +225,7 @@ class LDBTestCase(unittest.TestCase):
         self.assertRunOK("checkconsistency", "OK")
 
     def testTtlPutGet(self):
-        print("Running testTtlPutGet...")
+        print "Running testTtlPutGet..."
         self.assertRunOK("put a1 b1 --ttl --create_if_missing", "OK")
         self.assertRunOK("scan --hex", "0x6131 : 0x6231", True)
         self.assertRunOK("dump --ttl ", "a1 ==> b1", True)
@@ -239,7 +240,7 @@ class LDBTestCase(unittest.TestCase):
         self.assertRunOK("checkconsistency", "OK")
 
     def testInvalidCmdLines(self):  # noqa: F811 T25377293 Grandfathered in
-        print("Running testInvalidCmdLines...")
+        print "Running testInvalidCmdLines..."
         # db not specified
         self.assertRunFAILFull("put 0x6133 0x6233 --hex --create_if_missing")
         # No param called he
@@ -250,7 +251,7 @@ class LDBTestCase(unittest.TestCase):
         self.assertRunFAIL("put 0x6133 0x6233 --hex=Boo --create_if_missing")
 
     def testDumpLoad(self):
-        print("Running testDumpLoad...")
+        print "Running testDumpLoad..."
         self.assertRunOK("batchput --create_if_missing x1 y1 x2 y2 x3 y3 x4 y4",
                 "OK")
         self.assertRunOK("scan", "x1 : y1\nx2 : y2\nx3 : y3\nx4 : y4")
@@ -341,7 +342,7 @@ class LDBTestCase(unittest.TestCase):
             "--db=%s --create_if_missing" % origDbPath, dumpFilePath))
 
     def testIDumpBasics(self):
-        print("Running testIDumpBasics...")
+        print "Running testIDumpBasics..."
         self.assertRunOK("put a val --create_if_missing", "OK")
         self.assertRunOK("put b val", "OK")
         self.assertRunOK(
@@ -353,7 +354,7 @@ class LDBTestCase(unittest.TestCase):
                 "'a' seq:1, type:1 => val\nInternal keys in range: 1")
 
     def testMiscAdminTask(self):
-        print("Running testMiscAdminTask...")
+        print "Running testMiscAdminTask..."
         # These tests need to be improved; for example with asserts about
         # whether compaction or level reduction actually took place.
         self.assertRunOK("batchput --create_if_missing x1 y1 x2 y2 x3 y3 x4 y4",
@@ -389,7 +390,7 @@ class LDBTestCase(unittest.TestCase):
         self.assertRunOK("scan", "x1 : y1\nx2 : y2\nx3 : y3\nx4 : y4")
 
     def testCheckConsistency(self):
-        print("Running testCheckConsistency...")
+        print "Running testCheckConsistency..."
 
         dbPath = os.path.join(self.TMP_DIR, self.DB_NAME)
         self.assertRunOK("put x1 y1 --create_if_missing", "OK")
@@ -413,7 +414,7 @@ class LDBTestCase(unittest.TestCase):
             params, dumpFile))
 
     def testDumpLiveFiles(self):
-        print("Running testDumpLiveFiles...")
+        print "Running testDumpLiveFiles..."
 
         dbPath = os.path.join(self.TMP_DIR, self.DB_NAME)
         self.assertRunOK("put x1 y1 --create_if_missing", "OK")
@@ -438,7 +439,7 @@ class LDBTestCase(unittest.TestCase):
         return 0 == run_err_null("cp " + src + " " + dest)
 
     def testManifestDump(self):
-        print("Running testManifestDump...")
+        print "Running testManifestDump..."
         dbPath = os.path.join(self.TMP_DIR, self.DB_NAME)
         self.assertRunOK("put 1 1 --create_if_missing", "OK")
         self.assertRunOK("put 2 2", "OK")
@@ -474,7 +475,7 @@ class LDBTestCase(unittest.TestCase):
                              isPattern=True)
 
     def testSSTDump(self):
-        print("Running testSSTDump...")
+        print "Running testSSTDump..."
 
         dbPath = os.path.join(self.TMP_DIR, self.DB_NAME)
         self.assertRunOK("put sst1 sst1_val --create_if_missing", "OK")
@@ -494,7 +495,7 @@ class LDBTestCase(unittest.TestCase):
                              isPattern=True)
 
     def testWALDump(self):
-        print("Running testWALDump...")
+        print "Running testWALDump..."
 
         dbPath = os.path.join(self.TMP_DIR, self.DB_NAME)
         self.assertRunOK("put wal1 wal1_val --create_if_missing", "OK")
@@ -514,7 +515,7 @@ class LDBTestCase(unittest.TestCase):
                              isPattern=True)
 
     def testListColumnFamilies(self):
-        print("Running testListColumnFamilies...")
+        print "Running testListColumnFamilies..."
         self.assertRunOK("put x1 y1 --create_if_missing", "OK")
         cmd = "list_column_families | grep -v \"Column families\""
         # Test on valid dbPath.
@@ -523,7 +524,7 @@ class LDBTestCase(unittest.TestCase):
         self.assertRunFAIL(cmd)
 
     def testColumnFamilies(self):
-        print("Running testColumnFamilies...")
+        print "Running testColumnFamilies..."
         dbPath = os.path.join(self.TMP_DIR, self.DB_NAME)  # noqa: F841 T25377293 Grandfathered in
         self.assertRunOK("put cf1_1 1 --create_if_missing", "OK")
         self.assertRunOK("put cf1_2 2 --create_if_missing", "OK")
@@ -558,7 +559,7 @@ class LDBTestCase(unittest.TestCase):
         self.assertRunFAIL("drop_column_family four")
 
     def testIngestExternalSst(self):
-        print("Running testIngestExternalSst...")
+        print "Running testIngestExternalSst..."
 
         # Dump, load, write external sst and ingest it in another db
         dbPath = os.path.join(self.TMP_DIR, "db1")

tools/write_stress_runner.py

@@ -1,7 +1,5 @@
-#!/usr/bin/env python
+#!/usr/bin/env python2
 # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
-from __future__ import absolute_import, division, print_function, unicode_literals
-
 import subprocess
 import argparse
 import random
@@ -12,7 +10,7 @@ import sys
 def generate_runtimes(total_runtime):
     # combination of short runtimes and long runtimes, with heavier
     # weight on short runtimes
-    possible_runtimes_sec = list(range(1, 10)) + list(range(1, 20)) + [100, 1000]
+    possible_runtimes_sec = range(1, 10) + range(1, 20) + [100, 1000]
     runtimes = []
     while total_runtime > 0:
         chosen = random.choice(possible_runtimes_sec)
@@ -24,7 +22,7 @@ def generate_runtimes(total_runtime):
 
 def main(args):
     runtimes = generate_runtimes(int(args.runtime_sec))
-    print("Going to execute write stress for " + str(runtimes))  # noqa: E999 T25377293 Grandfathered in
+    print "Going to execute write stress for " + str(runtimes)  # noqa: E999 T25377293 Grandfathered in
     first_time = True
 
     for runtime in runtimes:

util/hash_map.h

@@ -36,7 +36,7 @@ class HashMap {
     return it != bucket.end();
   }
 
-  void Insert(K key, const V& value) {
+  void Insert(K key, V value) {
     auto& bucket = table_[key % size];
     bucket.push_back({key, value});
   }

util/threadpool_imp.cc

@@ -9,6 +9,9 @@
 
 #include "util/threadpool_imp.h"
 
+#include "monitoring/thread_status_util.h"
+#include "port/port.h"
+
 #ifndef OS_WIN
 #  include <unistd.h>
 #endif
@@ -28,10 +31,6 @@
 #include <thread>
 #include <vector>
 
-#include "monitoring/thread_status_util.h"
-#include "port/port.h"
-#include "test_util/sync_point.h"
-
 namespace ROCKSDB_NAMESPACE {
 
 void ThreadPoolImpl::PthreadCall(const char* label, int result) {
@@ -263,10 +262,6 @@ void ThreadPoolImpl::Impl::BGThread(size_t thread_id) {
     (void)decrease_io_priority;  // avoid 'unused variable' error
     (void)decrease_cpu_priority;
 #endif
-
-    TEST_SYNC_POINT_CALLBACK("ThreadPoolImpl::Impl::BGThread:BeforeRun",
-                             &priority_);
-
     func();
   }
 }

utilities/blob_db/blob_db_impl.cc

@@ -1483,7 +1483,7 @@ Status BlobDBImpl::GetRawBlobFromFile(const Slice& key, uint64_t file_number,
 
   // Allocate the buffer. This is safe in C++11
   std::string buf;
-  AlignedBuf aligned_buf;
+  std::unique_ptr<const char[]> internal_buf;
 
   // A partial blob record contain checksum, key and value.
   Slice blob_record;
@@ -1492,11 +1492,11 @@ Status BlobDBImpl::GetRawBlobFromFile(const Slice& key, uint64_t file_number,
     StopWatch read_sw(env_, statistics_, BLOB_DB_BLOB_FILE_READ_MICROS);
     if (reader->use_direct_io()) {
       s = reader->Read(record_offset, static_cast<size_t>(record_size),
-                       &blob_record, nullptr, &aligned_buf);
+                      &blob_record, nullptr, &internal_buf);
     } else {
       buf.reserve(static_cast<size_t>(record_size));
       s = reader->Read(record_offset, static_cast<size_t>(record_size),
-                       &blob_record, &buf[0], nullptr);
+                      &blob_record, &buf[0], nullptr);
     }
     RecordTick(statistics_, BLOB_DB_BLOB_FILE_BYTES_READ, blob_record.size());
   }

utilities/blob_db/blob_file.cc

@@ -139,11 +139,11 @@ Status BlobFile::ReadFooter(BlobLogFooter* bf) {
 
   Slice result;
   std::string buf;
-  AlignedBuf aligned_buf;
+  std::unique_ptr<const char[]> internal_buf;
   Status s;
   if (ra_file_reader_->use_direct_io()) {
     s = ra_file_reader_->Read(footer_offset, BlobLogFooter::kSize, &result,
-                              nullptr, &aligned_buf);
+                              nullptr, &internal_buf);
   } else {
     buf.reserve(BlobLogFooter::kSize + 10);
     s = ra_file_reader_->Read(footer_offset, BlobLogFooter::kSize, &result,
@@ -263,11 +263,11 @@ Status BlobFile::ReadMetadata(Env* env, const EnvOptions& env_options) {
 
   // Read file header.
   std::string header_buf;
-  AlignedBuf aligned_buf;
+  std::unique_ptr<const char[]> internal_buf;
   Slice header_slice;
   if (file_reader->use_direct_io()) {
     s = file_reader->Read(0, BlobLogHeader::kSize, &header_slice, nullptr,
-                          &aligned_buf);
+                          &internal_buf);
   } else {
     header_buf.reserve(BlobLogHeader::kSize);
     s = file_reader->Read(0, BlobLogHeader::kSize, &header_slice,
@@ -306,14 +306,12 @@ Status BlobFile::ReadMetadata(Env* env, const EnvOptions& env_options) {
   std::string footer_buf;
   Slice footer_slice;
   if (file_reader->use_direct_io()) {
-    s = file_reader->Read(file_size - BlobLogFooter::kSize,
-                          BlobLogFooter::kSize, &footer_slice, nullptr,
-                          &aligned_buf);
+    s = file_reader->Read(file_size - BlobLogFooter::kSize, BlobLogFooter::kSize,
+                          &footer_slice, nullptr, &internal_buf);
   } else {
     footer_buf.reserve(BlobLogFooter::kSize);
-    s = file_reader->Read(file_size - BlobLogFooter::kSize,
-                          BlobLogFooter::kSize, &footer_slice, &footer_buf[0],
-                          nullptr);
+    s = file_reader->Read(file_size - BlobLogFooter::kSize, BlobLogFooter::kSize,
+                          &footer_slice, &footer_buf[0], nullptr);
   }
   if (!s.ok()) {
     ROCKS_LOG_ERROR(info_log_,

utilities/write_batch_with_index/write_batch_with_index.cc

@@ -982,8 +982,7 @@ void WriteBatchWithIndex::MultiGetFromBatchAndDB(
 
     assert(result == WriteBatchWithIndexInternal::Result::kMergeInProgress ||
            result == WriteBatchWithIndexInternal::Result::kNotFound);
-    key_context.emplace_back(column_family, keys[i], &values[i],
-                             /*timestamp*/ nullptr, &statuses[i]);
+    key_context.emplace_back(column_family, keys[i], &values[i], &statuses[i]);
     merges.emplace_back(result, std::move(merge_context));
   }