Making platform 007 (gcc 7) default in build_detect_platform.sh (#5947)

Summary:
Making platform 007 (gcc 7) default in build_detect_platform.sh.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5947

Differential Revision: D18038837

Pulled By: vjnadimpalli

fbshipit-source-id: 9ac2ddaa93bf328a416faec028970e039886378e

CMakeLists.txt

@@ -336,18 +336,12 @@ endif()
 
 # Used to run CI build and tests so we can run faster
 option(OPTDBG "Build optimized debug build with MSVC" OFF)
-option(WITH_RUNTIME_DEBUG "build with debug version of runtime library" ON)
 if(MSVC)
   if(OPTDBG)
     message(STATUS "Debug optimization is enabled")
-    set(CMAKE_CXX_FLAGS_DEBUG "/Oxt")
+    set(CMAKE_CXX_FLAGS_DEBUG "/Oxt /${RUNTIME_LIBRARY}d")
   else()
-    set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /Od /RTC1 /Gm")
-  endif()
-  if(WITH_RUNTIME_DEBUG)
-    set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /${RUNTIME_LIBRARY}d")
-  else()
-    set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /${RUNTIME_LIBRARY}")
+    set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /Od /RTC1 /Gm /${RUNTIME_LIBRARY}d")
   endif()
   set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} /Oxt /Zp8 /Gm- /Gy /${RUNTIME_LIBRARY}")
 
@@ -946,7 +940,6 @@ if(WITH_TESTS)
         util/hash_test.cc
         util/heap_test.cc
         util/rate_limiter_test.cc
-        util/repeatable_thread_test.cc
         util/slice_transform_test.cc
         util/timer_queue_test.cc
         util/thread_list_test.cc
@@ -989,7 +982,6 @@ if(WITH_TESTS)
   set(BENCHMARKS
     cache/cache_bench.cc
     memtable/memtablerep_bench.cc
-    db/range_del_aggregator_bench.cc
     tools/db_bench.cc
     table/table_reader_bench.cc
     utilities/column_aware_encoding_exp.cc

HISTORY.md

@@ -1,33 +1,17 @@
 # Rocksdb Change Log
-## 5.17.4 (12/05/2018)
-### Bug Fixes
-* Fix a deadlock caused by compaction and file ingestion waiting for each other in the event of write stalls.
 
-# 5.17.2 (10/24/2018)
+# 5.16.6 (11/24/2018)
 ### Bug Fixes
 * Fix the bug that WriteBatchWithIndex's SeekForPrev() doesn't see the entries with the same key.
 
-# 5.17.1 (10/16/2018)
+## 5.16.5 (10/16/2018)
 ### Bug Fixes
 * Fix slow flush/compaction when DB contains many snapshots. The problem became noticeable to us in DBs with 100,000+ snapshots, though it will affect others at different thresholds.
 * Properly set the stop key for a truncated manual CompactRange
-* Fix corner case where a write group leader blocked due to write stall blocks other writers in queue with WriteOptions::no_slowdown set.
-
-### New Features
-* Introduced CacheAllocator, which lets the user specify custom allocator for memory in block cache.
-
-## 5.17.0 (10/05/2018)
-### Public API Change
-* `OnTableFileCreated` will now be called for empty files generated during compaction. In that case, `TableFileCreationInfo::file_path` will be "(nil)" and `TableFileCreationInfo::file_size` will be zero.
-* Add `FlushOptions::allow_write_stall`, which controls whether Flush calls start working immediately, even if it causes user writes to stall, or will wait until flush can be performed without causing write stall (similar to `CompactRangeOptions::allow_write_stall`). Note that the default value is false, meaning we add delay to Flush calls until stalling can be avoided when possible. This is behavior change compared to previous RocksDB versions, where Flush calls didn't check if they might cause stall or not.
-* Application using PessimisticTransactionDB is expected to rollback/commit recovered transactions before starting new ones. This assumption is used to skip concurrency control during recovery.
-
-### New Features
-* TransactionOptions::skip_concurrency_control allows pessimistic transactions to skip the overhead of concurrency control. Could be used for optimizing certain transactions or during recovery.
 
+## 5.16.4 (10/10/2018)
 ### Bug Fixes
-* Avoid creating empty SSTs and subsequently deleting them in certain cases during compaction.
-* Sync CURRENT file contents during checkpoint.
+* Fix corner case where a write group leader blocked due to write stall blocks other writers in queue with WriteOptions::no_slowdown set.
 
 ## 5.16.3 (10/1/2018)
 ### Bug Fixes
@@ -45,9 +29,11 @@
 
 ## 5.16.0 (8/21/2018)
 ### Public API Change
+* `OnTableFileCreated` will now be called for empty files generated during compaction. In that case, `TableFileCreationInfo::file_path` will be "(nil)" and `TableFileCreationInfo::file_size` will be zero.
+* Add `FlushOptions::allow_write_stall`, which controls whether Flush calls start working immediately, even if it causes user writes to stall, or will wait until flush can be performed without causing write stall (similar to `CompactRangeOptions::allow_write_stall`). Note that the default value is false, meaning we add delay to Flush calls until stalling can be avoided when possible. This is behavior change compared to previous RocksDB versions, where Flush calls didn't check if they might cause stall or not.
+
 * The merge operands are passed to `MergeOperator::ShouldMerge` in the reversed order relative to how they were merged (passed to FullMerge or FullMergeV2) for performance reasons
 * GetAllKeyVersions() to take an extra argument of `max_num_ikeys`.
-* Using ZSTD dictionary trainer (i.e., setting `CompressionOptions::zstd_max_train_bytes` to a nonzero value) now requires ZSTD version 1.1.3 or later.
 
 ### New Features
 * Changes the format of index blocks by delta encoding the index values, which are the block handles. This saves the encoding of BlockHandle::offset of the non-head index entries in each restart interval. The feature is backward compatible but not forward compatible. It is disabled by default unless format_version 4 or above is used.
@@ -56,6 +42,7 @@
 
 ### Bug Fixes
 * Fix a bug in misreporting the estimated partition index size in properties block.
+* Avoid creating empty SSTs and subsequently deleting them in certain cases during compaction.
 
 ## 5.15.0 (7/17/2018)
 ### Public API Change
@@ -191,8 +178,7 @@
 * `BackupableDBOptions::max_valid_backups_to_open == 0` now means no backups will be opened during BackupEngine initialization. Previously this condition disabled limiting backups opened.
 * `DBOptions::preserve_deletes` is a new option that allows one to specify that DB should not drop tombstones for regular deletes if they have sequence number larger than what was set by the new API call `DB::SetPreserveDeletesSequenceNumber(SequenceNumber seqnum)`. Disabled by default.
 * API call `DB::SetPreserveDeletesSequenceNumber(SequenceNumber seqnum)` was added, users who wish to preserve deletes are expected to periodically call this function to advance the cutoff seqnum (all deletes made before this seqnum can be dropped by DB). It's user responsibility to figure out how to advance the seqnum in the way so the tombstones are kept for the desired period of time, yet are eventually processed in time and don't eat up too much space.
-* `ReadOptions::iter_start_seqnum` was added;
-if set to something > 0 user will see 2 changes in iterators behavior 1) only keys written with sequence larger than this parameter would be returned and 2) the `Slice` returned by iter->key() now points to the memory that keep User-oriented representation of the internal key, rather than user key. New struct `FullKey` was added to represent internal keys, along with a new helper function `ParseFullKey(const Slice& internal_key, FullKey* result);`.
+* `ReadOptions::iter_start_seqnum` was added; if set to something > 0 user will see 2 changes in iterators behavior 1) only keys written with sequence larger than this parameter would be returned and 2) the `Slice` returned by iter->key() now points to the memory that keep User-oriented representation of the internal key, rather than user key. New struct `FullKey` was added to represent internal keys, along with a new helper function `ParseFullKey(const Slice& internal_key, FullKey* result);`.
 * Deprecate trash_dir param in NewSstFileManager, right now we will rename deleted files to <name>.trash instead of moving them to trash directory
 * Allow setting a custom trash/DB size ratio limit in the SstFileManager, after which files that are to be scheduled for deletion are deleted immediately, regardless of any delete ratelimit.
 * Return an error on write if write_options.sync = true and write_options.disableWAL = true to warn user of inconsistent options. Previously we will not write to WAL and not respecting the sync options in this case.

Makefile

@@ -533,7 +533,6 @@ TESTS = \
 	write_unprepared_transaction_test \
 	db_universal_compaction_test \
 	trace_analyzer_test \
-	repeatable_thread_test \
 
 PARALLEL_TEST = \
 	backupable_db_test \
@@ -583,7 +582,7 @@ TEST_LIBS = \
 	librocksdb_env_basic_test.a
 
 # TODO: add back forward_iterator_bench, after making it build in all environemnts.
-BENCHMARKS = db_bench table_reader_bench cache_bench memtablerep_bench column_aware_encoding_exp persistent_cache_bench range_del_aggregator_bench
+BENCHMARKS = db_bench table_reader_bench cache_bench memtablerep_bench column_aware_encoding_exp persistent_cache_bench
 
 # if user didn't config LIBNAME, set the default
 ifeq ($(LIBNAME),)
@@ -1003,10 +1002,8 @@ unity.a: unity.o
 	$(AM_V_AR)rm -f $@
 	$(AM_V_at)$(AR) $(ARFLAGS) $@ unity.o
 
-
-TOOLLIBOBJECTS = $(TOOL_LIB_SOURCES:.cc=.o)
 # try compiling db_test with unity
-unity_test: db/db_test.o db/db_test_util.o $(TESTHARNESS) $(TOOLLIBOBJECTS) unity.a
+unity_test: db/db_test.o db/db_test_util.o $(TESTHARNESS) unity.a
 	$(AM_LINK)
 	./unity_test
 
@@ -1551,15 +1548,9 @@ lua_test: utilities/lua/rocks_lua_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTH
 range_del_aggregator_test: db/range_del_aggregator_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)
 
-range_del_aggregator_bench: db/range_del_aggregator_bench.o $(LIBOBJECTS) $(TESTUTIL)
-	$(AM_LINK)
-
 blob_db_test: utilities/blob_db/blob_db_test.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)
 
-repeatable_thread_test: util/repeatable_thread_test.o $(LIBOBJECTS) $(TESTHARNESS)
-	$(AM_LINK)
-
 #-------------------------------------------------
 # make install related stuff
 INSTALL_PATH ?= /usr/local

TARGETS

@@ -292,7 +292,7 @@ cpp_library(
         "utilities/write_batch_with_index/write_batch_with_index.cc",
         "utilities/write_batch_with_index/write_batch_with_index_internal.cc",
     ],
-    auto_headers = AutoHeaders.RECURSIVE_GLOB,
+    headers = AutoHeaders.RECURSIVE_GLOB,
     arch_preprocessor_flags = rocksdb_arch_preprocessor_flags,
     compiler_flags = rocksdb_compiler_flags,
     preprocessor_flags = rocksdb_preprocessor_flags,
@@ -314,7 +314,7 @@ cpp_library(
         "utilities/col_buf_encoder.cc",
         "utilities/column_aware_encoding_util.cc",
     ],
-    auto_headers = AutoHeaders.RECURSIVE_GLOB,
+    headers = AutoHeaders.RECURSIVE_GLOB,
     arch_preprocessor_flags = rocksdb_arch_preprocessor_flags,
     compiler_flags = rocksdb_compiler_flags,
     preprocessor_flags = rocksdb_preprocessor_flags,
@@ -326,10 +326,9 @@ cpp_library(
     name = "rocksdb_tools_lib",
     srcs = [
         "tools/db_bench_tool.cc",
-        "tools/trace_analyzer_tool.cc",
         "util/testutil.cc",
     ],
-    auto_headers = AutoHeaders.RECURSIVE_GLOB,
+    headers = AutoHeaders.RECURSIVE_GLOB,
     arch_preprocessor_flags = rocksdb_arch_preprocessor_flags,
     compiler_flags = rocksdb_compiler_flags,
     preprocessor_flags = rocksdb_preprocessor_flags,
@@ -340,7 +339,7 @@ cpp_library(
 cpp_library(
     name = "env_basic_test_lib",
     srcs = ["env/env_basic_test.cc"],
-    auto_headers = AutoHeaders.RECURSIVE_GLOB,
+    headers = AutoHeaders.RECURSIVE_GLOB,
     arch_preprocessor_flags = rocksdb_arch_preprocessor_flags,
     compiler_flags = rocksdb_compiler_flags,
     preprocessor_flags = rocksdb_preprocessor_flags,
@@ -935,11 +934,6 @@ ROCKS_TESTS = [
         "db/repair_test.cc",
         "serial",
     ],
-    [
-        "repeatable_thread_test",
-        "util/repeatable_thread_test.cc",
-        "serial",
-    ],
     [
         "sim_cache_test",
         "utilities/simulator_cache/sim_cache_test.cc",
@@ -1087,19 +1081,19 @@ if not is_opt_mode:
         ttype = "gtest" if test_cfg[2] == "parallel" else "simple"
         test_bin = test_name + "_bin"
 
-        cpp_binary(
-            name = test_bin,
-            srcs = [test_cc],
-            arch_preprocessor_flags = rocksdb_arch_preprocessor_flags,
-            compiler_flags = rocksdb_compiler_flags,
-            preprocessor_flags = rocksdb_preprocessor_flags,
-            deps = [":rocksdb_test_lib"],
-            external_deps = rocksdb_external_deps,
+        cpp_binary (
+          name = test_bin,
+          srcs = [test_cc],
+          deps = [":rocksdb_test_lib"],
+          preprocessor_flags = rocksdb_preprocessor_flags,
+          arch_preprocessor_flags = rocksdb_arch_preprocessor_flags,
+          compiler_flags = rocksdb_compiler_flags,
+          external_deps = rocksdb_external_deps,
         )
 
         custom_unittest(
-            name = test_name,
-            command = [TEST_RUNNER, BUCK_BINS + test_bin],
-            type = ttype,
-            deps = [":" + test_bin],
+          name = test_name,
+          type = ttype,
+          deps = [":" + test_bin],
+          command = [TEST_RUNNER, BUCK_BINS + test_bin]
         )

USERS.md

@@ -12,7 +12,6 @@ At Facebook, we use RocksDB as storage engines in multiple data management servi
 6. LogDevice -- a distributed data store for logs [2]
 
 [1] https://research.facebook.com/publications/realtime-data-processing-at-facebook/
-
 [2] https://code.facebook.com/posts/357056558062811/logdevice-a-distributed-data-store-for-logs/
 
 ## LinkedIn
@@ -89,6 +88,3 @@ LzLabs is using RocksDB as a storage engine in their multi-database distributed
 
 ## ProfaneDB
 [ProfaneDB](https://profanedb.gitlab.io/) is a database for Protocol Buffers, and uses RocksDB for storage. It is accessible via gRPC, and the schema is defined using directly `.proto` files.
-
-## IOTA Foundation
- [IOTA Foundation](https://www.iota.org/) is using RocksDB in the [IOTA Reference Implementation (IRI)](https://github.com/iotaledger/iri) to store the local state of the Tangle. The Tangle is the first open-source distributed ledger powering the future of the Internet of Things.

buckifier/buckify_rocksdb.py

@@ -109,14 +109,12 @@ def generate_targets(repo_path):
         "rocksdb_test_lib",
         src_mk.get("MOCK_LIB_SOURCES", []) +
         src_mk.get("TEST_LIB_SOURCES", []) +
-        src_mk.get("EXP_LIB_SOURCES", []) +
-        src_mk.get("ANALYZER_LIB_SOURCES", []),
+        src_mk.get("EXP_LIB_SOURCES", []),
         [":rocksdb_lib"])
     # rocksdb_tools_lib
     TARGETS.add_library(
         "rocksdb_tools_lib",
         src_mk.get("BENCH_LIB_SOURCES", []) +
-        src_mk.get("ANALYZER_LIB_SOURCES", []) +
         ["util/testutil.cc"],
         [":rocksdb_lib"])
 

buckifier/targets_builder.py

@@ -10,7 +10,7 @@ def pretty_list(lst, indent=8):
 
     if len(lst) == 1:
         return "\"%s\"" % lst[0]
-
+    
     separator = "\",\n%s\"" % (" " * indent)
     res = separator.join(sorted(lst))
     res = "\n" + (" " * indent) + "\"" + res + "\",\n" + (" " * (indent - 4))
@@ -31,16 +31,13 @@ class TARGETSBuilder:
         self.targets_file.close()
 
     def add_library(self, name, srcs, deps=None, headers=None):
-        headers_attr_prefix = ""
         if headers is None:
-            headers_attr_prefix = "auto_"
             headers = "AutoHeaders.RECURSIVE_GLOB"
-        self.targets_file.write(targets_cfg.library_template.format(
-            name=name,
-            srcs=pretty_list(srcs),
-            headers_attr_prefix=headers_attr_prefix,
-            headers=headers,
-            deps=pretty_list(deps)))
+        self.targets_file.write(targets_cfg.library_template % (
+            name,
+            pretty_list(srcs),
+            headers,
+            pretty_list(deps)))
         self.total_lib = self.total_lib + 1
 
     def add_binary(self, name, srcs, deps=None):

buckifier/targets_cfg.py

@@ -2,7 +2,8 @@ from __future__ import absolute_import
 from __future__ import division
 from __future__ import print_function
 from __future__ import unicode_literals
-rocksdb_target_header = """load("@fbcode_macros//build_defs:auto_headers.bzl", "AutoHeaders")
+rocksdb_target_header = """
+load("@fbcode_macros//build_defs:auto_headers.bzl", "AutoHeaders")
 
 REPO_PATH = package_name() + "/"
 
@@ -75,13 +76,13 @@ if is_opt_mode:
 
 library_template = """
 cpp_library(
-    name = "{name}",
-    srcs = [{srcs}],
-    {headers_attr_prefix}headers = {headers},
+    name = "%s",
+    srcs = [%s],
+    headers = %s,
     arch_preprocessor_flags = rocksdb_arch_preprocessor_flags,
     compiler_flags = rocksdb_compiler_flags,
     preprocessor_flags = rocksdb_preprocessor_flags,
-    deps = [{deps}],
+    deps = [%s],
     external_deps = rocksdb_external_deps,
 )
 """
@@ -120,20 +121,21 @@ if not is_opt_mode:
         ttype = "gtest" if test_cfg[2] == "parallel" else "simple"
         test_bin = test_name + "_bin"
 
-        cpp_binary(
-            name = test_bin,
-            srcs = [test_cc],
-            arch_preprocessor_flags = rocksdb_arch_preprocessor_flags,
-            compiler_flags = rocksdb_compiler_flags,
-            preprocessor_flags = rocksdb_preprocessor_flags,
-            deps = [":rocksdb_test_lib"],
-            external_deps = rocksdb_external_deps,
+        cpp_binary (
+          name = test_bin,
+          srcs = [test_cc],
+          deps = [":rocksdb_test_lib"],
+          preprocessor_flags = rocksdb_preprocessor_flags,
+          arch_preprocessor_flags = rocksdb_arch_preprocessor_flags,
+          compiler_flags = rocksdb_compiler_flags,
+          external_deps = rocksdb_external_deps,
         )
 
         custom_unittest(
-            name = test_name,
-            command = [TEST_RUNNER, BUCK_BINS + test_bin],
-            type = ttype,
-            deps = [":" + test_bin],
+          name = test_name,
+          type = ttype,
+          deps = [":" + test_bin],
+          command = [TEST_RUNNER, BUCK_BINS + test_bin]
         )
+
 """

db/builder.cc

@@ -121,8 +121,8 @@ Status BuildTable(
       file->SetIOPriority(io_priority);
       file->SetWriteLifeTimeHint(write_hint);
 
-      file_writer.reset(new WritableFileWriter(
-          std::move(file), fname, env_options, ioptions.statistics));
+      file_writer.reset(new WritableFileWriter(std::move(file), env_options,
+                                               ioptions.statistics));
       builder = NewTableBuilder(
           ioptions, mutable_cf_options, internal_comparator,
           int_tbl_prop_collector_factories, column_family_id,

db/c.cc

@@ -33,7 +33,6 @@
 #include "rocksdb/utilities/backupable_db.h"
 #include "rocksdb/utilities/checkpoint.h"
 #include "rocksdb/utilities/db_ttl.h"
-#include "rocksdb/utilities/memory_util.h"
 #include "rocksdb/utilities/optimistic_transaction_db.h"
 #include "rocksdb/utilities/transaction.h"
 #include "rocksdb/utilities/transaction_db.h"
@@ -42,10 +41,6 @@
 #include "rocksdb/perf_context.h"
 #include "utilities/merge_operators.h"
 
-#include <vector>
-#include <unordered_set>
-#include <map>
-
 using rocksdb::BytewiseComparator;
 using rocksdb::Cache;
 using rocksdb::ColumnFamilyDescriptor;
@@ -113,12 +108,8 @@ using rocksdb::TransactionLogIterator;
 using rocksdb::BatchResult;
 using rocksdb::PerfLevel;
 using rocksdb::PerfContext;
-using rocksdb::MemoryUtil;
 
 using std::shared_ptr;
-using std::vector;
-using std::unordered_set;
-using std::map;
 
 extern "C" {
 
@@ -2411,7 +2402,7 @@ void rocksdb_options_set_bytes_per_sync(
 
 void rocksdb_options_set_writable_file_max_buffer_size(rocksdb_options_t* opt,
                                                        uint64_t v) {
-  opt->rep.writable_file_max_buffer_size = static_cast<size_t>(v);
+  opt->rep.writable_file_max_buffer_size = v;
 }
 
 void rocksdb_options_set_allow_concurrent_memtable_write(rocksdb_options_t* opt,
@@ -2442,13 +2433,11 @@ void rocksdb_options_set_max_write_buffer_number_to_maintain(
   opt->rep.max_write_buffer_number_to_maintain = n;
 }
 
-void rocksdb_options_set_enable_pipelined_write(rocksdb_options_t* opt,
-                                                unsigned char v) {
+void rocksdb_options_set_enable_pipelined_write(rocksdb_options_t* opt, unsigned char v) {
   opt->rep.enable_pipelined_write = v;
 }
 
-void rocksdb_options_set_max_subcompactions(rocksdb_options_t* opt,
-                                            uint32_t n) {
+void rocksdb_options_set_max_subcompactions(rocksdb_options_t* opt, uint32_t n) {
   opt->rep.max_subcompactions = n;
 }
 
@@ -4110,98 +4099,6 @@ const char* rocksdb_pinnableslice_value(const rocksdb_pinnableslice_t* v,
   *vlen = v->rep.size();
   return v->rep.data();
 }
-
-// container to keep databases and caches in order to use rocksdb::MemoryUtil
-struct rocksdb_memory_consumers_t {
-  std::vector<rocksdb_t*> dbs;
-  std::unordered_set<rocksdb_cache_t*> caches;
-};
-
-// initializes new container of memory consumers
-rocksdb_memory_consumers_t* rocksdb_memory_consumers_create() {
-  return new rocksdb_memory_consumers_t;
-}
-
-// adds datatabase to the container of memory consumers
-void rocksdb_memory_consumers_add_db(rocksdb_memory_consumers_t* consumers,
-                                     rocksdb_t* db) {
-  consumers->dbs.push_back(db);
-}
-
-// adds cache to the container of memory consumers
-void rocksdb_memory_consumers_add_cache(rocksdb_memory_consumers_t* consumers,
-                                        rocksdb_cache_t* cache) {
-  consumers->caches.insert(cache);
-}
-
-// deletes container with memory consumers
-void rocksdb_memory_consumers_destroy(rocksdb_memory_consumers_t* consumers) {
-  delete consumers;
-}
-
-// contains memory usage statistics provided by rocksdb::MemoryUtil
-struct rocksdb_memory_usage_t {
-  uint64_t mem_table_total;
-  uint64_t mem_table_unflushed;
-  uint64_t mem_table_readers_total;
-  uint64_t cache_total;
-};
-
-// estimates amount of memory occupied by consumers (dbs and caches)
-rocksdb_memory_usage_t* rocksdb_approximate_memory_usage_create(
-    rocksdb_memory_consumers_t* consumers, char** errptr) {
-
-  vector<DB*> dbs;
-  for (auto db : consumers->dbs) {
-    dbs.push_back(db->rep);
-  }
-
-  unordered_set<const Cache*> cache_set;
-  for (auto cache : consumers->caches) {
-    cache_set.insert(const_cast<const Cache*>(cache->rep.get()));
-  }
-
-  std::map<rocksdb::MemoryUtil::UsageType, uint64_t> usage_by_type;
-
-  auto status = MemoryUtil::GetApproximateMemoryUsageByType(dbs, cache_set,
-                                                            &usage_by_type);
-  if (SaveError(errptr, status)) {
-    return nullptr;
-  }
-
-  auto result = new rocksdb_memory_usage_t;
-  result->mem_table_total = usage_by_type[MemoryUtil::kMemTableTotal];
-  result->mem_table_unflushed = usage_by_type[MemoryUtil::kMemTableUnFlushed];
-  result->mem_table_readers_total = usage_by_type[MemoryUtil::kTableReadersTotal];
-  result->cache_total = usage_by_type[MemoryUtil::kCacheTotal];
-  return result;
-}
-
-uint64_t rocksdb_approximate_memory_usage_get_mem_table_total(
-    rocksdb_memory_usage_t* memory_usage) {
-  return memory_usage->mem_table_total;
-}
-
-uint64_t rocksdb_approximate_memory_usage_get_mem_table_unflushed(
-    rocksdb_memory_usage_t* memory_usage) {
-  return memory_usage->mem_table_unflushed;
-}
-
-uint64_t rocksdb_approximate_memory_usage_get_mem_table_readers_total(
-    rocksdb_memory_usage_t* memory_usage) {
-  return memory_usage->mem_table_readers_total;
-}
-
-uint64_t rocksdb_approximate_memory_usage_get_cache_total(
-    rocksdb_memory_usage_t* memory_usage) {
-  return memory_usage->cache_total;
-}
-
-// deletes container with memory usage estimates
-void rocksdb_approximate_memory_usage_destroy(rocksdb_memory_usage_t* usage) {
-  delete usage;
-}
-
 }  // end extern "C"
 
 #endif  // !ROCKSDB_LITE

db/c_test.c

@@ -19,8 +19,11 @@
 
 // Can not use port/port.h macros as this is a c file
 #ifdef OS_WIN
+
 #include <windows.h>
 
+#define snprintf _snprintf
+
 // Ok for uniqueness
 int geteuid() {
   int result = 0;
@@ -31,11 +34,6 @@ int geteuid() {
   return result;
 }
 
-// VS < 2015
-#if defined(_MSC_VER) && (_MSC_VER < 1900)
-#define snprintf _snprintf
-#endif
-
 #endif
 
 const char* phase = "";
@@ -49,19 +47,12 @@ static void StartPhase(const char* name) {
   fprintf(stderr, "=== Test %s\n", name);
   phase = name;
 }
-#ifdef _MSC_VER
-#pragma warning(push)
-#pragma warning (disable: 4996) // getenv security warning
-#endif
 static const char* GetTempDir(void) {
     const char* ret = getenv("TEST_TMPDIR");
     if (ret == NULL || ret[0] == '\0')
         ret = "/tmp";
     return ret;
 }
-#ifdef _MSC_VER
-#pragma warning(pop)
-#endif
 
 #define CheckNoError(err)                                               \
   if ((err) != NULL) {                                                  \
@@ -652,7 +643,7 @@ int main(int argc, char** argv) {
     rocksdb_sstfilewriter_t* writer =
         rocksdb_sstfilewriter_create(env_opt, io_options);
 
-    remove(sstfilename);
+    unlink(sstfilename);
     rocksdb_sstfilewriter_open(writer, sstfilename, &err);
     CheckNoError(err);
     rocksdb_sstfilewriter_put(writer, "sstk1", 5, "v1", 2, &err);
@@ -673,7 +664,7 @@ int main(int argc, char** argv) {
     CheckGet(db, roptions, "sstk2", "v2");
     CheckGet(db, roptions, "sstk3", "v3");
 
-    remove(sstfilename);
+    unlink(sstfilename);
     rocksdb_sstfilewriter_open(writer, sstfilename, &err);
     CheckNoError(err);
     rocksdb_sstfilewriter_put(writer, "sstk2", 5, "v4", 2, &err);
@@ -1343,47 +1334,6 @@ int main(int argc, char** argv) {
     rocksdb_destroy_db(options, dbname, &err);
   }
 
-  // Check memory usage stats
-  StartPhase("approximate_memory_usage");
-  {
-    // Create database
-    db = rocksdb_open(options, dbname, &err);
-    CheckNoError(err);
-
-    rocksdb_memory_consumers_t* consumers;
-    consumers = rocksdb_memory_consumers_create();
-    rocksdb_memory_consumers_add_db(consumers, db);
-    rocksdb_memory_consumers_add_cache(consumers, cache);
-
-    // take memory usage report before write-read operation
-    rocksdb_memory_usage_t* mu1;
-    mu1 = rocksdb_approximate_memory_usage_create(consumers, &err);
-    CheckNoError(err);
-
-    // Put data (this should affect memtables)
-    rocksdb_put(db, woptions, "memory", 6, "test", 4, &err);
-    CheckNoError(err);
-    CheckGet(db, roptions, "memory", "test");
-
-    // take memory usage report after write-read operation
-    rocksdb_memory_usage_t* mu2;
-    mu2 = rocksdb_approximate_memory_usage_create(consumers, &err);
-    CheckNoError(err);
-
-    // amount of memory used within memtables should grow
-    CheckCondition(rocksdb_approximate_memory_usage_get_mem_table_total(mu2) >=
-                   rocksdb_approximate_memory_usage_get_mem_table_total(mu1));
-    CheckCondition(rocksdb_approximate_memory_usage_get_mem_table_unflushed(mu2) >=
-                   rocksdb_approximate_memory_usage_get_mem_table_unflushed(mu1));
-
-    rocksdb_memory_consumers_destroy(consumers);
-    rocksdb_approximate_memory_usage_destroy(mu1);
-    rocksdb_approximate_memory_usage_destroy(mu2);
-    rocksdb_close(db);
-    rocksdb_destroy_db(options, dbname, &err);
-    CheckNoError(err);
-  }
-
   StartPhase("cuckoo_options");
   {
     rocksdb_cuckoo_table_options_t* cuckoo_options;
@@ -1725,7 +1675,7 @@ int main(int argc, char** argv) {
     db = rocksdb_open(options, dbname, &err);
     CheckNoError(err);
   }
-
+  
   StartPhase("cleanup");
   rocksdb_close(db);
   rocksdb_options_destroy(options);

db/compact_files_test.cc

@@ -309,14 +309,6 @@ TEST_F(CompactFilesTest, CompactionFilterWithGetSv) {
 }
 
 TEST_F(CompactFilesTest, SentinelCompressionType) {
-  if (!Zlib_Supported()) {
-    fprintf(stderr, "zlib compression not supported, skip this test\n");
-    return;
-  }
-  if (!Snappy_Supported()) {
-    fprintf(stderr, "snappy compression not supported, skip this test\n");
-    return;
-  }
   // Check that passing `CompressionType::kDisableCompressionOption` to
   // `CompactFiles` causes it to use the column family compression options.
   for (auto compaction_style :

db/compacted_db_impl.cc

@@ -25,12 +25,22 @@ CompactedDBImpl::~CompactedDBImpl() {
 }
 
 size_t CompactedDBImpl::FindFile(const Slice& key) {
+  size_t left = 0;
   size_t right = files_.num_files - 1;
-  auto cmp = [&](const FdWithKeyRange& f, const Slice& k) -> bool {
-    return user_comparator_->Compare(ExtractUserKey(f.largest_key), k) < 0;
-  };
-  return static_cast<size_t>(std::lower_bound(files_.files,
-                            files_.files + right, key, cmp) - files_.files);
+  while (left < right) {
+    size_t mid = (left + right) >> 1;
+    const FdWithKeyRange& f = files_.files[mid];
+    if (user_comparator_->Compare(ExtractUserKey(f.largest_key), key) < 0) {
+      // Key at "mid.largest" is < "target".  Therefore all
+      // files at or before "mid" are uninteresting.
+      left = mid + 1;
+    } else {
+      // Key at "mid.largest" is >= "target".  Therefore all files
+      // after "mid" are uninteresting.
+      right = mid;
+    }
+  }
+  return right;
 }
 
 Status CompactedDBImpl::Get(const ReadOptions& options, ColumnFamilyHandle*,

db/compaction_iterator.cc

@@ -18,8 +18,8 @@ CompactionIterator::CompactionIterator(
     SequenceNumber earliest_write_conflict_snapshot,
     const SnapshotChecker* snapshot_checker, Env* env,
     bool report_detailed_time, bool expect_valid_internal_key,
-    RangeDelAggregator* range_del_agg, const Compaction* compaction,
-    const CompactionFilter* compaction_filter,
+    RangeDelAggregator* range_del_agg,
+    const Compaction* compaction, const CompactionFilter* compaction_filter,
     const std::atomic<bool>* shutting_down,
     const SequenceNumber preserve_deletes_seqnum)
     : CompactionIterator(
@@ -511,31 +511,6 @@ void CompactionIterator::NextFromInput() {
         ++iter_stats_.num_optimized_del_drop_obsolete;
       }
       input_->Next();
-    } else if ((ikey_.type == kTypeDeletion) && bottommost_level_ &&
-               ikeyNotNeededForIncrementalSnapshot()) {
-      // Handle the case where we have a delete key at the bottom most level
-      // We can skip outputting the key iff there are no subsequent puts for this
-      // key
-      ParsedInternalKey next_ikey;
-      input_->Next();
-      // Skip over all versions of this key that happen to occur in the same snapshot
-      // range as the delete
-      while (input_->Valid() &&
-             ParseInternalKey(input_->key(), &next_ikey) &&
-             cmp_->Equal(ikey_.user_key, next_ikey.user_key) &&
-             (prev_snapshot == 0 || next_ikey.sequence > prev_snapshot ||
-              (snapshot_checker_ != nullptr &&
-               UNLIKELY(!snapshot_checker_->IsInSnapshot(next_ikey.sequence,
-                                                         prev_snapshot))))) {
-        input_->Next();
-      }
-      // If you find you still need to output a row with this key, we need to output the
-      // delete too
-      if (input_->Valid() && ParseInternalKey(input_->key(), &next_ikey) &&
-          cmp_->Equal(ikey_.user_key, next_ikey.user_key)) {
-        valid_ = true;
-        at_next_ = true;
-      }
     } else if (ikey_.type == kTypeMerge) {
       if (!merge_helper_->HasOperator()) {
         status_ = Status::InvalidArgument(

db/compaction_iterator_test.cc

@@ -247,8 +247,9 @@ class CompactionIteratorTest : public testing::TestWithParam<bool> {
     c_iter_.reset(new CompactionIterator(
         iter_.get(), cmp_, merge_helper_.get(), last_sequence, &snapshots_,
         earliest_write_conflict_snapshot, snapshot_checker_.get(),
-        Env::Default(), false /* report_detailed_time */, false,
-        range_del_agg_.get(), std::move(compaction), filter, &shutting_down_));
+        Env::Default(), false /* report_detailed_time */,
+        false, range_del_agg_.get(), std::move(compaction), filter,
+        &shutting_down_));
   }
 
   void AddSnapshot(SequenceNumber snapshot,
@@ -671,12 +672,8 @@ TEST_P(CompactionIteratorTest, ZeroOutSequenceAtBottomLevel) {
 TEST_P(CompactionIteratorTest, RemoveDeletionAtBottomLevel) {
   AddSnapshot(1);
   RunTest({test::KeyStr("a", 1, kTypeDeletion),
-           test::KeyStr("b", 3, kTypeDeletion),
-           test::KeyStr("b", 1, kTypeValue)},
-          {"", "", ""},
-          {test::KeyStr("b", 3, kTypeDeletion),
-           test::KeyStr("b", 0, kTypeValue)},
-          {"", ""},
+           test::KeyStr("b", 2, kTypeDeletion)},
+          {"", ""}, {test::KeyStr("b", 2, kTypeDeletion)}, {""},
           kMaxSequenceNumber /*last_commited_seq*/, nullptr /*merge_operator*/,
           nullptr /*compaction_filter*/, true /*bottommost_level*/);
 }
@@ -845,26 +842,13 @@ TEST_F(CompactionIteratorWithSnapshotCheckerTest,
       {test::KeyStr("a", 1, kTypeDeletion), test::KeyStr("b", 2, kTypeDeletion),
        test::KeyStr("c", 3, kTypeDeletion)},
       {"", "", ""},
-      {},
+      {test::KeyStr("b", 2, kTypeDeletion),
+       test::KeyStr("c", 3, kTypeDeletion)},
       {"", ""}, kMaxSequenceNumber /*last_commited_seq*/,
       nullptr /*merge_operator*/, nullptr /*compaction_filter*/,
       true /*bottommost_level*/);
 }
 
-TEST_F(CompactionIteratorWithSnapshotCheckerTest,
-       NotRemoveDeletionIfValuePresentToEarlierSnapshot) {
-  AddSnapshot(2,1);
-  RunTest(
-      {test::KeyStr("a", 4, kTypeDeletion), test::KeyStr("a", 1, kTypeValue),
-          test::KeyStr("b", 3, kTypeValue)},
-      {"", "", ""},
-      {test::KeyStr("a", 4, kTypeDeletion), test::KeyStr("a", 0, kTypeValue),
-            test::KeyStr("b", 3, kTypeValue)},
-      {"", "", ""}, kMaxSequenceNumber /*last_commited_seq*/,
-      nullptr /*merge_operator*/, nullptr /*compaction_filter*/,
-      true /*bottommost_level*/);
-}
-
 TEST_F(CompactionIteratorWithSnapshotCheckerTest,
        NotRemoveSingleDeletionIfNotVisibleToEarliestSnapshot) {
   AddSnapshot(2, 1);

db/compaction_job.cc

@@ -593,6 +593,10 @@ Status CompactionJob::Run() {
     thread.join();
   }
 
+  if (output_directory_) {
+    output_directory_->Fsync();
+  }
+
   compaction_stats_.micros = env_->NowMicros() - start_micros;
   MeasureTime(stats_, COMPACTION_TIME, compaction_stats_.micros);
 
@@ -607,10 +611,6 @@ Status CompactionJob::Run() {
     }
   }
 
-  if (status.ok() && output_directory_) {
-    status = output_directory_->Fsync();
-  }
-
   if (status.ok()) {
     thread_pool.clear();
     std::vector<const FileMetaData*> files_meta;
@@ -1309,7 +1309,9 @@ Status CompactionJob::FinishCompactionOutputFile(
     // VersionEdit.
     assert(!sub_compact->outputs.empty());
     sub_compact->outputs.pop_back();
-    meta = nullptr;
+    sub_compact->builder.reset();
+    sub_compact->current_output_file_size = 0;
+    return s;
   }
 
   if (s.ok() && (current_entries > 0 || tp.num_range_deletions > 0)) {
@@ -1463,9 +1465,8 @@ Status CompactionJob::OpenCompactionOutputFile(
   writable_file->SetWriteLifeTimeHint(write_hint_);
   writable_file->SetPreallocationBlockSize(static_cast<size_t>(
       sub_compact->compaction->OutputFilePreallocationSize()));
-  sub_compact->outfile.reset(
-      new WritableFileWriter(std::move(writable_file), fname, env_options_,
-                             db_options_.statistics.get()));
+  sub_compact->outfile.reset(new WritableFileWriter(
+      std::move(writable_file), env_options_, db_options_.statistics.get()));
 
   // If the Column family flag is to only optimize filters for hits,
   // we can skip creating filters if this is the bottommost_level where

db/compaction_job_test.cc

@@ -79,7 +79,7 @@ class CompactionJobTest : public testing::Test {
         shutting_down_(false),
         preserve_deletes_seqnum_(0),
         mock_table_factory_(new mock::MockTableFactory()),
-        error_handler_(nullptr, db_options_, &mutex_) {
+        error_handler_(db_options_, &mutex_) {
     EXPECT_OK(env_->CreateDirIfMissing(dbname_));
     db_options_.db_paths.emplace_back(dbname_,
                                       std::numeric_limits<uint64_t>::max());
@@ -205,7 +205,7 @@ class CompactionJobTest : public testing::Test {
         manifest, &file, env_->OptimizeForManifestWrite(env_options_));
     ASSERT_OK(s);
     unique_ptr<WritableFileWriter> file_writer(
-        new WritableFileWriter(std::move(file), manifest, env_options_));
+        new WritableFileWriter(std::move(file), env_options_));
     {
       log::Writer log(std::move(file_writer), 0, false);
       std::string record;

db/compaction_picker.cc

@@ -49,7 +49,7 @@ bool FindIntraL0Compaction(const std::vector<FileMetaData*>& level_files,
   // increasing.
   size_t new_compact_bytes_per_del_file = 0;
   for (span_len = 1; span_len < level_files.size(); ++span_len) {
-    compact_bytes += static_cast<size_t>(level_files[span_len]->fd.file_size);
+    compact_bytes += level_files[span_len]->fd.file_size;
     new_compact_bytes_per_del_file = compact_bytes / span_len;
     if (level_files[span_len]->being_compacted ||
         new_compact_bytes_per_del_file > compact_bytes_per_del_file) {

db/db_compaction_test.cc

@@ -120,12 +120,13 @@ class SstStatsCollector : public EventListener {
  public:
   SstStatsCollector() : num_ssts_creation_started_(0) {}
 
-  void OnTableFileCreationStarted(
-      const TableFileCreationBriefInfo& /* info */) override {
+  void OnTableFileCreationStarted(const TableFileCreationBriefInfo& /* info */) override {
     ++num_ssts_creation_started_;
   }
 
-  int num_ssts_creation_started() { return num_ssts_creation_started_; }
+  int num_ssts_creation_started() {
+    return num_ssts_creation_started_;
+  }
 
  private:
   std::atomic<int> num_ssts_creation_started_;
@@ -2477,7 +2478,6 @@ TEST_P(DBCompactionTestWithParam, ManualLevelCompactionOutputPathId) {
 
     // Compaction range overlaps files
     Compact(1, "p1", "p9", 1);
-    ASSERT_OK(dbfull()->TEST_WaitForCompact());
     ASSERT_EQ("0,1", FilesPerLevel(1));
     ASSERT_EQ(1, GetSstFileCount(options.db_paths[1].path));
     ASSERT_EQ(0, GetSstFileCount(options.db_paths[0].path));
@@ -2493,7 +2493,6 @@ TEST_P(DBCompactionTestWithParam, ManualLevelCompactionOutputPathId) {
 
     // Compact just the new range
     Compact(1, "b", "f", 1);
-    ASSERT_OK(dbfull()->TEST_WaitForCompact());
     ASSERT_EQ("0,2", FilesPerLevel(1));
     ASSERT_EQ(2, GetSstFileCount(options.db_paths[1].path));
     ASSERT_EQ(0, GetSstFileCount(options.db_paths[0].path));
@@ -2510,7 +2509,6 @@ TEST_P(DBCompactionTestWithParam, ManualLevelCompactionOutputPathId) {
     compact_options.target_path_id = 1;
     compact_options.exclusive_manual_compaction = exclusive_manual_compaction_;
     db_->CompactRange(compact_options, handles_[1], nullptr, nullptr);
-    ASSERT_OK(dbfull()->TEST_WaitForCompact());
 
     ASSERT_EQ("0,1", FilesPerLevel(1));
     ASSERT_EQ(1, GetSstFileCount(options.db_paths[1].path));
@@ -4005,83 +4003,6 @@ TEST_F(DBCompactionTest, PartialManualCompaction) {
   dbfull()->CompactRange(cro, nullptr, nullptr);
 }
 
-// FixFileIngestionCompactionDeadlock tests and verifies that compaction and
-// file ingestion do not cause deadlock in the event of write stall triggered
-// by number of L0 files reaching level0_stop_writes_trigger.
-TEST_P(DBCompactionTestWithParam, FixFileIngestionCompactionDeadlock) {
-  const int kNumKeysPerFile = 100;
-  // Generate SST files.
-  Options options = CurrentOptions();
-
-  // Generate an external SST file containing a single key, i.e. 99
-  std::string sst_files_dir = dbname_ + "/sst_files/";
-  test::DestroyDir(env_, sst_files_dir);
-  ASSERT_OK(env_->CreateDir(sst_files_dir));
-  SstFileWriter sst_writer(EnvOptions(), options);
-  const std::string sst_file_path = sst_files_dir + "test.sst";
-  ASSERT_OK(sst_writer.Open(sst_file_path));
-  ASSERT_OK(sst_writer.Put(Key(kNumKeysPerFile - 1), "value"));
-  ASSERT_OK(sst_writer.Finish());
-
-  SyncPoint::GetInstance()->DisableProcessing();
-  SyncPoint::GetInstance()->ClearAllCallBacks();
-  SyncPoint::GetInstance()->LoadDependency({
-      {"DBImpl::IngestExternalFile:AfterIncIngestFileCounter",
-       "BackgroundCallCompaction:0"},
-  });
-  SyncPoint::GetInstance()->EnableProcessing();
-
-  options.write_buffer_size = 110 << 10;  // 110KB
-  options.level0_file_num_compaction_trigger =
-      options.level0_stop_writes_trigger;
-  options.max_subcompactions = max_subcompactions_;
-  options.memtable_factory.reset(new SpecialSkipListFactory(kNumKeysPerFile));
-  DestroyAndReopen(options);
-  Random rnd(301);
-
-  // Generate level0_stop_writes_trigger L0 files to trigger write stop
-  for (int i = 0; i != options.level0_file_num_compaction_trigger; ++i) {
-    for (int j = 0; j != kNumKeysPerFile; ++j) {
-      ASSERT_OK(Put(Key(j), RandomString(&rnd, 990)));
-    }
-    if (0 == i) {
-      // When we reach here, the memtables have kNumKeysPerFile keys. Note that
-      // flush is not yet triggered. We need to write an extra key so that the
-      // write path will call PreprocessWrite and flush the previous key-value
-      // pairs to e flushed. After that, there will be the newest key in the
-      // memtable, and a bunch of L0 files. Since there is already one key in
-      // the memtable, then for i = 1, 2, ..., we do not have to write this
-      // extra key to trigger flush.
-      ASSERT_OK(Put("", ""));
-    }
-    dbfull()->TEST_WaitForFlushMemTable();
-    ASSERT_EQ(NumTableFilesAtLevel(0 /*level*/, 0 /*cf*/), i + 1);
-  }
-  // When we reach this point, there will be level0_stop_writes_trigger L0
-  // files and one extra key (99) in memory, which overlaps with the external
-  // SST file. Write stall triggers, and can be cleared only after compaction
-  // reduces the number of L0 files.
-
-  // Compaction will also be triggered since we have reached the threshold for
-  // auto compaction. Note that compaction may begin after the following file
-  // ingestion thread and waits for ingestion to finish.
-
-  // Thread to ingest file with overlapping key range with the current
-  // memtable. Consequently ingestion will trigger a flush. The flush MUST
-  // proceed without waiting for the write stall condition to clear, otherwise
-  // deadlock can happen.
-  port::Thread ingestion_thr([&]() {
-    IngestExternalFileOptions ifo;
-    Status s = db_->IngestExternalFile({sst_file_path}, ifo);
-    ASSERT_OK(s);
-  });
-
-  // More write to trigger write stop
-  ingestion_thr.join();
-  ASSERT_OK(dbfull()->TEST_WaitForCompact());
-  Close();
-}
-
 #endif // !defined(ROCKSDB_LITE)
 }  // namespace rocksdb
 

db/db_impl.cc

@@ -215,11 +215,9 @@ DBImpl::DBImpl(const DBOptions& options, const std::string& dbname,
       // requires a custom gc for compaction, we use that to set use_custom_gc_
       // as well.
       use_custom_gc_(seq_per_batch),
-      shutdown_initiated_(false),
-      own_sfm_(options.sst_file_manager == nullptr),
       preserve_deletes_(options.preserve_deletes),
       closed_(false),
-      error_handler_(this, immutable_db_options_, &mutex_) {
+      error_handler_(immutable_db_options_, &mutex_) {
   // !batch_per_trx_ implies seq_per_batch_ because it is only unset for
   // WriteUnprepared, which should use seq_per_batch_.
   assert(batch_per_txn_ || seq_per_batch_);
@@ -261,62 +259,16 @@ Status DBImpl::Resume() {
     return Status::OK();
   }
 
-  if (error_handler_.IsRecoveryInProgress()) {
-    // Don't allow a mix of manual and automatic recovery
-    return Status::Busy();
-  }
-
-  mutex_.Unlock();
-  Status s = error_handler_.RecoverFromBGError(true);
-  mutex_.Lock();
-  return s;
-}
-
-// This function implements the guts of recovery from a background error. It
-// is eventually called for both manual as well as automatic recovery. It does
-// the following -
-// 1. Wait for currently scheduled background flush/compaction to exit, in
-//    order to inadvertently causing an error and thinking recovery failed
-// 2. Flush memtables if there's any data for all the CFs. This may result
-//    another error, which will be saved by error_handler_ and reported later
-//    as the recovery status
-// 3. Find and delete any obsolete files
-// 4. Schedule compactions if needed for all the CFs. This is needed as the
-//    flush in the prior step might have been a no-op for some CFs, which
-//    means a new super version wouldn't have been installed
-Status DBImpl::ResumeImpl() {
-  mutex_.AssertHeld();
-  WaitForBackgroundWork();
-
-  Status bg_error = error_handler_.GetBGError();
-  Status s;
-  if (shutdown_initiated_) {
-    // Returning shutdown status to SFM during auto recovery will cause it
-    // to abort the recovery and allow the shutdown to progress
-    s = Status::ShutdownInProgress();
-  }
-  if (s.ok() && bg_error.severity() > Status::Severity::kHardError) {
+  Status s = error_handler_.GetBGError();
+  if (s.severity() > Status::Severity::kHardError) {
     ROCKS_LOG_INFO(immutable_db_options_.info_log,
         "DB resume requested but failed due to Fatal/Unrecoverable error");
-    s = bg_error;
-  }
-
-  // We cannot guarantee consistency of the WAL. So force flush Memtables of
-  // all the column families
-  if (s.ok()) {
-    s = FlushAllCFs(FlushReason::kErrorRecovery);
-    if (!s.ok()) {
-      ROCKS_LOG_INFO(immutable_db_options_.info_log,
-                     "DB resume requested but failed due to Flush failure [%s]",
-                     s.ToString().c_str());
-    }
+    return s;
   }
 
   JobContext job_context(0);
   FindObsoleteFiles(&job_context, true);
-  if (s.ok()) {
-    s = error_handler_.ClearBGError();
-  }
+  error_handler_.ClearBGError();
   mutex_.Unlock();
 
   job_context.manifest_file_number = 1;
@@ -325,36 +277,13 @@ Status DBImpl::ResumeImpl() {
   }
   job_context.Clean();
 
-  if (s.ok()) {
-    ROCKS_LOG_INFO(immutable_db_options_.info_log, "Successfully resumed DB");
-  }
+  ROCKS_LOG_INFO(immutable_db_options_.info_log, "Successfully resumed DB");
   mutex_.Lock();
-  // Check for shutdown again before scheduling further compactions,
-  // since we released and re-acquired the lock above
-  if (shutdown_initiated_) {
-    s = Status::ShutdownInProgress();
-  }
-  if (s.ok()) {
-    for (auto cfd : *versions_->GetColumnFamilySet()) {
-      SchedulePendingCompaction(cfd);
-    }
-    MaybeScheduleFlushOrCompaction();
-  }
-
-  // Wake up any waiters - in this case, it could be the shutdown thread
-  bg_cv_.SignalAll();
+  MaybeScheduleFlushOrCompaction();
 
   // No need to check BGError again. If something happened, event listener would be
   // notified and the operation causing it would have failed
-  return s;
-}
-
-void DBImpl::WaitForBackgroundWork() {
-  // Wait for background work to finish
-  while (bg_bottom_compaction_scheduled_ || bg_compaction_scheduled_ ||
-         bg_flush_scheduled_) {
-    bg_cv_.Wait();
-  }
+  return Status::OK();
 }
 
 // Will lock the mutex_,  will wait for completion if wait is true
@@ -384,20 +313,14 @@ void DBImpl::CancelAllBackgroundWork(bool wait) {
   if (!wait) {
     return;
   }
-  WaitForBackgroundWork();
+  // Wait for background work to finish
+  while (bg_bottom_compaction_scheduled_ || bg_compaction_scheduled_ ||
+         bg_flush_scheduled_) {
+    bg_cv_.Wait();
+  }
 }
 
 Status DBImpl::CloseHelper() {
-  // Guarantee that there is no background error recovery in progress before
-  // continuing with the shutdown
-  mutex_.Lock();
-  shutdown_initiated_ = true;
-  error_handler_.CancelErrorRecovery();
-  while (error_handler_.IsRecoveryInProgress()) {
-    bg_cv_.Wait();
-  }
-  mutex_.Unlock();
-
   // CancelAllBackgroundWork called with false means we just set the shutdown
   // marker. After this we do a variant of the waiting and unschedule work
   // (to consider: moving all the waiting into CancelAllBackgroundWork(true))
@@ -415,8 +338,7 @@ Status DBImpl::CloseHelper() {
   // Wait for background work to finish
   while (bg_bottom_compaction_scheduled_ || bg_compaction_scheduled_ ||
          bg_flush_scheduled_ || bg_purge_scheduled_ ||
-         pending_purge_obsolete_files_ ||
-         error_handler_.IsRecoveryInProgress()) {
+         pending_purge_obsolete_files_) {
     TEST_SYNC_POINT("DBImpl::~DBImpl:WaitJob");
     bg_cv_.Wait();
   }
@@ -426,12 +348,9 @@ Status DBImpl::CloseHelper() {
   flush_scheduler_.Clear();
 
   while (!flush_queue_.empty()) {
-    const FlushRequest& flush_req = PopFirstFromFlushQueue();
-    for (const auto& iter : flush_req) {
-      ColumnFamilyData* cfd = iter.first;
-      if (cfd->Unref()) {
-        delete cfd;
-      }
+    auto cfd = PopFirstFromFlushQueue();
+    if (cfd->Unref()) {
+      delete cfd;
     }
   }
   while (!compaction_queue_.empty()) {
@@ -521,17 +440,6 @@ Status DBImpl::CloseHelper() {
   ROCKS_LOG_INFO(immutable_db_options_.info_log, "Shutdown complete");
   LogFlush(immutable_db_options_.info_log);
 
-#ifndef ROCKSDB_LITE
-  // If the sst_file_manager was allocated by us during DB::Open(), ccall
-  // Close() on it before closing the info_log. Otherwise, background thread
-  // in SstFileManagerImpl might try to log something
-  if (immutable_db_options_.sst_file_manager && own_sfm_) {
-    auto sfm = static_cast<SstFileManagerImpl*>(
-        immutable_db_options_.sst_file_manager.get());
-    sfm->Close();
-  }
-#endif // ROCKSDB_LITE
-
   if (immutable_db_options_.info_log && own_info_log_) {
     Status s = immutable_db_options_.info_log->Close();
     if (ret.ok()) {
@@ -2502,7 +2410,7 @@ Status DBImpl::GetDbIdentity(std::string& identity) const {
   if (!s.ok()) {
     return s;
   }
-  char* buffer = reinterpret_cast<char*>(alloca(static_cast<size_t>(file_size)));
+  char* buffer = reinterpret_cast<char*>(alloca(file_size));
   Slice id;
   s = id_file_reader->Read(static_cast<size_t>(file_size), &id, buffer);
   if (!s.ok()) {
@@ -3059,7 +2967,6 @@ Status DBImpl::IngestExternalFile(
     }
 
     num_running_ingest_file_++;
-    TEST_SYNC_POINT("DBImpl::IngestExternalFile:AfterIncIngestFileCounter");
 
     // We cannot ingest a file into a dropped CF
     if (cfd->IsDropped()) {
@@ -3075,9 +2982,7 @@ Status DBImpl::IngestExternalFile(
                                &need_flush);
       if (status.ok() && need_flush) {
         mutex_.Unlock();
-        FlushOptions flush_opts;
-        flush_opts.allow_write_stall = true;
-        status = FlushMemTable(cfd, flush_opts,
+        status = FlushMemTable(cfd, FlushOptions(),
                                FlushReason::kExternalFileIngestion,
                                true /* writes_stopped */);
         mutex_.Lock();
@@ -3238,8 +3143,7 @@ Status DBImpl::TraceIteratorSeek(const uint32_t& cf_id, const Slice& key) {
   return s;
 }
 
-Status DBImpl::TraceIteratorSeekForPrev(const uint32_t& cf_id,
-                                        const Slice& key) {
+Status DBImpl::TraceIteratorSeekForPrev(const uint32_t& cf_id, const Slice& key) {
   Status s;
   if (tracer_) {
     InstrumentedMutexLock lock(&trace_mutex_);

db/db_impl.h

@@ -22,9 +22,9 @@
 #include "db/column_family.h"
 #include "db/compaction_job.h"
 #include "db/dbformat.h"
+#include "db/external_sst_file_ingestion_job.h"
 #include "db/error_handler.h"
 #include "db/event_helpers.h"
-#include "db/external_sst_file_ingestion_job.h"
 #include "db/flush_job.h"
 #include "db/flush_scheduler.h"
 #include "db/internal_stats.h"
@@ -464,7 +464,6 @@ class DBImpl : public DB {
 
   int TEST_BGCompactionsAllowed() const;
   int TEST_BGFlushesAllowed() const;
-  size_t TEST_GetWalPreallocateBlockSize(uint64_t write_buffer_size) const;
 
 #endif  // NDEBUG
 
@@ -796,7 +795,6 @@ class DBImpl : public DB {
 
  private:
   friend class DB;
-  friend class ErrorHandler;
   friend class InternalStats;
   friend class PessimisticTransaction;
   friend class TransactionBaseImpl;
@@ -848,8 +846,6 @@ class DBImpl : public DB {
                  bool read_only = false, bool error_if_log_file_exist = false,
                  bool error_if_data_exists_in_logs = false);
 
-  Status ResumeImpl();
-
   void MaybeIgnoreError(Status* s) const;
 
   const Status CreateArchivalDirectory();
@@ -889,41 +885,12 @@ class DBImpl : public DB {
   Status SyncClosedLogs(JobContext* job_context);
 
   // Flush the in-memory write buffer to storage.  Switches to a new
-  // log-file/memtable and writes a new descriptor iff successful. Then
-  // installs a new super version for the column family.
+  // log-file/memtable and writes a new descriptor iff successful.
   Status FlushMemTableToOutputFile(ColumnFamilyData* cfd,
                                    const MutableCFOptions& mutable_cf_options,
                                    bool* madeProgress, JobContext* job_context,
-                                   SuperVersionContext* superversion_context,
                                    LogBuffer* log_buffer);
 
-  // Argument required by background flush thread.
-  struct BGFlushArg {
-    BGFlushArg()
-        : cfd_(nullptr), memtable_id_(0), superversion_context_(nullptr) {}
-    BGFlushArg(ColumnFamilyData* cfd, uint64_t memtable_id,
-               SuperVersionContext* superversion_context)
-        : cfd_(cfd),
-          memtable_id_(memtable_id),
-          superversion_context_(superversion_context) {}
-
-    // Column family to flush.
-    ColumnFamilyData* cfd_;
-    // Maximum ID of memtable to flush. In this column family, memtables with
-    // IDs smaller than this value must be flushed before this flush completes.
-    uint64_t memtable_id_;
-    // Pointer to a SuperVersionContext object. After flush completes, RocksDB
-    // installs a new superversion for the column family. This operation
-    // requires a SuperVersionContext object (currently embedded in JobContext).
-    SuperVersionContext* superversion_context_;
-  };
-
-  // Flush the memtables of (multiple) column families to multiple files on
-  // persistent storage.
-  Status FlushMemTablesToOutputFiles(
-      const autovector<BGFlushArg>& bg_flush_args, bool* made_progress,
-      JobContext* job_context, LogBuffer* log_buffer);
-
   // REQUIRES: log_numbers are sorted in ascending order
   Status RecoverLogFiles(const std::vector<uint64_t>& log_numbers,
                          SequenceNumber* next_sequence, bool read_only);
@@ -936,12 +903,6 @@ class DBImpl : public DB {
   Status WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd,
                                      MemTable* mem, VersionEdit* edit);
 
-  // Restore alive_log_files_ and total_log_size_ after recovery.
-  // It needs to run only when there's no flush during recovery
-  // (e.g. avoid_flush_during_recovery=true). May also trigger flush
-  // in case total_log_size > max_total_wal_size.
-  Status RestoreAliveLogFiles(const std::vector<uint64_t>& log_numbers);
-
   // num_bytes: for slowdown case, delay time is calculated based on
   //            `num_bytes` going through.
   Status DelayWrite(uint64_t num_bytes, const WriteOptions& write_options);
@@ -951,7 +912,8 @@ class DBImpl : public DB {
 
   Status ScheduleFlushes(WriteContext* context);
 
-  Status SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context);
+  Status SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context,
+                        FlushReason flush_reason = FlushReason::kOthers);
 
   // Force current memtable contents to be flushed.
   Status FlushMemTable(ColumnFamilyData* cfd, const FlushOptions& options,
@@ -966,13 +928,7 @@ class DBImpl : public DB {
   // gets flush. Otherwise, wait until the column family don't have any
   // memtable pending flush.
   Status WaitForFlushMemTable(ColumnFamilyData* cfd,
-                              const uint64_t* flush_memtable_id = nullptr) {
-    return WaitForFlushMemTables({cfd}, {flush_memtable_id});
-  }
-  // Wait for memtables to be flushed for multiple column families.
-  Status WaitForFlushMemTables(
-      const autovector<ColumnFamilyData*>& cfds,
-      const autovector<const uint64_t*>& flush_memtable_ids);
+                              const uint64_t* flush_memtable_id = nullptr);
 
   // REQUIRES: mutex locked
   Status SwitchWAL(WriteContext* write_context);
@@ -1028,17 +984,7 @@ class DBImpl : public DB {
   ColumnFamilyData* GetColumnFamilyDataByName(const std::string& cf_name);
 
   void MaybeScheduleFlushOrCompaction();
-
-  // A flush request specifies the column families to flush as well as the
-  // largest memtable id to persist for each column family. Once all the
-  // memtables whose IDs are smaller than or equal to this per-column-family
-  // specified value, this flush request is considered to have completed its
-  // work of flushing this column family. After completing the work for all
-  // column families in this request, this flush is considered complete.
-  typedef std::vector<std::pair<ColumnFamilyData*, uint64_t>> FlushRequest;
-
-  void SchedulePendingFlush(const FlushRequest& req, FlushReason flush_reason);
-
+  void SchedulePendingFlush(ColumnFamilyData* cfd, FlushReason flush_reason);
   void SchedulePendingCompaction(ColumnFamilyData* cfd);
   void SchedulePendingPurge(std::string fname, std::string dir_to_sync,
                             FileType type, uint64_t number, int job_id);
@@ -1057,10 +1003,9 @@ class DBImpl : public DB {
                               LogBuffer* log_buffer,
                               PrepickedCompaction* prepicked_compaction);
   Status BackgroundFlush(bool* madeProgress, JobContext* job_context,
-                         LogBuffer* log_buffer, FlushReason* reason);
+                         LogBuffer* log_buffer);
 
-  bool EnoughRoomForCompaction(ColumnFamilyData* cfd,
-                               const std::vector<CompactionInputFiles>& inputs,
+  bool EnoughRoomForCompaction(const std::vector<CompactionInputFiles>& inputs,
                                bool* sfm_bookkeeping, LogBuffer* log_buffer);
 
   void PrintStatistics();
@@ -1081,7 +1026,8 @@ class DBImpl : public DB {
   // helper functions for adding and removing from flush & compaction queues
   void AddToCompactionQueue(ColumnFamilyData* cfd);
   ColumnFamilyData* PopFirstFromCompactionQueue();
-  FlushRequest PopFirstFromFlushQueue();
+  void AddToFlushQueue(ColumnFamilyData* cfd, FlushReason flush_reason);
+  ColumnFamilyData* PopFirstFromFlushQueue();
 
   // helper function to call after some of the logs_ were synced
   void MarkLogsSynced(uint64_t up_to, bool synced_dir, const Status& status);
@@ -1094,10 +1040,6 @@ class DBImpl : public DB {
 
   Status CloseHelper();
 
-  Status FlushAllCFs(FlushReason flush_reason);
-
-  void WaitForBackgroundWork();
-
   // table_cache_ provides its own synchronization
   std::shared_ptr<Cache> table_cache_;
 
@@ -1318,7 +1260,7 @@ class DBImpl : public DB {
   // in MaybeScheduleFlushOrCompaction()
   // invariant(column family present in flush_queue_ <==>
   // ColumnFamilyData::pending_flush_ == true)
-  std::deque<FlushRequest> flush_queue_;
+  std::deque<ColumnFamilyData*> flush_queue_;
   // invariant(column family present in compaction_queue_ <==>
   // ColumnFamilyData::pending_compaction_ == true)
   std::deque<ColumnFamilyData*> compaction_queue_;
@@ -1542,16 +1484,6 @@ class DBImpl : public DB {
   // flush/compaction and if it is not provided vis SnapshotChecker, we should
   // disable gc to be safe.
   const bool use_custom_gc_;
-  // Flag to indicate that the DB instance shutdown has been initiated. This
-  // different from shutting_down_ atomic in that it is set at the beginning
-  // of shutdown sequence, specifically in order to prevent any background
-  // error recovery from going on in parallel. The latter, shutting_down_,
-  // is set a little later during the shutdown after scheduling memtable
-  // flushes
-  bool shutdown_initiated_;
-  // Flag to indicate whether sst_file_manager object was allocated in
-  // DB::Open() or passed to us
-  bool own_sfm_;
 
   // Clients must periodically call SetPreserveDeletesSequenceNumber()
   // to advance this seqnum. Default value is 0 which means ALL deletes are

db/db_impl_compaction_flush.cc

@@ -26,7 +26,7 @@
 namespace rocksdb {
 
 bool DBImpl::EnoughRoomForCompaction(
-    ColumnFamilyData* cfd, const std::vector<CompactionInputFiles>& inputs,
+    const std::vector<CompactionInputFiles>& inputs,
     bool* sfm_reserved_compact_space, LogBuffer* log_buffer) {
   // Check if we have enough room to do the compaction
   bool enough_room = true;
@@ -34,17 +34,12 @@ bool DBImpl::EnoughRoomForCompaction(
   auto sfm = static_cast<SstFileManagerImpl*>(
       immutable_db_options_.sst_file_manager.get());
   if (sfm) {
-    // Pass the current bg_error_ to SFM so it can decide what checks to
-    // perform. If this DB instance hasn't seen any error yet, the SFM can be
-    // optimistic and not do disk space checks
-    enough_room =
-        sfm->EnoughRoomForCompaction(cfd, inputs, error_handler_.GetBGError());
+    enough_room = sfm->EnoughRoomForCompaction(inputs);
     if (enough_room) {
       *sfm_reserved_compact_space = true;
     }
   }
 #else
-  (void)cfd;
   (void)inputs;
   (void)sfm_reserved_compact_space;
 #endif  // ROCKSDB_LITE
@@ -109,8 +104,7 @@ Status DBImpl::SyncClosedLogs(JobContext* job_context) {
 
 Status DBImpl::FlushMemTableToOutputFile(
     ColumnFamilyData* cfd, const MutableCFOptions& mutable_cf_options,
-    bool* made_progress, JobContext* job_context,
-    SuperVersionContext* superversion_context, LogBuffer* log_buffer) {
+    bool* made_progress, JobContext* job_context, LogBuffer* log_buffer) {
   mutex_.AssertHeld();
   assert(cfd->imm()->NumNotFlushed() != 0);
   assert(cfd->imm()->IsFlushPending());
@@ -166,8 +160,8 @@ Status DBImpl::FlushMemTableToOutputFile(
   }
 
   if (s.ok()) {
-    InstallSuperVersionAndScheduleWork(cfd, superversion_context,
-                                       mutable_cf_options);
+    InstallSuperVersionAndScheduleWork(
+        cfd, &job_context->superversion_contexts[0], mutable_cf_options);
     if (made_progress) {
       *made_progress = 1;
     }
@@ -206,25 +200,6 @@ Status DBImpl::FlushMemTableToOutputFile(
   return s;
 }
 
-Status DBImpl::FlushMemTablesToOutputFiles(
-    const autovector<BGFlushArg>& bg_flush_args, bool* made_progress,
-    JobContext* job_context, LogBuffer* log_buffer) {
-  Status s;
-  for (auto& arg : bg_flush_args) {
-    ColumnFamilyData* cfd = arg.cfd_;
-    const MutableCFOptions& mutable_cf_options =
-        *cfd->GetLatestMutableCFOptions();
-    SuperVersionContext* superversion_context = arg.superversion_context_;
-    s = FlushMemTableToOutputFile(cfd, mutable_cf_options, made_progress,
-                                  job_context, superversion_context,
-                                  log_buffer);
-    if (!s.ok()) {
-      break;
-    }
-  }
-  return s;
-}
-
 void DBImpl::NotifyOnFlushBegin(ColumnFamilyData* cfd, FileMetaData* file_meta,
                                 const MutableCFOptions& mutable_cf_options,
                                 int job_id, TableProperties prop) {
@@ -589,7 +564,7 @@ Status DBImpl::CompactFilesImpl(
   bool sfm_reserved_compact_space = false;
   // First check if we have enough room to do the compaction
   bool enough_room = EnoughRoomForCompaction(
-      cfd, input_files, &sfm_reserved_compact_space, log_buffer);
+      input_files, &sfm_reserved_compact_space, log_buffer);
 
   if (!enough_room) {
     // m's vars will get set properly at the end of this function,
@@ -919,68 +894,6 @@ Status DBImpl::Flush(const FlushOptions& flush_options,
   return s;
 }
 
-
-Status DBImpl::FlushAllCFs(FlushReason flush_reason) {
-  Status s;
-  WriteContext context;
-  WriteThread::Writer w;
-
-  mutex_.AssertHeld();
-  write_thread_.EnterUnbatched(&w, &mutex_);
-
-  FlushRequest flush_req;
-  for (auto cfd : *versions_->GetColumnFamilySet()) {
-    if (cfd->imm()->NumNotFlushed() == 0 && cfd->mem()->IsEmpty() &&
-        cached_recoverable_state_empty_.load()) {
-      // Nothing to flush
-      continue;
-    }
-
-    // SwitchMemtable() will release and reacquire mutex during execution
-    s = SwitchMemtable(cfd, &context);
-    if (!s.ok()) {
-      break;
-    }
-
-    cfd->imm()->FlushRequested();
-
-    flush_req.emplace_back(cfd, cfd->imm()->GetLatestMemTableID());
-  }
-
-  // schedule flush
-  if (s.ok() && !flush_req.empty()) {
-    SchedulePendingFlush(flush_req, flush_reason);
-    MaybeScheduleFlushOrCompaction();
-  }
-
-  write_thread_.ExitUnbatched(&w);
-
-  if (s.ok()) {
-    for (auto& flush : flush_req) {
-      auto cfd = flush.first;
-      auto flush_memtable_id = flush.second;
-      while (cfd->imm()->NumNotFlushed() > 0 &&
-             cfd->imm()->GetEarliestMemTableID() <= flush_memtable_id) {
-        if (!error_handler_.GetRecoveryError().ok()) {
-          break;
-        }
-        if (cfd->IsDropped()) {
-          // FlushJob cannot flush a dropped CF, if we did not break here
-          // we will loop forever since cfd->imm()->NumNotFlushed() will never
-          // drop to zero
-          continue;
-        }
-        cfd->Ref();
-        bg_cv_.Wait();
-        cfd->Unref();
-      }
-    }
-  }
-
-  flush_req.clear();
-  return s;
-}
-
 Status DBImpl::RunManualCompaction(ColumnFamilyData* cfd, int input_level,
                                    int output_level, uint32_t output_path_id,
                                    uint32_t max_subcompactions,
@@ -1124,45 +1037,39 @@ Status DBImpl::FlushMemTable(ColumnFamilyData* cfd,
       return s;
     }
   }
-  FlushRequest flush_req;
   {
     WriteContext context;
     InstrumentedMutexLock guard_lock(&mutex_);
 
+    if (cfd->imm()->NumNotFlushed() == 0 && cfd->mem()->IsEmpty() &&
+        cached_recoverable_state_empty_.load()) {
+      // Nothing to flush
+      return Status::OK();
+    }
+
     WriteThread::Writer w;
     if (!writes_stopped) {
       write_thread_.EnterUnbatched(&w, &mutex_);
     }
 
-    if (cfd->imm()->NumNotFlushed() != 0 || !cfd->mem()->IsEmpty() ||
-        !cached_recoverable_state_empty_.load()) {
-      s = SwitchMemtable(cfd, &context);
-      flush_memtable_id = cfd->imm()->GetLatestMemTableID();
-      flush_req.emplace_back(cfd, flush_memtable_id);
-    }
-
-    if (s.ok() && !flush_req.empty()) {
-      for (auto& elem : flush_req) {
-        ColumnFamilyData* loop_cfd = elem.first;
-        loop_cfd->imm()->FlushRequested();
-      }
-      SchedulePendingFlush(flush_req, flush_reason);
-      MaybeScheduleFlushOrCompaction();
-    }
+    // SwitchMemtable() will release and reacquire mutex during execution
+    s = SwitchMemtable(cfd, &context);
+    flush_memtable_id = cfd->imm()->GetLatestMemTableID();
 
     if (!writes_stopped) {
       write_thread_.ExitUnbatched(&w);
     }
+
+    cfd->imm()->FlushRequested();
+
+    // schedule flush
+    SchedulePendingFlush(cfd, flush_reason);
+    MaybeScheduleFlushOrCompaction();
   }
 
   if (s.ok() && flush_options.wait) {
-    autovector<ColumnFamilyData*> cfds;
-    autovector<const uint64_t*> flush_memtable_ids;
-    for (auto& iter : flush_req) {
-      cfds.push_back(iter.first);
-      flush_memtable_ids.push_back(&(iter.second));
-    }
-    s = WaitForFlushMemTables(cfds, flush_memtable_ids);
+    // Wait until the compaction completes
+    s = WaitForFlushMemTable(cfd, &flush_memtable_id);
   }
   TEST_SYNC_POINT("FlushMemTableFinished");
   return s;
@@ -1231,49 +1138,25 @@ Status DBImpl::WaitUntilFlushWouldNotStallWrites(ColumnFamilyData* cfd,
   return Status::OK();
 }
 
-// Wait for memtables to be flushed for multiple column families.
-// let N = cfds.size()
-// for i in [0, N),
-//  1) if flush_memtable_ids[i] is not null, then the memtables with lower IDs
-//     have to be flushed for THIS column family;
-//  2) if flush_memtable_ids[i] is null, then all memtables in THIS column
-//     family have to be flushed.
-// Finish waiting when ALL column families finish flushing memtables.
-Status DBImpl::WaitForFlushMemTables(
-    const autovector<ColumnFamilyData*>& cfds,
-    const autovector<const uint64_t*>& flush_memtable_ids) {
-  int num = static_cast<int>(cfds.size());
+Status DBImpl::WaitForFlushMemTable(ColumnFamilyData* cfd,
+                                    const uint64_t* flush_memtable_id) {
+  Status s;
   // Wait until the compaction completes
   InstrumentedMutexLock l(&mutex_);
-  while (!error_handler_.IsDBStopped()) {
+  while (cfd->imm()->NumNotFlushed() > 0 && !error_handler_.IsDBStopped() &&
+         (flush_memtable_id == nullptr ||
+          cfd->imm()->GetEarliestMemTableID() <= *flush_memtable_id)) {
     if (shutting_down_.load(std::memory_order_acquire)) {
       return Status::ShutdownInProgress();
     }
-    // Number of column families that have been dropped.
-    int num_dropped = 0;
-    // Number of column families that have finished flush.
-    int num_finished = 0;
-    for (int i = 0; i < num; ++i) {
-      if (cfds[i]->IsDropped()) {
-        ++num_dropped;
-      } else if (cfds[i]->imm()->NumNotFlushed() == 0 ||
-                 (flush_memtable_ids[i] != nullptr &&
-                  cfds[i]->imm()->GetEarliestMemTableID() >
-                      *flush_memtable_ids[i])) {
-        ++num_finished;
-      }
-    }
-    if (1 == num_dropped && 1 == num) {
+    if (cfd->IsDropped()) {
+      // FlushJob cannot flush a dropped CF, if we did not break here
+      // we will loop forever since cfd->imm()->NumNotFlushed() will never
+      // drop to zero
       return Status::InvalidArgument("Cannot flush a dropped CF");
     }
-    // Column families involved in this flush request have either been dropped
-    // or finished flush. Then it's time to finish waiting.
-    if (num_dropped + num_finished == num) {
-      break;
-    }
     bg_cv_.Wait();
   }
-  Status s;
   if (error_handler_.IsDBStopped()) {
     s = error_handler_.GetBGError();
   }
@@ -1303,12 +1186,6 @@ void DBImpl::MaybeScheduleFlushOrCompaction() {
   if (bg_work_paused_ > 0) {
     // we paused the background work
     return;
-  } else if (error_handler_.IsBGWorkStopped() &&
-      !error_handler_.IsRecoveryInProgress()) {
-    // There has been a hard error and this call is not part of the recovery
-    // sequence. Bail out here so we don't get into an endless loop of
-    // scheduling BG work which will again call this function
-    return;
   } else if (shutting_down_.load(std::memory_order_acquire)) {
     // DB is being deleted; no more background compactions
     return;
@@ -1318,6 +1195,7 @@ void DBImpl::MaybeScheduleFlushOrCompaction() {
       env_->GetBackgroundThreads(Env::Priority::HIGH) == 0;
   while (!is_flush_pool_empty && unscheduled_flushes_ > 0 &&
          bg_flush_scheduled_ < bg_job_limits.max_flushes) {
+    unscheduled_flushes_--;
     bg_flush_scheduled_++;
     env_->Schedule(&DBImpl::BGWorkFlush, this, Env::Priority::HIGH, this);
   }
@@ -1328,6 +1206,7 @@ void DBImpl::MaybeScheduleFlushOrCompaction() {
     while (unscheduled_flushes_ > 0 &&
            bg_flush_scheduled_ + bg_compaction_scheduled_ <
                bg_job_limits.max_flushes) {
+      unscheduled_flushes_--;
       bg_flush_scheduled_++;
       env_->Schedule(&DBImpl::BGWorkFlush, this, Env::Priority::LOW, this);
     }
@@ -1336,12 +1215,6 @@ void DBImpl::MaybeScheduleFlushOrCompaction() {
   if (bg_compaction_paused_ > 0) {
     // we paused the background compaction
     return;
-  } else if (error_handler_.IsBGWorkStopped()) {
-    // Compaction is not part of the recovery sequence from a hard error. We
-    // might get here because recovery might do a flush and install a new
-    // super version, which will try to schedule pending compactions. Bail
-    // out here and let the higher level recovery handle compactions
-    return;
   }
 
   if (HasExclusiveManualCompaction()) {
@@ -1410,28 +1283,30 @@ ColumnFamilyData* DBImpl::PopFirstFromCompactionQueue() {
   return cfd;
 }
 
-DBImpl::FlushRequest DBImpl::PopFirstFromFlushQueue() {
-  assert(!flush_queue_.empty());
-  FlushRequest flush_req = flush_queue_.front();
-  assert(unscheduled_flushes_ >= static_cast<int>(flush_req.size()));
-  unscheduled_flushes_ -= static_cast<int>(flush_req.size());
-  flush_queue_.pop_front();
-  // TODO: need to unset flush reason?
-  return flush_req;
+void DBImpl::AddToFlushQueue(ColumnFamilyData* cfd, FlushReason flush_reason) {
+  assert(!cfd->queued_for_flush());
+  cfd->Ref();
+  flush_queue_.push_back(cfd);
+  cfd->set_queued_for_flush(true);
+  cfd->SetFlushReason(flush_reason);
 }
 
-void DBImpl::SchedulePendingFlush(const FlushRequest& flush_req,
+ColumnFamilyData* DBImpl::PopFirstFromFlushQueue() {
+  assert(!flush_queue_.empty());
+  auto cfd = *flush_queue_.begin();
+  flush_queue_.pop_front();
+  assert(cfd->queued_for_flush());
+  cfd->set_queued_for_flush(false);
+  // TODO: need to unset flush reason?
+  return cfd;
+}
+
+void DBImpl::SchedulePendingFlush(ColumnFamilyData* cfd,
                                   FlushReason flush_reason) {
-  if (flush_req.empty()) {
-    return;
+  if (!cfd->queued_for_flush() && cfd->imm()->IsFlushPending()) {
+    AddToFlushQueue(cfd, flush_reason);
+    ++unscheduled_flushes_;
   }
-  for (auto& iter : flush_req) {
-    ColumnFamilyData* cfd = iter.first;
-    cfd->Ref();
-    cfd->SetFlushReason(flush_reason);
-  }
-  unscheduled_flushes_ += static_cast<int>(flush_req.size());
-  flush_queue_.push_back(flush_req);
 }
 
 void DBImpl::SchedulePendingCompaction(ColumnFamilyData* cfd) {
@@ -1499,18 +1374,15 @@ void DBImpl::UnscheduleCallback(void* arg) {
 }
 
 Status DBImpl::BackgroundFlush(bool* made_progress, JobContext* job_context,
-                               LogBuffer* log_buffer, FlushReason* reason) {
+                               LogBuffer* log_buffer) {
   mutex_.AssertHeld();
 
   Status status;
-  *reason = FlushReason::kOthers;
-  // If BG work is stopped due to an error, but a recovery is in progress,
-  // that means this flush is part of the recovery. So allow it to go through
   if (!error_handler_.IsBGWorkStopped()) {
     if (shutting_down_.load(std::memory_order_acquire)) {
       status = Status::ShutdownInProgress();
     }
-  } else if (!error_handler_.IsRecoveryInProgress()) {
+  } else {
     status = error_handler_.GetBGError();
   }
 
@@ -1518,58 +1390,40 @@ Status DBImpl::BackgroundFlush(bool* made_progress, JobContext* job_context,
     return status;
   }
 
-  autovector<BGFlushArg> bg_flush_args;
-  std::vector<SuperVersionContext>& superversion_contexts =
-      job_context->superversion_contexts;
+  ColumnFamilyData* cfd = nullptr;
   while (!flush_queue_.empty()) {
     // This cfd is already referenced
-    const FlushRequest& flush_req = PopFirstFromFlushQueue();
-    superversion_contexts.clear();
-    superversion_contexts.reserve(flush_req.size());
+    auto first_cfd = PopFirstFromFlushQueue();
 
-    for (const auto& iter : flush_req) {
-      ColumnFamilyData* cfd = iter.first;
-      if (cfd->IsDropped() || !cfd->imm()->IsFlushPending()) {
-        // can't flush this CF, try next one
-        if (cfd->Unref()) {
-          delete cfd;
-        }
-        continue;
+    if (first_cfd->IsDropped() || !first_cfd->imm()->IsFlushPending()) {
+      // can't flush this CF, try next one
+      if (first_cfd->Unref()) {
+        delete first_cfd;
       }
-      superversion_contexts.emplace_back(SuperVersionContext(true));
-      bg_flush_args.emplace_back(cfd, iter.second,
-                                 &(superversion_contexts.back()));
-    }
-    if (!bg_flush_args.empty()) {
-      break;
+      continue;
     }
+
+    // found a flush!
+    cfd = first_cfd;
+    break;
   }
 
-  if (!bg_flush_args.empty()) {
+  if (cfd != nullptr) {
+    const MutableCFOptions mutable_cf_options =
+        *cfd->GetLatestMutableCFOptions();
     auto bg_job_limits = GetBGJobLimits();
-    for (const auto& arg : bg_flush_args) {
-      ColumnFamilyData* cfd = arg.cfd_;
-      ROCKS_LOG_BUFFER(
-          log_buffer,
-          "Calling FlushMemTableToOutputFile with column "
-          "family [%s], flush slots available %d, compaction slots available "
-          "%d, "
-          "flush slots scheduled %d, compaction slots scheduled %d",
-          cfd->GetName().c_str(), bg_job_limits.max_flushes,
-          bg_job_limits.max_compactions, bg_flush_scheduled_,
-          bg_compaction_scheduled_);
-    }
-    status = FlushMemTablesToOutputFiles(bg_flush_args, made_progress,
-                                         job_context, log_buffer);
-    // All the CFDs in the FlushReq must have the same flush reason, so just
-    // grab the first one
-    *reason = bg_flush_args[0].cfd_->GetFlushReason();
-    for (auto& arg : bg_flush_args) {
-      ColumnFamilyData* cfd = arg.cfd_;
-      if (cfd->Unref()) {
-        delete cfd;
-        arg.cfd_ = nullptr;
-      }
+    ROCKS_LOG_BUFFER(
+        log_buffer,
+        "Calling FlushMemTableToOutputFile with column "
+        "family [%s], flush slots available %d, compaction slots available %d, "
+        "flush slots scheduled %d, compaction slots scheduled %d",
+        cfd->GetName().c_str(), bg_job_limits.max_flushes,
+        bg_job_limits.max_compactions, bg_flush_scheduled_,
+        bg_compaction_scheduled_);
+    status = FlushMemTableToOutputFile(cfd, mutable_cf_options, made_progress,
+                                       job_context, log_buffer);
+    if (cfd->Unref()) {
+      delete cfd;
     }
   }
   return status;
@@ -1590,12 +1444,9 @@ void DBImpl::BackgroundCallFlush() {
 
     auto pending_outputs_inserted_elem =
         CaptureCurrentFileNumberInPendingOutputs();
-    FlushReason reason;
 
-    Status s =
-        BackgroundFlush(&made_progress, &job_context, &log_buffer, &reason);
-    if (!s.ok() && !s.IsShutdownInProgress() &&
-        reason != FlushReason::kErrorRecovery) {
+    Status s = BackgroundFlush(&made_progress, &job_context, &log_buffer);
+    if (!s.ok() && !s.IsShutdownInProgress()) {
       // Wait a little bit before retrying background flush in
       // case this is an environmental problem and we do not want to
       // chew up resources for failed flushes for the duration of
@@ -1785,11 +1636,6 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
     }
   } else {
     status = error_handler_.GetBGError();
-    // If we get here, it means a hard error happened after this compaction
-    // was scheduled by MaybeScheduleFlushOrCompaction(), but before it got
-    // a chance to execute. Since we didn't pop a cfd from the compaction
-    // queue, increment unscheduled_compactions_
-    unscheduled_compactions_++;
   }
 
   if (!status.ok()) {
@@ -1825,7 +1671,7 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
     } else {
       // First check if we have enough room to do the compaction
       bool enough_room = EnoughRoomForCompaction(
-          m->cfd, *(c->inputs()), &sfm_reserved_compact_space, log_buffer);
+          *(c->inputs()), &sfm_reserved_compact_space, log_buffer);
 
       if (!enough_room) {
         // Then don't do the compaction
@@ -1888,7 +1734,7 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
 
       if (c != nullptr) {
         bool enough_room = EnoughRoomForCompaction(
-            cfd, *(c->inputs()), &sfm_reserved_compact_space, log_buffer);
+            *(c->inputs()), &sfm_reserved_compact_space, log_buffer);
 
         if (!enough_room) {
           // Then don't do the compaction
@@ -2097,6 +1943,8 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
     NotifyOnCompactionCompleted(c->column_family_data(), c.get(), status,
                                 compaction_job_stats, job_context->job_id);
   }
+  // this will unref its input_version and column_family_data
+  c.reset();
 
   if (status.ok() || status.IsCompactionTooLarge()) {
     // Done
@@ -2106,26 +1954,7 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
     ROCKS_LOG_WARN(immutable_db_options_.info_log, "Compaction error: %s",
                    status.ToString().c_str());
     error_handler_.SetBGError(status, BackgroundErrorReason::kCompaction);
-    if (c != nullptr && !is_manual && !error_handler_.IsBGWorkStopped()) {
-      // Put this cfd back in the compaction queue so we can retry after some
-      // time
-      auto cfd = c->column_family_data();
-      assert(cfd != nullptr);
-      // Since this compaction failed, we need to recompute the score so it
-      // takes the original input files into account
-      c->column_family_data()
-          ->current()
-          ->storage_info()
-          ->ComputeCompactionScore(*(c->immutable_cf_options()),
-                                   *(c->mutable_cf_options()));
-      if (!cfd->queued_for_compaction()) {
-        AddToCompactionQueue(cfd);
-        ++unscheduled_compactions_;
-      }
-    }
   }
-  // this will unref its input_version and column_family_data
-  c.reset();
 
   if (is_manual) {
     ManualCompactionState* m = manual_compaction;
@@ -2274,10 +2103,7 @@ bool DBImpl::MCOverlap(ManualCompactionState* m, ManualCompactionState* m1) {
 
 void DBImpl::InstallSuperVersionAndScheduleWork(
     ColumnFamilyData* cfd, SuperVersionContext* sv_context,
-    const MutableCFOptions& mutable_cf_options,
-    FlushReason /* flush_reason */) {
-  // TODO(yanqin) investigate if 'flush_reason' can be removed since it's not
-  // used.
+    const MutableCFOptions& mutable_cf_options, FlushReason flush_reason) {
   mutex_.AssertHeld();
 
   // Update max_total_in_memory_state_
@@ -2296,6 +2122,7 @@ void DBImpl::InstallSuperVersionAndScheduleWork(
 
   // Whenever we install new SuperVersion, we might need to issue new flushes or
   // compactions.
+  SchedulePendingFlush(cfd, flush_reason);
   SchedulePendingCompaction(cfd);
   MaybeScheduleFlushOrCompaction();
 

db/db_impl_debug.cc

@@ -137,7 +137,7 @@ Status DBImpl::TEST_WaitForCompact(bool wait_unscheduled) {
   while ((bg_bottom_compaction_scheduled_ || bg_compaction_scheduled_ ||
           bg_flush_scheduled_ ||
           (wait_unscheduled && unscheduled_compactions_)) &&
-         (error_handler_.GetBGError() == Status::OK())) {
+         !error_handler_.IsDBStopped()) {
     bg_cv_.Wait();
   }
   return error_handler_.GetBGError();
@@ -237,11 +237,5 @@ SequenceNumber DBImpl::TEST_GetLastVisibleSequence() const {
   }
 }
 
-size_t DBImpl::TEST_GetWalPreallocateBlockSize(
-    uint64_t write_buffer_size) const {
-  InstrumentedMutexLock l(&mutex_);
-  return GetWalPreallocateBlockSize(write_buffer_size);
-}
-
 }  // namespace rocksdb
 #endif  // NDEBUG

db/db_impl_open.cc

@@ -134,15 +134,8 @@ DBOptions SanitizeOptions(const std::string& dbname, const DBOptions& src) {
   for (size_t i = 0; i < result.db_paths.size(); i++) {
     DeleteScheduler::CleanupDirectory(result.env, sfm, result.db_paths[i].path);
   }
-
-  // Create a default SstFileManager for purposes of tracking compaction size
-  // and facilitating recovery from out of space errors.
-  if (result.sst_file_manager.get() == nullptr) {
-    std::shared_ptr<SstFileManager> sst_file_manager(
-        NewSstFileManager(result.env, result.info_log));
-    result.sst_file_manager = sst_file_manager;
-  }
 #endif
+
   return result;
 }
 
@@ -239,7 +232,7 @@ Status DBImpl::NewDB() {
     file->SetPreallocationBlockSize(
         immutable_db_options_.manifest_preallocation_size);
     unique_ptr<WritableFileWriter> file_writer(
-        new WritableFileWriter(std::move(file), manifest, env_options));
+        new WritableFileWriter(std::move(file), env_options));
     log::Writer log(std::move(file_writer), 0, false);
     std::string record;
     new_db.EncodeTo(&record);
@@ -396,16 +389,6 @@ Status DBImpl::Recover(
       }
     }
   }
-
-  // Initial max_total_in_memory_state_ before recovery logs. Log recovery
-  // may check this value to decide whether to flush.
-  max_total_in_memory_state_ = 0;
-  for (auto cfd : *versions_->GetColumnFamilySet()) {
-    auto* mutable_cf_options = cfd->GetLatestMutableCFOptions();
-    max_total_in_memory_state_ += mutable_cf_options->write_buffer_size *
-                                  mutable_cf_options->max_write_buffer_number;
-  }
-
   if (s.ok()) {
     SequenceNumber next_sequence(kMaxSequenceNumber);
     default_cf_handle_ = new ColumnFamilyHandleImpl(
@@ -478,6 +461,14 @@ Status DBImpl::Recover(
     }
   }
 
+  // Initial value
+  max_total_in_memory_state_ = 0;
+  for (auto cfd : *versions_->GetColumnFamilySet()) {
+    auto* mutable_cf_options = cfd->GetLatestMutableCFOptions();
+    max_total_in_memory_state_ += mutable_cf_options->write_buffer_size *
+                                  mutable_cf_options->max_write_buffer_number;
+  }
+
   return s;
 }
 
@@ -607,7 +598,8 @@ Status DBImpl::RecoverLogFiles(const std::vector<uint64_t>& log_numbers,
     // to be skipped instead of propagating bad information (like overly
     // large sequence numbers).
     log::Reader reader(immutable_db_options_.info_log, std::move(file_reader),
-                       &reporter, true /*checksum*/, log_number);
+                       &reporter, true /*checksum*/, 0 /*initial_offset*/,
+                       log_number);
 
     // Determine if we should tolerate incomplete records at the tail end of the
     // Read all the records and add to a memtable
@@ -887,8 +879,18 @@ Status DBImpl::RecoverLogFiles(const std::vector<uint64_t>& log_numbers,
     }
   }
 
-  if (status.ok() && data_seen && !flushed) {
-    status = RestoreAliveLogFiles(log_numbers);
+  if (data_seen && !flushed) {
+    // Mark these as alive so they'll be considered for deletion later by
+    // FindObsoleteFiles()
+    if (two_write_queues_) {
+      log_write_mutex_.Lock();
+    }
+    for (auto log_number : log_numbers) {
+      alive_log_files_.push_back(LogFileNumberSize(log_number));
+    }
+    if (two_write_queues_) {
+      log_write_mutex_.Unlock();
+    }
   }
 
   event_logger_.Log() << "job" << job_id << "event"
@@ -897,60 +899,6 @@ Status DBImpl::RecoverLogFiles(const std::vector<uint64_t>& log_numbers,
   return status;
 }
 
-Status DBImpl::RestoreAliveLogFiles(const std::vector<uint64_t>& log_numbers) {
-  if (log_numbers.empty()) {
-    return Status::OK();
-  }
-  Status s;
-  mutex_.AssertHeld();
-  assert(immutable_db_options_.avoid_flush_during_recovery);
-  if (two_write_queues_) {
-    log_write_mutex_.Lock();
-  }
-  // Mark these as alive so they'll be considered for deletion later by
-  // FindObsoleteFiles()
-  total_log_size_ = 0;
-  log_empty_ = false;
-  for (auto log_number : log_numbers) {
-    LogFileNumberSize log(log_number);
-    std::string fname = LogFileName(immutable_db_options_.wal_dir, log_number);
-    // This gets the appear size of the logs, not including preallocated space.
-    s = env_->GetFileSize(fname, &log.size);
-    if (!s.ok()) {
-      break;
-    }
-    total_log_size_ += log.size;
-    alive_log_files_.push_back(log);
-    // We preallocate space for logs, but then after a crash and restart, those
-    // preallocated space are not needed anymore. It is likely only the last
-    // log has such preallocated space, so we only truncate for the last log.
-    if (log_number == log_numbers.back()) {
-      std::unique_ptr<WritableFile> last_log;
-      Status truncate_status = env_->ReopenWritableFile(
-          fname, &last_log,
-          env_->OptimizeForLogWrite(
-              env_options_,
-              BuildDBOptions(immutable_db_options_, mutable_db_options_)));
-      if (truncate_status.ok()) {
-        truncate_status = last_log->Truncate(log.size);
-      }
-      if (truncate_status.ok()) {
-        truncate_status = last_log->Close();
-      }
-      // Not a critical error if fail to truncate.
-      if (!truncate_status.ok()) {
-        ROCKS_LOG_WARN(immutable_db_options_.info_log,
-                       "Failed to truncate log #%" PRIu64 ": %s", log_number,
-                       truncate_status.ToString().c_str());
-      }
-    }
-  }
-  if (two_write_queues_) {
-    log_write_mutex_.Unlock();
-  }
-  return s;
-}
-
 Status DBImpl::WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd,
                                            MemTable* mem, VersionEdit* edit) {
   mutex_.AssertHeld();
@@ -1103,12 +1051,6 @@ Status DBImpl::Open(const DBOptions& db_options, const std::string& dbname,
         break;
       }
     }
-
-    // For recovery from NoSpace() error, we can only handle
-    // the case where the database is stored in a single path
-    if (paths.size() <= 1) {
-      impl->error_handler_.EnableAutoRecovery();
-    }
   }
 
   if (!s.ok()) {
@@ -1133,10 +1075,10 @@ Status DBImpl::Open(const DBOptions& db_options, const std::string& dbname,
         impl->immutable_db_options_.env->OptimizeForLogWrite(
             soptions, BuildDBOptions(impl->immutable_db_options_,
                                      impl->mutable_db_options_));
-    std::string log_fname =
-        LogFileName(impl->immutable_db_options_.wal_dir, new_log_number);
-    s = NewWritableFile(impl->immutable_db_options_.env, log_fname, &lfile,
-                        opt_env_options);
+    s = NewWritableFile(
+        impl->immutable_db_options_.env,
+        LogFileName(impl->immutable_db_options_.wal_dir, new_log_number),
+        &lfile, opt_env_options);
     if (s.ok()) {
       lfile->SetWriteLifeTimeHint(write_hint);
       lfile->SetPreallocationBlockSize(
@@ -1144,8 +1086,8 @@ Status DBImpl::Open(const DBOptions& db_options, const std::string& dbname,
       {
         InstrumentedMutexLock wl(&impl->log_write_mutex_);
         impl->logfile_number_ = new_log_number;
-        unique_ptr<WritableFileWriter> file_writer(new WritableFileWriter(
-            std::move(lfile), log_fname, opt_env_options));
+        unique_ptr<WritableFileWriter> file_writer(
+            new WritableFileWriter(std::move(lfile), opt_env_options));
         impl->logs_.emplace_back(
             new_log_number,
             new log::Writer(
@@ -1272,14 +1214,6 @@ Status DBImpl::Open(const DBOptions& db_options, const std::string& dbname,
         }
       }
     }
-
-    // Reserve some disk buffer space. This is a heuristic - when we run out
-    // of disk space, this ensures that there is atleast write_buffer_size
-    // amount of free space before we resume DB writes. In low disk space
-    // conditions, we want to avoid a lot of small L0 files due to frequent
-    // WAL write failures and resultant forced flushes
-    sfm->ReserveDiskBuffer(max_write_buffer_size,
-                           impl->immutable_db_options_.db_paths[0].path);
   }
 #endif  // !ROCKSDB_LITE
 

db/db_impl_readonly.cc

@@ -31,8 +31,6 @@ Status DBImplReadOnly::Get(const ReadOptions& read_options,
                            ColumnFamilyHandle* column_family, const Slice& key,
                            PinnableSlice* pinnable_val) {
   assert(pinnable_val != nullptr);
-  // TODO: stopwatch DB_GET needed?, perf timer needed?
-  PERF_TIMER_GUARD(get_snapshot_time);
   Status s;
   SequenceNumber snapshot = versions_->LastSequence();
   auto cfh = reinterpret_cast<ColumnFamilyHandleImpl*>(column_family);
@@ -47,22 +45,14 @@ Status DBImplReadOnly::Get(const ReadOptions& read_options,
   MergeContext merge_context;
   RangeDelAggregator range_del_agg(cfd->internal_comparator(), snapshot);
   LookupKey lkey(key, snapshot);
-  PERF_TIMER_STOP(get_snapshot_time);
   if (super_version->mem->Get(lkey, pinnable_val->GetSelf(), &s, &merge_context,
                               &range_del_agg, read_options)) {
     pinnable_val->PinSelf();
-    RecordTick(stats_, MEMTABLE_HIT);
   } else {
     PERF_TIMER_GUARD(get_from_output_files_time);
     super_version->current->Get(read_options, lkey, pinnable_val, &s,
                                 &merge_context, &range_del_agg);
-    RecordTick(stats_, MEMTABLE_MISS);
   }
-  RecordTick(stats_, NUMBER_KEYS_READ);
-  size_t size = pinnable_val->size();
-  RecordTick(stats_, BYTES_READ, size);
-  MeasureTime(stats_, BYTES_PER_READ, size);
-  PERF_COUNTER_ADD(get_read_bytes, size);
   return s;
 }
 

db/db_impl_write.cc

@@ -710,10 +710,6 @@ Status DBImpl::PreprocessWrite(const WriteOptions& write_options,
   assert(write_context != nullptr && need_log_sync != nullptr);
   Status status;
 
-  if (error_handler_.IsDBStopped()) {
-    status = error_handler_.GetBGError();
-  }
-
   PERF_TIMER_GUARD(write_scheduling_flushes_compactions_time);
 
   assert(!single_column_family_mode_ ||
@@ -732,6 +728,10 @@ Status DBImpl::PreprocessWrite(const WriteOptions& write_options,
     status = HandleWriteBufferFull(write_context);
   }
 
+  if (UNLIKELY(status.ok())) {
+    status = error_handler_.GetBGError();
+  }
+
   if (UNLIKELY(status.ok() && !flush_scheduler_.Empty())) {
     status = ScheduleFlushes(write_context);
   }
@@ -1064,7 +1064,6 @@ Status DBImpl::SwitchWAL(WriteContext* write_context) {
                  oldest_alive_log, total_log_size_.load(), GetMaxTotalWalSize());
   // no need to refcount because drop is happening in write thread, so can't
   // happen while we're in the write thread
-  FlushRequest flush_req;
   for (auto cfd : *versions_->GetColumnFamilySet()) {
     if (cfd->IsDropped()) {
       continue;
@@ -1074,14 +1073,11 @@ Status DBImpl::SwitchWAL(WriteContext* write_context) {
       if (!status.ok()) {
         break;
       }
-      flush_req.emplace_back(cfd, cfd->imm()->GetLatestMemTableID());
       cfd->imm()->FlushRequested();
+      SchedulePendingFlush(cfd, FlushReason::kWriteBufferManager);
     }
   }
-  if (status.ok()) {
-    SchedulePendingFlush(flush_req, FlushReason::kWriteBufferManager);
-    MaybeScheduleFlushOrCompaction();
-  }
+  MaybeScheduleFlushOrCompaction();
   return status;
 }
 
@@ -1120,26 +1116,14 @@ Status DBImpl::HandleWriteBufferFull(WriteContext* write_context) {
       }
     }
   }
-
-  autovector<ColumnFamilyData*> cfds;
   if (cfd_picked != nullptr) {
-    cfds.push_back(cfd_picked);
-  }
-  FlushRequest flush_req;
-  for (const auto cfd : cfds) {
-    cfd->Ref();
-    status = SwitchMemtable(cfd, write_context);
-    cfd->Unref();
-    if (!status.ok()) {
-      break;
+    status = SwitchMemtable(cfd_picked, write_context,
+                            FlushReason::kWriteBufferFull);
+    if (status.ok()) {
+      cfd_picked->imm()->FlushRequested();
+      SchedulePendingFlush(cfd_picked, FlushReason::kWriteBufferFull);
+      MaybeScheduleFlushOrCompaction();
     }
-    uint64_t flush_memtable_id = cfd->imm()->GetLatestMemTableID();
-    cfd->imm()->FlushRequested();
-    flush_req.emplace_back(cfd, flush_memtable_id);
-  }
-  if (status.ok()) {
-    SchedulePendingFlush(flush_req, FlushReason::kWriteBufferFull);
-    MaybeScheduleFlushOrCompaction();
   }
   return status;
 }
@@ -1189,11 +1173,7 @@ Status DBImpl::DelayWrite(uint64_t num_bytes,
       write_thread_.EndWriteStall();
     }
 
-    // Don't wait if there's a background error, even if its a soft error. We
-    // might wait here indefinitely as the background compaction may never
-    // finish successfully, resulting in the stall condition lasting
-    // indefinitely
-    while (error_handler_.GetBGError().ok() && write_controller_.IsStopped()) {
+    while (!error_handler_.IsDBStopped() && write_controller_.IsStopped()) {
       if (write_options.no_slowdown) {
         return Status::Incomplete("Write stall");
       }
@@ -1214,19 +1194,7 @@ Status DBImpl::DelayWrite(uint64_t num_bytes,
     RecordTick(stats_, STALL_MICROS, time_delayed);
   }
 
-  // If DB is not in read-only mode and write_controller is not stopping
-  // writes, we can ignore any background errors and allow the write to
-  // proceed
-  Status s;
-  if (write_controller_.IsStopped()) {
-    // If writes are still stopped, it means we bailed due to a background
-    // error
-    s = Status::Incomplete(error_handler_.GetBGError().ToString());
-  }
-  if (error_handler_.IsDBStopped()) {
-    s = error_handler_.GetBGError();
-  }
-  return s;
+  return error_handler_.GetBGError();
 }
 
 Status DBImpl::ThrottleLowPriWritesIfNeeded(const WriteOptions& write_options,
@@ -1261,28 +1229,16 @@ Status DBImpl::ThrottleLowPriWritesIfNeeded(const WriteOptions& write_options,
 
 Status DBImpl::ScheduleFlushes(WriteContext* context) {
   ColumnFamilyData* cfd;
-  FlushRequest flush_req;
-  Status status;
   while ((cfd = flush_scheduler_.TakeNextColumnFamily()) != nullptr) {
-    status = SwitchMemtable(cfd, context);
-    bool should_schedule = true;
+    auto status = SwitchMemtable(cfd, context, FlushReason::kWriteBufferFull);
     if (cfd->Unref()) {
       delete cfd;
-      should_schedule = false;
     }
     if (!status.ok()) {
-      break;
-    }
-    if (should_schedule) {
-      uint64_t flush_memtable_id = cfd->imm()->GetLatestMemTableID();
-      flush_req.emplace_back(cfd, flush_memtable_id);
+      return status;
     }
   }
-  if (status.ok()) {
-    SchedulePendingFlush(flush_req, FlushReason::kWriteBufferFull);
-    MaybeScheduleFlushOrCompaction();
-  }
-  return status;
+  return Status::OK();
 }
 
 #ifndef ROCKSDB_LITE
@@ -1303,7 +1259,8 @@ void DBImpl::NotifyOnMemTableSealed(ColumnFamilyData* /*cfd*/,
 
 // REQUIRES: mutex_ is held
 // REQUIRES: this thread is currently at the front of the writer queue
-Status DBImpl::SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context) {
+Status DBImpl::SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context,
+                              FlushReason flush_reason) {
   mutex_.AssertHeld();
   WriteThread::Writer nonmem_w;
   if (two_write_queues_) {
@@ -1372,8 +1329,6 @@ Status DBImpl::SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context) {
   auto write_hint = CalculateWALWriteHint();
   mutex_.Unlock();
   {
-    std::string log_fname =
-        LogFileName(immutable_db_options_.wal_dir, new_log_number);
     if (creating_new_log) {
       EnvOptions opt_env_opt =
           env_->OptimizeForLogWrite(env_options_, db_options);
@@ -1381,12 +1336,14 @@ Status DBImpl::SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context) {
         ROCKS_LOG_INFO(immutable_db_options_.info_log,
                        "reusing log %" PRIu64 " from recycle list\n",
                        recycle_log_number);
-        std::string old_log_fname =
-            LogFileName(immutable_db_options_.wal_dir, recycle_log_number);
-        s = env_->ReuseWritableFile(log_fname, old_log_fname, &lfile,
-                                    opt_env_opt);
+        s = env_->ReuseWritableFile(
+            LogFileName(immutable_db_options_.wal_dir, new_log_number),
+            LogFileName(immutable_db_options_.wal_dir, recycle_log_number),
+            &lfile, opt_env_opt);
       } else {
-        s = NewWritableFile(env_, log_fname, &lfile, opt_env_opt);
+        s = NewWritableFile(
+            env_, LogFileName(immutable_db_options_.wal_dir, new_log_number),
+            &lfile, opt_env_opt);
       }
       if (s.ok()) {
         // Our final size should be less than write_buffer_size
@@ -1397,7 +1354,7 @@ Status DBImpl::SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context) {
         lfile->SetPreallocationBlockSize(preallocate_block_size);
         lfile->SetWriteLifeTimeHint(write_hint);
         unique_ptr<WritableFileWriter> file_writer(
-            new WritableFileWriter(std::move(lfile), log_fname, opt_env_opt));
+            new WritableFileWriter(std::move(lfile), opt_env_opt));
         new_log = new log::Writer(
             std::move(file_writer), new_log_number,
             immutable_db_options_.recycle_log_file_num > 0, manual_wal_flush_);
@@ -1475,7 +1432,7 @@ Status DBImpl::SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context) {
   new_mem->Ref();
   cfd->SetMemtable(new_mem);
   InstallSuperVersionAndScheduleWork(cfd, &context->superversion_context,
-                                     mutable_cf_options);
+                                     mutable_cf_options, flush_reason);
   if (two_write_queues_) {
     nonmem_write_thread_.ExitUnbatched(&nonmem_w);
   }

db/db_table_properties_test.cc

@@ -252,14 +252,13 @@ TEST_F(DBTablePropertiesTest, GetColumnFamilyNameProperty) {
 }
 
 TEST_F(DBTablePropertiesTest, DeletionTriggeredCompactionMarking) {
-  int kNumKeys = 1000;
-  int kWindowSize = 100;
-  int kNumDelsTrigger = 90;
-  std::shared_ptr<TablePropertiesCollectorFactory> compact_on_del =
-    NewCompactOnDeletionCollectorFactory(kWindowSize, kNumDelsTrigger);
+  const int kNumKeys = 1000;
+  const int kWindowSize = 100;
+  const int kNumDelsTrigger = 90;
 
   Options opts = CurrentOptions();
-  opts.table_properties_collector_factories.emplace_back(compact_on_del);
+  opts.table_properties_collector_factories.emplace_back(
+      NewCompactOnDeletionCollectorFactory(kWindowSize, kNumDelsTrigger));
   Reopen(opts);
 
   // add an L1 file to prevent tombstones from dropping due to obsolescence
@@ -281,48 +280,6 @@ TEST_F(DBTablePropertiesTest, DeletionTriggeredCompactionMarking) {
   dbfull()->TEST_WaitForCompact();
   ASSERT_EQ(0, NumTableFilesAtLevel(0));
   ASSERT_GT(NumTableFilesAtLevel(1), 0);
-
-  // Change the window size and deletion trigger and ensure new values take
-  // effect
-  kWindowSize = 50;
-  kNumDelsTrigger = 40;
-  static_cast<CompactOnDeletionCollectorFactory*>
-    (compact_on_del.get())->SetWindowSize(kWindowSize);
-  static_cast<CompactOnDeletionCollectorFactory*>
-    (compact_on_del.get())->SetDeletionTrigger(kNumDelsTrigger);
-  for (int i = 0; i < kNumKeys; ++i) {
-    if (i >= kNumKeys - kWindowSize &&
-        i < kNumKeys - kWindowSize + kNumDelsTrigger) {
-      Delete(Key(i));
-    } else {
-      Put(Key(i), "val");
-    }
-  }
-  Flush();
-
-  dbfull()->TEST_WaitForCompact();
-  ASSERT_EQ(0, NumTableFilesAtLevel(0));
-  ASSERT_GT(NumTableFilesAtLevel(1), 0);
-
-  // Change the window size to disable delete triggered compaction
-  kWindowSize = 0;
-  static_cast<CompactOnDeletionCollectorFactory*>
-    (compact_on_del.get())->SetWindowSize(kWindowSize);
-  static_cast<CompactOnDeletionCollectorFactory*>
-    (compact_on_del.get())->SetDeletionTrigger(kNumDelsTrigger);
-  for (int i = 0; i < kNumKeys; ++i) {
-    if (i >= kNumKeys - kWindowSize &&
-        i < kNumKeys - kWindowSize + kNumDelsTrigger) {
-      Delete(Key(i));
-    } else {
-      Put(Key(i), "val");
-    }
-  }
-  Flush();
-
-  dbfull()->TEST_WaitForCompact();
-  ASSERT_EQ(1, NumTableFilesAtLevel(0));
-
 }
 
 }  // namespace rocksdb

db/db_test_util.h

@@ -315,9 +315,6 @@ class SpecialEnv : public EnvWrapper {
         }
       }
       uint64_t GetFileSize() override { return base_->GetFileSize(); }
-      Status Allocate(uint64_t offset, uint64_t len) override {
-        return base_->Allocate(offset, len);
-      }
 
      private:
       SpecialEnv* env_;
@@ -371,9 +368,6 @@ class SpecialEnv : public EnvWrapper {
       bool IsSyncThreadSafe() const override {
         return env_->is_wal_sync_thread_safe_.load();
       }
-      Status Allocate(uint64_t offset, uint64_t len) override {
-        return base_->Allocate(offset, len);
-      }
 
      private:
       SpecialEnv* env_;
@@ -579,7 +573,7 @@ class SpecialEnv : public EnvWrapper {
 
   std::atomic<bool> is_wal_sync_thread_safe_{true};
 
-  std::atomic<size_t> compaction_readahead_size_{};
+  std::atomic<size_t> compaction_readahead_size_;
 };
 
 class MockTimeEnv : public EnvWrapper {

db/db_wal_test.cc

@@ -18,15 +18,6 @@ namespace rocksdb {
 class DBWALTest : public DBTestBase {
  public:
   DBWALTest() : DBTestBase("/db_wal_test") {}
-
-#if defined(ROCKSDB_PLATFORM_POSIX)
-  uint64_t GetAllocatedFileSize(std::string file_name) {
-    struct stat sbuf;
-    int err = stat(file_name.c_str(), &sbuf);
-    assert(err == 0);
-    return sbuf.st_blocks * 512;
-  }
-#endif
 };
 
 // A SpecialEnv enriched to give more insight about deleted files
@@ -824,7 +815,7 @@ class RecoveryTestHelper {
       unique_ptr<WritableFile> file;
       ASSERT_OK(db_options.env->NewWritableFile(fname, &file, env_options));
       unique_ptr<WritableFileWriter> file_writer(
-          new WritableFileWriter(std::move(file), fname, env_options));
+          new WritableFileWriter(std::move(file), env_options));
       current_log_writer.reset(
           new log::Writer(std::move(file_writer), current_log_number,
                           db_options.recycle_log_file_num > 0));
@@ -1338,99 +1329,6 @@ TEST_F(DBWALTest, RecoverFromCorruptedWALWithoutFlush) {
   }
 }
 
-// Tests that total log size is recovered if we set
-// avoid_flush_during_recovery=true.
-// Flush should trigger if max_total_wal_size is reached.
-TEST_F(DBWALTest, RestoreTotalLogSizeAfterRecoverWithoutFlush) {
-  class TestFlushListener : public EventListener {
-   public:
-    std::atomic<int> count{0};
-
-    TestFlushListener() = default;
-
-    void OnFlushBegin(DB* /*db*/, const FlushJobInfo& flush_job_info) override {
-      count++;
-      assert(FlushReason::kWriteBufferManager == flush_job_info.flush_reason);
-    }
-  };
-  std::shared_ptr<TestFlushListener> test_listener =
-      std::make_shared<TestFlushListener>();
-
-  constexpr size_t kKB = 1024;
-  constexpr size_t kMB = 1024 * 1024;
-  Options options = CurrentOptions();
-  options.avoid_flush_during_recovery = true;
-  options.max_total_wal_size = 1 * kMB;
-  options.listeners.push_back(test_listener);
-  // Have to open DB in multi-CF mode to trigger flush when
-  // max_total_wal_size is reached.
-  CreateAndReopenWithCF({"one"}, options);
-  // Write some keys and we will end up with one log file which is slightly
-  // smaller than 1MB.
-  std::string value_100k(100 * kKB, 'v');
-  std::string value_300k(300 * kKB, 'v');
-  ASSERT_OK(Put(0, "foo", "v1"));
-  for (int i = 0; i < 9; i++) {
-    ASSERT_OK(Put(1, "key" + ToString(i), value_100k));
-  }
-  // Get log files before reopen.
-  VectorLogPtr log_files_before;
-  ASSERT_OK(dbfull()->GetSortedWalFiles(log_files_before));
-  ASSERT_EQ(1, log_files_before.size());
-  uint64_t log_size_before = log_files_before[0]->SizeFileBytes();
-  ASSERT_GT(log_size_before, 900 * kKB);
-  ASSERT_LT(log_size_before, 1 * kMB);
-  ReopenWithColumnFamilies({"default", "one"}, options);
-  // Write one more value to make log larger than 1MB.
-  ASSERT_OK(Put(1, "bar", value_300k));
-  // Get log files again. A new log file will be opened.
-  VectorLogPtr log_files_after_reopen;
-  ASSERT_OK(dbfull()->GetSortedWalFiles(log_files_after_reopen));
-  ASSERT_EQ(2, log_files_after_reopen.size());
-  ASSERT_EQ(log_files_before[0]->LogNumber(),
-            log_files_after_reopen[0]->LogNumber());
-  ASSERT_GT(log_files_after_reopen[0]->SizeFileBytes() +
-                log_files_after_reopen[1]->SizeFileBytes(),
-            1 * kMB);
-  // Write one more key to trigger flush.
-  ASSERT_OK(Put(0, "foo", "v2"));
-  dbfull()->TEST_WaitForFlushMemTable();
-  // Flushed two column families.
-  ASSERT_EQ(2, test_listener->count.load());
-}
-
-#if defined(ROCKSDB_PLATFORM_POSIX)
-#if defined(ROCKSDB_FALLOCATE_PRESENT)
-// Tests that we will truncate the preallocated space of the last log from
-// previous.
-TEST_F(DBWALTest, TruncateLastLogAfterRecoverWithoutFlush) {
-  constexpr size_t kKB = 1024;
-  Options options = CurrentOptions();
-  options.avoid_flush_during_recovery = true;
-  DestroyAndReopen(options);
-  size_t preallocated_size =
-      dbfull()->TEST_GetWalPreallocateBlockSize(options.write_buffer_size);
-  ASSERT_OK(Put("foo", "v1"));
-  VectorLogPtr log_files_before;
-  ASSERT_OK(dbfull()->GetSortedWalFiles(log_files_before));
-  ASSERT_EQ(1, log_files_before.size());
-  auto& file_before = log_files_before[0];
-  ASSERT_LT(file_before->SizeFileBytes(), 1 * kKB);
-  // The log file has preallocated space.
-  ASSERT_GE(GetAllocatedFileSize(dbname_ + file_before->PathName()),
-            preallocated_size);
-  Reopen(options);
-  VectorLogPtr log_files_after;
-  ASSERT_OK(dbfull()->GetSortedWalFiles(log_files_after));
-  ASSERT_EQ(1, log_files_after.size());
-  ASSERT_LT(log_files_after[0]->SizeFileBytes(), 1 * kKB);
-  // The preallocated space should be truncated.
-  ASSERT_LT(GetAllocatedFileSize(dbname_ + file_before->PathName()),
-            preallocated_size);
-}
-#endif  // ROCKSDB_FALLOCATE_PRESENT
-#endif  // ROCKSDB_PLATFORM_POSIX
-
 #endif  // ROCKSDB_LITE
 
 TEST_F(DBWALTest, WalTermTest) {

db/error_handler.cc

@@ -4,9 +4,7 @@
 //  (found in the LICENSE.Apache file in the root directory).
 //
 #include "db/error_handler.h"
-#include "db/db_impl.h"
 #include "db/event_helpers.h"
-#include "util/sst_file_manager_impl.h"
 
 namespace rocksdb {
 
@@ -35,7 +33,7 @@ std::map<std::tuple<BackgroundErrorReason, Status::Code, Status::SubCode, bool>,
         // Errors during BG flush
         {std::make_tuple(BackgroundErrorReason::kFlush, Status::Code::kIOError,
                          Status::SubCode::kNoSpace, true),
-         Status::Severity::kHardError},
+         Status::Severity::kSoftError},
         {std::make_tuple(BackgroundErrorReason::kFlush, Status::Code::kIOError,
                          Status::SubCode::kNoSpace, false),
          Status::Severity::kNoError},
@@ -46,11 +44,11 @@ std::map<std::tuple<BackgroundErrorReason, Status::Code, Status::SubCode, bool>,
         {std::make_tuple(BackgroundErrorReason::kWriteCallback,
                          Status::Code::kIOError, Status::SubCode::kNoSpace,
                          true),
-         Status::Severity::kHardError},
+         Status::Severity::kFatalError},
         {std::make_tuple(BackgroundErrorReason::kWriteCallback,
                          Status::Code::kIOError, Status::SubCode::kNoSpace,
                          false),
-         Status::Severity::kHardError},
+         Status::Severity::kFatalError},
 };
 
 std::map<std::tuple<BackgroundErrorReason, Status::Code, bool>, Status::Severity>
@@ -120,45 +118,6 @@ std::map<std::tuple<BackgroundErrorReason, bool>, Status::Severity>
           Status::Severity::kFatalError},
 };
 
-void ErrorHandler::CancelErrorRecovery() {
-#ifndef ROCKSDB_LITE
-  db_mutex_->AssertHeld();
-
-  // We'll release the lock before calling sfm, so make sure no new
-  // recovery gets scheduled at that point
-  auto_recovery_ = false;
-  SstFileManagerImpl* sfm = reinterpret_cast<SstFileManagerImpl*>(
-      db_options_.sst_file_manager.get());
-  if (sfm) {
-    // This may or may not cancel a pending recovery
-    db_mutex_->Unlock();
-    bool cancelled = sfm->CancelErrorRecovery(this);
-    db_mutex_->Lock();
-    if (cancelled) {
-      recovery_in_prog_ = false;
-    }
-  }
-#endif
-}
-
-// This is the main function for looking at an error during a background
-// operation and deciding the severity, and error recovery strategy. The high
-// level algorithm is as follows -
-// 1. Classify the severity of the error based on the ErrorSeverityMap,
-//    DefaultErrorSeverityMap and DefaultReasonMap defined earlier
-// 2. Call a Status code specific override function to adjust the severity
-//    if needed. The reason for this is our ability to recover may depend on
-//    the exact options enabled in DBOptions
-// 3. Determine if auto recovery is possible. A listener notification callback
-//    is called, which can disable the auto recovery even if we decide its
-//    feasible
-// 4. For Status::NoSpace() errors, rely on SstFileManagerImpl to control
-//    the actual recovery. If no sst file manager is specified in DBOptions,
-//    a default one is allocated during DB::Open(), so there will always be
-//    one.
-// This can also get called as part of a recovery operation. In that case, we
-// also track the error seperately in recovery_error_ so we can tell in the
-// end whether recovery succeeded or not
 Status ErrorHandler::SetBGError(const Status& bg_err, BackgroundErrorReason reason) {
   db_mutex_->AssertHeld();
 
@@ -166,12 +125,6 @@ Status ErrorHandler::SetBGError(const Status& bg_err, BackgroundErrorReason reas
     return Status::OK();
   }
 
-  // Check if recovery is currently in progress. If it is, we will save this
-  // error so we can check it at the end to see if recovery succeeded or not
-  if (recovery_in_prog_ && recovery_error_.ok()) {
-    recovery_error_ = bg_err;
-  }
-
   bool paranoid = db_options_.paranoid_checks;
   Status::Severity sev = Status::Severity::kFatalError;
   Status new_bg_err;
@@ -203,143 +156,15 @@ Status ErrorHandler::SetBGError(const Status& bg_err, BackgroundErrorReason reas
   }
 
   new_bg_err = Status(bg_err, sev);
-
-  bool auto_recovery = auto_recovery_;
-  if (new_bg_err.severity() >= Status::Severity::kFatalError && auto_recovery) {
-    auto_recovery = false;
-    ;
-  }
-
-  // Allow some error specific overrides
-  if (new_bg_err == Status::NoSpace()) {
-    new_bg_err = OverrideNoSpaceError(new_bg_err, &auto_recovery);
-  }
-
   if (!new_bg_err.ok()) {
     Status s = new_bg_err;
-    EventHelpers::NotifyOnBackgroundError(db_options_.listeners, reason, &s,
-                                          db_mutex_, &auto_recovery);
+    EventHelpers::NotifyOnBackgroundError(db_options_.listeners, reason, &s, db_mutex_);
     if (!s.ok() && (s.severity() > bg_error_.severity())) {
       bg_error_ = s;
-    } else {
-      // This error is less severe than previously encountered error. Don't
-      // take any further action
-      return bg_error_;
     }
   }
 
-  if (auto_recovery) {
-    recovery_in_prog_ = true;
-
-    // Kick-off error specific recovery
-    if (bg_error_ == Status::NoSpace()) {
-      RecoverFromNoSpace();
-    }
-  }
   return bg_error_;
 }
 
-Status ErrorHandler::OverrideNoSpaceError(Status bg_error,
-                                          bool* auto_recovery) {
-#ifndef ROCKSDB_LITE
-  if (bg_error.severity() >= Status::Severity::kFatalError) {
-    return bg_error;
-  }
-
-  if (db_options_.sst_file_manager.get() == nullptr) {
-    // We rely on SFM to poll for enough disk space and recover
-    *auto_recovery = false;
-    return bg_error;
-  }
-
-  if (db_options_.allow_2pc &&
-      (bg_error.severity() <= Status::Severity::kSoftError)) {
-    // Don't know how to recover, as the contents of the current WAL file may
-    // be inconsistent, and it may be needed for 2PC. If 2PC is not enabled,
-    // we can just flush the memtable and discard the log
-    *auto_recovery = false;
-    return Status(bg_error, Status::Severity::kFatalError);
-  }
-
-  {
-    uint64_t free_space;
-    if (db_options_.env->GetFreeSpace(db_options_.db_paths[0].path,
-                                      &free_space) == Status::NotSupported()) {
-      *auto_recovery = false;
-    }
-  }
-
-  return bg_error;
-#else
-  (void)auto_recovery;
-  return Status(bg_error, Status::Severity::kFatalError);
-#endif
-}
-
-void ErrorHandler::RecoverFromNoSpace() {
-#ifndef ROCKSDB_LITE
-  SstFileManagerImpl* sfm =
-      reinterpret_cast<SstFileManagerImpl*>(db_options_.sst_file_manager.get());
-
-  // Inform SFM of the error, so it can kick-off the recovery
-  if (sfm) {
-    sfm->StartErrorRecovery(this, bg_error_);
-  }
-#endif
-}
-
-Status ErrorHandler::ClearBGError() {
-#ifndef ROCKSDB_LITE
-  db_mutex_->AssertHeld();
-
-  // Signal that recovery succeeded
-  if (recovery_error_.ok()) {
-    Status old_bg_error = bg_error_;
-    bg_error_ = Status::OK();
-    recovery_in_prog_ = false;
-    EventHelpers::NotifyOnErrorRecoveryCompleted(db_options_.listeners,
-                                                 old_bg_error, db_mutex_);
-  }
-  return recovery_error_;
-#else
-  return bg_error_;
-#endif
-}
-
-Status ErrorHandler::RecoverFromBGError(bool is_manual) {
-#ifndef ROCKSDB_LITE
-  InstrumentedMutexLock l(db_mutex_);
-  if (is_manual) {
-    // If its a manual recovery and there's a background recovery in progress
-    // return busy status
-    if (recovery_in_prog_) {
-      return Status::Busy();
-    }
-    recovery_in_prog_ = true;
-  }
-
-  if (bg_error_.severity() == Status::Severity::kSoftError) {
-    // Simply clear the background error and return
-    recovery_error_ = Status::OK();
-    return ClearBGError();
-  }
-
-  // Reset recovery_error_. We will use this to record any errors that happen
-  // during the recovery process. While recovering, the only operations that
-  // can generate background errors should be the flush operations
-  recovery_error_ = Status::OK();
-  Status s = db_->ResumeImpl();
-  // For manual recover, shutdown, and fatal error  cases, set
-  // recovery_in_prog_ to false. For automatic background recovery, leave it
-  // as is regardless of success or failure as it will be retried
-  if (is_manual || s.IsShutdownInProgress() ||
-      bg_error_.severity() >= Status::Severity::kFatalError) {
-    recovery_in_prog_ = false;
-  }
-  return s;
-#else
-  (void)is_manual;
-  return bg_error_;
-#endif
-}
 }

db/error_handler.h

@@ -11,65 +11,42 @@
 
 namespace rocksdb {
 
-class DBImpl;
-
 class ErrorHandler {
   public:
-   ErrorHandler(DBImpl* db, const ImmutableDBOptions& db_options,
-                InstrumentedMutex* db_mutex)
-       : db_(db),
-         db_options_(db_options),
-         bg_error_(Status::OK()),
-         recovery_error_(Status::OK()),
-         db_mutex_(db_mutex),
-         auto_recovery_(false),
-         recovery_in_prog_(false) {}
-   ~ErrorHandler() {}
+    ErrorHandler(const ImmutableDBOptions& db_options,
+        InstrumentedMutex* db_mutex)
+      : db_options_(db_options),
+        bg_error_(Status::OK()),
+        db_mutex_(db_mutex)
+      {}
+    ~ErrorHandler() {}
 
-   void EnableAutoRecovery() { auto_recovery_ = true; }
+    Status::Severity GetErrorSeverity(BackgroundErrorReason reason,
+        Status::Code code, Status::SubCode subcode);
 
-   Status::Severity GetErrorSeverity(BackgroundErrorReason reason,
-                                     Status::Code code,
-                                     Status::SubCode subcode);
+    Status SetBGError(const Status& bg_err, BackgroundErrorReason reason);
 
-   Status SetBGError(const Status& bg_err, BackgroundErrorReason reason);
+    Status GetBGError()
+    {
+      return bg_error_;
+    }
 
-   Status GetBGError() { return bg_error_; }
+    void ClearBGError() {
+      bg_error_ = Status::OK();
+    }
 
-   Status GetRecoveryError() { return recovery_error_; }
-
-   Status ClearBGError();
-
-   bool IsDBStopped() {
-     return !bg_error_.ok() &&
-            bg_error_.severity() >= Status::Severity::kHardError;
+    bool IsDBStopped() {
+      return !bg_error_.ok();
     }
 
     bool IsBGWorkStopped() {
-      return !bg_error_.ok() &&
-             (bg_error_.severity() >= Status::Severity::kHardError ||
-              !auto_recovery_);
+      return !bg_error_.ok();
     }
 
-    bool IsRecoveryInProgress() { return recovery_in_prog_; }
-
-    Status RecoverFromBGError(bool is_manual = false);
-    void CancelErrorRecovery();
-
-   private:
-    DBImpl* db_;
+  private:
     const ImmutableDBOptions& db_options_;
     Status bg_error_;
-    // A seperate Status variable used to record any errors during the
-    // recovery process from hard errors
-    Status recovery_error_;
     InstrumentedMutex* db_mutex_;
-    // A flag indicating whether automatic recovery from errors is enabled
-    bool auto_recovery_;
-    bool recovery_in_prog_;
-
-    Status OverrideNoSpaceError(Status bg_error, bool* auto_recovery);
-    void RecoverFromNoSpace();
 };
 
 }

db/error_handler_test.cc

@@ -6,12 +6,9 @@
 // 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.
-#ifndef ROCKSDB_LITE
-
 #include "db/db_test_util.h"
 #include "port/stack_trace.h"
 #include "rocksdb/perf_context.h"
-#include "rocksdb/sst_file_manager.h"
 #include "util/fault_injection_test_env.h"
 #if !defined(ROCKSDB_LITE)
 #include "util/sync_point.h"
@@ -36,139 +33,36 @@ class DBErrorHandlingEnv : public EnvWrapper {
     bool trig_io_error;
 };
 
-class ErrorHandlerListener : public EventListener {
- public:
-  ErrorHandlerListener()
-      : mutex_(),
-        cv_(&mutex_),
-        no_auto_recovery_(false),
-        recovery_complete_(false),
-        file_creation_started_(false),
-        override_bg_error_(false),
-        file_count_(0),
-        fault_env_(nullptr) {}
-
-  void OnTableFileCreationStarted(
-      const TableFileCreationBriefInfo& /*ti*/) override {
-    InstrumentedMutexLock l(&mutex_);
-    file_creation_started_ = true;
-    if (file_count_ > 0) {
-      if (--file_count_ == 0) {
-        fault_env_->SetFilesystemActive(false, file_creation_error_);
-        file_creation_error_ = Status::OK();
-      }
-    }
-    cv_.SignalAll();
-  }
-
-  void OnErrorRecoveryBegin(BackgroundErrorReason /*reason*/,
-                            Status /*bg_error*/,
-                            bool* auto_recovery) override {
-    if (*auto_recovery && no_auto_recovery_) {
-      *auto_recovery = false;
-    }
-  }
-
-  void OnErrorRecoveryCompleted(Status /*old_bg_error*/) override {
-    InstrumentedMutexLock l(&mutex_);
-    recovery_complete_ = true;
-    cv_.SignalAll();
-  }
-
-  bool WaitForRecovery(uint64_t /*abs_time_us*/) {
-    InstrumentedMutexLock l(&mutex_);
-    while (!recovery_complete_) {
-      cv_.Wait(/*abs_time_us*/);
-    }
-    if (recovery_complete_) {
-      recovery_complete_ = false;
-      return true;
-    }
-    return false;
-  }
-
-  void WaitForTableFileCreationStarted(uint64_t /*abs_time_us*/) {
-    InstrumentedMutexLock l(&mutex_);
-    while (!file_creation_started_) {
-      cv_.Wait(/*abs_time_us*/);
-    }
-    file_creation_started_ = false;
-  }
-
-  void OnBackgroundError(BackgroundErrorReason /*reason*/,
-                         Status* bg_error) override {
-    if (override_bg_error_) {
-      *bg_error = bg_error_;
-      override_bg_error_ = false;
-    }
-  }
-
-  void EnableAutoRecovery(bool enable = true) { no_auto_recovery_ = !enable; }
-
-  void OverrideBGError(Status bg_err) {
-    bg_error_ = bg_err;
-    override_bg_error_ = true;
-  }
-
-  void InjectFileCreationError(FaultInjectionTestEnv* env, int file_count,
-                               Status s) {
-    fault_env_ = env;
-    file_count_ = file_count;
-    file_creation_error_ = s;
-  }
-
- private:
-  InstrumentedMutex mutex_;
-  InstrumentedCondVar cv_;
-  bool no_auto_recovery_;
-  bool recovery_complete_;
-  bool file_creation_started_;
-  bool override_bg_error_;
-  int file_count_;
-  Status file_creation_error_;
-  Status bg_error_;
-  FaultInjectionTestEnv* fault_env_;
-};
-
 TEST_F(DBErrorHandlingTest, FLushWriteError) {
   std::unique_ptr<FaultInjectionTestEnv> fault_env(
       new FaultInjectionTestEnv(Env::Default()));
-  std::shared_ptr<ErrorHandlerListener> listener(new ErrorHandlerListener());
   Options options = GetDefaultOptions();
   options.create_if_missing = true;
   options.env = fault_env.get();
-  options.listeners.emplace_back(listener);
   Status s;
-
-  listener->EnableAutoRecovery(false);
   DestroyAndReopen(options);
 
-  Put(Key(0), "val");
+  Put(Key(0), "va;");
   SyncPoint::GetInstance()->SetCallBack(
       "FlushJob::Start", [&](void *) {
     fault_env->SetFilesystemActive(false, Status::NoSpace("Out of space"));
   });
   SyncPoint::GetInstance()->EnableProcessing();
   s = Flush();
-  ASSERT_EQ(s.severity(), rocksdb::Status::Severity::kHardError);
-  SyncPoint::GetInstance()->DisableProcessing();
+  ASSERT_EQ(s.severity(), rocksdb::Status::Severity::kSoftError);
   fault_env->SetFilesystemActive(true);
   s = dbfull()->Resume();
   ASSERT_EQ(s, Status::OK());
 
-  Reopen(options);
-  ASSERT_EQ("val", Get(Key(0)));
   Destroy(options);
 }
 
 TEST_F(DBErrorHandlingTest, CompactionWriteError) {
   std::unique_ptr<FaultInjectionTestEnv> fault_env(
       new FaultInjectionTestEnv(Env::Default()));
-  std::shared_ptr<ErrorHandlerListener> listener(new ErrorHandlerListener());
   Options options = GetDefaultOptions();
   options.create_if_missing = true;
   options.level0_file_num_compaction_trigger = 2;
-  options.listeners.emplace_back(listener);
   options.env = fault_env.get();
   Status s;
   DestroyAndReopen(options);
@@ -178,10 +72,6 @@ TEST_F(DBErrorHandlingTest, CompactionWriteError) {
   s = Flush();
   ASSERT_EQ(s, Status::OK());
 
-  listener->OverrideBGError(
-      Status(Status::NoSpace(), Status::Severity::kHardError)
-      );
-  listener->EnableAutoRecovery(false);
   rocksdb::SyncPoint::GetInstance()->LoadDependency(
       {{"FlushMemTableFinished", "BackgroundCallCompaction:0"}});
   rocksdb::SyncPoint::GetInstance()->SetCallBack(
@@ -195,7 +85,7 @@ TEST_F(DBErrorHandlingTest, CompactionWriteError) {
   ASSERT_EQ(s, Status::OK());
 
   s = dbfull()->TEST_WaitForCompact();
-  ASSERT_EQ(s.severity(), rocksdb::Status::Severity::kHardError);
+  ASSERT_EQ(s.severity(), rocksdb::Status::Severity::kSoftError);
 
   fault_env->SetFilesystemActive(true);
   s = dbfull()->Resume();
@@ -239,439 +129,6 @@ TEST_F(DBErrorHandlingTest, CorruptionError) {
   Destroy(options);
 }
 
-TEST_F(DBErrorHandlingTest, AutoRecoverFlushError) {
-  std::unique_ptr<FaultInjectionTestEnv> fault_env(
-      new FaultInjectionTestEnv(Env::Default()));
-  std::shared_ptr<ErrorHandlerListener> listener(new ErrorHandlerListener());
-  Options options = GetDefaultOptions();
-  options.create_if_missing = true;
-  options.env = fault_env.get();
-  options.listeners.emplace_back(listener);
-  Status s;
-
-  listener->EnableAutoRecovery();
-  DestroyAndReopen(options);
-
-  Put(Key(0), "val");
-  SyncPoint::GetInstance()->SetCallBack("FlushJob::Start", [&](void*) {
-    fault_env->SetFilesystemActive(false, Status::NoSpace("Out of space"));
-  });
-  SyncPoint::GetInstance()->EnableProcessing();
-  s = Flush();
-  ASSERT_EQ(s.severity(), rocksdb::Status::Severity::kHardError);
-  SyncPoint::GetInstance()->DisableProcessing();
-  fault_env->SetFilesystemActive(true);
-  ASSERT_EQ(listener->WaitForRecovery(5000000), true);
-
-  s = Put(Key(1), "val");
-  ASSERT_EQ(s, Status::OK());
-
-  Reopen(options);
-  ASSERT_EQ("val", Get(Key(0)));
-  ASSERT_EQ("val", Get(Key(1)));
-  Destroy(options);
-}
-
-TEST_F(DBErrorHandlingTest, FailRecoverFlushError) {
-  std::unique_ptr<FaultInjectionTestEnv> fault_env(
-      new FaultInjectionTestEnv(Env::Default()));
-  std::shared_ptr<ErrorHandlerListener> listener(new ErrorHandlerListener());
-  Options options = GetDefaultOptions();
-  options.create_if_missing = true;
-  options.env = fault_env.get();
-  options.listeners.emplace_back(listener);
-  Status s;
-
-  listener->EnableAutoRecovery();
-  DestroyAndReopen(options);
-
-  Put(Key(0), "val");
-  SyncPoint::GetInstance()->SetCallBack("FlushJob::Start", [&](void*) {
-    fault_env->SetFilesystemActive(false, Status::NoSpace("Out of space"));
-  });
-  SyncPoint::GetInstance()->EnableProcessing();
-  s = Flush();
-  ASSERT_EQ(s.severity(), rocksdb::Status::Severity::kHardError);
-  // We should be able to shutdown the database while auto recovery is going
-  // on in the background
-  Close();
-  DestroyDB(dbname_, options);
-}
-
-TEST_F(DBErrorHandlingTest, WALWriteError) {
-  std::unique_ptr<FaultInjectionTestEnv> fault_env(
-      new FaultInjectionTestEnv(Env::Default()));
-  std::shared_ptr<ErrorHandlerListener> listener(new ErrorHandlerListener());
-  Options options = GetDefaultOptions();
-  options.create_if_missing = true;
-  options.writable_file_max_buffer_size = 32768;
-  options.env = fault_env.get();
-  options.listeners.emplace_back(listener);
-  Status s;
-  Random rnd(301);
-
-  listener->EnableAutoRecovery();
-  DestroyAndReopen(options);
-
-  {
-    WriteBatch batch;
-
-    for (auto i = 0; i<100; ++i) {
-      batch.Put(Key(i), RandomString(&rnd, 1024));
-    }
-
-    WriteOptions wopts;
-    wopts.sync = true;
-    ASSERT_EQ(dbfull()->Write(wopts, &batch), Status::OK());
-  };
-
-  {
-    WriteBatch batch;
-    int write_error = 0;
-
-    for (auto i = 100; i<199; ++i) {
-      batch.Put(Key(i), RandomString(&rnd, 1024));
-    }
-
-    SyncPoint::GetInstance()->SetCallBack("WritableFileWriter::Append:BeforePrepareWrite", [&](void*) {
-      write_error++;
-      if (write_error > 2) {
-        fault_env->SetFilesystemActive(false, Status::NoSpace("Out of space"));
-      }
-    });
-    SyncPoint::GetInstance()->EnableProcessing();
-    WriteOptions wopts;
-    wopts.sync = true;
-    s = dbfull()->Write(wopts, &batch);
-    ASSERT_EQ(s, s.NoSpace());
-  }
-  SyncPoint::GetInstance()->DisableProcessing();
-  fault_env->SetFilesystemActive(true);
-  ASSERT_EQ(listener->WaitForRecovery(5000000), true);
-  for (auto i=0; i<199; ++i) {
-    if (i < 100) {
-      ASSERT_NE(Get(Key(i)), "NOT_FOUND");
-    } else {
-      ASSERT_EQ(Get(Key(i)), "NOT_FOUND");
-    }
-  }
-  Reopen(options);
-  for (auto i=0; i<199; ++i) {
-    if (i < 100) {
-      ASSERT_NE(Get(Key(i)), "NOT_FOUND");
-    } else {
-      ASSERT_EQ(Get(Key(i)), "NOT_FOUND");
-    }
-  }
-  Close();
-}
-
-TEST_F(DBErrorHandlingTest, MultiCFWALWriteError) {
-  std::unique_ptr<FaultInjectionTestEnv> fault_env(
-      new FaultInjectionTestEnv(Env::Default()));
-  std::shared_ptr<ErrorHandlerListener> listener(new ErrorHandlerListener());
-  Options options = GetDefaultOptions();
-  options.create_if_missing = true;
-  options.writable_file_max_buffer_size = 32768;
-  options.env = fault_env.get();
-  options.listeners.emplace_back(listener);
-  Status s;
-  Random rnd(301);
-
-  listener->EnableAutoRecovery();
-  CreateAndReopenWithCF({"one", "two", "three"}, options);
-
-  {
-    WriteBatch batch;
-
-    for (auto i = 1; i < 4; ++i) {
-      for (auto j = 0; j < 100; ++j) {
-        batch.Put(handles_[i], Key(j), RandomString(&rnd, 1024));
-      }
-    }
-
-    WriteOptions wopts;
-    wopts.sync = true;
-    ASSERT_EQ(dbfull()->Write(wopts, &batch), Status::OK());
-  };
-
-  {
-    WriteBatch batch;
-    int write_error = 0;
-
-    // Write to one CF
-    for (auto i = 100; i < 199; ++i) {
-      batch.Put(handles_[2], Key(i), RandomString(&rnd, 1024));
-    }
-
-    SyncPoint::GetInstance()->SetCallBack(
-        "WritableFileWriter::Append:BeforePrepareWrite", [&](void*) {
-          write_error++;
-          if (write_error > 2) {
-            fault_env->SetFilesystemActive(false,
-                                           Status::NoSpace("Out of space"));
-          }
-        });
-    SyncPoint::GetInstance()->EnableProcessing();
-    WriteOptions wopts;
-    wopts.sync = true;
-    s = dbfull()->Write(wopts, &batch);
-    ASSERT_EQ(s, s.NoSpace());
-  }
-  SyncPoint::GetInstance()->DisableProcessing();
-  fault_env->SetFilesystemActive(true);
-  ASSERT_EQ(listener->WaitForRecovery(5000000), true);
-
-  for (auto i = 1; i < 4; ++i) {
-    // Every CF should have been flushed
-    ASSERT_EQ(NumTableFilesAtLevel(0, i), 1);
-  }
-
-  for (auto i = 1; i < 4; ++i) {
-    for (auto j = 0; j < 199; ++j) {
-      if (j < 100) {
-        ASSERT_NE(Get(i, Key(j)), "NOT_FOUND");
-      } else {
-        ASSERT_EQ(Get(i, Key(j)), "NOT_FOUND");
-      }
-    }
-  }
-  ReopenWithColumnFamilies({"default", "one", "two", "three"}, options);
-  for (auto i = 1; i < 4; ++i) {
-    for (auto j = 0; j < 199; ++j) {
-      if (j < 100) {
-        ASSERT_NE(Get(i, Key(j)), "NOT_FOUND");
-      } else {
-        ASSERT_EQ(Get(i, Key(j)), "NOT_FOUND");
-      }
-    }
-  }
-  Close();
-}
-
-TEST_F(DBErrorHandlingTest, MultiDBCompactionError) {
-  FaultInjectionTestEnv* def_env = new FaultInjectionTestEnv(Env::Default());
-  std::vector<std::unique_ptr<FaultInjectionTestEnv>> fault_env;
-  std::vector<Options> options;
-  std::vector<std::shared_ptr<ErrorHandlerListener>> listener;
-  std::vector<DB*> db;
-  std::shared_ptr<SstFileManager> sfm(NewSstFileManager(def_env));
-  int kNumDbInstances = 3;
-  Random rnd(301);
-
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    listener.emplace_back(new ErrorHandlerListener());
-    options.emplace_back(GetDefaultOptions());
-    fault_env.emplace_back(new FaultInjectionTestEnv(Env::Default()));
-    options[i].create_if_missing = true;
-    options[i].level0_file_num_compaction_trigger = 2;
-    options[i].writable_file_max_buffer_size = 32768;
-    options[i].env = fault_env[i].get();
-    options[i].listeners.emplace_back(listener[i]);
-    options[i].sst_file_manager = sfm;
-    DB* dbptr;
-    char buf[16];
-
-    listener[i]->EnableAutoRecovery();
-    // Setup for returning error for the 3rd SST, which would be level 1
-    listener[i]->InjectFileCreationError(fault_env[i].get(), 3,
-                                         Status::NoSpace("Out of space"));
-    snprintf(buf, sizeof(buf), "_%d", i);
-    DestroyDB(dbname_ + std::string(buf), options[i]);
-    ASSERT_EQ(DB::Open(options[i], dbname_ + std::string(buf), &dbptr),
-              Status::OK());
-    db.emplace_back(dbptr);
-  }
-
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    WriteBatch batch;
-
-    for (auto j = 0; j <= 100; ++j) {
-      batch.Put(Key(j), RandomString(&rnd, 1024));
-    }
-
-    WriteOptions wopts;
-    wopts.sync = true;
-    ASSERT_EQ(db[i]->Write(wopts, &batch), Status::OK());
-    ASSERT_EQ(db[i]->Flush(FlushOptions()), Status::OK());
-  }
-
-  def_env->SetFilesystemActive(false, Status::NoSpace("Out of space"));
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    WriteBatch batch;
-
-    // Write to one CF
-    for (auto j = 100; j < 199; ++j) {
-      batch.Put(Key(j), RandomString(&rnd, 1024));
-    }
-
-    WriteOptions wopts;
-    wopts.sync = true;
-    ASSERT_EQ(db[i]->Write(wopts, &batch), Status::OK());
-    ASSERT_EQ(db[i]->Flush(FlushOptions()), Status::OK());
-  }
-
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    Status s = static_cast<DBImpl*>(db[i])->TEST_WaitForCompact(true);
-    ASSERT_EQ(s.severity(), Status::Severity::kSoftError);
-    fault_env[i]->SetFilesystemActive(true);
-  }
-
-  def_env->SetFilesystemActive(true);
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    std::string prop;
-    ASSERT_EQ(listener[i]->WaitForRecovery(5000000), true);
-    EXPECT_TRUE(db[i]->GetProperty(
-        "rocksdb.num-files-at-level" + NumberToString(0), &prop));
-    EXPECT_EQ(atoi(prop.c_str()), 0);
-    EXPECT_TRUE(db[i]->GetProperty(
-        "rocksdb.num-files-at-level" + NumberToString(1), &prop));
-    EXPECT_EQ(atoi(prop.c_str()), 1);
-  }
-
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    char buf[16];
-    snprintf(buf, sizeof(buf), "_%d", i);
-    delete db[i];
-    fault_env[i]->SetFilesystemActive(true);
-    if (getenv("KEEP_DB")) {
-      printf("DB is still at %s%s\n", dbname_.c_str(), buf);
-    } else {
-      Status s = DestroyDB(dbname_ + std::string(buf), options[i]);
-    }
-  }
-  options.clear();
-  sfm.reset();
-  delete def_env;
-}
-
-TEST_F(DBErrorHandlingTest, MultiDBVariousErrors) {
-  FaultInjectionTestEnv* def_env = new FaultInjectionTestEnv(Env::Default());
-  std::vector<std::unique_ptr<FaultInjectionTestEnv>> fault_env;
-  std::vector<Options> options;
-  std::vector<std::shared_ptr<ErrorHandlerListener>> listener;
-  std::vector<DB*> db;
-  std::shared_ptr<SstFileManager> sfm(NewSstFileManager(def_env));
-  int kNumDbInstances = 3;
-  Random rnd(301);
-
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    listener.emplace_back(new ErrorHandlerListener());
-    options.emplace_back(GetDefaultOptions());
-    fault_env.emplace_back(new FaultInjectionTestEnv(Env::Default()));
-    options[i].create_if_missing = true;
-    options[i].level0_file_num_compaction_trigger = 2;
-    options[i].writable_file_max_buffer_size = 32768;
-    options[i].env = fault_env[i].get();
-    options[i].listeners.emplace_back(listener[i]);
-    options[i].sst_file_manager = sfm;
-    DB* dbptr;
-    char buf[16];
-
-    listener[i]->EnableAutoRecovery();
-    switch (i) {
-      case 0:
-        // Setup for returning error for the 3rd SST, which would be level 1
-        listener[i]->InjectFileCreationError(fault_env[i].get(), 3,
-                                             Status::NoSpace("Out of space"));
-        break;
-      case 1:
-        // Setup for returning error after the 1st SST, which would result
-        // in a hard error
-        listener[i]->InjectFileCreationError(fault_env[i].get(), 2,
-                                             Status::NoSpace("Out of space"));
-        break;
-      default:
-        break;
-    }
-    snprintf(buf, sizeof(buf), "_%d", i);
-    DestroyDB(dbname_ + std::string(buf), options[i]);
-    ASSERT_EQ(DB::Open(options[i], dbname_ + std::string(buf), &dbptr),
-              Status::OK());
-    db.emplace_back(dbptr);
-  }
-
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    WriteBatch batch;
-
-    for (auto j = 0; j <= 100; ++j) {
-      batch.Put(Key(j), RandomString(&rnd, 1024));
-    }
-
-    WriteOptions wopts;
-    wopts.sync = true;
-    ASSERT_EQ(db[i]->Write(wopts, &batch), Status::OK());
-    ASSERT_EQ(db[i]->Flush(FlushOptions()), Status::OK());
-  }
-
-  def_env->SetFilesystemActive(false, Status::NoSpace("Out of space"));
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    WriteBatch batch;
-
-    // Write to one CF
-    for (auto j = 100; j < 199; ++j) {
-      batch.Put(Key(j), RandomString(&rnd, 1024));
-    }
-
-    WriteOptions wopts;
-    wopts.sync = true;
-    ASSERT_EQ(db[i]->Write(wopts, &batch), Status::OK());
-    if (i != 1) {
-      ASSERT_EQ(db[i]->Flush(FlushOptions()), Status::OK());
-    } else {
-      ASSERT_EQ(db[i]->Flush(FlushOptions()), Status::NoSpace());
-    }
-  }
-
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    Status s = static_cast<DBImpl*>(db[i])->TEST_WaitForCompact(true);
-    switch (i) {
-      case 0:
-        ASSERT_EQ(s.severity(), Status::Severity::kSoftError);
-        break;
-      case 1:
-        ASSERT_EQ(s.severity(), Status::Severity::kHardError);
-        break;
-      case 2:
-        ASSERT_EQ(s, Status::OK());
-        break;
-    }
-    fault_env[i]->SetFilesystemActive(true);
-  }
-
-  def_env->SetFilesystemActive(true);
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    std::string prop;
-    if (i < 2) {
-      ASSERT_EQ(listener[i]->WaitForRecovery(5000000), true);
-    }
-    if (i == 1) {
-      ASSERT_EQ(static_cast<DBImpl*>(db[i])->TEST_WaitForCompact(true),
-                Status::OK());
-    }
-    EXPECT_TRUE(db[i]->GetProperty(
-        "rocksdb.num-files-at-level" + NumberToString(0), &prop));
-    EXPECT_EQ(atoi(prop.c_str()), 0);
-    EXPECT_TRUE(db[i]->GetProperty(
-        "rocksdb.num-files-at-level" + NumberToString(1), &prop));
-    EXPECT_EQ(atoi(prop.c_str()), 1);
-  }
-
-  for (auto i = 0; i < kNumDbInstances; ++i) {
-    char buf[16];
-    snprintf(buf, sizeof(buf), "_%d", i);
-    fault_env[i]->SetFilesystemActive(true);
-    delete db[i];
-    if (getenv("KEEP_DB")) {
-      printf("DB is still at %s%s\n", dbname_.c_str(), buf);
-    } else {
-      DestroyDB(dbname_ + std::string(buf), options[i]);
-    }
-  }
-  options.clear();
-  delete def_env;
-}
-
 }  // namespace rocksdb
 
 int main(int argc, char** argv) {
@@ -679,13 +136,3 @@ int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();
 }
-
-#else
-#include <stdio.h>
-
-int main(int /*argc*/, char** /*argv*/) {
-  fprintf(stderr, "SKIPPED as Cuckoo table is not supported in ROCKSDB_LITE\n");
-  return 0;
-}
-
-#endif  // ROCKSDB_LITE

db/event_helpers.cc

@@ -40,8 +40,8 @@ void EventHelpers::NotifyTableFileCreationStarted(
 
 void EventHelpers::NotifyOnBackgroundError(
     const std::vector<std::shared_ptr<EventListener>>& listeners,
-    BackgroundErrorReason reason, Status* bg_error, InstrumentedMutex* db_mutex,
-    bool* auto_recovery) {
+    BackgroundErrorReason reason, Status* bg_error,
+    InstrumentedMutex* db_mutex) {
 #ifndef ROCKSDB_LITE
   if (listeners.size() == 0U) {
     return;
@@ -51,9 +51,6 @@ void EventHelpers::NotifyOnBackgroundError(
   db_mutex->Unlock();
   for (auto& listener : listeners) {
     listener->OnBackgroundError(reason, bg_error);
-    if (*auto_recovery) {
-      listener->OnErrorRecoveryBegin(reason, *bg_error, auto_recovery);
-    }
   }
   db_mutex->Lock();
 #else
@@ -61,7 +58,6 @@ void EventHelpers::NotifyOnBackgroundError(
   (void)reason;
   (void)bg_error;
   (void)db_mutex;
-  (void)auto_recovery;
 #endif  // ROCKSDB_LITE
 }
 
@@ -171,25 +167,4 @@ void EventHelpers::LogAndNotifyTableFileDeletion(
 #endif  // !ROCKSDB_LITE
 }
 
-void EventHelpers::NotifyOnErrorRecoveryCompleted(
-    const std::vector<std::shared_ptr<EventListener>>& listeners,
-    Status old_bg_error, InstrumentedMutex* db_mutex) {
-#ifndef ROCKSDB_LITE
-  if (listeners.size() == 0U) {
-    return;
-  }
-  db_mutex->AssertHeld();
-  // release lock while notifying events
-  db_mutex->Unlock();
-  for (auto& listener : listeners) {
-    listener->OnErrorRecoveryCompleted(old_bg_error);
-  }
-  db_mutex->Lock();
-#else
-  (void)listeners;
-  (void)old_bg_error;
-  (void)db_mutex;
-#endif  // ROCKSDB_LITE
-}
-
 }  // namespace rocksdb

db/event_helpers.h

@@ -28,7 +28,7 @@ class EventHelpers {
   static void NotifyOnBackgroundError(
       const std::vector<std::shared_ptr<EventListener>>& listeners,
       BackgroundErrorReason reason, Status* bg_error,
-      InstrumentedMutex* db_mutex, bool* auto_recovery);
+      InstrumentedMutex* db_mutex);
   static void LogAndNotifyTableFileCreationFinished(
       EventLogger* event_logger,
       const std::vector<std::shared_ptr<EventListener>>& listeners,
@@ -41,9 +41,6 @@ class EventHelpers {
       uint64_t file_number, const std::string& file_path,
       const Status& status, const std::string& db_name,
       const std::vector<std::shared_ptr<EventListener>>& listeners);
-  static void NotifyOnErrorRecoveryCompleted(
-      const std::vector<std::shared_ptr<EventListener>>& listeners,
-      Status bg_error, InstrumentedMutex* db_mutex);
 
  private:
   static void LogAndNotifyTableFileCreation(

db/external_sst_file_ingestion_job.cc

@@ -344,7 +344,7 @@ Status ExternalSstFileIngestionJob::GetIngestedFileInfo(
       file_to_ingest->global_seqno_offset = 0;
       return Status::Corruption("Was not able to find file global seqno field");
     }
-    file_to_ingest->global_seqno_offset = static_cast<size_t>(offsets_iter->second);
+    file_to_ingest->global_seqno_offset = offsets_iter->second;
   } else if (file_to_ingest->version == 1) {
     // SST file V1 should not have global seqno field
     assert(seqno_iter == uprops.end());

db/flush_job.cc

@@ -78,8 +78,6 @@ const char* GetFlushReasonString (FlushReason flush_reason) {
       return "Auto Compaction";
     case FlushReason::kManualFlush:
       return "Manual Flush";
-    case FlushReason::kErrorRecovery:
-      return "Error Recovery";
     default:
       return "Invalid";
   }
@@ -373,8 +371,8 @@ Status FlushJob::WriteLevel0Table() {
                    s.ToString().c_str(),
                    meta_.marked_for_compaction ? " (needs compaction)" : "");
 
-    if (s.ok() && output_file_directory_ != nullptr) {
-      s = output_file_directory_->Fsync();
+    if (output_file_directory_ != nullptr) {
+      output_file_directory_->Fsync();
     }
     TEST_SYNC_POINT("FlushJob::WriteLevel0Table");
     db_mutex_->Lock();

db/flush_job_test.cc

@@ -62,7 +62,7 @@ class FlushJobTest : public testing::Test {
         manifest, &file, env_->OptimizeForManifestWrite(env_options_));
     ASSERT_OK(s);
     unique_ptr<WritableFileWriter> file_writer(
-        new WritableFileWriter(std::move(file), manifest, EnvOptions()));
+        new WritableFileWriter(std::move(file), EnvOptions()));
     {
       log::Writer log(std::move(file_writer), 0, false);
       std::string record;

db/forward_iterator.cc

@@ -916,13 +916,21 @@ bool ForwardIterator::TEST_CheckDeletedIters(int* pdeleted_iters,
 uint32_t ForwardIterator::FindFileInRange(
     const std::vector<FileMetaData*>& files, const Slice& internal_key,
     uint32_t left, uint32_t right) {
-  auto cmp = [&](const FileMetaData* f, const Slice& key) -> bool {
-    return cfd_->internal_comparator().InternalKeyComparator::Compare(
-            f->largest.Encode(), key) < 0;
-  };
-  const auto &b = files.begin();
-  return static_cast<uint32_t>(std::lower_bound(b + left,
-                                 b + right, internal_key, cmp) - b);
+  while (left < right) {
+    uint32_t mid = (left + right) / 2;
+    const FileMetaData* f = files[mid];
+    if (cfd_->internal_comparator().InternalKeyComparator::Compare(
+          f->largest.Encode(), internal_key) < 0) {
+      // Key at "mid.largest" is < "target".  Therefore all
+      // files at or before "mid" are uninteresting.
+      left = mid + 1;
+    } else {
+      // Key at "mid.largest" is >= "target".  Therefore all files
+      // after "mid" are uninteresting.
+      right = mid;
+    }
+  }
+  return right;
 }
 
 void ForwardIterator::DeleteIterator(InternalIterator* iter, bool is_arena) {

db/listener_test.cc

@@ -882,13 +882,10 @@ TEST_F(EventListenerTest, BackgroundErrorListenerFailedCompactionTest) {
   ASSERT_EQ(1, listener->counter());
 
   // trigger flush so compaction is triggered again; this time it succeeds
-  // The previous failed compaction may get retried automatically, so we may
-  // be left with 0 or 1 files in level 1, depending on when the retry gets
-  // scheduled
   ASSERT_OK(Put("key0", "val"));
   ASSERT_OK(dbfull()->TEST_WaitForFlushMemTable());
   ASSERT_OK(dbfull()->TEST_WaitForCompact());
-  ASSERT_LE(1, NumTableFilesAtLevel(0));
+  ASSERT_EQ(0, NumTableFilesAtLevel(0));
 }
 
 }  // namespace rocksdb

db/log_format.h

@@ -37,9 +37,9 @@ static const unsigned int kBlockSize = 32768;
 // Header is checksum (4 bytes), length (2 bytes), type (1 byte)
 static const int kHeaderSize = 4 + 2 + 1;
 
-// Recyclable header is checksum (4 bytes), length (2 bytes), type (1 byte),
-// log number (4 bytes).
-static const int kRecyclableHeaderSize = 4 + 2 + 1 + 4;
+// Recyclable header is checksum (4 bytes), type (1 byte), log number
+// (4 bytes), length (2 bytes).
+static const int kRecyclableHeaderSize = 4 + 1 + 4 + 2;
 
 }  // namespace log
 }  // namespace rocksdb

db/log_reader.cc

@@ -24,7 +24,7 @@ Reader::Reporter::~Reporter() {
 
 Reader::Reader(std::shared_ptr<Logger> info_log,
                unique_ptr<SequentialFileReader>&& _file, Reporter* reporter,
-               bool checksum, uint64_t log_num)
+               bool checksum, uint64_t initial_offset, uint64_t log_num)
     : info_log_(info_log),
       file_(std::move(_file)),
       reporter_(reporter),
@@ -36,6 +36,7 @@ Reader::Reader(std::shared_ptr<Logger> info_log,
       eof_offset_(0),
       last_record_offset_(0),
       end_of_buffer_offset_(0),
+      initial_offset_(initial_offset),
       log_number_(log_num),
       recycled_(false) {}
 
@@ -43,6 +44,29 @@ Reader::~Reader() {
   delete[] backing_store_;
 }
 
+bool Reader::SkipToInitialBlock() {
+  size_t initial_offset_in_block = initial_offset_ % kBlockSize;
+  uint64_t block_start_location = initial_offset_ - initial_offset_in_block;
+
+  // Don't search a block if we'd be in the trailer
+  if (initial_offset_in_block > kBlockSize - 6) {
+    block_start_location += kBlockSize;
+  }
+
+  end_of_buffer_offset_ = block_start_location;
+
+  // Skip to start of first block that can contain the initial record
+  if (block_start_location > 0) {
+    Status skip_status = file_->Skip(block_start_location);
+    if (!skip_status.ok()) {
+      ReportDrop(static_cast<size_t>(block_start_location), skip_status);
+      return false;
+    }
+  }
+
+  return true;
+}
+
 // For kAbsoluteConsistency, on clean shutdown we don't expect any error
 // in the log files.  For other modes, we can ignore only incomplete records
 // in the last log file, which are presumably due to a write in progress
@@ -52,6 +76,12 @@ Reader::~Reader() {
 // restrict the inconsistency to only the last log
 bool Reader::ReadRecord(Slice* record, std::string* scratch,
                         WALRecoveryMode wal_recovery_mode) {
+  if (last_record_offset_ < initial_offset_) {
+    if (!SkipToInitialBlock()) {
+      return false;
+    }
+  }
+
   scratch->clear();
   record->clear();
   bool in_fragmented_record = false;
@@ -269,7 +299,8 @@ void Reader::ReportCorruption(size_t bytes, const char* reason) {
 }
 
 void Reader::ReportDrop(size_t bytes, const Status& reason) {
-  if (reporter_ != nullptr) {
+  if (reporter_ != nullptr &&
+      end_of_buffer_offset_ - buffer_.size() - bytes >= initial_offset_) {
     reporter_->Corruption(bytes, reason);
   }
 }
@@ -286,7 +317,7 @@ bool Reader::ReadMore(size_t* drop_size, int *error) {
       read_error_ = true;
       *error = kEof;
       return false;
-    } else if (buffer_.size() < static_cast<size_t>(kBlockSize)) {
+    } else if (buffer_.size() < (size_t)kBlockSize) {
       eof_ = true;
       eof_offset_ = buffer_.size();
     }
@@ -311,7 +342,7 @@ bool Reader::ReadMore(size_t* drop_size, int *error) {
 unsigned int Reader::ReadPhysicalRecord(Slice* result, size_t* drop_size) {
   while (true) {
     // We need at least the minimum header size
-    if (buffer_.size() < static_cast<size_t>(kHeaderSize)) {
+    if (buffer_.size() < (size_t)kHeaderSize) {
       int r;
       if (!ReadMore(drop_size, &r)) {
         return r;
@@ -332,7 +363,7 @@ unsigned int Reader::ReadPhysicalRecord(Slice* result, size_t* drop_size) {
       }
       header_size = kRecyclableHeaderSize;
       // We need enough for the larger header
-      if (buffer_.size() < static_cast<size_t>(kRecyclableHeaderSize)) {
+      if (buffer_.size() < (size_t)kRecyclableHeaderSize) {
         int r;
         if (!ReadMore(drop_size, &r)) {
           return r;
@@ -386,6 +417,13 @@ unsigned int Reader::ReadPhysicalRecord(Slice* result, size_t* drop_size) {
 
     buffer_.remove_prefix(header_size + length);
 
+    // Skip physical record that started before initial_offset_
+    if (end_of_buffer_offset_ - buffer_.size() - header_size - length <
+        initial_offset_) {
+      result->clear();
+      return kBadRecord;
+    }
+
     *result = Slice(header + header_size, length);
     return type;
   }

db/log_reader.h

@@ -50,10 +50,14 @@ class Reader {
   // live while this Reader is in use.
   //
   // If "checksum" is true, verify checksums if available.
+  //
+  // The Reader will start reading at the first record located at physical
+  // position >= initial_offset within the file.
   Reader(std::shared_ptr<Logger> info_log,
-         // @lint-ignore TXT2 T25377293 Grandfathered in
-         unique_ptr<SequentialFileReader>&& file, Reporter* reporter,
-         bool checksum, uint64_t log_num);
+  // @lint-ignore TXT2 T25377293 Grandfathered in
+	 unique_ptr<SequentialFileReader>&& file,
+         Reporter* reporter, bool checksum, uint64_t initial_offset,
+         uint64_t log_num);
 
   ~Reader();
 
@@ -104,6 +108,9 @@ class Reader {
   // Offset of the first location past the end of buffer_.
   uint64_t end_of_buffer_offset_;
 
+  // Offset at which to start looking for the first record to return
+  uint64_t const initial_offset_;
+
   // which log number this is
   uint64_t const log_number_;
 
@@ -117,6 +124,7 @@ class Reader {
     // Currently there are three situations in which this happens:
     // * The record has an invalid CRC (ReadPhysicalRecord reports a drop)
     // * The record is a 0-length record (No drop is reported)
+    // * The record is below constructor's initial_offset (No drop is reported)
     kBadRecord = kMaxRecordType + 2,
     // Returned when we fail to read a valid header.
     kBadHeader = kMaxRecordType + 3,
@@ -128,6 +136,11 @@ class Reader {
     kBadRecordChecksum = kMaxRecordType + 6,
   };
 
+  // Skips all blocks that are completely before "initial_offset_".
+  //
+  // Returns true on success. Handles reporting.
+  bool SkipToInitialBlock();
+
   // Return type, or one of the preceding special values
   unsigned int ReadPhysicalRecord(Slice* result, size_t* drop_size);
 

db/log_test.cc

@@ -159,12 +159,12 @@ class LogTest : public ::testing::TestWithParam<int> {
   LogTest()
       : reader_contents_(),
         dest_holder_(test::GetWritableFileWriter(
-            new test::StringSink(&reader_contents_), "" /* don't care */)),
+            new test::StringSink(&reader_contents_))),
         source_holder_(test::GetSequentialFileReader(
             new StringSource(reader_contents_), "" /* file name */)),
         writer_(std::move(dest_holder_), 123, GetParam()),
-        reader_(nullptr, std::move(source_holder_), &report_,
-                true /* checksum */, 123 /* log_number */) {
+        reader_(nullptr, std::move(source_holder_), &report_, true /*checksum*/,
+                0 /*initial_offset*/, 123) {
     int header_size = GetParam() ? kRecyclableHeaderSize : kHeaderSize;
     initial_offset_last_record_offsets_[0] = 0;
     initial_offset_last_record_offsets_[1] = header_size + 10000;
@@ -266,6 +266,36 @@ class LogTest : public ::testing::TestWithParam<int> {
     }
   }
 
+  void CheckOffsetPastEndReturnsNoRecords(uint64_t offset_past_end) {
+    WriteInitialOffsetLog();
+    unique_ptr<SequentialFileReader> file_reader(test::GetSequentialFileReader(
+        new StringSource(reader_contents_), "" /* fname */));
+    unique_ptr<Reader> offset_reader(
+        new Reader(nullptr, std::move(file_reader), &report_,
+                   true /*checksum*/, WrittenBytes() + offset_past_end, 123));
+    Slice record;
+    std::string scratch;
+    ASSERT_TRUE(!offset_reader->ReadRecord(&record, &scratch));
+  }
+
+  void CheckInitialOffsetRecord(uint64_t initial_offset,
+                                int expected_record_offset) {
+    WriteInitialOffsetLog();
+    unique_ptr<SequentialFileReader> file_reader(test::GetSequentialFileReader(
+        new StringSource(reader_contents_), "" /* fname */));
+    unique_ptr<Reader> offset_reader(
+        new Reader(nullptr, std::move(file_reader), &report_,
+                   true /*checksum*/, initial_offset, 123));
+    Slice record;
+    std::string scratch;
+    ASSERT_TRUE(offset_reader->ReadRecord(&record, &scratch));
+    ASSERT_EQ(initial_offset_record_sizes_[expected_record_offset],
+              record.size());
+    ASSERT_EQ(initial_offset_last_record_offsets_[expected_record_offset],
+              offset_reader->LastRecordOffset());
+    ASSERT_EQ((char)('a' + expected_record_offset), record.data()[0]);
+  }
+
 };
 
 size_t LogTest::initial_offset_record_sizes_[] =
@@ -560,6 +590,55 @@ TEST_P(LogTest, ErrorJoinsRecords) {
   }
 }
 
+TEST_P(LogTest, ReadStart) { CheckInitialOffsetRecord(0, 0); }
+
+TEST_P(LogTest, ReadSecondOneOff) { CheckInitialOffsetRecord(1, 1); }
+
+TEST_P(LogTest, ReadSecondTenThousand) { CheckInitialOffsetRecord(10000, 1); }
+
+TEST_P(LogTest, ReadSecondStart) {
+  int header_size = GetParam() ? kRecyclableHeaderSize : kHeaderSize;
+  CheckInitialOffsetRecord(10000 + header_size, 1);
+}
+
+TEST_P(LogTest, ReadThirdOneOff) {
+  int header_size = GetParam() ? kRecyclableHeaderSize : kHeaderSize;
+  CheckInitialOffsetRecord(10000 + header_size + 1, 2);
+}
+
+TEST_P(LogTest, ReadThirdStart) {
+  int header_size = GetParam() ? kRecyclableHeaderSize : kHeaderSize;
+  CheckInitialOffsetRecord(20000 + 2 * header_size, 2);
+}
+
+TEST_P(LogTest, ReadFourthOneOff) {
+  int header_size = GetParam() ? kRecyclableHeaderSize : kHeaderSize;
+  CheckInitialOffsetRecord(20000 + 2 * header_size + 1, 3);
+}
+
+TEST_P(LogTest, ReadFourthFirstBlockTrailer) {
+  CheckInitialOffsetRecord(log::kBlockSize - 4, 3);
+}
+
+TEST_P(LogTest, ReadFourthMiddleBlock) {
+  CheckInitialOffsetRecord(log::kBlockSize + 1, 3);
+}
+
+TEST_P(LogTest, ReadFourthLastBlock) {
+  CheckInitialOffsetRecord(2 * log::kBlockSize + 1, 3);
+}
+
+TEST_P(LogTest, ReadFourthStart) {
+  int header_size = GetParam() ? kRecyclableHeaderSize : kHeaderSize;
+  CheckInitialOffsetRecord(
+      2 * (header_size + 1000) + (2 * log::kBlockSize - 1000) + 3 * header_size,
+      3);
+}
+
+TEST_P(LogTest, ReadEnd) { CheckOffsetPastEndReturnsNoRecords(0); }
+
+TEST_P(LogTest, ReadPastEnd) { CheckOffsetPastEndReturnsNoRecords(5); }
+
 TEST_P(LogTest, ClearEofSingleBlock) {
   Write("foo");
   Write("bar");
@@ -639,8 +718,7 @@ TEST_P(LogTest, Recycle) {
     Write("xxxxxxxxxxxxxxxx");
   }
   unique_ptr<WritableFileWriter> dest_holder(test::GetWritableFileWriter(
-      new test::OverwritingStringSink(get_reader_contents()),
-      "" /* don't care */));
+      new test::OverwritingStringSink(get_reader_contents())));
   Writer recycle_writer(std::move(dest_holder), 123, true);
   recycle_writer.AddRecord(Slice("foooo"));
   recycle_writer.AddRecord(Slice("bar"));

db/malloc_stats.cc

@@ -18,11 +18,9 @@ namespace rocksdb {
 #ifdef ROCKSDB_JEMALLOC
 #ifdef __FreeBSD__
 #include <malloc_np.h>
+#define je_malloc_stats_print malloc_stats_print
 #else
 #include "jemalloc/jemalloc.h"
-#ifdef JEMALLOC_NO_RENAME
-#define malloc_stats_print je_malloc_stats_print
-#endif
 #endif
 
 typedef struct {
@@ -50,7 +48,7 @@ void DumpMallocStats(std::string* stats) {
   std::unique_ptr<char[]> buf{new char[kMallocStatusLen + 1]};
   mstat.cur = buf.get();
   mstat.end = buf.get() + kMallocStatusLen;
-  malloc_stats_print(GetJemallocStatus, &mstat, "");
+  je_malloc_stats_print(GetJemallocStatus, &mstat, "");
   stats->append(buf.get());
 }
 #else

db/memtable.h

@@ -336,14 +336,6 @@ class MemTable {
     mem_tracker_.DoneAllocating();
   }
 
-  // Notify the underlying storage that all data it contained has been
-  // persisted.
-  // REQUIRES: external synchronization to prevent simultaneous
-  // operations on the same MemTable.
-  void MarkFlushed() {
-    table_->MarkFlushed();
-  }
-
   // return true if the current MemTableRep supports merge operator.
   bool IsMergeOperatorSupported() const {
     return table_->IsMergeOperatorSupported();

db/memtable_list.cc

@@ -248,7 +248,6 @@ void MemTableListVersion::Remove(MemTable* m,
   assert(refs_ == 1);  // only when refs_ == 1 is MemTableListVersion mutable
   memlist_.remove(m);
 
-  m->MarkFlushed();
   if (max_write_buffer_number_to_maintain_ > 0) {
     memlist_history_.push_front(m);
     TrimHistory(to_delete);

db/merge_helper.h

@@ -3,7 +3,8 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 //
-#pragma once
+#ifndef MERGE_HELPER_H
+#define MERGE_HELPER_H
 
 #include <deque>
 #include <string>
@@ -192,3 +193,5 @@ class MergeOutputIterator {
 };
 
 } // namespace rocksdb
+
+#endif

db/perf_context_test.cc

@@ -469,7 +469,7 @@ void ProfileQueries(bool enabled_time = false) {
     ASSERT_GT(hist_num_memtable_checked.Average(), 0);
     // In read-only mode Get(), no super version operation is needed
     ASSERT_EQ(hist_get_post_process.Average(), 0);
-    ASSERT_GT(hist_get_snapshot.Average(), 0);
+    ASSERT_EQ(hist_get_snapshot.Average(), 0);
 
     ASSERT_GT(hist_mget.Average(), 0);
     ASSERT_GT(hist_mget_snapshot.Average(), 0);

db/range_del_aggregator.cc

@@ -76,9 +76,7 @@ class UncollapsedRangeDelMap : public RangeDelMap {
     return false;
   }
 
-  void AddTombstone(RangeTombstone tombstone) override {
-    rep_.emplace(tombstone);
-  }
+  void AddTombstone(RangeTombstone tombstone) override { rep_.emplace(tombstone); }
 
   size_t Size() const override { return rep_.size(); }
 
@@ -173,9 +171,7 @@ class CollapsedRangeDelMap : public RangeDelMap {
   const Comparator* ucmp_;
 
  public:
-  explicit CollapsedRangeDelMap(const Comparator* ucmp) 
-    : rep_(stl_wrappers::LessOfComparator(ucmp)), 
-      ucmp_(ucmp) {
+  CollapsedRangeDelMap(const Comparator* ucmp) : ucmp_(ucmp) {
     InvalidatePosition();
   }
 
@@ -269,36 +265,22 @@ class CollapsedRangeDelMap : public RangeDelMap {
       //     2:    c---   OR   2:    c---   OR   2:    c---   OR   2: c------
       //     1: A--C           1:                1: A------        1: C------
       //                ^                 ^                 ^                  ^
+      // Insert a new transition at the new tombstone's start point, or raise
+      // the existing transition at that point to the new tombstone's seqno.
       end_seq = prev_seq();
-      Rep::iterator pit;
-      if (it != rep_.begin() && (pit = std::prev(it)) != rep_.begin() &&
-          ucmp_->Compare(pit->first, t.start_key_) == 0 && std::prev(pit)->second == t.seq_) {
-        // The new tombstone starts at the end of an existing tombstone with an
-        // identical seqno:
-        //
-        //     3:
-        //     2: A--C---
-        //     1:
-        //                ^
-        // Merge the tombstones by removing the existing tombstone's end key.
-        it = rep_.erase(std::prev(it));
-      } else {
-        // Insert a new transition at the new tombstone's start point, or raise
-        // the existing transition at that point to the new tombstone's seqno.
-        rep_[t.start_key_] = t.seq_;  // operator[] will overwrite existing entry
-      }
+      rep_[t.start_key_] = t.seq_;  // operator[] will overwrite existing entry
     } else {
       // The new tombstone's start point is covered by an existing tombstone:
       //
-      //      3: A-----   OR    3: C------   OR
-      //      2:   c---         2: c------         2: C------
-      //                ^                  ^                  ^
+      //      3: A-----   OR    3: C------
+      //      2:   c---         2: c------
+      //                ^                  ^
       // Do nothing.
     }
 
     // Look at all the existing transitions that overlap the new tombstone.
     while (it != rep_.end() && ucmp_->Compare(it->first, t.end_key_) < 0) {
-      if (t.seq_ >= it->second) {
+      if (t.seq_ > it->second) {
         // The transition is to an existing tombstone that the new tombstone
         // covers. Save the covered tombstone's seqno. We'll need to return to
         // it if the new tombstone ends before the existing tombstone.
@@ -342,29 +324,15 @@ class CollapsedRangeDelMap : public RangeDelMap {
     }
 
     if (t.seq_ == prev_seq()) {
-      // The new tombstone is unterminated in the map.
-      if (it != rep_.end() && t.seq_ == it->second && ucmp_->Compare(it->first, t.end_key_) == 0) {
-        // The new tombstone ends at the start of another tombstone with an
-        // identical seqno. Merge the tombstones by removing the existing
-        // tombstone's start key.
-        rep_.erase(it);
-      } else if (end_seq == prev_seq() || (it != rep_.end() && end_seq == it->second)) {
-        // The new tombstone is implicitly ended because its end point is
-        // contained within an existing tombstone with the same seqno:
-        //
-        //     2: ---k--N
-        //              ^
-      } else {
-        // The new tombstone needs an explicit end point.
-        //
-        //     3:             OR   3: --G       OR   3: --G   K--
-        //     2: C-------k        2:   G---k        2:   G---k
-        //                  ^                 ^               ^
-        // Install one that returns to the last seqno we covered. Because end
-        // keys are exclusive, if there's an existing transition at t.end_key_,
-        // it takes precedence over the transition that we install here.
-        rep_.emplace(t.end_key_, end_seq);  // emplace is a noop if existing entry
-      }
+      // The new tombstone is unterminated in the map:
+      //
+      //     3:             OR   3: --G       OR   3: --G   K--
+      //     2: C-------k        2:   G---k        2:   G---k
+      //                  ^                 ^               ^
+      // End it now, returning to the last seqno we covered. Because end keys
+      // are exclusive, if there's an existing transition at t.end_key_, it
+      // takes precedence over the transition that we install here.
+      rep_.emplace(t.end_key_, end_seq);  // emplace is a noop if existing entry
     } else {
       // The new tombstone is implicitly ended because its end point is covered
       // by an existing tombstone with a higher seqno.
@@ -510,17 +478,18 @@ Status RangeDelAggregator::AddTombstones(
       }
     }
     if (largest != nullptr) {
-      // To safely truncate the range tombstone's end key, it must extend past
-      // the largest key in the sstable (which may have been extended to the
-      // smallest key in the next sstable), and largest must be a tombstone
-      // sentinel key. A range tombstone may straddle two sstables and not be
-      // the tombstone sentinel key in the first sstable if a user-key also
-      // straddles the sstables (possible if there is a snapshot between the
-      // two versions of the user-key), in which case we cannot truncate the
-      // range tombstone.
-      if (icmp_.user_comparator()->Compare(tombstone.end_key_,
-                                           largest->user_key()) > 0 &&
-          GetInternalKeySeqno(largest->Encode()) == kMaxSequenceNumber) {
+      // This is subtly correct despite the discrepancy between
+      // FileMetaData::largest being inclusive while RangeTombstone::end_key_
+      // is exclusive. A tombstone will only extend past the bounds of an
+      // sstable if its end-key is the largest key in the table. If that
+      // occurs, the largest key for the table is set based on the smallest
+      // key in the next table in the level. In that case, largest->user_key()
+      // is not actually a key in the current table and thus we can use it as
+      // the exclusive end-key for the tombstone.
+      if (icmp_.user_comparator()->Compare(
+              tombstone.end_key_, largest->user_key()) > 0) {
+        // The largest key should be a tombstone sentinel key.
+        assert(GetInternalKeySeqno(largest->Encode()) == kMaxSequenceNumber);
         tombstone.end_key_ = largest->user_key();
       }
     }

db/range_del_aggregator_bench.cc

@@ -1,244 +0,0 @@
-//  Copyright (c) 2018-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).
-
-#ifndef GFLAGS
-#include <cstdio>
-int main() {
-  fprintf(stderr, "Please install gflags to run rocksdb tools\n");
-  return 1;
-}
-#else
-
-#include <iostream>
-#include <iomanip>
-#include <memory>
-#include <random>
-#include <set>
-#include <string>
-#include <vector>
-
-#include "db/range_del_aggregator.h"
-#include "rocksdb/comparator.h"
-#include "rocksdb/env.h"
-#include "util/coding.h"
-#include "util/random.h"
-#include "util/stop_watch.h"
-#include "util/testutil.h"
-
-#include "util/gflags_compat.h"
-
-using GFLAGS_NAMESPACE::ParseCommandLineFlags;
-
-DEFINE_int32(num_range_tombstones, 1000, "number of range tombstones created");
-
-DEFINE_int32(num_runs, 10000, "number of test runs");
-
-DEFINE_int32(tombstone_start_upper_bound, 1000,
-             "exclusive upper bound on range tombstone start keys");
-
-DEFINE_int32(should_delete_upper_bound, 1000,
-             "exclusive upper bound on keys passed to ShouldDelete");
-
-DEFINE_double(tombstone_width_mean, 100.0, "average range tombstone width");
-
-DEFINE_double(tombstone_width_stddev, 0.0,
-              "standard deviation of range tombstone width");
-
-DEFINE_bool(use_collapsed, true, "use the collapsed range tombstone map");
-
-DEFINE_int32(seed, 0, "random number generator seed");
-
-DEFINE_int32(should_deletes_per_run, 1, "number of ShouldDelete calls per run");
-
-DEFINE_int32(add_tombstones_per_run, 1,
-             "number of AddTombstones calls per run");
-
-namespace {
-
-struct Stats {
-  uint64_t time_add_tombstones = 0;
-  uint64_t time_first_should_delete = 0;
-  uint64_t time_rest_should_delete = 0;
-};
-
-std::ostream& operator<<(std::ostream& os, const Stats& s) {
-  std::ios fmt_holder(nullptr);
-  fmt_holder.copyfmt(os);
-
-  os << std::left;
-  os << std::setw(25) << "AddTombstones: "
-     << s.time_add_tombstones /
-            (FLAGS_add_tombstones_per_run * FLAGS_num_runs * 1.0e3)
-     << " us\n";
-  os << std::setw(25) << "ShouldDelete (first): "
-     << s.time_first_should_delete / (FLAGS_num_runs * 1.0e3) << " us\n";
-  if (FLAGS_should_deletes_per_run > 1) {
-    os << std::setw(25) << "ShouldDelete (rest): "
-       << s.time_rest_should_delete /
-              ((FLAGS_should_deletes_per_run - 1) * FLAGS_num_runs * 1.0e3)
-       << " us\n";
-  }
-
-  os.copyfmt(fmt_holder);
-  return os;
-}
-
-}  // anonymous namespace
-
-namespace rocksdb {
-
-namespace {
-
-// A wrapper around RangeTombstones and the underlying data of its start and end
-// keys.
-struct PersistentRangeTombstone {
-  std::string start_key;
-  std::string end_key;
-  RangeTombstone tombstone;
-
-  PersistentRangeTombstone(std::string start, std::string end,
-                           SequenceNumber seq)
-      : start_key(std::move(start)), end_key(std::move(end)) {
-    tombstone = RangeTombstone(start_key, end_key, seq);
-  }
-
-  PersistentRangeTombstone() = default;
-
-  PersistentRangeTombstone(const PersistentRangeTombstone& t) { *this = t; }
-
-  PersistentRangeTombstone& operator=(const PersistentRangeTombstone& t) {
-    start_key = t.start_key;
-    end_key = t.end_key;
-    tombstone = RangeTombstone(start_key, end_key, t.tombstone.seq_);
-
-    return *this;
-  }
-
-  PersistentRangeTombstone(PersistentRangeTombstone&& t) noexcept { *this = t; }
-
-  PersistentRangeTombstone& operator=(PersistentRangeTombstone&& t) {
-    start_key = std::move(t.start_key);
-    end_key = std::move(t.end_key);
-    tombstone = RangeTombstone(start_key, end_key, t.tombstone.seq_);
-
-    return *this;
-  }
-};
-
-struct TombstoneStartKeyComparator {
-  explicit TombstoneStartKeyComparator(const Comparator* c) : cmp(c) {}
-
-  bool operator()(const RangeTombstone& a, const RangeTombstone& b) const {
-    return cmp->Compare(a.start_key_, b.start_key_) < 0;
-  }
-
-  const Comparator* cmp;
-};
-
-std::unique_ptr<InternalIterator> MakeRangeDelIterator(
-    const std::vector<PersistentRangeTombstone>& range_dels) {
-  std::vector<std::string> keys, values;
-  for (const auto& range_del : range_dels) {
-    auto key_and_value = range_del.tombstone.Serialize();
-    keys.push_back(key_and_value.first.Encode().ToString());
-    values.push_back(key_and_value.second.ToString());
-  }
-  return std::unique_ptr<test::VectorIterator>(
-      new test::VectorIterator(keys, values));
-}
-
-// convert long to a big-endian slice key
-static std::string Key(int64_t val) {
-  std::string little_endian_key;
-  std::string big_endian_key;
-  PutFixed64(&little_endian_key, val);
-  assert(little_endian_key.size() == sizeof(val));
-  big_endian_key.resize(sizeof(val));
-  for (size_t i = 0; i < sizeof(val); ++i) {
-    big_endian_key[i] = little_endian_key[sizeof(val) - 1 - i];
-  }
-  return big_endian_key;
-}
-
-}  // anonymous namespace
-
-}  // namespace rocksdb
-
-int main(int argc, char** argv) {
-  ParseCommandLineFlags(&argc, &argv, true);
-
-  Stats stats;
-  rocksdb::Random64 rnd(FLAGS_seed);
-  std::default_random_engine random_gen(FLAGS_seed);
-  std::normal_distribution<double> normal_dist(FLAGS_tombstone_width_mean,
-                                               FLAGS_tombstone_width_stddev);
-  std::vector<std::vector<rocksdb::PersistentRangeTombstone> >
-      all_persistent_range_tombstones(FLAGS_add_tombstones_per_run);
-  for (int i = 0; i < FLAGS_add_tombstones_per_run; i++) {
-    all_persistent_range_tombstones[i] =
-        std::vector<rocksdb::PersistentRangeTombstone>(
-            FLAGS_num_range_tombstones);
-  }
-  auto mode = FLAGS_use_collapsed
-                  ? rocksdb::RangeDelPositioningMode::kForwardTraversal
-                  : rocksdb::RangeDelPositioningMode::kFullScan;
-
-  for (int i = 0; i < FLAGS_num_runs; i++) {
-    auto icmp = rocksdb::InternalKeyComparator(rocksdb::BytewiseComparator());
-    rocksdb::RangeDelAggregator range_del_agg(icmp, {} /* snapshots */,
-                                              FLAGS_use_collapsed);
-
-    for (auto& persistent_range_tombstones : all_persistent_range_tombstones) {
-      // TODO(abhimadan): consider whether creating the range tombstones right
-      // before AddTombstones is artificially warming the cache compared to
-      // real workloads.
-      for (int j = 0; j < FLAGS_num_range_tombstones; j++) {
-        uint64_t start = rnd.Uniform(FLAGS_tombstone_start_upper_bound);
-        uint64_t end = start + std::max(1.0, normal_dist(random_gen));
-        persistent_range_tombstones[j] = rocksdb::PersistentRangeTombstone(
-            rocksdb::Key(start), rocksdb::Key(end), j);
-      }
-
-      auto range_del_iter =
-          rocksdb::MakeRangeDelIterator(persistent_range_tombstones);
-      rocksdb::StopWatchNano stop_watch_add_tombstones(rocksdb::Env::Default(),
-                                                       true /* auto_start */);
-      range_del_agg.AddTombstones(std::move(range_del_iter));
-      stats.time_add_tombstones += stop_watch_add_tombstones.ElapsedNanos();
-    }
-
-    rocksdb::ParsedInternalKey parsed_key;
-    parsed_key.sequence = FLAGS_num_range_tombstones / 2;
-    parsed_key.type = rocksdb::kTypeValue;
-
-    uint64_t first_key = rnd.Uniform(FLAGS_should_delete_upper_bound -
-                                     FLAGS_should_deletes_per_run + 1);
-
-    for (int j = 0; j < FLAGS_should_deletes_per_run; j++) {
-      std::string key_string = rocksdb::Key(first_key + j);
-      parsed_key.user_key = key_string;
-
-      rocksdb::StopWatchNano stop_watch_should_delete(rocksdb::Env::Default(),
-          true /* auto_start */);
-      range_del_agg.ShouldDelete(parsed_key, mode);
-      uint64_t call_time = stop_watch_should_delete.ElapsedNanos();
-
-      if (j == 0) {
-        stats.time_first_should_delete += call_time;
-      } else {
-        stats.time_rest_should_delete += call_time;
-      }
-    }
-  }
-
-  std::cout << "=========================\n"
-            << "Results:\n"
-            << "=========================\n"
-            << stats;
-
-  return 0;
-}
-
-#endif  // GFLAGS

db/range_del_aggregator_test.cc

@@ -27,7 +27,7 @@ enum Direction {
   kReverse,
 };
 
-static auto bytewise_icmp = InternalKeyComparator(BytewiseComparator());
+static auto icmp = InternalKeyComparator(BytewiseComparator());
 
 void AddTombstones(RangeDelAggregator* range_del_agg,
                    const std::vector<RangeTombstone>& range_dels,
@@ -66,8 +66,8 @@ void VerifyRangeDels(
     const std::vector<RangeTombstone>& range_dels_in,
     const std::vector<ExpectedPoint>& expected_points,
     const std::vector<RangeTombstone>& expected_collapsed_range_dels,
-    const InternalKey* smallest = nullptr, const InternalKey* largest = nullptr,
-    const InternalKeyComparator& icmp = bytewise_icmp) {
+    const InternalKey* smallest = nullptr,
+    const InternalKey* largest = nullptr) {
   // Test same result regardless of which order the range deletions are added
   // and regardless of collapsed mode.
   for (bool collapsed : {false, true}) {
@@ -164,14 +164,6 @@ TEST_F(RangeDelAggregatorTest, OverlapAboveMiddle) {
                   {{"a", "b", 5}, {"b", "c", 10}, {"c", "d", 5}});
 }
 
-TEST_F(RangeDelAggregatorTest, OverlapAboveMiddleReverse) {
-  VerifyRangeDels({{"d", "a", 5}, {"c", "b", 10}},
-                  {{"z", 0}, {"d", 5}, {"c", 10}, {"b", 5}, {"a", 0}},
-                  {{"d", "c", 5}, {"c", "b", 10}, {"b", "a", 5}},
-                  nullptr /* smallest */, nullptr /* largest */,
-                  InternalKeyComparator(ReverseBytewiseComparator()));
-}
-
 TEST_F(RangeDelAggregatorTest, OverlapFully) {
   VerifyRangeDels({{"a", "d", 10}, {"b", "c", 5}},
                   {{" ", 0}, {"a", 10}, {"d", 0}}, {{"a", "d", 10}});
@@ -208,30 +200,6 @@ TEST_F(RangeDelAggregatorTest, GapsBetweenRanges) {
                   {{"a", "b", 5}, {"c", "d", 10}, {"e", "f", 15}});
 }
 
-TEST_F(RangeDelAggregatorTest, IdenticalSameSeqNo) {
-  VerifyRangeDels({{"a", "b", 5}, {"a", "b", 5}},
-                  {{" ", 0}, {"a", 5}, {"b", 0}},
-                  {{"a", "b", 5}});
-}
-
-TEST_F(RangeDelAggregatorTest, ContiguousSameSeqNo) {
-  VerifyRangeDels({{"a", "b", 5}, {"b", "c", 5}},
-                  {{" ", 0}, {"a", 5}, {"b", 5}, {"c", 0}},
-                  {{"a", "c", 5}});
-}
-
-TEST_F(RangeDelAggregatorTest, OverlappingSameSeqNo) {
-  VerifyRangeDels({{"a", "c", 5}, {"b", "d", 5}},
-                  {{" ", 0}, {"a", 5}, {"b", 5}, {"c", 5}, {"d", 0}},
-                  {{"a", "d", 5}});
-}
-
-TEST_F(RangeDelAggregatorTest, CoverSameSeqNo) {
-  VerifyRangeDels({{"a", "d", 5}, {"b", "c", 5}},
-                  {{" ", 0}, {"a", 5}, {"b", 5}, {"c", 5}, {"d", 0}},
-                  {{"a", "d", 5}});
-}
-
 // Note the Cover* tests also test cases where tombstones are inserted under a
 // larger one when VerifyRangeDels() runs them in reverse
 TEST_F(RangeDelAggregatorTest, CoverMultipleFromLeft) {
@@ -267,14 +235,14 @@ TEST_F(RangeDelAggregatorTest, AlternateMultipleAboveBelow) {
 
 TEST_F(RangeDelAggregatorTest, MergingIteratorAllEmptyStripes) {
   for (bool collapsed : {true, false}) {
-    RangeDelAggregator range_del_agg(bytewise_icmp, {1, 2}, collapsed);
+    RangeDelAggregator range_del_agg(icmp, {1, 2}, collapsed);
     VerifyRangeDelIter(range_del_agg.NewIterator().get(), {});
   }
 }
 
 TEST_F(RangeDelAggregatorTest, MergingIteratorOverlappingStripes) {
   for (bool collapsed : {true, false}) {
-    RangeDelAggregator range_del_agg(bytewise_icmp, {5, 15, 25, 35}, collapsed);
+    RangeDelAggregator range_del_agg(icmp, {5, 15, 25, 35}, collapsed);
     AddTombstones(
         &range_del_agg,
         {{"d", "e", 10}, {"aa", "b", 20}, {"c", "d", 30}, {"a", "b", 10}});
@@ -285,8 +253,7 @@ TEST_F(RangeDelAggregatorTest, MergingIteratorOverlappingStripes) {
 }
 
 TEST_F(RangeDelAggregatorTest, MergingIteratorSeek) {
-  RangeDelAggregator range_del_agg(bytewise_icmp, {5, 15},
-                                   true /* collapsed */);
+  RangeDelAggregator range_del_agg(icmp, {5, 15}, true /* collapsed */);
   AddTombstones(&range_del_agg, {{"a", "c", 10},
                                  {"b", "c", 11},
                                  {"f", "g", 10},
@@ -333,21 +300,6 @@ TEST_F(RangeDelAggregatorTest, TruncateTombstones) {
       &smallest, &largest);
 }
 
-TEST_F(RangeDelAggregatorTest, OverlappingLargestKeyTruncateTombstones) {
-  const InternalKey smallest("b", 1, kTypeRangeDeletion);
-  const InternalKey largest(
-      "e", 3,  // could happen if "e" is in consecutive sstables
-      kTypeValue);
-  VerifyRangeDels(
-      {{"a", "c", 10}, {"d", "f", 10}},
-      {{"a", 10, true},  // truncated
-       {"b", 10, false}, // not truncated
-       {"d", 10, false}, // not truncated
-       {"e", 10, false}}, // not truncated
-      {{"b", "c", 10}, {"d", "f", 10}},
-      &smallest, &largest);
-}
-
 }  // namespace rocksdb
 
 int main(int argc, char** argv) {

db/repair.cc

@@ -353,7 +353,7 @@ class Repairer {
     // propagating bad information (like overly large sequence
     // numbers).
     log::Reader reader(db_options_.info_log, std::move(lfile_reader), &reporter,
-                       true /*enable checksum*/, log);
+                       true /*enable checksum*/, 0 /*initial_offset*/, log);
 
     // Initialize per-column family memtables
     for (auto* cfd : *vset_.GetColumnFamilySet()) {

db/repair_test.cc

@@ -74,7 +74,7 @@ TEST_F(RepairTest, CorruptManifest) {
 
   Close();
   ASSERT_OK(env_->FileExists(manifest_path));
-  CreateFile(env_, manifest_path, "blah", false /* use_fsync */);
+  CreateFile(env_, manifest_path, "blah");
   ASSERT_OK(RepairDB(dbname_, CurrentOptions()));
   Reopen(CurrentOptions());
 
@@ -153,7 +153,7 @@ TEST_F(RepairTest, CorruptSst) {
   Flush();
   auto sst_path = GetFirstSstPath();
   ASSERT_FALSE(sst_path.empty());
-  CreateFile(env_, sst_path, "blah", false /* use_fsync */);
+  CreateFile(env_, sst_path, "blah");
 
   Close();
   ASSERT_OK(RepairDB(dbname_, CurrentOptions()));

db/snapshot_checker.h

@@ -19,9 +19,8 @@ class SnapshotChecker {
 class DisableGCSnapshotChecker : public SnapshotChecker {
  public:
   virtual ~DisableGCSnapshotChecker() {}
-  virtual bool IsInSnapshot(
-      SequenceNumber /*sequence*/,
-      SequenceNumber /*snapshot_sequence*/) const override {
+  virtual bool IsInSnapshot(SequenceNumber /*sequence*/,
+                            SequenceNumber /*snapshot_sequence*/) const override {
     // By returning false, we prevent all the values from being GCed
     return false;
   }

db/table_properties_collector_test.cc

@@ -46,8 +46,7 @@ void MakeBuilder(const Options& options, const ImmutableCFOptions& ioptions,
                  std::unique_ptr<WritableFileWriter>* writable,
                  std::unique_ptr<TableBuilder>* builder) {
   unique_ptr<WritableFile> wf(new test::StringSink);
-  writable->reset(
-      new WritableFileWriter(std::move(wf), "" /* don't care */, EnvOptions()));
+  writable->reset(new WritableFileWriter(std::move(wf), EnvOptions()));
   int unknown_level = -1;
   builder->reset(NewTableBuilder(
       ioptions, moptions, internal_comparator, int_tbl_prop_collector_factories,

db/transaction_log_impl.cc

@@ -104,7 +104,7 @@ void TransactionLogIteratorImpl::SeekToStartSequence(
   if (files_->size() <= startFileIndex) {
     return;
   }
-  Status s = OpenLogReader(files_->at(static_cast<size_t>(startFileIndex)).get());
+  Status s = OpenLogReader(files_->at(startFileIndex).get());
   if (!s.ok()) {
     currentStatus_ = s;
     reporter_.Info(currentStatus_.ToString().c_str());
@@ -312,9 +312,9 @@ Status TransactionLogIteratorImpl::OpenLogReader(const LogFile* logFile) {
     return s;
   }
   assert(file);
-  currentLogReader_.reset(
-      new log::Reader(options_->info_log, std::move(file), &reporter_,
-                      read_options_.verify_checksums_, logFile->LogNumber()));
+  currentLogReader_.reset(new log::Reader(
+      options_->info_log, std::move(file), &reporter_,
+      read_options_.verify_checksums_, 0, logFile->LogNumber()));
   return Status::OK();
 }
 }  //  namespace rocksdb

db/version_builder.cc

@@ -324,6 +324,8 @@ class VersionBuilder::Rep {
       // Merge the set of added files with the set of pre-existing files.
       // Drop any deleted files.  Store the result in *v.
       const auto& base_files = base_vstorage_->LevelFiles(level);
+      auto base_iter = base_files.begin();
+      auto base_end = base_files.end();
       const auto& unordered_added_files = levels_[level].added_files;
       vstorage->Reserve(level,
                         base_files.size() + unordered_added_files.size());
@@ -337,27 +339,30 @@ class VersionBuilder::Rep {
       std::sort(added_files.begin(), added_files.end(), cmp);
 
 #ifndef NDEBUG
-      FileMetaData* prev_added_file = nullptr;
-      for (const auto& added : added_files) {
-        if (level > 0 && prev_added_file != nullptr) {
-          assert(base_vstorage_->InternalComparator()->Compare(
-                     prev_added_file->smallest, added->smallest) <= 0);
-        }
-        prev_added_file = added;
-      }
+      FileMetaData* prev_file = nullptr;
 #endif
 
-      auto base_iter = base_files.begin();
-      auto base_end = base_files.end();
-      auto added_iter = added_files.begin();
-      auto added_end = added_files.end();
-      while (added_iter != added_end || base_iter != base_end) {
-        if (base_iter == base_end ||
-                (added_iter != added_end && cmp(*added_iter, *base_iter))) {
-          MaybeAddFile(vstorage, level, *added_iter++);
-        } else {
-          MaybeAddFile(vstorage, level, *base_iter++);
+      for (const auto& added : added_files) {
+#ifndef NDEBUG
+        if (level > 0 && prev_file != nullptr) {
+          assert(base_vstorage_->InternalComparator()->Compare(
+                     prev_file->smallest, added->smallest) <= 0);
         }
+        prev_file = added;
+#endif
+
+        // Add all smaller files listed in base_
+        for (auto bpos = std::upper_bound(base_iter, base_end, added, cmp);
+             base_iter != bpos; ++base_iter) {
+          MaybeAddFile(vstorage, level, *base_iter);
+        }
+
+        MaybeAddFile(vstorage, level, added);
+      }
+
+      // Add remaining base files
+      for (; base_iter != base_end; ++base_iter) {
+        MaybeAddFile(vstorage, level, *base_iter);
       }
     }
 

db/version_set.cc

@@ -21,7 +21,6 @@
 #include <string>
 #include <unordered_map>
 #include <vector>
-#include <list>
 #include "db/compaction.h"
 #include "db/internal_stats.h"
 #include "db/log_reader.h"
@@ -63,12 +62,20 @@ int FindFileInRange(const InternalKeyComparator& icmp,
     const Slice& key,
     uint32_t left,
     uint32_t right) {
-  auto cmp = [&](const FdWithKeyRange& f, const Slice& k) -> bool {
-    return icmp.InternalKeyComparator::Compare(f.largest_key, k) < 0;
-  };
-  const auto &b = file_level.files;
-  return static_cast<int>(std::lower_bound(b + left,
-                                           b + right, key, cmp) - b);
+  while (left < right) {
+    uint32_t mid = (left + right) / 2;
+    const FdWithKeyRange& f = file_level.files[mid];
+    if (icmp.InternalKeyComparator::Compare(f.largest_key, key) < 0) {
+      // Key at "mid.largest" is < "target".  Therefore all
+      // files at or before "mid" are uninteresting.
+      left = mid + 1;
+    } else {
+      // Key at "mid.largest" is >= "target".  Therefore all files
+      // after "mid" are uninteresting.
+      right = mid;
+    }
+  }
+  return right;
 }
 
 Status OverlapWithIterator(const Comparator* ucmp,
@@ -888,16 +895,13 @@ void Version::GetColumnFamilyMetaData(ColumnFamilyMetaData* cf_meta) {
         assert(!ioptions->cf_paths.empty());
         file_path = ioptions->cf_paths.back().path;
       }
-      files.emplace_back(SstFileMetaData{
-          MakeTableFileName("", file->fd.GetNumber()),
-          file_path,
-          static_cast<size_t>(file->fd.GetFileSize()),
-          file->fd.smallest_seqno,
-          file->fd.largest_seqno,
-          file->smallest.user_key().ToString(),
+      files.emplace_back(
+          MakeTableFileName("", file->fd.GetNumber()), file_path,
+          file->fd.GetFileSize(), file->fd.smallest_seqno,
+          file->fd.largest_seqno, file->smallest.user_key().ToString(),
           file->largest.user_key().ToString(),
           file->stats.num_reads_sampled.load(std::memory_order_relaxed),
-          file->being_compacted});
+          file->being_compacted);
       level_size += file->fd.GetFileSize();
     }
     cf_meta->levels.emplace_back(
@@ -2035,6 +2039,13 @@ void VersionStorageInfo::GetOverlappingInputs(
   }
 
   inputs->clear();
+  Slice user_begin, user_end;
+  if (begin != nullptr) {
+    user_begin = begin->user_key();
+  }
+  if (end != nullptr) {
+    user_end = end->user_key();
+  }
   if (file_index) {
     *file_index = -1;
   }
@@ -2051,59 +2062,33 @@ void VersionStorageInfo::GetOverlappingInputs(
     *next_smallest = nullptr;
   }
 
-  Slice user_begin, user_end;
-  if (begin != nullptr) {
-    user_begin = begin->user_key();
-  }
-  if (end != nullptr) {
-    user_end = end->user_key();
-  }
-
-  // index stores the file index need to check.
-  std::list<size_t> index;
-  for (size_t i = 0; i < level_files_brief_[level].num_files; i++) {
-    index.emplace_back(i);
-  }
-
-  while (!index.empty()) {
-    bool found_overlapping_file = false;
-    auto iter = index.begin();
-    while (iter != index.end()) {
-      FdWithKeyRange* f = &(level_files_brief_[level].files[*iter]);
-      const Slice file_start = ExtractUserKey(f->smallest_key);
-      const Slice file_limit = ExtractUserKey(f->largest_key);
-      if (begin != nullptr && user_cmp->Compare(file_limit, user_begin) < 0) {
-        // "f" is completely before specified range; skip it
-        iter++;
-      } else if (end != nullptr && user_cmp->Compare(file_start, user_end) > 0) {
-        // "f" is completely after specified range; skip it
-        iter++;
-      } else {
-        // if overlap
-        inputs->emplace_back(files_[level][*iter]);
-        found_overlapping_file = true;
-        // record the first file index.
-        if (file_index && *file_index == -1) {
-          *file_index = static_cast<int>(*iter);
-        }
-        // the related file is overlap, erase to avoid checking again.
-        iter = index.erase(iter);
-        if (expand_range) {
-          if (begin != nullptr &&
-              user_cmp->Compare(file_start, user_begin) < 0) {
-            user_begin = file_start;
-          }
-          if (end != nullptr &&
-              user_cmp->Compare(file_limit, user_end) > 0) {
-            user_end = file_limit;
-          }
+  for (size_t i = 0; i < level_files_brief_[level].num_files; ) {
+    FdWithKeyRange* f = &(level_files_brief_[level].files[i++]);
+    const Slice file_start = ExtractUserKey(f->smallest_key);
+    const Slice file_limit = ExtractUserKey(f->largest_key);
+    if (begin != nullptr && user_cmp->Compare(file_limit, user_begin) < 0) {
+      // "f" is completely before specified range; skip it
+    } else if (end != nullptr && user_cmp->Compare(file_start, user_end) > 0) {
+      // "f" is completely after specified range; skip it
+    } else {
+      inputs->push_back(files_[level][i-1]);
+      if (level == 0 && expand_range) {
+        // Level-0 files may overlap each other.  So check if the newly
+        // added file has expanded the range.  If so, restart search.
+        if (begin != nullptr && user_cmp->Compare(file_start, user_begin) < 0) {
+          user_begin = file_start;
+          inputs->clear();
+          i = 0;
+        } else if (end != nullptr
+            && user_cmp->Compare(file_limit, user_end) > 0) {
+          user_end = file_limit;
+          inputs->clear();
+          i = 0;
         }
+      } else if (file_index) {
+        *file_index = static_cast<int>(i) - 1;
       }
     }
-    // if all the files left are not overlap, break
-    if (!found_overlapping_file) {
-      break;
-    }
   }
 }
 
@@ -2934,17 +2919,16 @@ Status VersionSet::ProcessManifestWrites(
       // create new manifest file
       ROCKS_LOG_INFO(db_options_->info_log, "Creating manifest %" PRIu64 "\n",
                      pending_manifest_file_number_);
-      std::string descriptor_fname =
-          DescriptorFileName(dbname_, pending_manifest_file_number_);
       unique_ptr<WritableFile> descriptor_file;
-      s = NewWritableFile(env_, descriptor_fname, &descriptor_file,
-                          opt_env_opts);
+      s = NewWritableFile(
+          env_, DescriptorFileName(dbname_, pending_manifest_file_number_),
+          &descriptor_file, opt_env_opts);
       if (s.ok()) {
         descriptor_file->SetPreallocationBlockSize(
             db_options_->manifest_preallocation_size);
 
-        unique_ptr<WritableFileWriter> file_writer(new WritableFileWriter(
-            std::move(descriptor_file), descriptor_fname, opt_env_opts));
+        unique_ptr<WritableFileWriter> file_writer(
+            new WritableFileWriter(std::move(descriptor_file), opt_env_opts));
         descriptor_log_.reset(
             new log::Writer(std::move(file_writer), 0, false));
         s = WriteSnapshot(descriptor_log_.get());
@@ -3454,7 +3438,7 @@ Status VersionSet::Recover(
     VersionSet::LogReporter reporter;
     reporter.status = &s;
     log::Reader reader(nullptr, std::move(manifest_file_reader), &reporter,
-                       true /* checksum */, 0 /* log_number */);
+                       true /*checksum*/, 0 /*initial_offset*/, 0);
     Slice record;
     std::string scratch;
     std::vector<VersionEdit> replay_buffer;
@@ -3659,8 +3643,8 @@ Status VersionSet::ListColumnFamilies(std::vector<std::string>* column_families,
   column_family_names.insert({0, kDefaultColumnFamilyName});
   VersionSet::LogReporter reporter;
   reporter.status = &s;
-  log::Reader reader(nullptr, std::move(file_reader), &reporter,
-                     true /* checksum */, 0 /* log_number */);
+  log::Reader reader(nullptr, std::move(file_reader), &reporter, true /*checksum*/,
+                     0 /*initial_offset*/, 0);
   Slice record;
   std::string scratch;
   while (reader.ReadRecord(&record, &scratch) && s.ok()) {
@@ -3820,7 +3804,7 @@ Status VersionSet::DumpManifest(Options& options, std::string& dscname,
     VersionSet::LogReporter reporter;
     reporter.status = &s;
     log::Reader reader(nullptr, std::move(file_reader), &reporter,
-                       true /* checksum */, 0 /* log_number */);
+                       true /*checksum*/, 0 /*initial_offset*/, 0);
     Slice record;
     std::string scratch;
     while (reader.ReadRecord(&record, &scratch) && s.ok()) {
@@ -4371,7 +4355,7 @@ void VersionSet::GetLiveFilesMetaData(std::vector<LiveFileMetaData>* metadata) {
         }
         filemetadata.name = MakeTableFileName("", file->fd.GetNumber());
         filemetadata.level = level;
-        filemetadata.size = static_cast<size_t>(file->fd.GetFileSize());
+        filemetadata.size = file->fd.GetFileSize();
         filemetadata.smallestkey = file->smallest.user_key().ToString();
         filemetadata.largestkey = file->largest.user_key().ToString();
         filemetadata.smallest_seqno = file->fd.smallest_seqno;

db/version_set_test.cc

@@ -566,7 +566,7 @@ class ManifestWriterTest : public testing::Test {
         manifest, &file, env_->OptimizeForManifestWrite(env_options_));
     ASSERT_OK(s);
     unique_ptr<WritableFileWriter> file_writer(
-        new WritableFileWriter(std::move(file), manifest, env_options_));
+        new WritableFileWriter(std::move(file), env_options_));
     {
       log::Writer log(std::move(file_writer), 0, false);
       std::string record;

db/wal_manager.cc

@@ -237,7 +237,7 @@ void WalManager::PurgeObsoleteWALFiles() {
   }
 
   size_t const files_keep_num =
-      static_cast<size_t>(db_options_.wal_size_limit_mb * 1024 * 1024 / log_file_size);
+      db_options_.wal_size_limit_mb * 1024 * 1024 / log_file_size;
   if (log_files_num <= files_keep_num) {
     return;
   }
@@ -352,7 +352,7 @@ Status WalManager::RetainProbableWalFiles(VectorLogPtr& all_logs,
   // Binary Search. avoid opening all files.
   while (end >= start) {
     int64_t mid = start + (end - start) / 2;  // Avoid overflow.
-    SequenceNumber current_seq_num = all_logs.at(static_cast<size_t>(mid))->StartSequence();
+    SequenceNumber current_seq_num = all_logs.at(mid)->StartSequence();
     if (current_seq_num == target) {
       end = mid;
       break;
@@ -363,7 +363,7 @@ Status WalManager::RetainProbableWalFiles(VectorLogPtr& all_logs,
     }
   }
   // end could be -ve.
-  size_t start_index = static_cast<size_t>(std::max(static_cast<int64_t>(0), end));
+  size_t start_index = std::max(static_cast<int64_t>(0), end);
   // The last wal file is always included
   all_logs.erase(all_logs.begin(), all_logs.begin() + start_index);
   return Status::OK();
@@ -457,7 +457,7 @@ Status WalManager::ReadFirstLine(const std::string& fname,
   reporter.status = &status;
   reporter.ignore_error = !db_options_.paranoid_checks;
   log::Reader reader(db_options_.info_log, std::move(file_reader), &reporter,
-                     true /*checksum*/, number);
+                     true /*checksum*/, 0 /*initial_offset*/, number);
   std::string scratch;
   Slice record;
 

db/wal_manager_test.cc

@@ -79,7 +79,7 @@ class WalManagerTest : public testing::Test {
     unique_ptr<WritableFile> file;
     ASSERT_OK(env_->NewWritableFile(fname, &file, env_options_));
     unique_ptr<WritableFileWriter> file_writer(
-        new WritableFileWriter(std::move(file), fname, env_options_));
+        new WritableFileWriter(std::move(file), env_options_));
     current_log_writer_.reset(new log::Writer(std::move(file_writer), 0, false));
   }
 
@@ -130,7 +130,7 @@ TEST_F(WalManagerTest, ReadFirstRecordCache) {
   ASSERT_EQ(s, 0U);
 
   unique_ptr<WritableFileWriter> file_writer(
-      new WritableFileWriter(std::move(file), path, EnvOptions()));
+      new WritableFileWriter(std::move(file), EnvOptions()));
   log::Writer writer(std::move(file_writer), 1,
                      db_options_.recycle_log_file_num > 0);
   WriteBatch batch;

docs/_data/authors.yml

@@ -60,7 +60,3 @@ sagar0:
 lightmark:
   full_name: Aaron Gao
   fbid: 1351549072
-
-fgwu:
-  full_name: Fenggang Wu
-  fbid: 100002297362180

docs/_posts/2018-08-23-data-block-hash-index.markdown

@@ -1,118 +0,0 @@
----
-title: Improving Point-Lookup Using Data Block Hash Index
-layout: post
-author: fgwu
-category: blog
----
-We've designed and implemented a _data block hash index_ in RocksDB that has the benefit of both reducing the CPU util and increasing the throughput for point lookup queries with a reasonable and tunable space overhead. 
-
-Specifially, we append a compact hash table to the end of the data block for efficient indexing. It is backward compatible with the data base created without this feature. After turned on the hash index feature, existing data will be gradually converted to the hash index format.
-
-Benchmarks with `db_bench`  show the CPU utilization of one of the main functions in the point lookup code path, `DataBlockIter::Seek()`, is reduced by 21.8%, and the overall RocksDB throughput is increased by 10% under purely cached workloads, at an overhead of 4.6% more space. Shadow testing with Facebook production traffic shows good CPU improvements too.
-
-
-### How to use it
-Two new options are added as part of this feature: `BlockBasedTableOptions::data_block_index_type` and `BlockBasedTableOptions::data_block_hash_table_util_ratio`.
-
-The hash index is disabled by default unless `BlockBasedTableOptions::data_block_index_type` is set to `data_block_index_type = kDataBlockBinaryAndHash`. The hash table utilization ratio is adjustable using `BlockBasedTableOptions::data_block_hash_table_util_ratio`, which is valid only if `data_block_index_type = kDataBlockBinaryAndHash`.
-
-
-```
-// the definitions can be found in include/rocksdb/table.h
-
-// The index type that will be used for the data block.
-enum DataBlockIndexType : char {
-  kDataBlockBinarySearch = 0,  // traditional block type
-  kDataBlockBinaryAndHash = 1, // additional hash index
-};
-
-// Set to kDataBlockBinaryAndHash to enable hash index
-DataBlockIndexType data_block_index_type = kDataBlockBinarySearch;
-
-// #entries/#buckets. It is valid only when data_block_hash_index_type is
-// kDataBlockBinaryAndHash.
-double data_block_hash_table_util_ratio = 0.75;
-
-```
-
-
-### Data Block Hash Index Design
-
-Current data block format groups adjacent keys together as a restart interval. One block consists of multiple restart intervals. The byte offset of the beginning of each restart interval, i.e. a restart point, is stored in an array called restart interval index or binary seek index. RocksDB does a binary search when performing point lookup for keys in data blocks to find the right restart interval the key may reside. We will use binary seek and binary search interchangeably in this post.
-
-In order to find the right location where the key may reside using binary search, multiple key parsing and comparison are needed. Each binary search branching triggers CPU cache miss, causing much CPU utilization. We have seen that this binary search takes up considerable CPU in production use-cases.
-
-![](/static/images/data-block-hash-index/block-format-binary-seek.png)
-
-We implemented a hash map at the end of the block to index the key to reduce the CPU overhead of the binary search. The hash index is just an array of pointers pointing into the binary seek index.
-
-![](/static/images/data-block-hash-index/block-format-hash-index.png)
-
-
-Each array element is considered as a hash bucket when storing the location of a key (or more precisely, the restart index of the restart interval where the key resides). When multiple keys happen to hash into the same bucket (hash collision), we just mark the bucket as “collision”. So that when later querying on that key, the hash table lookup knows that there was a hash collision happened so it can fall back to the traditional binary search to find the location of the key.
-
-We define hash table utilization ratio as the #keys/#buckets. If a utilization ratio is 0.5 and there are 100 buckets, 50 keys are stored in the bucket. The less the util ratio, the less hash collision, and the less chance for a point lookup falls back to binary seek (fall back ratio) due to the collision. So a small util ratio has more benefit to reduce the CPU time but introduces more space overhead.
-
-Space overhead depends on the util ratio. Each bucket is a `uint8_t`  (i.e. one byte). For a util ratio of 1, the space overhead is 1Byte per key, the fall back ratio observed is ~52%.
-
-![](/static/images/data-block-hash-index/hash-index-data-structure.png)
-
-### Things that Need Attention
-
-**Customized Comparator**
-
-Hash index will hash different keys (keys with different content, or byte sequence) into different hash values. This assumes the comparator will not treat different keys as equal if they have different content. 
-
-The default bytewise comparator orders the keys in alphabetical order and works well with hash index, as different keys will never be regarded as equal. However, some specially crafted comparators will do. For example, say, a `StringToIntComparator` can convert a string into an integer, and use the integer to perform the comparison. Key string “16” and “0x10” is equal to each other as seen by this `StringToIntComparator`, but they probably hash to different value. Later queries to one form of the key will not be able to find the existing key been stored in the other format.
-
-We add a new function member to the comparator interface: 
-
-```
-virtual bool CanKeysWithDifferentByteContentsBeEqual() const { return true; }
-```
-
-
-Every comparator implementation should override this function and specify the behavior of the comparator. If a comparator can regard different keys equal, the function returns true, and as a result the hash index feature will not be enabled, and vice versa.
-
-NOTE: to use the hash index feature, one should 1) have a comparator that can never treat different keys as equal; and 2) override the `CanKeysWithDifferentByteContentsBeEqual()` function to return `false`, so the hash index can be enabled.
-
-
-**Util Ratio's Impact on Data Block Cache**
-
-Adding the hash index to the end of the data block essentially takes up the data block cache space, making the effective data block cache size smaller and increasing the data block cache miss ratio. Therefore, a very small util ratio will result in a large data block cache miss ratio, and the extra I/O may drag down the throughput gain achieved by the hash index lookup. Besides, when compression is enabled, cache miss also incurs data block decompression, which is CPU-consuming. Therefore the CPU may even increase if using a too small util ratio. The best util ratio depends on workloads, cache to data ratio, disk bandwidth/latency etc. In our experiment, we found util ratio = 0.5 ~ 1 is a good range to explore that brings both CPU and throughput gains.
-
-
-### Limitations
-
-As we use `uint8_t` to store binary seek index, i.e. restart interval index, the total number of restart intervals cannot be more than 253 (we reserved  255 and 254 as special flags). For blocks having a larger number of restart intervals, the hash index will not be created and the point lookup will be done by traditional binary seek.
-
-Data block hash index only supports point lookup. We do not support range lookup. Range lookup request will fall back to BinarySeek.
-
-RocksDB supports many types of records, such as `Put`, `Delete`, `Merge`, etc (visit [here](https://github.com/facebook/rocksdb/wiki/rocksdb-basics) for more information). Currently we only support `Put` and `Delete`, but not `Merge`. Internally we have a limited set of supported record types:
-
-
-```
-kPutRecord,          <=== supported
-kDeleteRecord,       <=== supported
-kSingleDeleteRecord, <=== supported
-kTypeBlobIndex,      <=== supported
-```
-
-For records not supported, the searching process will fall back to the traditional binary seek. 
-
-
-
-### Evaluation
-To evaluate the CPU util reduction and isolate other factors such as disk I/O and block decompression, we first evaluate the hash idnex in a purely cached workload. We observe that the CPU utilization of one of the main functions in the point lookup code path, DataBlockIter::Seek(), is reduced by 21.8% and the overall throughput is increased by 10% at an overhead of 4.6% more space.
-
-However, general worload is not always purely cached. So we also evaluate the performance under different cache space pressure. In the following test, we use `db_bench` with RocksDB deployed on SSDs. The total DB size is 5~6GB, and it is about 14GB if decompressed. Different block cache sizes are used, ranging from 14GB down to 2GB, with an increasing cache miss ratio.
-
-Orange bars are representing our hash index performance. We use a hash util ratio of 1.0 in this test. Block size are set to 16KiB with the restart interval as 16.
-
-![](/static/images/data-block-hash-index/perf-throughput.png)
-![](/static/images/data-block-hash-index/perf-cache-miss.png)
-
-We can see that if cache size is greater than 8GB, hash index can bring throughput gain. Cache size greater than 8GB can be translated to a cache miss ratio smaller than 40%. So if the workload has a cache miss ratio smaller than 40%, hash index is able to increase the throughput.
-
-Besides, shadow testing with Facebook production traffic shows good CPU improvements too.
-

docs/feed.xml

@@ -6,7 +6,7 @@ layout: null
   <channel>
     <title>{{ site.title | xml_escape }}</title>
     <description>{{ site.description | xml_escape }}</description>
-    <link>https://rocksdb.org/feed.xml</link>
+    <link>{{ absolute_url }}/</link>
     <atom:link href="{{ '/feed.xml' | absolute_url }}" rel="self" type="application/rss+xml"/>
     <pubDate>{{ site.time | date_to_rfc822 }}</pubDate>
     <lastBuildDate>{{ site.time | date_to_rfc822 }}</lastBuildDate>

env/env_posix.cc

@@ -25,7 +25,6 @@
 #include <sys/syscall.h>
 #include <sys/sysmacros.h>
 #endif
-#include <sys/statvfs.h>
 #include <sys/time.h>
 #include <sys/types.h>
 #include <time.h>
@@ -103,18 +102,6 @@ class PosixFileLock : public FileLock {
   std::string filename;
 };
 
-int cloexec_flags(int flags, const EnvOptions* options) {
-  // If the system supports opening the file with cloexec enabled,
-  // do so, as this avoids a race condition if a db is opened around
-  // the same time that a child process is forked
-#ifdef O_CLOEXEC
-  if (options == nullptr || options->set_fd_cloexec) {
-    flags |= O_CLOEXEC;
-  }
-#endif
-  return flags;
-}
-
 class PosixEnv : public Env {
  public:
   PosixEnv();
@@ -146,7 +133,7 @@ class PosixEnv : public Env {
                                    const EnvOptions& options) override {
     result->reset();
     int fd = -1;
-    int flags = cloexec_flags(O_RDONLY, &options);
+    int flags = O_RDONLY;
     FILE* file = nullptr;
 
     if (options.use_direct_reads && !options.use_mmap_reads) {
@@ -197,8 +184,7 @@ class PosixEnv : public Env {
     result->reset();
     Status s;
     int fd;
-    int flags = cloexec_flags(O_RDONLY, &options);
-
+    int flags = O_RDONLY;
     if (options.use_direct_reads && !options.use_mmap_reads) {
 #ifdef ROCKSDB_LITE
       return Status::IOError(fname, "Direct I/O not supported in RocksDB lite");
@@ -280,8 +266,6 @@ class PosixEnv : public Env {
       flags |= O_WRONLY;
     }
 
-    flags = cloexec_flags(flags, &options);
-
     do {
       IOSTATS_TIMER_GUARD(open_nanos);
       fd = open(fname.c_str(), flags, GetDBFileMode(allow_non_owner_access_));
@@ -370,8 +354,6 @@ class PosixEnv : public Env {
       flags |= O_WRONLY;
     }
 
-    flags = cloexec_flags(flags, &options);
-
     do {
       IOSTATS_TIMER_GUARD(open_nanos);
       fd = open(old_fname.c_str(), flags,
@@ -433,12 +415,9 @@ class PosixEnv : public Env {
                                  unique_ptr<RandomRWFile>* result,
                                  const EnvOptions& options) override {
     int fd = -1;
-    int flags = cloexec_flags(O_RDWR, &options);
-
     while (fd < 0) {
       IOSTATS_TIMER_GUARD(open_nanos);
-
-      fd = open(fname.c_str(), flags, GetDBFileMode(allow_non_owner_access_));
+      fd = open(fname.c_str(), O_RDWR, GetDBFileMode(allow_non_owner_access_));
       if (fd < 0) {
         // Error while opening the file
         if (errno == EINTR) {
@@ -458,11 +437,9 @@ class PosixEnv : public Env {
       unique_ptr<MemoryMappedFileBuffer>* result) override {
     int fd = -1;
     Status status;
-    int flags = cloexec_flags(O_RDWR, nullptr);
-
     while (fd < 0) {
       IOSTATS_TIMER_GUARD(open_nanos);
-      fd = open(fname.c_str(), flags, 0644);
+      fd = open(fname.c_str(), O_RDWR, 0644);
       if (fd < 0) {
         // Error while opening the file
         if (errno == EINTR) {
@@ -500,10 +477,9 @@ class PosixEnv : public Env {
                               unique_ptr<Directory>* result) override {
     result->reset();
     int fd;
-    int flags = cloexec_flags(0, nullptr);
     {
       IOSTATS_TIMER_GUARD(open_nanos);
-      fd = open(name.c_str(), flags);
+      fd = open(name.c_str(), 0);
     }
     if (fd < 0) {
       return IOError("While open directory", name, errno);
@@ -520,8 +496,7 @@ class PosixEnv : public Env {
       return Status::OK();
     }
 
-    int err = errno;
-    switch (err) {
+    switch (errno) {
       case EACCES:
       case ELOOP:
       case ENAMETOOLONG:
@@ -529,8 +504,8 @@ class PosixEnv : public Env {
       case ENOTDIR:
         return Status::NotFound();
       default:
-        assert(err == EIO || err == ENOMEM);
-        return Status::IOError("Unexpected error(" + ToString(err) +
+        assert(result == EIO || result == ENOMEM);
+        return Status::IOError("Unexpected error(" + ToString(result) +
                                ") accessing file `" + fname + "' ");
     }
   }
@@ -688,11 +663,9 @@ class PosixEnv : public Env {
     }
 
     int fd;
-    int flags = cloexec_flags(O_RDWR | O_CREAT, nullptr);
-
     {
       IOSTATS_TIMER_GUARD(open_nanos);
-      fd = open(fname.c_str(), flags, 0644);
+      fd = open(fname.c_str(), O_RDWR | O_CREAT, 0644);
     }
     if (fd < 0) {
       result = IOError("while open a file for lock", fname, errno);
@@ -778,30 +751,12 @@ class PosixEnv : public Env {
     return gettid(pthread_self());
   }
 
-  virtual Status GetFreeSpace(const std::string& fname,
-                              uint64_t* free_space) override {
-    struct statvfs sbuf;
-
-    if (statvfs(fname.c_str(), &sbuf) < 0) {
-      return IOError("While doing statvfs", fname, errno);
-    }
-
-    *free_space = ((uint64_t)sbuf.f_bsize * sbuf.f_bfree);
-    return Status::OK();
-  }
-
   virtual Status NewLogger(const std::string& fname,
                            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
-          );
+      f = fopen(fname.c_str(), "w");
     }
     if (f == nullptr) {
       result->reset();

env/mock_env.cc

@@ -201,7 +201,7 @@ class MockSequentialFile : public SequentialFile {
     if (n > available) {
       n = available;
     }
-    pos_ += static_cast<size_t>(n);
+    pos_ += n;
     return Status::OK();
   }
 

env/posix_logger.h

@@ -165,6 +165,7 @@ class PosixLogger : public Logger {
 
       size_t sz = fwrite(base, 1, write_size, file_);
       flush_pending_ = true;
+      assert(sz == write_size);
       if (sz > 0) {
         log_size_ += write_size;
       }

include/rocksdb/c.h

@@ -42,6 +42,9 @@
   (5) All of the pointer arguments must be non-NULL.
 */
 
+#ifndef STORAGE_ROCKSDB_INCLUDE_C_H_
+#define STORAGE_ROCKSDB_INCLUDE_C_H_
+
 #pragma once
 
 #ifdef _WIN32
@@ -123,8 +126,6 @@ typedef struct rocksdb_transaction_t rocksdb_transaction_t;
 typedef struct rocksdb_checkpoint_t rocksdb_checkpoint_t;
 typedef struct rocksdb_wal_iterator_t rocksdb_wal_iterator_t;
 typedef struct rocksdb_wal_readoptions_t rocksdb_wal_readoptions_t;
-typedef struct rocksdb_memory_consumers_t rocksdb_memory_consumers_t;
-typedef struct rocksdb_memory_usage_t rocksdb_memory_usage_t;
 
 /* DB operations */
 
@@ -1674,33 +1675,8 @@ extern ROCKSDB_LIBRARY_API void rocksdb_pinnableslice_destroy(
 extern ROCKSDB_LIBRARY_API const char* rocksdb_pinnableslice_value(
     const rocksdb_pinnableslice_t* t, size_t* vlen);
 
-extern ROCKSDB_LIBRARY_API rocksdb_memory_consumers_t*
-    rocksdb_memory_consumers_create();
-extern ROCKSDB_LIBRARY_API void rocksdb_memory_consumers_add_db(
-    rocksdb_memory_consumers_t* consumers, rocksdb_t* db);
-extern ROCKSDB_LIBRARY_API void rocksdb_memory_consumers_add_cache(
-    rocksdb_memory_consumers_t* consumers, rocksdb_cache_t* cache);
-extern ROCKSDB_LIBRARY_API void rocksdb_memory_consumers_destroy(
-    rocksdb_memory_consumers_t* consumers);
-extern ROCKSDB_LIBRARY_API rocksdb_memory_usage_t*
-rocksdb_approximate_memory_usage_create(rocksdb_memory_consumers_t* consumers,
-                                       char** errptr);
-extern ROCKSDB_LIBRARY_API void rocksdb_approximate_memory_usage_destroy(
-    rocksdb_memory_usage_t* usage);
-
-extern ROCKSDB_LIBRARY_API uint64_t
-rocksdb_approximate_memory_usage_get_mem_table_total(
-    rocksdb_memory_usage_t* memory_usage);
-extern ROCKSDB_LIBRARY_API uint64_t
-rocksdb_approximate_memory_usage_get_mem_table_unflushed(
-    rocksdb_memory_usage_t* memory_usage);
-extern ROCKSDB_LIBRARY_API uint64_t
-rocksdb_approximate_memory_usage_get_mem_table_readers_total(
-    rocksdb_memory_usage_t* memory_usage);
-extern ROCKSDB_LIBRARY_API uint64_t
-rocksdb_approximate_memory_usage_get_cache_total(
-    rocksdb_memory_usage_t* memory_usage);
-
 #ifdef __cplusplus
 }  /* end extern "C" */
 #endif
+
+#endif  /* STORAGE_ROCKSDB_INCLUDE_C_H_ */

include/rocksdb/cleanable.h

@@ -16,7 +16,8 @@
 // non-const method, all threads accessing the same Iterator must use
 // external synchronization.
 
-#pragma once
+#ifndef INCLUDE_ROCKSDB_CLEANABLE_H_
+#define INCLUDE_ROCKSDB_CLEANABLE_H_
 
 namespace rocksdb {
 
@@ -77,3 +78,5 @@ class Cleanable {
 };
 
 }  // namespace rocksdb
+
+#endif  // INCLUDE_ROCKSDB_CLEANABLE_H_

include/rocksdb/compaction_filter.h

@@ -6,7 +6,8 @@
 // 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
+#ifndef STORAGE_ROCKSDB_INCLUDE_COMPACTION_FILTER_H_
+#define STORAGE_ROCKSDB_INCLUDE_COMPACTION_FILTER_H_
 
 #include <cassert>
 #include <memory>
@@ -205,3 +206,5 @@ class CompactionFilterFactory {
 };
 
 }  // namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_COMPACTION_FILTER_H_

include/rocksdb/comparator.h

@@ -6,7 +6,8 @@
 // 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
+#ifndef STORAGE_ROCKSDB_INCLUDE_COMPARATOR_H_
+#define STORAGE_ROCKSDB_INCLUDE_COMPARATOR_H_
 
 #include <string>
 
@@ -91,3 +92,5 @@ extern const Comparator* BytewiseComparator();
 extern const Comparator* ReverseBytewiseComparator();
 
 }  // namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_COMPARATOR_H_

include/rocksdb/db.h

@@ -6,7 +6,8 @@
 // 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
+#ifndef STORAGE_ROCKSDB_INCLUDE_DB_H_
+#define STORAGE_ROCKSDB_INCLUDE_DB_H_
 
 #include <stdint.h>
 #include <stdio.h>
@@ -949,14 +950,14 @@ class DB {
   // GetLiveFiles followed by GetSortedWalFiles can generate a lossless backup
 
   // Retrieve the list of all files in the database. The files are
-  // relative to the dbname and are not absolute paths. Despite being relative
-  // paths, the file names begin with "/". The valid size of the manifest file
-  // is returned in manifest_file_size. The manifest file is an ever growing
-  // file, but only the portion specified by manifest_file_size is valid for
-  // this snapshot. Setting flush_memtable to true does Flush before recording
-  // the live files. Setting flush_memtable to false is useful when we don't
-  // want to wait for flush which may have to wait for compaction to complete
-  // taking an indeterminate time.
+  // relative to the dbname and are not absolute paths. The valid size of the
+  // manifest file is returned in manifest_file_size. The manifest file is an
+  // ever growing file, but only the portion specified by manifest_file_size is
+  // valid for this snapshot.
+  // Setting flush_memtable to true does Flush before recording the live files.
+  // Setting flush_memtable to false is useful when we don't want to wait for
+  // flush which may have to wait for compaction to complete taking an
+  // indeterminate time.
   //
   // In case you have multiple column families, even if flush_memtable is true,
   // you still need to call GetSortedWalFiles after GetLiveFiles to compensate
@@ -1220,3 +1221,5 @@ Status RepairDB(const std::string& dbname, const Options& options);
 #endif
 
 }  // namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_DB_H_

include/rocksdb/env.h

@@ -14,7 +14,8 @@
 // All Env implementations are safe for concurrent access from
 // multiple threads without any external synchronization.
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_INCLUDE_ENV_H_
+#define STORAGE_ROCKSDB_INCLUDE_ENV_H_
 
 #include <stdint.h>
 #include <cstdarg>
@@ -477,15 +478,6 @@ class Env {
   // Returns the ID of the current thread.
   virtual uint64_t GetThreadID() const;
 
-// This seems to clash with a macro on Windows, so #undef it here
-#undef GetFreeSpace
-
-  // Get the amount of free disk space
-  virtual Status GetFreeSpace(const std::string& /*path*/,
-                              uint64_t* /*diskfree*/) {
-    return Status::NotSupported();
-  }
-
  protected:
   // The pointer to an internal structure that will update the
   // status of each thread.
@@ -1275,3 +1267,5 @@ Status NewHdfsEnv(Env** hdfs_env, const std::string& fsname);
 Env* NewTimedEnv(Env* base_env);
 
 }  // namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_ENV_H_

include/rocksdb/filter_policy.h

@@ -17,7 +17,8 @@
 // Most people will want to use the builtin bloom filter support (see
 // NewBloomFilterPolicy() below).
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_INCLUDE_FILTER_POLICY_H_
+#define STORAGE_ROCKSDB_INCLUDE_FILTER_POLICY_H_
 
 #include <memory>
 #include <stdexcept>
@@ -148,3 +149,5 @@ class FilterPolicy {
 extern const FilterPolicy* NewBloomFilterPolicy(int bits_per_key,
     bool use_block_based_builder = true);
 }
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_FILTER_POLICY_H_

include/rocksdb/iterator.h

@@ -16,7 +16,8 @@
 // non-const method, all threads accessing the same Iterator must use
 // external synchronization.
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_INCLUDE_ITERATOR_H_
+#define STORAGE_ROCKSDB_INCLUDE_ITERATOR_H_
 
 #include <string>
 #include "rocksdb/cleanable.h"
@@ -118,3 +119,5 @@ extern Iterator* NewEmptyIterator();
 extern Iterator* NewErrorIterator(const Status& status);
 
 }  // namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_ITERATOR_H_

include/rocksdb/ldb_tool.h

@@ -2,8 +2,8 @@
 //  This source code is licensed under both the GPLv2 (found in the
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
-#pragma once
 #ifndef ROCKSDB_LITE
+#pragma once
 #include <string>
 #include <vector>
 #include "rocksdb/db.h"

include/rocksdb/listener.h

@@ -27,7 +27,6 @@ enum class TableFileCreationReason {
   kFlush,
   kCompaction,
   kRecovery,
-  kMisc,
 };
 
 struct TableFileCreationBriefInfo {
@@ -104,7 +103,6 @@ enum class FlushReason : int {
   kDeleteFiles = 0x08,
   kAutoCompaction = 0x09,
   kManualFlush = 0x0a,
-  kErrorRecovery = 0xb,
 };
 
 enum class BackgroundErrorReason {
@@ -395,21 +393,6 @@ class EventListener {
   // returns.  Otherwise, RocksDB may be blocked.
   virtual void OnStallConditionsChanged(const WriteStallInfo& /*info*/) {}
 
-  // A callback function for RocksDB which will be called just before
-  // starting the automatic recovery process for recoverable background
-  // errors, such as NoSpace(). The callback can suppress the automatic
-  // recovery by setting *auto_recovery to false. The database will then
-  // have to be transitioned out of read-only mode by calling DB::Resume()
-  virtual void OnErrorRecoveryBegin(BackgroundErrorReason /* reason */,
-                                    Status /* bg_error */,
-                                    bool* /* auto_recovery */) {}
-
-  // A callback function for RocksDB which will be called once the database
-  // is recovered from read-only mode after an error. When this is called, it
-  // means normal writes to the database can be issued and the user can
-  // initiate any further recovery actions needed
-  virtual void OnErrorRecoveryCompleted(Status /* old_bg_error */) {}
-
   virtual ~EventListener() {}
 };
 

include/rocksdb/memtablerep.h

@@ -144,14 +144,6 @@ class MemTableRep {
   // or any writes done directly to entries accessed through the iterator.)
   virtual void MarkReadOnly() { }
 
-  // Notify this table rep that it has been flushed to stable storage.
-  // By default, does nothing.
-  //
-  // Invariant: MarkReadOnly() is called, before MarkFlushed().
-  // Note that this method if overridden, should not run for an extended period
-  // of time. Otherwise, RocksDB may be blocked.
-  virtual void MarkFlushed() { }
-
   // Look up key from the mem table, since the first key in the mem table whose
   // user_key matches the one given k, call the function callback_func(), with
   // callback_args directly forwarded as the first parameter, and the mem table

include/rocksdb/merge_operator.h

@@ -3,7 +3,8 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_INCLUDE_MERGE_OPERATOR_H_
+#define STORAGE_ROCKSDB_INCLUDE_MERGE_OPERATOR_H_
 
 #include <deque>
 #include <memory>
@@ -240,3 +241,5 @@ class AssociativeMergeOperator : public MergeOperator {
 };
 
 }  // namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_MERGE_OPERATOR_H_

include/rocksdb/metadata.h

@@ -65,7 +65,7 @@ struct SstFileMetaData {
         num_reads_sampled(0),
         being_compacted(false) {}
   SstFileMetaData(const std::string& _file_name, const std::string& _path,
-                  size_t _size, SequenceNumber _smallest_seqno,
+                  uint64_t _size, SequenceNumber _smallest_seqno,
                   SequenceNumber _largest_seqno,
                   const std::string& _smallestkey,
                   const std::string& _largestkey, uint64_t _num_reads_sampled,
@@ -81,7 +81,7 @@ struct SstFileMetaData {
         being_compacted(_being_compacted) {}
 
   // File size in bytes.
-  size_t size;
+  uint64_t size;
   // The name of the file.
   std::string name;
   // The full path where the file locates.

include/rocksdb/options.h

@@ -6,7 +6,8 @@
 // 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
+#ifndef STORAGE_ROCKSDB_INCLUDE_OPTIONS_H_
+#define STORAGE_ROCKSDB_INCLUDE_OPTIONS_H_
 
 #include <stddef.h>
 #include <stdint.h>
@@ -1291,3 +1292,5 @@ struct IngestExternalFileOptions {
 struct TraceOptions {};
 
 }  // namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_OPTIONS_H_

include/rocksdb/perf_context.h

@@ -3,7 +3,8 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_INCLUDE_PERF_CONTEXT_H
+#define STORAGE_ROCKSDB_INCLUDE_PERF_CONTEXT_H
 
 #include <stdint.h>
 #include <string>
@@ -175,3 +176,5 @@ struct PerfContext {
 PerfContext* get_perf_context();
 
 }
+
+#endif

include/rocksdb/perf_level.h

@@ -3,7 +3,8 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
-#pragma once
+#ifndef INCLUDE_ROCKSDB_PERF_LEVEL_H_
+#define INCLUDE_ROCKSDB_PERF_LEVEL_H_
 
 #include <stdint.h>
 #include <string>
@@ -28,3 +29,5 @@ void SetPerfLevel(PerfLevel level);
 PerfLevel GetPerfLevel();
 
 }  // namespace rocksdb
+
+#endif  // INCLUDE_ROCKSDB_PERF_LEVEL_H_

include/rocksdb/slice.h

@@ -16,7 +16,8 @@
 // non-const method, all threads accessing the same Slice must use
 // external synchronization.
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_INCLUDE_SLICE_H_
+#define STORAGE_ROCKSDB_INCLUDE_SLICE_H_
 
 #include <assert.h>
 #include <cstdio>
@@ -47,7 +48,7 @@ class Slice {
 #ifdef __cpp_lib_string_view
   // Create a slice that refers to the same contents as "sv"
   /* implicit */
-  Slice(std::string_view sv) : data_(sv.data()), size_(sv.size()) {}
+  Slice(std::string_view sv) : data_(sv.data()), size_(sv.size()) { }
 #endif
 
   // Create a slice that refers to s[0,strlen(s)-1]
@@ -255,4 +256,6 @@ inline size_t Slice::difference_offset(const Slice& b) const {
   return off;
 }
 
-}  // namespace rocksdb
+}  // namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_SLICE_H_

include/rocksdb/slice_transform.h

@@ -12,7 +12,8 @@
 // define InDomain and InRange to determine which slices are in either
 // of these sets respectively.
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_INCLUDE_SLICE_TRANSFORM_H_
+#define STORAGE_ROCKSDB_INCLUDE_SLICE_TRANSFORM_H_
 
 #include <string>
 
@@ -99,3 +100,5 @@ extern const SliceTransform* NewCappedPrefixTransform(size_t cap_len);
 extern const SliceTransform* NewNoopTransform();
 
 }
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_SLICE_TRANSFORM_H_

include/rocksdb/sst_file_writer.h

@@ -3,10 +3,10 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
-#pragma once
-
 #ifndef ROCKSDB_LITE
 
+#pragma once
+
 #include <memory>
 #include <string>
 

include/rocksdb/statistics.h

@@ -3,7 +3,8 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_INCLUDE_STATISTICS_H_
+#define STORAGE_ROCKSDB_INCLUDE_STATISTICS_H_
 
 #include <atomic>
 #include <cstddef>
@@ -672,3 +673,5 @@ class Statistics {
 std::shared_ptr<Statistics> CreateDBStatistics();
 
 }  // namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_STATISTICS_H_

include/rocksdb/status.h

@@ -14,7 +14,8 @@
 // non-const method, all threads accessing the same Status must use
 // external synchronization.
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_INCLUDE_STATUS_H_
+#define STORAGE_ROCKSDB_INCLUDE_STATUS_H_
 
 #include <string>
 #include "rocksdb/slice.h"
@@ -347,3 +348,5 @@ inline bool Status::operator!=(const Status& rhs) const {
 }
 
 }  // namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_STATUS_H_

include/rocksdb/table.h

@@ -16,7 +16,6 @@
 //   https://github.com/facebook/rocksdb/wiki/A-Tutorial-of-RocksDB-SST-formats#wiki-examples
 
 #pragma once
-
 #include <memory>
 #include <string>
 #include <unordered_map>
@@ -239,12 +238,6 @@ struct BlockBasedTableOptions {
   // version 5.15, you should probably use this.
   // This option only affects newly written tables. When reading existing
   // tables, the information about version is read from the footer.
-  // 4 -- Can be read by RocksDB's versions since 5.16. Changes the way we
-  // encode the values in index blocks. If you don't plan to run RocksDB before
-  // version 5.16 and you are using index_block_restart_interval > 1, you should
-  // probably use this as it would reduce the index size.
-  // This option only affects newly written tables. When reading existing
-  // tables, the information about version is read from the footer.
   uint32_t format_version = 2;
 
   // Store index blocks on disk in compressed format. Changing this option to

include/rocksdb/transaction_log.h

@@ -3,7 +3,8 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_INCLUDE_TRANSACTION_LOG_ITERATOR_H_
+#define STORAGE_ROCKSDB_INCLUDE_TRANSACTION_LOG_ITERATOR_H_
 
 #include "rocksdb/status.h"
 #include "rocksdb/types.h"
@@ -120,3 +121,5 @@ class TransactionLogIterator {
   };
 };
 } //  namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_INCLUDE_TRANSACTION_LOG_ITERATOR_H_

include/rocksdb/types.h

@@ -3,7 +3,8 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_INCLUDE_TYPES_H_
+#define STORAGE_ROCKSDB_INCLUDE_TYPES_H_
 
 #include <stdint.h>
 #include "rocksdb/slice.h"
@@ -52,3 +53,5 @@ struct FullKey {
 bool ParseFullKey(const Slice& internal_key, FullKey* result);
 
 }  //  namespace rocksdb
+
+#endif //  STORAGE_ROCKSDB_INCLUDE_TYPES_H_

include/rocksdb/universal_compaction.h

@@ -3,7 +3,8 @@
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
 
-#pragma once
+#ifndef STORAGE_ROCKSDB_UNIVERSAL_COMPACTION_OPTIONS_H
+#define STORAGE_ROCKSDB_UNIVERSAL_COMPACTION_OPTIONS_H
 
 #include <stdint.h>
 #include <climits>
@@ -85,3 +86,5 @@ class CompactionOptionsUniversal {
 };
 
 }  // namespace rocksdb
+
+#endif  // STORAGE_ROCKSDB_UNIVERSAL_COMPACTION_OPTIONS_H

include/rocksdb/utilities/env_librados.h

@@ -2,8 +2,8 @@
 //  This source code is licensed under both the GPLv2 (found in the
 //  COPYING file in the root directory) and Apache 2.0 License
 //  (found in the LICENSE.Apache file in the root directory).
-
-#pragma once
+#ifndef ROCKSDB_UTILITIES_ENV_LIBRADOS_H
+#define ROCKSDB_UTILITIES_ENV_LIBRADOS_H
 
 #include <memory>
 #include <string>
@@ -173,3 +173,4 @@ class EnvLibrados : public EnvWrapper {
   friend class LibradosWritableFile;
 };
 }
+#endif

include/rocksdb/utilities/table_properties_collectors.h

@@ -5,60 +5,12 @@
 
 #pragma once
 #ifndef ROCKSDB_LITE
-#include <atomic>
 #include <memory>
 
 #include "rocksdb/table_properties.h"
 
 namespace rocksdb {
 
-// A factory of a table property collector that marks a SST
-// file as need-compaction when it observe at least "D" deletion
-// entries in any "N" consecutive entires.
-class CompactOnDeletionCollectorFactory
-    : public TablePropertiesCollectorFactory {
- public:
-  virtual ~CompactOnDeletionCollectorFactory() {}
-
-  virtual TablePropertiesCollector* CreateTablePropertiesCollector(
-      TablePropertiesCollectorFactory::Context context) override;
-
-  // Change the value of sliding_window_size "N"
-  // Setting it to 0 disables the delete triggered compaction
-  void SetWindowSize(size_t sliding_window_size) {
-    sliding_window_size_.store(sliding_window_size);
-  }
-
-  // Change the value of deletion_trigger "D"
-  void SetDeletionTrigger(size_t deletion_trigger) {
-    deletion_trigger_.store(deletion_trigger);
-  }
-
-  virtual const char* Name() const override {
-    return "CompactOnDeletionCollector";
-  }
-
- private:
-  friend std::shared_ptr<CompactOnDeletionCollectorFactory>
-    NewCompactOnDeletionCollectorFactory(
-        size_t sliding_window_size,
-        size_t deletion_trigger);
-  // A factory of a table property collector that marks a SST
-  // file as need-compaction when it observe at least "D" deletion
-  // entries in any "N" consecutive entires.
-  //
-  // @param sliding_window_size "N"
-  // @param deletion_trigger "D"
-  CompactOnDeletionCollectorFactory(
-      size_t sliding_window_size,
-      size_t deletion_trigger) :
-          sliding_window_size_(sliding_window_size),
-          deletion_trigger_(deletion_trigger) {}
-
-  std::atomic<size_t> sliding_window_size_;
-  std::atomic<size_t> deletion_trigger_;
-};
-
 // Creates a factory of a table property collector that marks a SST
 // file as need-compaction when it observe at least "D" deletion
 // entries in any "N" consecutive entires.
@@ -68,7 +20,7 @@ class CompactOnDeletionCollectorFactory
 //     than the specified size.
 // @param deletion_trigger "D".  Note that even when "N" is changed,
 //     the specified number for "D" will not be changed.
-extern std::shared_ptr<CompactOnDeletionCollectorFactory>
+extern std::shared_ptr<TablePropertiesCollectorFactory>
     NewCompactOnDeletionCollectorFactory(
         size_t sliding_window_size,
         size_t deletion_trigger);