remove warning as error.

This reverts commit 61720850f6.

.travis.yml

@@ -1,22 +1,17 @@
 sudo: false
-dist: trusty
 language: cpp
 os:
   - linux
   - osx
 compiler:
   - clang
-  - gcc
 jdk:
   - oraclejdk7
-cache:
-  - ccache
-  - apt
 
 addons:
    apt:
       sources: ['ubuntu-toolchain-r-test', 'llvm-toolchain-precise-3.6']
-      packages: ['clang-3.6' , 'g++-6', 'zlib1g-dev', 'libbz2-dev', 'libsnappy-dev', 'curl', 'libgflags-dev']
+      packages: ['clang-3.6' , 'zlib1g-dev', 'libbz2-dev', 'libsnappy-dev', 'curl', 'gflags']
 env:
   # Run all tests before db_block_cache_test (db_test, db_test2)
   - JOB_NAME=unittests ROCKSDBTESTS_END=db_block_cache_test
@@ -27,20 +22,17 @@ env:
   # Build ROCKSDB_LITE
   - JOB_NAME=lite_build
 
-matrix:
-  exclude:
-  - os: osx
-    compiler: gcc
+install:
+  # Build gflags
+  # TODO(noetzli): Remove when gflags available through Travis
+  - pushd /tmp/ && curl -L https://github.com/gflags/gflags/archive/v2.1.2.tar.gz -o gflags.tar.gz && tar xfz gflags.tar.gz && cd gflags-2.1.2 && cmake . && make && popd
 
 before_script:
-  - if [[ "${TRAVIS_OS_NAME}" == 'linux' && "${CXX}" == 'clang++' ]]; then CXX=clang++-3.6; fi
-  # test one linux g++ build with g++-6
-  - if [[ "${TRAVIS_OS_NAME}" == 'linux' && "${CXX}" == 'g++' && "${JOB_NAME}" == 'unittests' ]]; then CXX=g++-6; fi
+  - if [[ "${TRAVIS_OS_NAME}" == 'linux' ]]; then CXX=clang++-3.6; fi
   # Limit the maximum number of open file descriptors to 8192
   - ulimit -n 8192 || true
 
 script:
-  - ${CXX} --version
   - if [[ "${JOB_NAME}" == 'unittests' ]]; then OPT=-DTRAVIS V=1 make -j4 check_some; fi
   - if [[ "${JOB_NAME}" == 'java_test' ]]; then OPT=-DTRAVIS V=1 make clean jclean rocksdbjava jtest; fi
   - if [[ "${JOB_NAME}" == 'lite_build' ]]; then OPT="-DTRAVIS -DROCKSDB_LITE" V=1 make -j4 static_lib; fi

DEFAULT_OPTIONS_HISTORY.md

@@ -1,4 +1,8 @@
 # RocksDB default options change log
+## 5.0 (11/17/2016)
+* Options::allow_concurrent_memtable_write and Options::enable_write_thread_adaptive_yield are now true by default
+* Options.level0_stop_writes_trigger default value changes from 24 to 32.
+
 ## 4.8.0 (5/2/2016)
 * options.max_open_files changes from 5000 to -1. It improves performance, but users need to set file descriptor limit to be large enough and watch memory usage for index and bloom filters.
 * options.base_background_compactions changes from max_background_compactions to 1. When users set higher max_background_compactions but the write throughput is not high, the writes are less spiky to disks.

HISTORY.md

@@ -1,11 +1,9 @@
 # Rocksdb Change Log
-## 5.1.0 (01/13/2017)
-### Public API Change
-* Support dynamically change `delete_obsolete_files_period_micros` option via SetDBOptions().
-* Added EventListener::OnExternalFileIngested which will be called when IngestExternalFile() add a file successfully.
-* BackupEngine::Open and BackupEngineReadOnly::Open now always return error statuses matching those of the backup Env.
-
-### Bug Fixes
+## Unreleased
+### Bug fixes
+* Fix the data corruption bug in the case that concurrent memtable write is enabled and 2PC is used.
+* OptimizeForPointLookup() doesn't work with the default DB setting of allow_concurrent_memtable_write=true. Fix it.
+* Fix a 2PC related bug where WAL files size grow too large.
 * Fix the bug that if 2PC is enabled, checkpoints may loss some recent transactions.
 * When file copying is needed when creating checkpoints or bulk loading files, fsync the file after the file copying.
 

Makefile

@@ -6,9 +6,6 @@
 
 #-----------------------------------------------
 
-BASH_EXISTS := $(shell which bash)
-SHELL := $(shell which bash)
-
 CLEAN_FILES = # deliberately empty, so we can append below.
 CFLAGS += ${EXTRA_CFLAGS}
 CXXFLAGS += ${EXTRA_CXXFLAGS}
@@ -140,7 +137,7 @@ endif
 
 AM_LINK = $(AM_V_CCLD)$(CXX) $^ $(EXEC_LDFLAGS) -o $@ $(LDFLAGS) $(COVERAGEFLAGS)
 # detect what platform we're building on
-dummy := $(shell (export ROCKSDB_ROOT="$(CURDIR)"; export PORTABLE="$(PORTABLE)"; "$(CURDIR)/build_tools/build_detect_platform" "$(CURDIR)/make_config.mk"))
+dummy := $(shell (export ROCKSDB_ROOT="$(CURDIR)"; "$(CURDIR)/build_tools/build_detect_platform" "$(CURDIR)/make_config.mk"))
 # this file is generated by the previous line to set build flags and sources
 include make_config.mk
 CLEAN_FILES += make_config.mk
@@ -223,6 +220,7 @@ default: all
 WARNING_FLAGS = -W -Wextra -Wall -Wsign-compare -Wshadow \
   -Wno-unused-parameter
 
+
 ifdef LUA_PATH
 
 ifndef LUA_INCLUDE
@@ -319,7 +317,6 @@ TESTS = \
 	db_properties_test \
 	db_table_properties_test \
 	autovector_test \
-	cleanable_test \
 	column_family_test \
 	table_properties_collector_test \
 	arena_test \
@@ -414,7 +411,6 @@ TESTS = \
 	persistent_cache_test \
 	statistics_test \
 	lua_test \
-	range_del_aggregator_test \
 	lru_cache_test \
 
 PARALLEL_TEST = \
@@ -1143,9 +1139,6 @@ full_filter_block_test: table/full_filter_block_test.o $(LIBOBJECTS) $(TESTHARNE
 log_test: db/log_test.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)
 
-cleanable_test: table/cleanable_test.o $(LIBOBJECTS) $(TESTHARNESS)
-	$(AM_LINK)
-
 table_test: table/table_test.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)
 
@@ -1308,9 +1301,6 @@ lru_cache_test: util/lru_cache_test.o $(LIBOBJECTS) $(TESTHARNESS)
 lua_test: utilities/lua/rocks_lua_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)
 
-range_del_aggregator_test: db/range_del_aggregator_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS)
-	$(AM_LINK)
-
 #-------------------------------------------------
 # make install related stuff
 INSTALL_PATH ?= /usr/local

USERS.md

@@ -7,7 +7,7 @@ At Facebook, we use RocksDB as storage engines in multiple data management servi
 2. MongoRocks -- https://github.com/mongodb-partners/mongo-rocks
 3. ZippyDB --  Facebook's distributed key-value store with Paxos-style replication, built on top of RocksDB.[*] https://www.youtube.com/watch?v=DfiN7pG0D0khtt
 4. Laser -- Laser is a high query throughput, low (millisecond) latency, key-value storage service built on top of RocksDB.[*]
-4. Dragon -- a distributed graph query engine. https://code.facebook.com/posts/1737605303120405/dragon-a-distributed-graph-query-engine/
+4. Dragan -- a distributed graph query engine. https://code.facebook.com/posts/1737605303120405/dragon-a-distributed-graph-query-engine/
 5. Stylus -- a low-level stream processing framework writtenin C++.[*]
 
 [*] https://research.facebook.com/publications/realtime-data-processing-at-facebook/
@@ -64,7 +64,7 @@ Pinterest's Object Retrieval System uses RocksDB for storage: https://www.youtub
 quasardb uses a heavily tuned RocksDB as its persistence layer.
 
 ## Netflix
-[Netflix](http://techblog.netflix.com/2016/05/application-data-caching-using-ssds.html) Netflix uses RocksDB on AWS EC2 instances with local SSD drives to cache application data.
+[Netflix](http://techblog.netflix.com/2016/05/application-data-caching-using-ssds.html) uses RocksDB on spinning disks to cache application data.
 
 ## TiKV
 [TiKV](https://github.com/pingcap/tikv) is a GEO-replicated, high-performance, distributed, transactional key-value database. TiKV is powered by Rust and Raft. TiKV uses RocksDB as its persistence layer.
@@ -74,6 +74,3 @@ quasardb uses a heavily tuned RocksDB as its persistence layer.
 
 ## Dgraph
 [Dgraph](https://github.com/dgraph-io/dgraph) is an open-source, scalable, distributed, low latency, high throughput Graph database .They use RocksDB to store state locally on a machine.
-
-## Uber
-[Uber](http://eng.uber.com/cherami/) uses RocksDB as a durable and and scalable task queue.

arcanist_util/config/RocksDBCommonHelper.php

@@ -21,7 +21,7 @@ function postURL($diffID, $url) {
   assert(is_numeric($diffID));
   assert(strlen($url) > 0);
 
-  $cmd = 'echo \'{"diff_id": ' . $diffID . ', '
+  $cmd = 'echo \'{"diff_id": "' . $diffID . '", '
          . '"name":"click here for sandcastle tests for D' . $diffID . '", '
          . '"link":"' . $url . '"}\' | '
          . 'arc call-conduit '
@@ -35,7 +35,7 @@ function buildUpdateTestStatusCmd($diffID, $test, $status) {
   assert(strlen($test) > 0);
   assert(strlen($status) > 0);
 
-  $cmd = 'echo \'{"diff_id": ' . $diffID . ', '
+  $cmd = 'echo \'{"diff_id": "' . $diffID . '", '
          . '"name":"' . $test . '", '
          . '"result":"' . $status . '"}\' | '
          . 'arc call-conduit '

build_tools/cont_integration.sh

@@ -8,7 +8,7 @@
 # for a certain interval.
 #
 
-SRC_GIT_REPO=/data/git/rocksdb-public
+SRC_GIT_REPO=/data/git/rocksdb
 error=0
 
 function log {

build_tools/fbcode_config.sh

@@ -86,7 +86,7 @@ fi
 CFLAGS+=" -DTBB"
 
 # use Intel SSE support for checksum calculations
-export USE_SSE=" -msse -msse4.2 "
+export USE_SSE=1
 
 BINUTILS="$BINUTILS_BASE/bin"
 AR="$BINUTILS/ar"

build_tools/format-diff.sh

@@ -11,8 +11,7 @@ if ! which $CLANG_FORMAT_DIFF &> /dev/null
 then
   echo "You didn't have clang-format-diff.py available in your computer!"
   echo "You can download it by running: "
-  echo "    curl --location http://goo.gl/iUW1u2 -o ${CLANG_FORMAT_DIFF}"
-  echo "and move ${CLANG_FORMAT_DIFF} to some directory within PATH=${PATH}"
+  echo "    curl http://goo.gl/iUW1u2"
   exit 128
 fi
 
@@ -51,15 +50,14 @@ fi
 set -e
 
 uncommitted_code=`git diff HEAD`
-LAST_MASTER=`git merge-base master HEAD`
 
 # If there's no uncommitted changes, we assume user are doing post-commit
-# format check, in which case we'll check the modified lines since last commit
-# from master. Otherwise, we'll check format of the uncommitted code only.
+# format check, in which case we'll check the modified lines from latest commit.
+# Otherwise, we'll check format of the uncommitted code only.
 if [ -z "$uncommitted_code" ]
 then
   # Check the format of last commit
-  diffs=$(git diff -U0 $LAST_MASTER^ | $CLANG_FORMAT_DIFF -p 1)
+  diffs=$(git diff -U0 HEAD^ | $CLANG_FORMAT_DIFF -p 1)
 else
   # Check the format of uncommitted lines,
   diffs=$(git diff -U0 HEAD | $CLANG_FORMAT_DIFF -p 1)
@@ -99,12 +97,7 @@ then
 fi
 
 # Do in-place format adjustment.
-if [ -z "$uncommitted_code" ]
-then
-  git diff -U0 $LAST_MASTER^ | $CLANG_FORMAT_DIFF -i -p 1
-else
-  git diff -U0 HEAD^ | $CLANG_FORMAT_DIFF -i -p 1
-fi
+git diff -U0 HEAD^ | $CLANG_FORMAT_DIFF -i -p 1
 echo "Files reformatted!"
 
 # Amend to last commit if user do the post-commit format check

db/c.cc

@@ -104,9 +104,6 @@ struct rocksdb_readoptions_t {
 };
 struct rocksdb_writeoptions_t    { WriteOptions      rep; };
 struct rocksdb_options_t         { Options           rep; };
-struct rocksdb_compactoptions_t {
-  CompactRangeOptions rep;
-};
 struct rocksdb_block_based_table_options_t  { BlockBasedTableOptions rep; };
 struct rocksdb_cuckoo_table_options_t  { CuckooTableOptions rep; };
 struct rocksdb_seqfile_t         { SequentialFile*   rep; };
@@ -448,11 +445,7 @@ rocksdb_backup_engine_t* rocksdb_backup_engine_open(
     const rocksdb_options_t* options, const char* path, char** errptr) {
   BackupEngine* be;
   if (SaveError(errptr, BackupEngine::Open(options->rep.env,
-                                           BackupableDBOptions(path,
-                                                               nullptr,
-                                                               true,
-                                                               options->rep.info_log.get()),
-                                           &be))) {
+                                           BackupableDBOptions(path), &be))) {
     return nullptr;
   }
   rocksdb_backup_engine_t* result = new rocksdb_backup_engine_t;
@@ -983,30 +976,6 @@ void rocksdb_compact_range_cf(
       (limit_key ? (b = Slice(limit_key, limit_key_len), &b) : nullptr));
 }
 
-void rocksdb_compact_range_opt(rocksdb_t* db, rocksdb_compactoptions_t* opt,
-                               const char* start_key, size_t start_key_len,
-                               const char* limit_key, size_t limit_key_len) {
-  Slice a, b;
-  db->rep->CompactRange(
-      opt->rep,
-      // Pass nullptr Slice if corresponding "const char*" is nullptr
-      (start_key ? (a = Slice(start_key, start_key_len), &a) : nullptr),
-      (limit_key ? (b = Slice(limit_key, limit_key_len), &b) : nullptr));
-}
-
-void rocksdb_compact_range_cf_opt(rocksdb_t* db,
-                                  rocksdb_column_family_handle_t* column_family,
-                                  rocksdb_compactoptions_t* opt,
-                                  const char* start_key, size_t start_key_len,
-                                  const char* limit_key, size_t limit_key_len) {
-  Slice a, b;
-  db->rep->CompactRange(
-      opt->rep, column_family->rep,
-      // Pass nullptr Slice if corresponding "const char*" is nullptr
-      (start_key ? (a = Slice(start_key, start_key_len), &a) : nullptr),
-      (limit_key ? (b = Slice(limit_key, limit_key_len), &b) : nullptr));
-}
-
 void rocksdb_flush(
     rocksdb_t* db,
     const rocksdb_flushoptions_t* options,
@@ -1254,53 +1223,6 @@ void rocksdb_writebatch_deletev_cf(
   b->rep.Delete(column_family->rep, SliceParts(key_slices.data(), num_keys));
 }
 
-void rocksdb_writebatch_delete_range(rocksdb_writebatch_t* b,
-                                     const char* start_key,
-                                     size_t start_key_len, const char* end_key,
-                                     size_t end_key_len) {
-  b->rep.DeleteRange(Slice(start_key, start_key_len),
-                     Slice(end_key, end_key_len));
-}
-
-void rocksdb_writebatch_delete_range_cf(
-    rocksdb_writebatch_t* b, rocksdb_column_family_handle_t* column_family,
-    const char* start_key, size_t start_key_len, const char* end_key,
-    size_t end_key_len) {
-  b->rep.DeleteRange(column_family->rep, Slice(start_key, start_key_len),
-                     Slice(end_key, end_key_len));
-}
-
-void rocksdb_writebatch_delete_rangev(rocksdb_writebatch_t* b, int num_keys,
-                                      const char* const* start_keys_list,
-                                      const size_t* start_keys_list_sizes,
-                                      const char* const* end_keys_list,
-                                      const size_t* end_keys_list_sizes) {
-  std::vector<Slice> start_key_slices(num_keys);
-  std::vector<Slice> end_key_slices(num_keys);
-  for (int i = 0; i < num_keys; i++) {
-    start_key_slices[i] = Slice(start_keys_list[i], start_keys_list_sizes[i]);
-    end_key_slices[i] = Slice(end_keys_list[i], end_keys_list_sizes[i]);
-  }
-  b->rep.DeleteRange(SliceParts(start_key_slices.data(), num_keys),
-                     SliceParts(end_key_slices.data(), num_keys));
-}
-
-void rocksdb_writebatch_delete_rangev_cf(
-    rocksdb_writebatch_t* b, rocksdb_column_family_handle_t* column_family,
-    int num_keys, const char* const* start_keys_list,
-    const size_t* start_keys_list_sizes, const char* const* end_keys_list,
-    const size_t* end_keys_list_sizes) {
-  std::vector<Slice> start_key_slices(num_keys);
-  std::vector<Slice> end_key_slices(num_keys);
-  for (int i = 0; i < num_keys; i++) {
-    start_key_slices[i] = Slice(start_keys_list[i], start_keys_list_sizes[i]);
-    end_key_slices[i] = Slice(end_keys_list[i], end_keys_list_sizes[i]);
-  }
-  b->rep.DeleteRange(column_family->rep,
-                     SliceParts(start_key_slices.data(), num_keys),
-                     SliceParts(end_key_slices.data(), num_keys));
-}
-
 void rocksdb_writebatch_put_log_data(
     rocksdb_writebatch_t* b,
     const char* blob, size_t len) {
@@ -1723,14 +1645,9 @@ void rocksdb_options_set_manifest_preallocation_size(
 void rocksdb_options_set_purge_redundant_kvs_while_flush(rocksdb_options_t* opt,
                                                          unsigned char v) {}
 
-void rocksdb_options_set_use_direct_reads(rocksdb_options_t* opt,
-                                          unsigned char v) {
-  opt->rep.use_direct_reads = v;
-}
-
-void rocksdb_options_set_use_direct_writes(rocksdb_options_t* opt,
-                                           unsigned char v) {
-  opt->rep.use_direct_writes = v;
+void rocksdb_options_set_allow_os_buffer(rocksdb_options_t* opt,
+                                         unsigned char v) {
+  opt->rep.allow_os_buffer = v;
 }
 
 void rocksdb_options_set_allow_mmap_reads(
@@ -2009,7 +1926,7 @@ rocksdb_ratelimiter_t* rocksdb_ratelimiter_create(
 
 void rocksdb_ratelimiter_destroy(rocksdb_ratelimiter_t *limiter) {
   if (limiter->rep) {
-    delete limiter->rep;
+	delete limiter->rep;
   }
   delete limiter;
 }
@@ -2251,16 +2168,6 @@ void rocksdb_readoptions_set_readahead_size(
   opt->rep.readahead_size = v;
 }
 
-void rocksdb_readoptions_set_pin_data(rocksdb_readoptions_t* opt,
-                                      unsigned char v) {
-  opt->rep.pin_data = v;
-}
-
-void rocksdb_readoptions_set_total_order_seek(rocksdb_readoptions_t* opt,
-                                              unsigned char v) {
-  opt->rep.total_order_seek = v;
-}
-
 rocksdb_writeoptions_t* rocksdb_writeoptions_create() {
   return new rocksdb_writeoptions_t;
 }
@@ -2278,28 +2185,6 @@ void rocksdb_writeoptions_disable_WAL(rocksdb_writeoptions_t* opt, int disable)
   opt->rep.disableWAL = disable;
 }
 
-rocksdb_compactoptions_t* rocksdb_compactoptions_create() {
-  return new rocksdb_compactoptions_t;
-}
-
-void rocksdb_compactoptions_destroy(rocksdb_compactoptions_t* opt) {
-  delete opt;
-}
-
-void rocksdb_compactoptions_set_exclusive_manual_compaction(
-    rocksdb_compactoptions_t* opt, unsigned char v) {
-  opt->rep.exclusive_manual_compaction = v;
-}
-
-void rocksdb_compactoptions_set_change_level(rocksdb_compactoptions_t* opt,
-                                             unsigned char v) {
-  opt->rep.change_level = v;
-}
-
-void rocksdb_compactoptions_set_target_level(rocksdb_compactoptions_t* opt,
-                                             int n) {
-  opt->rep.target_level = n;
-}
 
 rocksdb_flushoptions_t* rocksdb_flushoptions_create() {
   return new rocksdb_flushoptions_t;
@@ -2328,14 +2213,6 @@ void rocksdb_cache_set_capacity(rocksdb_cache_t* cache, size_t capacity) {
   cache->rep->SetCapacity(capacity);
 }
 
-size_t rocksdb_cache_get_usage(rocksdb_cache_t* cache) {
-  return cache->rep->GetUsage();
-}
-
-size_t rocksdb_cache_get_pinned_usage(rocksdb_cache_t* cache) {
-  return cache->rep->GetPinnedUsage();
-}
-
 rocksdb_env_t* rocksdb_create_default_env() {
   rocksdb_env_t* result = new rocksdb_env_t;
   result->rep = Env::Default();

db/c_test.c

@@ -288,7 +288,6 @@ int main(int argc, char** argv) {
   rocksdb_cache_t* cache;
   rocksdb_env_t* env;
   rocksdb_options_t* options;
-  rocksdb_compactoptions_t* coptions;
   rocksdb_block_based_table_options_t* table_options;
   rocksdb_readoptions_t* roptions;
   rocksdb_writeoptions_t* woptions;
@@ -340,14 +339,11 @@ int main(int argc, char** argv) {
 
   roptions = rocksdb_readoptions_create();
   rocksdb_readoptions_set_verify_checksums(roptions, 1);
-  rocksdb_readoptions_set_fill_cache(roptions, 1);
+  rocksdb_readoptions_set_fill_cache(roptions, 0);
 
   woptions = rocksdb_writeoptions_create();
   rocksdb_writeoptions_set_sync(woptions, 1);
 
-  coptions = rocksdb_compactoptions_create();
-  rocksdb_compactoptions_set_exclusive_manual_compaction(coptions, 1);
-
   StartPhase("destroy");
   rocksdb_destroy_db(options, dbname, &err);
   Free(&err);
@@ -429,95 +425,6 @@ int main(int argc, char** argv) {
   rocksdb_compact_range(db, "a", 1, "z", 1);
   CheckGet(db, roptions, "foo", "hello");
 
-  StartPhase("compactallopt");
-  rocksdb_compact_range_opt(db, coptions, NULL, 0, NULL, 0);
-  CheckGet(db, roptions, "foo", "hello");
-
-  StartPhase("compactrangeopt");
-  rocksdb_compact_range_opt(db, coptions, "a", 1, "z", 1);
-  CheckGet(db, roptions, "foo", "hello");
-
-  // Simple check cache usage
-  StartPhase("cache_usage");
-  {
-    rocksdb_readoptions_set_pin_data(roptions, 1);
-    rocksdb_iterator_t* iter = rocksdb_create_iterator(db, roptions);
-    rocksdb_iter_seek(iter, "foo", 3);
-
-    size_t usage = rocksdb_cache_get_usage(cache);
-    CheckCondition(usage > 0);
-
-    size_t pin_usage = rocksdb_cache_get_pinned_usage(cache);
-    CheckCondition(pin_usage > 0);
-
-    rocksdb_iter_next(iter);
-    rocksdb_iter_destroy(iter);
-    rocksdb_readoptions_set_pin_data(roptions, 0);
-  }
-
-  StartPhase("addfile");
-  {
-    rocksdb_envoptions_t* env_opt = rocksdb_envoptions_create();
-    rocksdb_options_t* io_options = rocksdb_options_create();
-    rocksdb_sstfilewriter_t* writer =
-        rocksdb_sstfilewriter_create(env_opt, io_options);
-
-    unlink(sstfilename);
-    rocksdb_sstfilewriter_open(writer, sstfilename, &err);
-    CheckNoError(err);
-    rocksdb_sstfilewriter_add(writer, "sstk1", 5, "v1", 2, &err);
-    CheckNoError(err);
-    rocksdb_sstfilewriter_add(writer, "sstk2", 5, "v2", 2, &err);
-    CheckNoError(err);
-    rocksdb_sstfilewriter_add(writer, "sstk3", 5, "v3", 2, &err);
-    CheckNoError(err);
-    rocksdb_sstfilewriter_finish(writer, &err);
-    CheckNoError(err);
-
-    rocksdb_ingestexternalfileoptions_t* ing_opt =
-        rocksdb_ingestexternalfileoptions_create();
-    const char* file_list[1] = {sstfilename};
-    rocksdb_ingest_external_file(db, file_list, 1, ing_opt, &err);
-    CheckNoError(err);
-    CheckGet(db, roptions, "sstk1", "v1");
-    CheckGet(db, roptions, "sstk2", "v2");
-    CheckGet(db, roptions, "sstk3", "v3");
-
-    unlink(sstfilename);
-    rocksdb_sstfilewriter_open(writer, sstfilename, &err);
-    CheckNoError(err);
-    rocksdb_sstfilewriter_add(writer, "sstk2", 5, "v4", 2, &err);
-    CheckNoError(err);
-    rocksdb_sstfilewriter_add(writer, "sstk22", 6, "v5", 2, &err);
-    CheckNoError(err);
-    rocksdb_sstfilewriter_add(writer, "sstk3", 5, "v6", 2, &err);
-    CheckNoError(err);
-    rocksdb_sstfilewriter_finish(writer, &err);
-    CheckNoError(err);
-
-    rocksdb_ingest_external_file(db, file_list, 1, ing_opt, &err);
-    CheckNoError(err);
-    CheckGet(db, roptions, "sstk1", "v1");
-    CheckGet(db, roptions, "sstk2", "v4");
-    CheckGet(db, roptions, "sstk22", "v5");
-    CheckGet(db, roptions, "sstk3", "v6");
-
-    rocksdb_ingestexternalfileoptions_destroy(ing_opt);
-    rocksdb_sstfilewriter_destroy(writer);
-    rocksdb_options_destroy(io_options);
-    rocksdb_envoptions_destroy(env_opt);
-
-    // Delete all keys we just ingested
-    rocksdb_delete(db, woptions, "sstk1", 5, &err);
-    CheckNoError(err);
-    rocksdb_delete(db, woptions, "sstk2", 5, &err);
-    CheckNoError(err);
-    rocksdb_delete(db, woptions, "sstk22", 6, &err);
-    CheckNoError(err);
-    rocksdb_delete(db, woptions, "sstk3", 5, &err);
-    CheckNoError(err);
-  }
-
   StartPhase("writebatch");
   {
     rocksdb_writebatch_t* wb = rocksdb_writebatch_create();
@@ -534,24 +441,6 @@ int main(int argc, char** argv) {
     int pos = 0;
     rocksdb_writebatch_iterate(wb, &pos, CheckPut, CheckDel);
     CheckCondition(pos == 3);
-    rocksdb_writebatch_clear(wb);
-    rocksdb_writebatch_put(wb, "bar", 3, "b", 1);
-    rocksdb_writebatch_put(wb, "bay", 3, "d", 1);
-    rocksdb_writebatch_delete_range(wb, "bar", 3, "bay", 3);
-    rocksdb_write(db, woptions, wb, &err);
-    CheckNoError(err);
-    CheckGet(db, roptions, "bar", NULL);
-    CheckGet(db, roptions, "bay", "d");
-    rocksdb_writebatch_clear(wb);
-    const char* start_list[1] = {"bay"};
-    const size_t start_sizes[1] = {3};
-    const char* end_list[1] = {"baz"};
-    const size_t end_sizes[1] = {3};
-    rocksdb_writebatch_delete_rangev(wb, 1, start_list, start_sizes, end_list,
-                                     end_sizes);
-    rocksdb_write(db, woptions, wb, &err);
-    CheckNoError(err);
-    CheckGet(db, roptions, "bay", NULL);
     rocksdb_writebatch_destroy(wb);
   }
 
@@ -690,6 +579,59 @@ int main(int argc, char** argv) {
     rocksdb_release_snapshot(db, snap);
   }
 
+  StartPhase("addfile");
+  {
+    rocksdb_envoptions_t* env_opt = rocksdb_envoptions_create();
+    rocksdb_options_t* io_options = rocksdb_options_create();
+    rocksdb_sstfilewriter_t* writer =
+        rocksdb_sstfilewriter_create(env_opt, io_options);
+
+    unlink(sstfilename);
+    rocksdb_sstfilewriter_open(writer, sstfilename, &err);
+    CheckNoError(err);
+    rocksdb_sstfilewriter_add(writer, "sstk1", 5, "v1", 2, &err);
+    CheckNoError(err);
+    rocksdb_sstfilewriter_add(writer, "sstk2", 5, "v2", 2, &err);
+    CheckNoError(err);
+    rocksdb_sstfilewriter_add(writer, "sstk3", 5, "v3", 2, &err);
+    CheckNoError(err);
+    rocksdb_sstfilewriter_finish(writer, &err);
+    CheckNoError(err);
+
+    rocksdb_ingestexternalfileoptions_t* ing_opt =
+        rocksdb_ingestexternalfileoptions_create();
+    const char* file_list[1] = {sstfilename};
+    rocksdb_ingest_external_file(db, file_list, 1, ing_opt, &err);
+    CheckNoError(err);
+    CheckGet(db, roptions, "sstk1", "v1");
+    CheckGet(db, roptions, "sstk2", "v2");
+    CheckGet(db, roptions, "sstk3", "v3");
+
+    unlink(sstfilename);
+    rocksdb_sstfilewriter_open(writer, sstfilename, &err);
+    CheckNoError(err);
+    rocksdb_sstfilewriter_add(writer, "sstk2", 5, "v4", 2, &err);
+    CheckNoError(err);
+    rocksdb_sstfilewriter_add(writer, "sstk22", 6, "v5", 2, &err);
+    CheckNoError(err);
+    rocksdb_sstfilewriter_add(writer, "sstk3", 5, "v6", 2, &err);
+    CheckNoError(err);
+    rocksdb_sstfilewriter_finish(writer, &err);
+    CheckNoError(err);
+
+    rocksdb_ingest_external_file(db, file_list, 1, ing_opt, &err);
+    CheckNoError(err);
+    CheckGet(db, roptions, "sstk1", "v1");
+    CheckGet(db, roptions, "sstk2", "v4");
+    CheckGet(db, roptions, "sstk22", "v5");
+    CheckGet(db, roptions, "sstk3", "v6");
+
+    rocksdb_ingestexternalfileoptions_destroy(ing_opt);
+    rocksdb_sstfilewriter_destroy(writer);
+    rocksdb_options_destroy(io_options);
+    rocksdb_envoptions_destroy(env_opt);
+  }
+
   StartPhase("repair");
   {
     // If we do not compact here, then the lazy deletion of
@@ -823,9 +765,9 @@ int main(int argc, char** argv) {
   {
     rocksdb_close(db);
     rocksdb_destroy_db(options, dbname, &err);
-    CheckNoError(err);
+    CheckNoError(err)
 
-    rocksdb_options_t* db_options = rocksdb_options_create();
+        rocksdb_options_t* db_options = rocksdb_options_create();
     rocksdb_options_set_create_if_missing(db_options, 1);
     db = rocksdb_open(db_options, dbname, &err);
     CheckNoError(err)
@@ -989,19 +931,6 @@ int main(int argc, char** argv) {
     CheckNoError(err);
     rocksdb_iter_destroy(iter);
 
-    rocksdb_readoptions_set_total_order_seek(roptions, 1);
-    iter = rocksdb_create_iterator(db, roptions);
-    CheckCondition(!rocksdb_iter_valid(iter));
-
-    rocksdb_iter_seek(iter, "ba", 2);
-    rocksdb_iter_get_error(iter, &err);
-    CheckNoError(err);
-    CheckCondition(rocksdb_iter_valid(iter));
-    CheckIter(iter, "bar1", "bar");
-
-    rocksdb_iter_destroy(iter);
-    rocksdb_readoptions_set_total_order_seek(roptions, 0);
-
     rocksdb_close(db);
     rocksdb_destroy_db(options, dbname, &err);
   }
@@ -1111,7 +1040,6 @@ int main(int argc, char** argv) {
   rocksdb_block_based_options_destroy(table_options);
   rocksdb_readoptions_destroy(roptions);
   rocksdb_writeoptions_destroy(woptions);
-  rocksdb_compactoptions_destroy(coptions);
   rocksdb_cache_destroy(cache);
   rocksdb_comparator_destroy(cmp);
   rocksdb_env_destroy(env);

db/compact_files_test.cc

@@ -253,61 +253,6 @@ TEST_F(CompactFilesTest, CapturingPendingFiles) {
   delete db;
 }
 
-TEST_F(CompactFilesTest, CompactionFilterWithGetSv) {
-  class FilterWithGet : public CompactionFilter {
-   public:
-    virtual bool Filter(int level, const Slice& key, const Slice& value,
-                        std::string* new_value,
-                        bool* value_changed) const override {
-      if (db_ == nullptr) {
-        return true;
-      }
-      std::string res;
-      db_->Get(ReadOptions(), "", &res);
-      return true;
-    }
-
-    void SetDB(DB* db) {
-      db_ = db;
-    }
-
-    virtual const char* Name() const override { return "FilterWithGet"; }
-
-   private:
-    DB* db_;
-  };
-
-
-  std::shared_ptr<FilterWithGet> cf(new FilterWithGet());
-
-  Options options;
-  options.create_if_missing = true;
-  options.compaction_filter = cf.get();
-
-  DB* db = nullptr;
-  DestroyDB(db_name_, options);
-  Status s = DB::Open(options, db_name_, &db);
-  ASSERT_OK(s);
-
-  cf->SetDB(db);
-
-  // Write one L0 file
-  db->Put(WriteOptions(), "K1", "V1");
-  db->Flush(FlushOptions());
-
-  // Compact all L0 files using CompactFiles
-  rocksdb::ColumnFamilyMetaData meta;
-  db->GetColumnFamilyMetaData(&meta);
-  for (auto& file : meta.levels[0].files) {
-    std::string fname = file.db_path + "/" + file.name;
-    ASSERT_OK(
-        db->CompactFiles(rocksdb::CompactionOptions(), {fname}, 0));
-  }
-
-
-  delete db;
-}
-
 }  // namespace rocksdb
 
 int main(int argc, char** argv) {

db/compaction.cc

@@ -150,8 +150,7 @@ Compaction::Compaction(VersionStorageInfo* vstorage,
                        bool _manual_compaction, double _score,
                        bool _deletion_compaction,
                        CompactionReason _compaction_reason)
-    : input_vstorage_(vstorage),
-      start_level_(_inputs[0].level),
+    : start_level_(_inputs[0].level),
       output_level_(_output_level),
       max_output_file_size_(_target_file_size),
       max_compaction_bytes_(_max_compaction_bytes),
@@ -205,10 +204,11 @@ Compaction::~Compaction() {
 }
 
 bool Compaction::InputCompressionMatchesOutput() const {
-  int base_level = input_vstorage_->base_level();
-  bool matches = (GetCompressionType(immutable_cf_options_, input_vstorage_,
-                                     mutable_cf_options_, start_level_,
-                                     base_level) == output_compression_);
+  VersionStorageInfo* vstorage = input_version_->storage_info();
+  int base_level = vstorage->base_level();
+  bool matches =
+      (GetCompressionType(*cfd_->ioptions(), vstorage, mutable_cf_options_,
+                          start_level_, base_level) == output_compression_);
   if (matches) {
     TEST_SYNC_POINT("Compaction::InputCompressionMatchesOutput:Matches");
     return true;
@@ -225,14 +225,15 @@ bool Compaction::IsTrivialMove() const {
   // filter to be applied to that level, and thus cannot be a trivial move.
 
   // Check if start level have files with overlapping ranges
-  if (start_level_ == 0 && input_vstorage_->level0_non_overlapping() == false) {
+  if (start_level_ == 0 &&
+      input_version_->storage_info()->level0_non_overlapping() == false) {
     // We cannot move files from L0 to L1 if the files are overlapping
     return false;
   }
 
   if (is_manual_compaction_ &&
-      (immutable_cf_options_.compaction_filter != nullptr ||
-       immutable_cf_options_.compaction_filter_factory != nullptr)) {
+      (cfd_->ioptions()->compaction_filter != nullptr ||
+       cfd_->ioptions()->compaction_filter_factory != nullptr)) {
     // This is a manual compaction and we have a compaction filter that should
     // be executed, we cannot do a trivial move
     return false;
@@ -240,34 +241,15 @@ bool Compaction::IsTrivialMove() const {
 
   // Used in universal compaction, where trivial move can be done if the
   // input files are non overlapping
-  if ((immutable_cf_options_.compaction_options_universal.allow_trivial_move) &&
+  if ((cfd_->ioptions()->compaction_options_universal.allow_trivial_move) &&
       (output_level_ != 0)) {
     return is_trivial_move_;
   }
 
-  if (!(start_level_ != output_level_ && num_input_levels() == 1 &&
+  return (start_level_ != output_level_ && num_input_levels() == 1 &&
           input(0, 0)->fd.GetPathId() == output_path_id() &&
-          InputCompressionMatchesOutput())) {
-    return false;
-  }
-
-  // assert inputs_.size() == 1
-
-  for (const auto& file : inputs_.front().files) {
-    std::vector<FileMetaData*> file_grand_parents;
-    if (output_level_ + 1 >= number_levels_) {
-      continue;
-    }
-    input_vstorage_->GetOverlappingInputs(output_level_ + 1, &file->smallest,
-                                          &file->largest, &file_grand_parents);
-    const auto compaction_size =
-        file->fd.GetFileSize() + TotalFileSize(file_grand_parents);
-    if (compaction_size > max_compaction_bytes_) {
-      return false;
-    }
-  }
-
-  return true;
+          InputCompressionMatchesOutput() &&
+          TotalFileSize(grandparents_) <= max_compaction_bytes_);
 }
 
 void Compaction::AddInputDeletions(VersionEdit* out_edit) {
@@ -290,7 +272,8 @@ bool Compaction::KeyNotExistsBeyondOutputLevel(
   // Maybe use binary search to find right entry instead of linear search?
   const Comparator* user_cmp = cfd_->user_comparator();
   for (int lvl = output_level_ + 1; lvl < number_levels_; lvl++) {
-    const std::vector<FileMetaData*>& files = input_vstorage_->LevelFiles(lvl);
+    const std::vector<FileMetaData*>& files =
+        input_version_->storage_info()->LevelFiles(lvl);
     for (; level_ptrs->at(lvl) < files.size(); level_ptrs->at(lvl)++) {
       auto* f = files[level_ptrs->at(lvl)];
       if (user_cmp->Compare(user_key, f->largest.user_key()) <= 0) {
@@ -362,7 +345,7 @@ void Compaction::ReleaseCompactionFiles(Status status) {
 
 void Compaction::ResetNextCompactionIndex() {
   assert(input_version_ != nullptr);
-  input_vstorage_->ResetNextCompactionIndex(start_level_);
+  input_version_->storage_info()->ResetNextCompactionIndex(start_level_);
 }
 
 namespace {

db/compaction.h

@@ -258,8 +258,6 @@ class Compaction {
   static bool IsFullCompaction(VersionStorageInfo* vstorage,
                                const std::vector<CompactionInputFiles>& inputs);
 
-  VersionStorageInfo* input_vstorage_;
-
   const int start_level_;    // the lowest level to be compacted
   const int output_level_;  // levels to which output files are stored
   uint64_t max_output_file_size_;

db/compaction_iterator.cc

@@ -16,14 +16,14 @@ CompactionIterator::CompactionIterator(
     SequenceNumber earliest_write_conflict_snapshot, Env* env,
     bool expect_valid_internal_key, RangeDelAggregator* range_del_agg,
     const Compaction* compaction, const CompactionFilter* compaction_filter,
-    const std::atomic<bool>* shutting_down)
+    LogBuffer* log_buffer)
     : CompactionIterator(
           input, cmp, merge_helper, last_sequence, snapshots,
           earliest_write_conflict_snapshot, env, expect_valid_internal_key,
           range_del_agg,
           std::unique_ptr<CompactionProxy>(
               compaction ? new CompactionProxy(compaction) : nullptr),
-          compaction_filter, shutting_down) {}
+          compaction_filter, log_buffer) {}
 
 CompactionIterator::CompactionIterator(
     InternalIterator* input, const Comparator* cmp, MergeHelper* merge_helper,
@@ -31,8 +31,7 @@ CompactionIterator::CompactionIterator(
     SequenceNumber earliest_write_conflict_snapshot, Env* env,
     bool expect_valid_internal_key, RangeDelAggregator* range_del_agg,
     std::unique_ptr<CompactionProxy> compaction,
-    const CompactionFilter* compaction_filter,
-    const std::atomic<bool>* shutting_down)
+    const CompactionFilter* compaction_filter, LogBuffer* log_buffer)
     : input_(input),
       cmp_(cmp),
       merge_helper_(merge_helper),
@@ -43,7 +42,7 @@ CompactionIterator::CompactionIterator(
       range_del_agg_(range_del_agg),
       compaction_(std::move(compaction)),
       compaction_filter_(compaction_filter),
-      shutting_down_(shutting_down),
+      log_buffer_(log_buffer),
       merge_out_iter_(merge_helper_) {
   assert(compaction_filter_ == nullptr || compaction_ != nullptr);
   bottommost_level_ =
@@ -137,7 +136,7 @@ void CompactionIterator::NextFromInput() {
   at_next_ = false;
   valid_ = false;
 
-  while (!valid_ && input_->Valid() && !IsShuttingDown()) {
+  while (!valid_ && input_->Valid()) {
     key_ = input_->key();
     value_ = input_->value();
     iter_stats_.num_input_records++;
@@ -218,8 +217,7 @@ void CompactionIterator::NextFromInput() {
         }
 
         if (filter == CompactionFilter::Decision::kRemove) {
-          // convert the current key to a delete; key_ is pointing into
-          // current_key_ at this point, so updating current_key_ updates key()
+          // convert the current key to a delete
           ikey_.type = kTypeDeletion;
           current_key_.UpdateInternalKey(ikey_.sequence, kTypeDeletion);
           // no value associated with delete
@@ -424,6 +422,7 @@ void CompactionIterator::NextFromInput() {
       input_->Next();
     } else if (ikey_.type == kTypeMerge) {
       if (!merge_helper_->HasOperator()) {
+        LogToBuffer(log_buffer_, "Options::merge_operator is null.");
         status_ = Status::InvalidArgument(
             "merge_operator is not properly initialized.");
         return;
@@ -434,14 +433,11 @@ void CompactionIterator::NextFromInput() {
       // have hit (A)
       // We encapsulate the merge related state machine in a different
       // object to minimize change to the existing flow.
-      Status s = merge_helper_->MergeUntil(input_, range_del_agg_,
-                                           prev_snapshot, bottommost_level_);
+      merge_helper_->MergeUntil(input_, range_del_agg_, prev_snapshot,
+                                bottommost_level_);
       merge_out_iter_.SeekToFirst();
 
-      if (!s.ok() && !s.IsMergeInProgress()) {
-        status_ = s;
-        return;
-      } else if (merge_out_iter_.Valid()) {
+      if (merge_out_iter_.Valid()) {
         // NOTE: key, value, and ikey_ refer to old entries.
         //       These will be correctly set below.
         key_ = merge_out_iter_.key();
@@ -470,8 +466,7 @@ void CompactionIterator::NextFromInput() {
     } else {
       // 1. new user key -OR-
       // 2. different snapshot stripe
-      bool should_delete = range_del_agg_->ShouldDelete(
-          key_, RangeDelAggregator::RangePositioningMode::kForwardTraversal);
+      bool should_delete = range_del_agg_->ShouldDelete(key_);
       if (should_delete) {
         ++iter_stats_.num_record_drop_hidden;
         ++iter_stats_.num_record_drop_range_del;
@@ -485,10 +480,6 @@ void CompactionIterator::NextFromInput() {
       input_->Seek(skip_until);
     }
   }
-
-  if (!valid_ && IsShuttingDown()) {
-    status_ = Status::ShutdownInProgress();
-  }
 }
 
 void CompactionIterator::PrepareOutput() {

db/compaction_iterator.h

@@ -17,6 +17,7 @@
 #include "db/pinned_iterators_manager.h"
 #include "db/range_del_aggregator.h"
 #include "rocksdb/compaction_filter.h"
+#include "util/log_buffer.h"
 
 namespace rocksdb {
 
@@ -60,7 +61,7 @@ class CompactionIterator {
                      RangeDelAggregator* range_del_agg,
                      const Compaction* compaction = nullptr,
                      const CompactionFilter* compaction_filter = nullptr,
-                     const std::atomic<bool>* shutting_down = nullptr);
+                     LogBuffer* log_buffer = nullptr);
 
   // Constructor with custom CompactionProxy, used for tests.
   CompactionIterator(InternalIterator* input, const Comparator* cmp,
@@ -71,7 +72,7 @@ class CompactionIterator {
                      RangeDelAggregator* range_del_agg,
                      std::unique_ptr<CompactionProxy> compaction,
                      const CompactionFilter* compaction_filter = nullptr,
-                     const std::atomic<bool>* shutting_down = nullptr);
+                     LogBuffer* log_buffer = nullptr);
 
   ~CompactionIterator();
 
@@ -124,7 +125,7 @@ class CompactionIterator {
   RangeDelAggregator* range_del_agg_;
   std::unique_ptr<CompactionProxy> compaction_;
   const CompactionFilter* compaction_filter_;
-  const std::atomic<bool>* shutting_down_;
+  LogBuffer* log_buffer_;
   bool bottommost_level_;
   bool valid_ = false;
   bool visible_at_tip_;
@@ -179,10 +180,5 @@ class CompactionIterator {
   // is in or beyond the last file checked during the previous call
   std::vector<size_t> level_ptrs_;
   CompactionIterationStats iter_stats_;
-
-  bool IsShuttingDown() {
-    // This is a best-effort facility, so memory_order_relaxed is sufficient.
-    return shutting_down_ && shutting_down_->load(std::memory_order_relaxed);
-  }
 };
 }  // namespace rocksdb

db/compaction_iterator_test.cc

@@ -13,64 +13,6 @@
 
 namespace rocksdb {
 
-// Expects no merging attempts.
-class NoMergingMergeOp : public MergeOperator {
- public:
-  bool FullMergeV2(const MergeOperationInput& merge_in,
-                   MergeOperationOutput* merge_out) const override {
-    ADD_FAILURE();
-    return false;
-  }
-  bool PartialMergeMulti(const Slice& key,
-                         const std::deque<Slice>& operand_list,
-                         std::string* new_value,
-                         Logger* logger) const override {
-    ADD_FAILURE();
-    return false;
-  }
-  const char* Name() const override {
-    return "CompactionIteratorTest NoMergingMergeOp";
-  }
-};
-
-// Compaction filter that gets stuck when it sees a particular key,
-// then gets unstuck when told to.
-// Always returns Decition::kRemove.
-class StallingFilter : public CompactionFilter {
- public:
-  virtual Decision FilterV2(int level, const Slice& key, ValueType t,
-                            const Slice& existing_value, std::string* new_value,
-                            std::string* skip_until) const override {
-    int k = std::atoi(key.ToString().c_str());
-    last_seen.store(k);
-    while (k >= stall_at.load()) {
-      std::this_thread::yield();
-    }
-    return Decision::kRemove;
-  }
-
-  const char* Name() const override {
-    return "CompactionIteratorTest StallingFilter";
-  }
-
-  // Wait until the filter sees a key >= k and stalls at that key.
-  // If `exact`, asserts that the seen key is equal to k.
-  void WaitForStall(int k, bool exact = true) {
-    stall_at.store(k);
-    while (last_seen.load() < k) {
-      std::this_thread::yield();
-    }
-    if (exact) {
-      EXPECT_EQ(k, last_seen.load());
-    }
-  }
-
-  // Filter will stall on key >= stall_at. Advance stall_at to unstall.
-  mutable std::atomic<int> stall_at{0};
-  // Last key the filter was called with.
-  mutable std::atomic<int> last_seen{0};
-};
-
 class LoggingForwardVectorIterator : public InternalIterator {
  public:
   struct Action {
@@ -146,15 +88,13 @@ class FakeCompaction : public CompactionIterator::CompactionProxy {
   virtual int level(size_t compaction_input_level) const { return 0; }
   virtual bool KeyNotExistsBeyondOutputLevel(
       const Slice& user_key, std::vector<size_t>* level_ptrs) const {
-    return key_not_exists_beyond_output_level;
+    return false;
   }
   virtual bool bottommost_level() const { return false; }
   virtual int number_levels() const { return 1; }
   virtual Slice GetLargestUserKey() const {
     return "\xff\xff\xff\xff\xff\xff\xff\xff\xff";
   }
-
-  bool key_not_exists_beyond_output_level = false;
 };
 
 class CompactionIteratorTest : public testing::Test {
@@ -176,19 +116,17 @@ class CompactionIteratorTest : public testing::Test {
 
     std::unique_ptr<CompactionIterator::CompactionProxy> compaction;
     if (filter) {
-      compaction_proxy_ = new FakeCompaction();
-      compaction.reset(compaction_proxy_);
+      compaction.reset(new FakeCompaction());
     }
 
     merge_helper_.reset(new MergeHelper(Env::Default(), cmp_, merge_op, filter,
-                                        nullptr, 0U, false, 0, 0, nullptr,
-                                        &shutting_down_));
+                                        nullptr, 0U, false, 0));
     iter_.reset(new LoggingForwardVectorIterator(ks, vs));
     iter_->SeekToFirst();
     c_iter_.reset(new CompactionIterator(
         iter_.get(), cmp_, merge_helper_.get(), last_sequence, &snapshots_,
         kMaxSequenceNumber, Env::Default(), false, range_del_agg_.get(),
-        std::move(compaction), filter, &shutting_down_));
+        std::move(compaction), filter));
   }
 
   void AddSnapshot(SequenceNumber snapshot) { snapshots_.push_back(snapshot); }
@@ -200,8 +138,6 @@ class CompactionIteratorTest : public testing::Test {
   std::unique_ptr<LoggingForwardVectorIterator> iter_;
   std::unique_ptr<CompactionIterator> c_iter_;
   std::unique_ptr<RangeDelAggregator> range_del_agg_;
-  std::atomic<bool> shutting_down_{false};
-  FakeCompaction* compaction_proxy_;
 };
 
 // It is possible that the output of the compaction iterator is empty even if
@@ -273,6 +209,26 @@ TEST_F(CompactionIteratorTest, RangeDeletionWithSnapshots) {
 }
 
 TEST_F(CompactionIteratorTest, CompactionFilterSkipUntil) {
+  // Expect no merging attempts.
+  class MergeOp : public MergeOperator {
+   public:
+    bool FullMergeV2(const MergeOperationInput& merge_in,
+                     MergeOperationOutput* merge_out) const override {
+      ADD_FAILURE();
+      return false;
+    }
+    bool PartialMergeMulti(const Slice& key,
+                           const std::deque<Slice>& operand_list,
+                           std::string* new_value,
+                           Logger* logger) const override {
+      ADD_FAILURE();
+      return false;
+    }
+    const char* Name() const override {
+      return "CompactionIteratorTest.CompactionFilterSkipUntil::MergeOp";
+    }
+  };
+
   class Filter : public CompactionFilter {
     virtual Decision FilterV2(int level, const Slice& key, ValueType t,
                               const Slice& existing_value,
@@ -330,7 +286,7 @@ TEST_F(CompactionIteratorTest, CompactionFilterSkipUntil) {
     }
   };
 
-  NoMergingMergeOp merge_op;
+  MergeOp merge_op;
   Filter filter;
   InitIterators(
       {test::KeyStr("a", 50, kTypeValue),  // keep
@@ -382,77 +338,6 @@ TEST_F(CompactionIteratorTest, CompactionFilterSkipUntil) {
   ASSERT_EQ(expected_actions, iter_->log);
 }
 
-TEST_F(CompactionIteratorTest, ShuttingDownInFilter) {
-  NoMergingMergeOp merge_op;
-  StallingFilter filter;
-  InitIterators(
-      {test::KeyStr("1", 1, kTypeValue), test::KeyStr("2", 2, kTypeValue),
-       test::KeyStr("3", 3, kTypeValue), test::KeyStr("4", 4, kTypeValue)},
-      {"v1", "v2", "v3", "v4"}, {}, {}, kMaxSequenceNumber, &merge_op, &filter);
-  // Don't leave tombstones (kTypeDeletion) for filtered keys.
-  compaction_proxy_->key_not_exists_beyond_output_level = true;
-
-  std::atomic<bool> seek_done{false};
-  std::thread compaction_thread([&] {
-    c_iter_->SeekToFirst();
-    EXPECT_FALSE(c_iter_->Valid());
-    EXPECT_TRUE(c_iter_->status().IsShutdownInProgress());
-    seek_done.store(true);
-  });
-
-  // Let key 1 through.
-  filter.WaitForStall(1);
-
-  // Shutdown during compaction filter call for key 2.
-  filter.WaitForStall(2);
-  shutting_down_.store(true);
-  EXPECT_FALSE(seek_done.load());
-
-  // Unstall filter and wait for SeekToFirst() to return.
-  filter.stall_at.store(3);
-  compaction_thread.join();
-  assert(seek_done.load());
-
-  // Check that filter was never called again.
-  EXPECT_EQ(2, filter.last_seen.load());
-}
-
-// Same as ShuttingDownInFilter, but shutdown happens during filter call for
-// a merge operand, not for a value.
-TEST_F(CompactionIteratorTest, ShuttingDownInMerge) {
-  NoMergingMergeOp merge_op;
-  StallingFilter filter;
-  InitIterators(
-      {test::KeyStr("1", 1, kTypeValue), test::KeyStr("2", 2, kTypeMerge),
-       test::KeyStr("3", 3, kTypeMerge), test::KeyStr("4", 4, kTypeValue)},
-      {"v1", "v2", "v3", "v4"}, {}, {}, kMaxSequenceNumber, &merge_op, &filter);
-  compaction_proxy_->key_not_exists_beyond_output_level = true;
-
-  std::atomic<bool> seek_done{false};
-  std::thread compaction_thread([&] {
-    c_iter_->SeekToFirst();
-    ASSERT_FALSE(c_iter_->Valid());
-    ASSERT_TRUE(c_iter_->status().IsShutdownInProgress());
-    seek_done.store(true);
-  });
-
-  // Let key 1 through.
-  filter.WaitForStall(1);
-
-  // Shutdown during compaction filter call for key 2.
-  filter.WaitForStall(2);
-  shutting_down_.store(true);
-  EXPECT_FALSE(seek_done.load());
-
-  // Unstall filter and wait for SeekToFirst() to return.
-  filter.stall_at.store(3);
-  compaction_thread.join();
-  assert(seek_done.load());
-
-  // Check that filter was never called again.
-  EXPECT_EQ(2, filter.last_seen.load());
-}
-
 }  // namespace rocksdb
 
 int main(int argc, char** argv) {

db/compaction_job.cc

@@ -264,7 +264,7 @@ void CompactionJob::AggregateStatistics() {
 CompactionJob::CompactionJob(
     int job_id, Compaction* compaction, const ImmutableDBOptions& db_options,
     const EnvOptions& env_options, VersionSet* versions,
-    const std::atomic<bool>* shutting_down, LogBuffer* log_buffer,
+    std::atomic<bool>* shutting_down, LogBuffer* log_buffer,
     Directory* db_directory, Directory* output_directory, Statistics* stats,
     InstrumentedMutex* db_mutex, Status* db_bg_error,
     std::vector<SequenceNumber> existing_snapshots,
@@ -724,8 +724,7 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
       mutable_cf_options->min_partial_merge_operands,
       false /* internal key corruption is expected */,
       existing_snapshots_.empty() ? 0 : existing_snapshots_.back(),
-      compact_->compaction->level(), db_options_.statistics.get(),
-      shutting_down_);
+      compact_->compaction->level(), db_options_.statistics.get());
 
   TEST_SYNC_POINT("CompactionJob::Run():Inprogress");
 
@@ -743,8 +742,7 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
   sub_compact->c_iter.reset(new CompactionIterator(
       input.get(), cfd->user_comparator(), &merge, versions_->LastSequence(),
       &existing_snapshots_, earliest_write_conflict_snapshot_, env_, false,
-      range_del_agg.get(), sub_compact->compaction, compaction_filter,
-      shutting_down_));
+      range_del_agg.get(), sub_compact->compaction, compaction_filter));
   auto c_iter = sub_compact->c_iter.get();
   c_iter->SeekToFirst();
   const auto& c_iter_stats = c_iter->iter_stats();
@@ -755,7 +753,10 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
   std::string compression_dict;
   compression_dict.reserve(cfd->ioptions()->compression_opts.max_dict_bytes);
 
-  while (status.ok() && !cfd->IsDropped() && c_iter->Valid()) {
+  // TODO(noetzli): check whether we could check !shutting_down_->... only
+  // only occasionally (see diff D42687)
+  while (status.ok() && !shutting_down_->load(std::memory_order_acquire) &&
+         !cfd->IsDropped() && c_iter->Valid()) {
     // Invariant: c_iter.status() is guaranteed to be OK if c_iter->Valid()
     // returns true.
     const Slice& key = c_iter->key();
@@ -902,36 +903,27 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
   RecordDroppedKeys(c_iter_stats, &sub_compact->compaction_job_stats);
   RecordCompactionIOStats();
 
-  if (status.ok() && (shutting_down_->load(std::memory_order_relaxed) ||
-                      cfd->IsDropped())) {
+  if (status.ok() &&
+      (shutting_down_->load(std::memory_order_acquire) || cfd->IsDropped())) {
     status = Status::ShutdownInProgress(
         "Database shutdown or Column family drop during compaction");
   }
-  if (status.ok()) {
-    status = input->status();
-  }
-  if (status.ok()) {
-    status = c_iter->status();
-  }
-
   if (status.ok() && sub_compact->builder == nullptr &&
       sub_compact->outputs.size() == 0 &&
       range_del_agg->ShouldAddTombstones(bottommost_level_)) {
     // handle subcompaction containing only range deletions
     status = OpenCompactionOutputFile(sub_compact);
   }
-
-  // Call FinishCompactionOutputFile() even if status is not ok: it needs to
-  // close the output file.
-  if (sub_compact->builder != nullptr) {
+  if (status.ok() && sub_compact->builder != nullptr) {
     CompactionIterationStats range_del_out_stats;
-    Status s = FinishCompactionOutputFile(
-        status, sub_compact, range_del_agg.get(), &range_del_out_stats);
-    if (status.ok()) {
-      status = s;
-    }
+    status =
+        FinishCompactionOutputFile(input->status(), sub_compact,
+                                   range_del_agg.get(), &range_del_out_stats);
     RecordDroppedKeys(range_del_out_stats, &sub_compact->compaction_job_stats);
   }
+  if (status.ok()) {
+    status = input->status();
+  }
 
   if (measure_io_stats_) {
     sub_compact->compaction_job_stats.file_write_nanos +=
@@ -1094,7 +1086,6 @@ Status CompactionJob::FinishCompactionOutputFile(
       event_logger_, cfd->ioptions()->listeners, dbname_, cfd->GetName(), fname,
       job_id_, meta->fd, tp, TableFileCreationReason::kCompaction, s);
 
-#ifndef ROCKSDB_LITE
   // Report new file to SstFileManagerImpl
   auto sfm =
       static_cast<SstFileManagerImpl*>(db_options_.sst_file_manager.get());
@@ -1112,7 +1103,6 @@ Status CompactionJob::FinishCompactionOutputFile(
       }
     }
   }
-#endif
 
   sub_compact->builder.reset();
   sub_compact->current_output_file_size = 0;

db/compaction_job.h

@@ -57,7 +57,7 @@ class CompactionJob {
   CompactionJob(int job_id, Compaction* compaction,
                 const ImmutableDBOptions& db_options,
                 const EnvOptions& env_options, VersionSet* versions,
-                const std::atomic<bool>* shutting_down, LogBuffer* log_buffer,
+                std::atomic<bool>* shutting_down, LogBuffer* log_buffer,
                 Directory* db_directory, Directory* output_directory,
                 Statistics* stats, InstrumentedMutex* db_mutex,
                 Status* db_bg_error,
@@ -131,7 +131,7 @@ class CompactionJob {
 
   Env* env_;
   VersionSet* versions_;
-  const std::atomic<bool>* shutting_down_;
+  std::atomic<bool>* shutting_down_;
   LogBuffer* log_buffer_;
   Directory* db_directory_;
   Directory* output_directory_;

db/compaction_picker.h

@@ -119,6 +119,9 @@ class CompactionPicker {
                                   const Slice& largest_user_key,
                                   int level) const;
 
+ protected:
+  int NumberLevels() const { return ioptions_.num_levels; }
+
   // Stores the minimal range that covers all entries in inputs in
   // *smallest, *largest.
   // REQUIRES: inputs is not empty
@@ -138,9 +141,6 @@ class CompactionPicker {
   void GetRange(const std::vector<CompactionInputFiles>& inputs,
                 InternalKey* smallest, InternalKey* largest) const;
 
- protected:
-  int NumberLevels() const { return ioptions_.num_levels; }
-
   // Add more files to the inputs on "level" to make sure that
   // no newer version of a key is compacted to "level+1" while leaving an older
   // version in a "level". Otherwise, any Get() will search "level" first,

db/compaction_picker_test.cc

@@ -1177,50 +1177,6 @@ TEST_F(CompactionPickerTest, MaxCompactionBytesNotHit) {
   ASSERT_EQ(5U, compaction->input(1, 0)->fd.GetNumber());
 }
 
-TEST_F(CompactionPickerTest, IsTrivialMoveOn) {
-  mutable_cf_options_.max_bytes_for_level_base = 10000u;
-  mutable_cf_options_.max_compaction_bytes = 10001u;
-  ioptions_.level_compaction_dynamic_level_bytes = false;
-  NewVersionStorage(6, kCompactionStyleLevel);
-  // A compaction should be triggered and pick file 2
-  Add(1, 1U, "100", "150", 3000U);
-  Add(1, 2U, "151", "200", 3001U);
-  Add(1, 3U, "201", "250", 3000U);
-  Add(1, 4U, "251", "300", 3000U);
-
-  Add(3, 5U, "120", "130", 7000U);
-  Add(3, 6U, "170", "180", 7000U);
-  Add(3, 5U, "220", "230", 7000U);
-  Add(3, 5U, "270", "280", 7000U);
-  UpdateVersionStorageInfo();
-
-  std::unique_ptr<Compaction> compaction(level_compaction_picker.PickCompaction(
-    cf_name_, mutable_cf_options_, vstorage_.get(), &log_buffer_));
-  ASSERT_TRUE(compaction.get() != nullptr);
-  ASSERT_TRUE(compaction->IsTrivialMove());
-}
-
-TEST_F(CompactionPickerTest, IsTrivialMoveOff) {
-  mutable_cf_options_.max_bytes_for_level_base = 1000000u;
-  mutable_cf_options_.max_compaction_bytes = 10000u;
-  ioptions_.level_compaction_dynamic_level_bytes = false;
-  NewVersionStorage(6, kCompactionStyleLevel);
-  // A compaction should be triggered and pick all files from level 1
-  Add(1, 1U, "100", "150", 300000U, 0, 0);
-  Add(1, 2U, "150", "200", 300000U, 0, 0);
-  Add(1, 3U, "200", "250", 300000U, 0, 0);
-  Add(1, 4U, "250", "300", 300000U, 0, 0);
-
-  Add(3, 5U, "120", "130", 6000U);
-  Add(3, 6U, "140", "150", 6000U);
-  UpdateVersionStorageInfo();
-
-  std::unique_ptr<Compaction> compaction(level_compaction_picker.PickCompaction(
-    cf_name_, mutable_cf_options_, vstorage_.get(), &log_buffer_));
-  ASSERT_TRUE(compaction.get() != nullptr);
-  ASSERT_FALSE(compaction->IsTrivialMove());
-}
-
 }  // namespace rocksdb
 
 int main(int argc, char** argv) {

db/db_dynamic_level_test.cc

@@ -406,7 +406,7 @@ TEST_F(DBTestDynamicLevel, DynamicLevelMaxBytesBaseInc) {
   env_->SetBackgroundThreads(1, Env::HIGH);
 }
 
-TEST_F(DBTestDynamicLevel, DISABLED_MigrateToDynamicLevelMaxBytesBase) {
+TEST_F(DBTestDynamicLevel, MigrateToDynamicLevelMaxBytesBase) {
   Random rnd(301);
   const int kMaxKey = 2000;
 

db/db_filesnapshot.cc

@@ -27,6 +27,7 @@
 #include "util/file_util.h"
 #include "util/mutexlock.h"
 #include "util/sync_point.h"
+#include "util/testharness.h"
 
 namespace rocksdb {
 

db/db_flush_test.cc

@@ -9,7 +9,6 @@
 
 #include "db/db_test_util.h"
 #include "port/stack_trace.h"
-#include "util/fault_injection_test_env.h"
 #include "util/sync_point.h"
 
 namespace rocksdb {
@@ -48,38 +47,6 @@ TEST_F(DBFlushTest, FlushWhileWritingManifest) {
 #endif  // ROCKSDB_LITE
 }
 
-TEST_F(DBFlushTest, SyncFail) {
-  std::unique_ptr<FaultInjectionTestEnv> fault_injection_env(
-      new FaultInjectionTestEnv(Env::Default()));
-  Options options;
-  options.disable_auto_compactions = true;
-  options.env = fault_injection_env.get();
-
-  SyncPoint::GetInstance()->LoadDependency(
-      {{"DBFlushTest::SyncFail:1", "DBImpl::SyncClosedLogs:Start"},
-       {"DBImpl::SyncClosedLogs:Failed", "DBFlushTest::SyncFail:2"}});
-  SyncPoint::GetInstance()->EnableProcessing();
-
-  Reopen(options);
-  Put("key", "value");
-  auto* cfd =
-      reinterpret_cast<ColumnFamilyHandleImpl*>(db_->DefaultColumnFamily())
-          ->cfd();
-  int refs_before = cfd->current()->TEST_refs();
-  FlushOptions flush_options;
-  flush_options.wait = false;
-  ASSERT_OK(dbfull()->Flush(flush_options));
-  fault_injection_env->SetFilesystemActive(false);
-  TEST_SYNC_POINT("DBFlushTest::SyncFail:1");
-  TEST_SYNC_POINT("DBFlushTest::SyncFail:2");
-  fault_injection_env->SetFilesystemActive(true);
-  dbfull()->TEST_WaitForFlushMemTable();
-  ASSERT_EQ("", FilesPerLevel());  // flush failed.
-  // Flush job should release ref count to current version.
-  ASSERT_EQ(refs_before, cfd->current()->TEST_refs());
-  Destroy(options);
-}
-
 }  // namespace rocksdb
 
 int main(int argc, char** argv) {

db/db_impl.cc

@@ -255,17 +255,11 @@ static Status ValidateOptions(
         "More than four DB paths are not supported yet. ");
   }
 
-  if (db_options.allow_mmap_reads && db_options.use_direct_reads) {
+  if (db_options.allow_mmap_reads && !db_options.allow_os_buffer) {
     // Protect against assert in PosixMMapReadableFile constructor
     return Status::NotSupported(
         "If memory mapped reads (allow_mmap_reads) are enabled "
-        "then direct I/O reads (use_direct_reads) must be disabled. ");
-  }
-
-  if (db_options.allow_mmap_writes && db_options.use_direct_writes) {
-    return Status::NotSupported(
-        "If memory mapped writes (allow_mmap_writes) are enabled "
-        "then direct I/O writes (use_direct_writes) must be disabled. ");
+        "then os caching (allow_os_buffer) must also be enabled. ");
   }
 
   return Status::OK();
@@ -338,7 +332,9 @@ DBImpl::DBImpl(const DBOptions& options, const std::string& dbname)
       num_running_flushes_(0),
       bg_purge_scheduled_(0),
       disable_delete_obsolete_files_(0),
-      delete_obsolete_files_last_run_(env_->NowMicros()),
+      delete_obsolete_files_next_run_(
+          env_->NowMicros() +
+          immutable_db_options_.delete_obsolete_files_period_micros),
       last_stats_dump_time_microsec_(0),
       next_job_id_(1),
       has_unpersisted_data_(false),
@@ -380,9 +376,6 @@ DBImpl::DBImpl(const DBOptions& options, const std::string& dbname)
 void DBImpl::CancelAllBackgroundWork(bool wait) {
   InstrumentedMutexLock l(&mutex_);
 
-  Log(InfoLogLevel::INFO_LEVEL, immutable_db_options_.info_log,
-      "Shutdown: canceling all background work");
-
   if (!shutting_down_.load(std::memory_order_acquire) &&
       has_unpersisted_data_ &&
       !mutable_db_options_.avoid_flush_during_shutdown) {
@@ -507,8 +500,6 @@ DBImpl::~DBImpl() {
     env_->UnlockFile(db_lock_);
   }
 
-  Log(InfoLogLevel::INFO_LEVEL, immutable_db_options_.info_log,
-      "Shutdown complete");
   LogFlush(immutable_db_options_.info_log);
 }
 
@@ -790,7 +781,7 @@ uint64_t DBImpl::MinLogNumberToKeep() {
 // Otherwise, gets obsolete files from VersionSet.
 // no_full_scan = true -- never do the full scan using GetChildren()
 // force = false -- don't force the full scan, except every
-//  mutable_db_options_.delete_obsolete_files_period_micros
+//  immutable_db_options_.delete_obsolete_files_period_micros
 // force = true -- force the full scan
 void DBImpl::FindObsoleteFiles(JobContext* job_context, bool force,
                                bool no_full_scan) {
@@ -807,15 +798,15 @@ void DBImpl::FindObsoleteFiles(JobContext* job_context, bool force,
   if (no_full_scan) {
     doing_the_full_scan = false;
   } else if (force ||
-             mutable_db_options_.delete_obsolete_files_period_micros == 0) {
+             immutable_db_options_.delete_obsolete_files_period_micros == 0) {
     doing_the_full_scan = true;
   } else {
     const uint64_t now_micros = env_->NowMicros();
-    if ((delete_obsolete_files_last_run_ +
-         mutable_db_options_.delete_obsolete_files_period_micros) <
-        now_micros) {
+    if (delete_obsolete_files_next_run_ < now_micros) {
       doing_the_full_scan = true;
-      delete_obsolete_files_last_run_ = now_micros;
+      delete_obsolete_files_next_run_ =
+          now_micros +
+          immutable_db_options_.delete_obsolete_files_period_micros;
     }
   }
 
@@ -1842,7 +1833,6 @@ Status DBImpl::WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd,
 }
 
 Status DBImpl::SyncClosedLogs(JobContext* job_context) {
-  TEST_SYNC_POINT("DBImpl::SyncClosedLogs:Start");
   mutex_.AssertHeld();
   autovector<log::Writer*, 1> logs_to_sync;
   uint64_t current_log_number = logfile_number_;
@@ -1879,7 +1869,6 @@ Status DBImpl::SyncClosedLogs(JobContext* job_context) {
     MarkLogsSynced(current_log_number - 1, true, s);
     if (!s.ok()) {
       bg_error_ = s;
-      TEST_SYNC_POINT("DBImpl::SyncClosedLogs:Failed");
       return s;
     }
   }
@@ -1930,8 +1919,6 @@ Status DBImpl::FlushMemTableToOutputFile(
   // is unlocked by the current thread.
   if (s.ok()) {
     s = flush_job.Run(&file_meta);
-  } else {
-    flush_job.Cancel();
   }
 
   if (s.ok()) {
@@ -1956,6 +1943,7 @@ Status DBImpl::FlushMemTableToOutputFile(
     // may temporarily unlock and lock the mutex.
     NotifyOnFlushCompleted(cfd, &file_meta, mutable_cf_options,
                            job_context->job_id, flush_job.GetTableProperties());
+#endif  // ROCKSDB_LITE
     auto sfm = static_cast<SstFileManagerImpl*>(
         immutable_db_options_.sst_file_manager.get());
     if (sfm) {
@@ -1969,7 +1957,6 @@ Status DBImpl::FlushMemTableToOutputFile(
             "DBImpl::FlushMemTableToOutputFile:MaxAllowedSpaceReached");
       }
     }
-#endif  // ROCKSDB_LITE
   }
   return s;
 }
@@ -2149,7 +2136,7 @@ Status DBImpl::CompactFiles(
                        immutable_db_options_.info_log.get());
 
   // Perform CompactFiles
-  SuperVersion* sv = cfd->GetReferencedSuperVersion(&mutex_);
+  SuperVersion* sv = GetAndRefSuperVersion(cfd);
   {
     InstrumentedMutexLock l(&mutex_);
 
@@ -2161,12 +2148,7 @@ Status DBImpl::CompactFiles(
                          input_file_names, output_level,
                          output_path_id, &job_context, &log_buffer);
   }
-  if (sv->Unref()) {
-    mutex_.Lock();
-    sv->Cleanup();
-    mutex_.Unlock();
-    delete sv;
-  }
+  ReturnAndCleanupSuperVersion(cfd, sv);
 
   // Find and delete obsolete files
   {
@@ -2771,7 +2753,7 @@ void DBImpl::MarkLogsSynced(
       ++it;
     }
   }
-  assert(!status.ok() || logs_.empty() || logs_[0].number > up_to ||
+  assert(logs_.empty() || logs_[0].number > up_to ||
          (logs_.size() == 1 && !logs_[0].getting_synced));
   log_sync_cv_.SignalAll();
 }
@@ -6118,7 +6100,6 @@ Status DB::Open(const DBOptions& db_options, const std::string& dbname,
   }
   impl->mutex_.Unlock();
 
-#ifndef ROCKSDB_LITE
   auto sfm = static_cast<SstFileManagerImpl*>(
       impl->immutable_db_options_.sst_file_manager.get());
   if (s.ok() && sfm) {
@@ -6137,7 +6118,6 @@ Status DB::Open(const DBOptions& db_options, const std::string& dbname,
       }
     }
   }
-#endif  // !ROCKSDB_LITE
 
   if (s.ok()) {
     Log(InfoLogLevel::INFO_LEVEL, impl->immutable_db_options_.info_log,
@@ -6368,7 +6348,7 @@ Status DBImpl::RenameTempFileToOptionsFile(const std::string& file_name) {
 #endif  // !ROCKSDB_LITE
 }
 
-#ifdef ROCKSDB_USING_THREAD_STATUS
+#if ROCKSDB_USING_THREAD_STATUS
 
 void DBImpl::NewThreadStatusCfInfo(
     ColumnFamilyData* cfd) const {

db/db_impl.h

@@ -988,9 +988,8 @@ class DBImpl : public DB {
   // without any synchronization
   int disable_delete_obsolete_files_;
 
-  // last time when DeleteObsoleteFiles with full scan was executed. Originaly
-  // initialized with startup time.
-  uint64_t delete_obsolete_files_last_run_;
+  // next time when we should run DeleteObsoleteFiles with full scan
+  uint64_t delete_obsolete_files_next_run_;
 
   // last time stats were dumped to LOG
   std::atomic<uint64_t> last_stats_dump_time_microsec_;

db/db_iter.cc

@@ -123,8 +123,7 @@ class DBIter: public Iterator {
         prefix_same_as_start_(prefix_same_as_start),
         pin_thru_lifetime_(pin_data),
         total_order_seek_(total_order_seek),
-        range_del_agg_(ioptions.internal_comparator, s,
-                       true /* collapse_deletions */) {
+        range_del_agg_(ioptions.internal_comparator, s) {
     RecordTick(statistics_, NO_ITERATORS);
     prefix_extractor_ = ioptions.prefix_extractor;
     max_skip_ = max_sequential_skip_in_iterations;
@@ -341,7 +340,6 @@ void DBIter::Next() {
 //
 // NOTE: In between, saved_key_ can point to a user key that has
 //       a delete marker or a sequence number higher than sequence_
-//       saved_key_ MUST have a proper user_key before calling this function
 //
 // The prefix_check parameter controls whether we check the iterated
 // keys against the prefix of the seeked key. Set to false when
@@ -413,9 +411,7 @@ void DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
             saved_key_.SetKey(
                 ikey.user_key,
                 !iter_->IsKeyPinned() || !pin_thru_lifetime_ /* copy */);
-            if (range_del_agg_.ShouldDelete(
-                    ikey, RangeDelAggregator::RangePositioningMode::
-                              kForwardTraversal)) {
+            if (range_del_agg_.ShouldDelete(ikey)) {
               // Arrange to skip all upcoming entries for this key since
               // they are hidden by this deletion.
               skipping = true;
@@ -430,9 +426,7 @@ void DBIter::FindNextUserEntryInternal(bool skipping, bool prefix_check) {
             saved_key_.SetKey(
                 ikey.user_key,
                 !iter_->IsKeyPinned() || !pin_thru_lifetime_ /* copy */);
-            if (range_del_agg_.ShouldDelete(
-                    ikey, RangeDelAggregator::RangePositioningMode::
-                              kForwardTraversal)) {
+            if (range_del_agg_.ShouldDelete(ikey)) {
               // Arrange to skip all upcoming entries for this key since
               // they are hidden by this deletion.
               skipping = true;
@@ -535,9 +529,7 @@ void DBIter::MergeValuesNewToOld() {
       // hit the next user key, stop right here
       break;
     } else if (kTypeDeletion == ikey.type || kTypeSingleDeletion == ikey.type ||
-               range_del_agg_.ShouldDelete(
-                   ikey, RangeDelAggregator::RangePositioningMode::
-                             kForwardTraversal)) {
+               range_del_agg_.ShouldDelete(ikey)) {
       // hit a delete with the same user key, stop right here
       // iter_ is positioned after delete
       iter_->Next();
@@ -606,7 +598,6 @@ void DBIter::ReverseToForward() {
   direction_ = kForward;
   if (!iter_->Valid()) {
     iter_->SeekToFirst();
-    range_del_agg_.InvalidateTombstoneMapPositions();
   }
 }
 
@@ -622,7 +613,6 @@ void DBIter::ReverseToBackward() {
     // previous key.
     if (!iter_->Valid()) {
       iter_->SeekToLast();
-      range_del_agg_.InvalidateTombstoneMapPositions();
     }
     ParsedInternalKey ikey;
     FindParseableKey(&ikey, kReverse);
@@ -719,9 +709,7 @@ bool DBIter::FindValueForCurrentKey() {
     last_key_entry_type = ikey.type;
     switch (last_key_entry_type) {
       case kTypeValue:
-        if (range_del_agg_.ShouldDelete(
-                ikey,
-                RangeDelAggregator::RangePositioningMode::kBackwardTraversal)) {
+        if (range_del_agg_.ShouldDelete(ikey)) {
           last_key_entry_type = kTypeRangeDeletion;
           PERF_COUNTER_ADD(internal_delete_skipped_count, 1);
         } else {
@@ -738,9 +726,7 @@ bool DBIter::FindValueForCurrentKey() {
         PERF_COUNTER_ADD(internal_delete_skipped_count, 1);
         break;
       case kTypeMerge:
-        if (range_del_agg_.ShouldDelete(
-                ikey,
-                RangeDelAggregator::RangePositioningMode::kBackwardTraversal)) {
+        if (range_del_agg_.ShouldDelete(ikey)) {
           merge_context_.Clear();
           last_key_entry_type = kTypeRangeDeletion;
           last_not_merge_type = last_key_entry_type;
@@ -818,8 +804,7 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() {
   FindParseableKey(&ikey, kForward);
 
   if (ikey.type == kTypeDeletion || ikey.type == kTypeSingleDeletion ||
-      range_del_agg_.ShouldDelete(
-          ikey, RangeDelAggregator::RangePositioningMode::kBackwardTraversal)) {
+      range_del_agg_.ShouldDelete(ikey)) {
     valid_ = false;
     return false;
   }
@@ -834,12 +819,9 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() {
   // in operands
   current_entry_is_merged_ = true;
   merge_context_.Clear();
-  while (
-      iter_->Valid() &&
-      user_comparator_->Equal(ikey.user_key, saved_key_.GetKey()) &&
-      ikey.type == kTypeMerge &&
-      !range_del_agg_.ShouldDelete(
-          ikey, RangeDelAggregator::RangePositioningMode::kBackwardTraversal)) {
+  while (iter_->Valid() &&
+         user_comparator_->Equal(ikey.user_key, saved_key_.GetKey()) &&
+         ikey.type == kTypeMerge && !range_del_agg_.ShouldDelete(ikey)) {
     merge_context_.PushOperand(iter_->value(),
                                iter_->IsValuePinned() /* operand_pinned */);
     PERF_COUNTER_ADD(internal_merge_count, 1);
@@ -851,8 +833,7 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() {
   if (!iter_->Valid() ||
       !user_comparator_->Equal(ikey.user_key, saved_key_.GetKey()) ||
       ikey.type == kTypeDeletion || ikey.type == kTypeSingleDeletion ||
-      range_del_agg_.ShouldDelete(
-          ikey, RangeDelAggregator::RangePositioningMode::kBackwardTraversal)) {
+      range_del_agg_.ShouldDelete(ikey)) {
     s = MergeHelper::TimedFullMerge(merge_operator_, saved_key_.GetKey(),
                                     nullptr, merge_context_.GetOperands(),
                                     &saved_value_, logger_, statistics_, env_,
@@ -947,12 +928,12 @@ void DBIter::Seek(const Slice& target) {
   StopWatch sw(env_, statistics_, DB_SEEK);
   ReleaseTempPinnedData();
   saved_key_.Clear();
+  // now savved_key is used to store internal key.
   saved_key_.SetInternalKey(target, sequence_);
 
   {
     PERF_TIMER_GUARD(seek_internal_seek_time);
     iter_->Seek(saved_key_.GetKey());
-    range_del_agg_.InvalidateTombstoneMapPositions();
   }
   RecordTick(statistics_, NUMBER_DB_SEEK);
   if (iter_->Valid()) {
@@ -961,9 +942,6 @@ void DBIter::Seek(const Slice& target) {
     }
     direction_ = kForward;
     ClearSavedValue();
-    // convert the InternalKey to UserKey in saved_key_ before
-    // passed to FindNextUserEntry
-    saved_key_.Reserve(saved_key_.Size() - 8);
     FindNextUserEntry(false /* not skipping */, prefix_same_as_start_);
     if (!valid_) {
       prefix_start_key_.clear();
@@ -995,7 +973,6 @@ void DBIter::SeekForPrev(const Slice& target) {
   {
     PERF_TIMER_GUARD(seek_internal_seek_time);
     iter_->SeekForPrev(saved_key_.GetKey());
-    range_del_agg_.InvalidateTombstoneMapPositions();
   }
 
   RecordTick(statistics_, NUMBER_DB_SEEK);
@@ -1037,13 +1014,10 @@ void DBIter::SeekToFirst() {
   {
     PERF_TIMER_GUARD(seek_internal_seek_time);
     iter_->SeekToFirst();
-    range_del_agg_.InvalidateTombstoneMapPositions();
   }
 
   RecordTick(statistics_, NUMBER_DB_SEEK);
   if (iter_->Valid()) {
-    saved_key_.SetKey(ExtractUserKey(iter_->key()),
-                      !iter_->IsKeyPinned() || !pin_thru_lifetime_ /* copy */);
     FindNextUserEntry(false /* not skipping */, false /* no prefix check */);
     if (statistics_ != nullptr) {
       if (valid_) {
@@ -1073,14 +1047,12 @@ void DBIter::SeekToLast() {
   {
     PERF_TIMER_GUARD(seek_internal_seek_time);
     iter_->SeekToLast();
-    range_del_agg_.InvalidateTombstoneMapPositions();
   }
   // When the iterate_upper_bound is set to a value,
   // it will seek to the last key before the
   // ReadOptions.iterate_upper_bound
   if (iter_->Valid() && iterate_upper_bound_ != nullptr) {
     SeekForPrev(*iterate_upper_bound_);
-    range_del_agg_.InvalidateTombstoneMapPositions();
     if (!Valid()) {
       return;
     } else if (user_comparator_->Equal(*iterate_upper_bound_, key())) {

db/db_iter_test.cc

@@ -1897,66 +1897,6 @@ TEST_F(DBIteratorTest, DBIterator12) {
   ASSERT_FALSE(db_iter->Valid());
 }
 
-TEST_F(DBIteratorTest, DBIterator13) {
-  Options options;
-  options.merge_operator = nullptr;
-
-  std::string key;
-  key.resize(9);
-  key.assign(9, static_cast<char>(0));
-  key[0] = 'b';
-
-  TestIterator* internal_iter = new TestIterator(BytewiseComparator());
-  internal_iter->AddPut(key, "0");
-  internal_iter->AddPut(key, "1");
-  internal_iter->AddPut(key, "2");
-  internal_iter->AddPut(key, "3");
-  internal_iter->AddPut(key, "4");
-  internal_iter->AddPut(key, "5");
-  internal_iter->AddPut(key, "6");
-  internal_iter->AddPut(key, "7");
-  internal_iter->AddPut(key, "8");
-  internal_iter->Finish();
-
-  std::unique_ptr<Iterator> db_iter(
-      NewDBIterator(env_, ImmutableCFOptions(options), BytewiseComparator(),
-                    internal_iter, 2, 3, 0));
-  db_iter->Seek("b");
-  ASSERT_TRUE(db_iter->Valid());
-  ASSERT_EQ(db_iter->key().ToString(), key);
-  ASSERT_EQ(db_iter->value().ToString(), "2");
-}
-
-TEST_F(DBIteratorTest, DBIterator14) {
-  Options options;
-  options.merge_operator = nullptr;
-
-  std::string key("b");
-  TestIterator* internal_iter = new TestIterator(BytewiseComparator());
-  internal_iter->AddPut("b", "0");
-  internal_iter->AddPut("b", "1");
-  internal_iter->AddPut("b", "2");
-  internal_iter->AddPut("b", "3");
-  internal_iter->AddPut("a", "4");
-  internal_iter->AddPut("a", "5");
-  internal_iter->AddPut("a", "6");
-  internal_iter->AddPut("c", "7");
-  internal_iter->AddPut("c", "8");
-  internal_iter->AddPut("c", "9");
-  internal_iter->Finish();
-
-  std::unique_ptr<Iterator> db_iter(
-      NewDBIterator(env_, ImmutableCFOptions(options), BytewiseComparator(),
-                    internal_iter, 4, 1, 0));
-  db_iter->Seek("b");
-  ASSERT_TRUE(db_iter->Valid());
-  ASSERT_EQ(db_iter->key().ToString(), "b");
-  ASSERT_EQ(db_iter->value().ToString(), "3");
-  db_iter->SeekToFirst();
-  ASSERT_EQ(db_iter->key().ToString(), "a");
-  ASSERT_EQ(db_iter->value().ToString(), "4");
-}
-
 class DBIterWithMergeIterTest : public testing::Test {
  public:
   DBIterWithMergeIterTest()

db/db_iterator_test.cc

@@ -7,8 +7,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 
-#include <functional>
-
 #include "db/db_test_util.h"
 #include "port/port.h"
 #include "port/stack_trace.h"

db/db_options_test.cc

@@ -316,47 +316,6 @@ TEST_F(DBOptionsTest, MaxTotalWalSizeChange) {
   }
 }
 
-static void assert_candidate_files_empty(DBImpl* dbfull, const bool empty) {
-  dbfull->TEST_LockMutex();
-  JobContext job_context(0);
-  dbfull->FindObsoleteFiles(&job_context, false);
-  ASSERT_EQ(empty, job_context.full_scan_candidate_files.empty());
-  job_context.Clean();
-  dbfull->TEST_UnlockMutex();
-}
-
-TEST_F(DBOptionsTest, DeleteObsoleteFilesPeriodChange) {
-  SpecialEnv env(Env::Default());
-  env.time_elapse_only_sleep_ = true;
-  Options options;
-  options.env = &env;
-  options.create_if_missing = true;
-  ASSERT_OK(TryReopen(options));
-
-  // Verify that candidate files set is empty when no full scan requested.
-  assert_candidate_files_empty(dbfull(), true);
-
-  ASSERT_OK(
-      dbfull()->SetDBOptions({{"delete_obsolete_files_period_micros", "0"}}));
-
-  // After delete_obsolete_files_period_micros updated to 0, the next call
-  // to FindObsoleteFiles should make a full scan
-  assert_candidate_files_empty(dbfull(), false);
-
-  ASSERT_OK(
-      dbfull()->SetDBOptions({{"delete_obsolete_files_period_micros", "20"}}));
-
-  assert_candidate_files_empty(dbfull(), true);
-
-  env.addon_time_.store(20);
-  assert_candidate_files_empty(dbfull(), true);
-
-  env.addon_time_.store(21);
-  assert_candidate_files_empty(dbfull(), false);
-
-  Close();
-}
-
 #endif  // ROCKSDB_LITE
 
 }  // namespace rocksdb

db/db_range_del_test.cc

@@ -41,7 +41,7 @@ TEST_F(DBRangeDelTest, NonBlockBasedTableNotSupported) {
 
 TEST_F(DBRangeDelTest, FlushOutputHasOnlyRangeTombstones) {
   ASSERT_OK(db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "dr1",
-                             "dr2"));
+                             "dr1"));
   ASSERT_OK(db_->Flush(FlushOptions()));
   ASSERT_EQ(1, NumTableFilesAtLevel(0));
 }
@@ -165,28 +165,6 @@ TEST_F(DBRangeDelTest, MaxCompactionBytesCutsOutputFiles) {
   db_->ReleaseSnapshot(snapshot);
 }
 
-TEST_F(DBRangeDelTest, SentinelsOmittedFromOutputFile) {
-  // Regression test for bug where sentinel range deletions (i.e., ones with
-  // sequence number of zero) were included in output files.
-  // snapshot protects range tombstone from dropping due to becoming obsolete.
-  const Snapshot* snapshot = db_->GetSnapshot();
-
-  // gaps between ranges creates sentinels in our internal representation
-  std::vector<std::pair<std::string, std::string>> range_dels = {{"a", "b"}, {"c", "d"}, {"e", "f"}};
-  for (const auto& range_del : range_dels) {
-    ASSERT_OK(db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(),
-                               range_del.first, range_del.second));
-  }
-  ASSERT_OK(db_->Flush(FlushOptions()));
-  ASSERT_EQ(1, NumTableFilesAtLevel(0));
-
-  std::vector<std::vector<FileMetaData>> files;
-  dbfull()->TEST_GetFilesMetaData(db_->DefaultColumnFamily(), &files);
-  ASSERT_GT(files[0][0].smallest_seqno, 0);
-
-  db_->ReleaseSnapshot(snapshot);
-}
-
 TEST_F(DBRangeDelTest, FlushRangeDelsSameStartKey) {
   db_->Put(WriteOptions(), "b1", "val");
   ASSERT_OK(
@@ -505,209 +483,6 @@ TEST_F(DBRangeDelTest, ObsoleteTombstoneCleanup) {
 }
 #endif  // ROCKSDB_LITE
 
-TEST_F(DBRangeDelTest, GetCoveredKeyFromMutableMemtable) {
-  db_->Put(WriteOptions(), "key", "val");
-  ASSERT_OK(
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "a", "z"));
-
-  ReadOptions read_opts;
-  std::string value;
-  ASSERT_TRUE(db_->Get(read_opts, "key", &value).IsNotFound());
-}
-
-TEST_F(DBRangeDelTest, GetCoveredKeyFromImmutableMemtable) {
-  Options opts = CurrentOptions();
-  opts.max_write_buffer_number = 3;
-  opts.min_write_buffer_number_to_merge = 2;
-  // SpecialSkipListFactory lets us specify maximum number of elements the
-  // memtable can hold. It switches the active memtable to immutable (flush is
-  // prevented by the above options) upon inserting an element that would
-  // overflow the memtable.
-  opts.memtable_factory.reset(new SpecialSkipListFactory(1));
-  Reopen(opts);
-
-  db_->Put(WriteOptions(), "key", "val");
-  ASSERT_OK(
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "a", "z"));
-  db_->Put(WriteOptions(), "blah", "val");
-
-  ReadOptions read_opts;
-  std::string value;
-  ASSERT_TRUE(db_->Get(read_opts, "key", &value).IsNotFound());
-}
-
-TEST_F(DBRangeDelTest, GetCoveredKeyFromSst) {
-  db_->Put(WriteOptions(), "key", "val");
-  // snapshot prevents key from being deleted during flush
-  const Snapshot* snapshot = db_->GetSnapshot();
-  ASSERT_OK(
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "a", "z"));
-  ASSERT_OK(db_->Flush(FlushOptions()));
-
-  ReadOptions read_opts;
-  std::string value;
-  ASSERT_TRUE(db_->Get(read_opts, "key", &value).IsNotFound());
-  db_->ReleaseSnapshot(snapshot);
-}
-
-TEST_F(DBRangeDelTest, GetCoveredMergeOperandFromMemtable) {
-  const int kNumMergeOps = 10;
-  Options opts = CurrentOptions();
-  opts.merge_operator = MergeOperators::CreateUInt64AddOperator();
-  Reopen(opts);
-
-  for (int i = 0; i < kNumMergeOps; ++i) {
-    std::string val;
-    PutFixed64(&val, i);
-    db_->Merge(WriteOptions(), "key", val);
-    if (i == kNumMergeOps / 2) {
-      // deletes [0, 5]
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "key",
-                       "key_");
-    }
-  }
-
-  ReadOptions read_opts;
-  std::string expected, actual;
-  ASSERT_OK(db_->Get(read_opts, "key", &actual));
-  PutFixed64(&expected, 30);  // 6+7+8+9
-  ASSERT_EQ(expected, actual);
-
-  expected.clear();
-  read_opts.ignore_range_deletions = true;
-  ASSERT_OK(db_->Get(read_opts, "key", &actual));
-  PutFixed64(&expected, 45);  // 0+1+2+...+9
-  ASSERT_EQ(expected, actual);
-}
-
-TEST_F(DBRangeDelTest, GetIgnoresRangeDeletions) {
-  Options opts = CurrentOptions();
-  opts.max_write_buffer_number = 4;
-  opts.min_write_buffer_number_to_merge = 3;
-  opts.memtable_factory.reset(new SpecialSkipListFactory(1));
-  Reopen(opts);
-
-  db_->Put(WriteOptions(), "sst_key", "val");
-  // snapshot prevents key from being deleted during flush
-  const Snapshot* snapshot = db_->GetSnapshot();
-  ASSERT_OK(
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "a", "z"));
-  ASSERT_OK(db_->Flush(FlushOptions()));
-  db_->Put(WriteOptions(), "imm_key", "val");
-  ASSERT_OK(
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "a", "z"));
-  db_->Put(WriteOptions(), "mem_key", "val");
-  ASSERT_OK(
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "a", "z"));
-
-  ReadOptions read_opts;
-  read_opts.ignore_range_deletions = true;
-  for (std::string key : {"sst_key", "imm_key", "mem_key"}) {
-    std::string value;
-    ASSERT_OK(db_->Get(read_opts, key, &value));
-  }
-  db_->ReleaseSnapshot(snapshot);
-}
-
-TEST_F(DBRangeDelTest, IteratorRemovesCoveredKeys) {
-  const int kNum = 200, kRangeBegin = 50, kRangeEnd = 150, kNumPerFile = 25;
-  Options opts = CurrentOptions();
-  opts.comparator = test::Uint64Comparator();
-  opts.memtable_factory.reset(new SpecialSkipListFactory(kNumPerFile));
-  Reopen(opts);
-
-  // Write half of the keys before the tombstone and half after the tombstone.
-  // Only covered keys (i.e., within the range and older than the tombstone)
-  // should be deleted.
-  for (int i = 0; i < kNum; ++i) {
-    if (i == kNum / 2) {
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(),
-                       GetNumericStr(kRangeBegin), GetNumericStr(kRangeEnd));
-    }
-    db_->Put(WriteOptions(), GetNumericStr(i), "val");
-  }
-  ReadOptions read_opts;
-  auto* iter = db_->NewIterator(read_opts);
-
-  int expected = 0;
-  for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
-    ASSERT_EQ(GetNumericStr(expected), iter->key());
-    if (expected == kRangeBegin - 1) {
-      expected = kNum / 2;
-    } else {
-      ++expected;
-    }
-  }
-  ASSERT_EQ(kNum, expected);
-  delete iter;
-}
-
-TEST_F(DBRangeDelTest, IteratorOverUserSnapshot) {
-  const int kNum = 200, kRangeBegin = 50, kRangeEnd = 150, kNumPerFile = 25;
-  Options opts = CurrentOptions();
-  opts.comparator = test::Uint64Comparator();
-  opts.memtable_factory.reset(new SpecialSkipListFactory(kNumPerFile));
-  Reopen(opts);
-
-  const Snapshot* snapshot = nullptr;
-  // Put a snapshot before the range tombstone, verify an iterator using that
-  // snapshot sees all inserted keys.
-  for (int i = 0; i < kNum; ++i) {
-    if (i == kNum / 2) {
-      snapshot = db_->GetSnapshot();
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(),
-                       GetNumericStr(kRangeBegin), GetNumericStr(kRangeEnd));
-    }
-    db_->Put(WriteOptions(), GetNumericStr(i), "val");
-  }
-  ReadOptions read_opts;
-  read_opts.snapshot = snapshot;
-  auto* iter = db_->NewIterator(read_opts);
-
-  int expected = 0;
-  for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
-    ASSERT_EQ(GetNumericStr(expected), iter->key());
-    ++expected;
-  }
-  ASSERT_EQ(kNum / 2, expected);
-  delete iter;
-  db_->ReleaseSnapshot(snapshot);
-}
-
-TEST_F(DBRangeDelTest, IteratorIgnoresRangeDeletions) {
-  Options opts = CurrentOptions();
-  opts.max_write_buffer_number = 4;
-  opts.min_write_buffer_number_to_merge = 3;
-  opts.memtable_factory.reset(new SpecialSkipListFactory(1));
-  Reopen(opts);
-
-  db_->Put(WriteOptions(), "sst_key", "val");
-  // snapshot prevents key from being deleted during flush
-  const Snapshot* snapshot = db_->GetSnapshot();
-  ASSERT_OK(
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "a", "z"));
-  ASSERT_OK(db_->Flush(FlushOptions()));
-  db_->Put(WriteOptions(), "imm_key", "val");
-  ASSERT_OK(
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "a", "z"));
-  db_->Put(WriteOptions(), "mem_key", "val");
-  ASSERT_OK(
-      db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "a", "z"));
-
-  ReadOptions read_opts;
-  read_opts.ignore_range_deletions = true;
-  auto* iter = db_->NewIterator(read_opts);
-  int i = 0;
-  std::string expected[] = {"imm_key", "mem_key", "sst_key"};
-  for (iter->SeekToFirst(); iter->Valid(); iter->Next(), ++i) {
-    std::string key;
-    ASSERT_EQ(expected[i], iter->key());
-  }
-  ASSERT_EQ(3, i);
-  delete iter;
-  db_->ReleaseSnapshot(snapshot);
-}
-
 }  // namespace rocksdb
 
 int main(int argc, char** argv) {

db/db_sst_test.cc

@@ -221,6 +221,8 @@ TEST_F(DBSSTTest, DeleteObsoleteFilesPendingOutputs) {
   listener->VerifyMatchedCount(1);
 }
 
+#endif  // ROCKSDB_LITE
+
 TEST_F(DBSSTTest, DBWithSstFileManager) {
   std::shared_ptr<SstFileManager> sst_file_manager(NewSstFileManager(env_));
   auto sfm = static_cast<SstFileManagerImpl*>(sst_file_manager.get());
@@ -285,6 +287,7 @@ TEST_F(DBSSTTest, DBWithSstFileManager) {
   rocksdb::SyncPoint::GetInstance()->DisableProcessing();
 }
 
+#ifndef ROCKSDB_LITE
 TEST_F(DBSSTTest, RateLimitedDelete) {
   Destroy(last_options_);
   rocksdb::SyncPoint::GetInstance()->LoadDependency({
@@ -479,6 +482,7 @@ TEST_F(DBSSTTest, DestroyDBWithRateLimitedDelete) {
   // We have deleted the 4 sst files in the delete_scheduler
   ASSERT_EQ(bg_delete_file, 4);
 }
+#endif  // ROCKSDB_LITE
 
 TEST_F(DBSSTTest, DBWithMaxSpaceAllowed) {
   std::shared_ptr<SstFileManager> sst_file_manager(NewSstFileManager(env_));
@@ -576,6 +580,7 @@ TEST_F(DBSSTTest, DBWithMaxSpaceAllowedRandomized) {
   ASSERT_GT(reached_max_space_on_compaction, 0);
 }
 
+#ifndef ROCKSDB_LITE
 TEST_F(DBSSTTest, OpenDBWithInfiniteMaxOpenFiles) {
   // Open DB with infinite max open files
   //  - First iteration use 1 thread to open files

db/db_test.cc

@@ -197,19 +197,6 @@ TEST_F(DBTest, MemEnvTest) {
 }
 #endif  // ROCKSDB_LITE
 
-TEST_F(DBTest, OpenWhenOpen) {
-  Options options = CurrentOptions();
-  options.env = env_;
-  rocksdb::DB* db2 = nullptr;
-  rocksdb::Status s = DB::Open(options, dbname_, &db2);
-
-  ASSERT_EQ(Status::Code::kIOError, s.code());
-  ASSERT_EQ(Status::SubCode::kNone, s.subcode());
-  ASSERT_TRUE(strstr(s.getState(), "lock ") != nullptr);
-
-  delete db2;
-}
-
 TEST_F(DBTest, WriteEmptyBatch) {
   Options options = CurrentOptions();
   options.env = env_;
@@ -3409,21 +3396,19 @@ TEST_F(DBTest, TableOptionsSanitizeTest) {
 TEST_F(DBTest, MmapAndBufferOptions) {
   Options options = CurrentOptions();
 
-  options.use_direct_reads = true;
+  options.allow_os_buffer = false;
   options.allow_mmap_reads = true;
   ASSERT_NOK(TryReopen(options));
 
   // All other combinations are acceptable
-  options.use_direct_reads = false;
+  options.allow_os_buffer = true;
   ASSERT_OK(TryReopen(options));
 
-  if (IsDirectIOSupported()) {
-    options.use_direct_reads = true;
-    options.allow_mmap_reads = false;
-    ASSERT_OK(TryReopen(options));
-  }
+  options.allow_os_buffer = false;
+  options.allow_mmap_reads = false;
+  ASSERT_OK(TryReopen(options));
 
-  options.use_direct_reads = false;
+  options.allow_os_buffer = true;
   ASSERT_OK(TryReopen(options));
 }
 #endif
@@ -3678,7 +3663,7 @@ TEST_F(DBTest, DynamicMemtableOptions) {
 }
 #endif  // ROCKSDB_LITE
 
-#ifdef ROCKSDB_USING_THREAD_STATUS
+#if ROCKSDB_USING_THREAD_STATUS
 namespace {
 void VerifyOperationCount(Env* env, ThreadStatus::OperationType op_type,
                           int expected_count) {
@@ -4985,7 +4970,7 @@ TEST_F(DBTest, MergeTestTime) {
 
   ASSERT_EQ(1, count);
   ASSERT_EQ(2000000, TestGetTickerCount(options, MERGE_OPERATION_TOTAL_TIME));
-#ifdef ROCKSDB_USING_THREAD_STATUS
+#if ROCKSDB_USING_THREAD_STATUS
   ASSERT_GT(TestGetTickerCount(options, FLUSH_WRITE_BYTES), 0);
 #endif  // ROCKSDB_USING_THREAD_STATUS
   this->env_->time_elapse_only_sleep_ = false;

db/db_test2.cc

@@ -1505,10 +1505,6 @@ class MockPersistentCache : public PersistentCache {
 
   bool IsCompressed() override { return is_compressed_; }
 
-  std::string GetPrintableOptions() const override {
-    return "MockPersistentCache";
-  }
-
   port::Mutex lock_;
   std::map<std::string, std::string> data_;
   const bool is_compressed_ = true;
@@ -2230,26 +2226,6 @@ TEST_F(DBTest2, OptimizeForPointLookup) {
 }
 #endif  // ROCKSDB_LITE
 
-TEST_F(DBTest2, DirectIO) {
-  if (!IsDirectIOSupported()) {
-    return;
-  }
-  Options options = CurrentOptions();
-  options.use_direct_reads = options.use_direct_writes = true;
-  options.allow_mmap_reads = options.allow_mmap_writes = false;
-  DestroyAndReopen(options);
-
-  ASSERT_OK(Put(Key(0), "a"));
-  ASSERT_OK(Put(Key(5), "a"));
-  ASSERT_OK(Flush());
-
-  ASSERT_OK(Put(Key(10), "a"));
-  ASSERT_OK(Put(Key(15), "a"));
-  ASSERT_OK(Flush());
-
-  ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
-  Reopen(options);
-}
 }  // namespace rocksdb
 
 int main(int argc, char** argv) {

db/db_test_util.cc

@@ -308,7 +308,6 @@ Options DBTestBase::CurrentOptions(
       break;
     case kManifestFileSize:
       options.max_manifest_file_size = 50;  // 50 bytes
-      break;
     case kPerfOptions:
       options.soft_rate_limit = 2.0;
       options.delayed_write_rate = 8 * 1024 * 1024;
@@ -478,22 +477,6 @@ Status DBTestBase::TryReopen(const Options& options) {
   return DB::Open(options, dbname_, &db_);
 }
 
-bool DBTestBase::IsDirectIOSupported() {
-  EnvOptions env_options;
-  env_options.use_mmap_writes = false;
-  env_options.use_direct_writes = true;
-  std::string tmp = TempFileName(dbname_, 999);
-  Status s;
-  {
-    unique_ptr<WritableFile> file;
-    s = env_->NewWritableFile(tmp, &file, env_options);
-  }
-  if (s.ok()) {
-    s = env_->DeleteFile(tmp);
-  }
-  return s.ok();
-}
-
 Status DBTestBase::Flush(int cf) {
   if (cf == 0) {
     return db_->Flush(FlushOptions());

db/db_test_util.h

@@ -231,20 +231,6 @@ class SpecialEnv : public EnvWrapper {
           return base_->Append(data);
         }
       }
-      Status PositionedAppend(const Slice& data, uint64_t offset) override {
-        if (env_->table_write_callback_) {
-          (*env_->table_write_callback_)();
-        }
-        if (env_->drop_writes_.load(std::memory_order_acquire)) {
-          // Drop writes on the floor
-          return Status::OK();
-        } else if (env_->no_space_.load(std::memory_order_acquire)) {
-          return Status::NoSpace("No space left on device");
-        } else {
-          env_->bytes_written_ += data.size();
-          return base_->PositionedAppend(data, offset);
-        }
-      }
       Status Truncate(uint64_t size) override { return base_->Truncate(size); }
       Status Close() override {
 // SyncPoint is not supported in Released Windows Mode.
@@ -271,9 +257,6 @@ class SpecialEnv : public EnvWrapper {
       Env::IOPriority GetIOPriority() override {
         return base_->GetIOPriority();
       }
-      bool use_direct_io() const override {
-        return base_->use_direct_io();
-      }
     };
     class ManifestFile : public WritableFile {
      public:
@@ -375,14 +358,7 @@ class SpecialEnv : public EnvWrapper {
       return Status::IOError("simulated write error");
     }
 
-    EnvOptions optimized = soptions;
-    if (strstr(f.c_str(), "MANIFEST") != nullptr ||
-        strstr(f.c_str(), "log") != nullptr) {
-      optimized.use_mmap_writes = false;
-      optimized.use_direct_writes = false;
-    }
-
-    Status s = target()->NewWritableFile(f, r, optimized);
+    Status s = target()->NewWritableFile(f, r, soptions);
     if (s.ok()) {
       if (strstr(f.c_str(), ".sst") != nullptr) {
         r->reset(new SSTableFile(this, std::move(*r)));
@@ -716,8 +692,6 @@ class DBTestBase : public testing::Test {
 
   Status TryReopen(const Options& options);
 
-  bool IsDirectIOSupported();
-
   Status Flush(int cf = 0);
 
   Status Put(const Slice& k, const Slice& v, WriteOptions wo = WriteOptions());
@@ -857,10 +831,10 @@ class DBTestBase : public testing::Test {
 
   std::vector<std::uint64_t> ListTableFiles(Env* env, const std::string& path);
 
-  void VerifyDBFromMap(
-      std::map<std::string, std::string> true_data,
-      size_t* total_reads_res = nullptr, bool tailing_iter = false,
-      std::map<std::string, Status> status = std::map<std::string, Status>());
+  void VerifyDBFromMap(std::map<std::string, std::string> true_data,
+                       size_t* total_reads_res = nullptr,
+                       bool tailing_iter = false,
+                       std::map<std::string, Status> status = {});
 
   void VerifyDBInternal(
       std::vector<std::pair<std::string, std::string>> true_data);

db/dbformat.h

@@ -528,11 +528,10 @@ struct RangeTombstone {
   Slice start_key_;
   Slice end_key_;
   SequenceNumber seq_;
-  RangeTombstone() = default;
-  RangeTombstone(Slice sk, Slice ek, SequenceNumber sn)
+  explicit RangeTombstone(Slice sk, Slice ek, SequenceNumber sn)
       : start_key_(sk), end_key_(ek), seq_(sn) {}
 
-  RangeTombstone(ParsedInternalKey parsed_key, Slice value) {
+  explicit RangeTombstone(ParsedInternalKey parsed_key, Slice value) {
     start_key_ = parsed_key.user_key;
     seq_ = parsed_key.sequence;
     end_key_ = value;

db/event_helpers.cc

@@ -128,7 +128,7 @@ void EventHelpers::LogAndNotifyTableFileDeletion(
   info.job_id = job_id;
   info.file_path = file_path;
   info.status = status;
-  for (auto& listener : listeners) {
+  for (auto listener : listeners) {
     listener->OnTableFileDeleted(info);
   }
 #endif  // !ROCKSDB_LITE

db/external_sst_file_test.cc

@@ -3,8 +3,6 @@
 //  LICENSE file in the root directory of this source tree. An additional grant
 //  of patent rights can be found in the PATENTS file in the same directory.
 
-#include <functional>
-
 #include "db/db_test_util.h"
 #include "port/port.h"
 #include "port/stack_trace.h"
@@ -141,9 +139,6 @@ TEST_F(ExternalSSTFileTest, Basic) {
 
     SstFileWriter sst_file_writer(EnvOptions(), options, options.comparator);
 
-    // Current file size should be 0 after sst_file_writer init and before open a file.
-    ASSERT_EQ(sst_file_writer.FileSize(), 0);
-
     // file1.sst (0 => 99)
     std::string file1 = sst_files_dir_ + "file1.sst";
     ASSERT_OK(sst_file_writer.Open(file1));
@@ -153,10 +148,6 @@ TEST_F(ExternalSSTFileTest, Basic) {
     ExternalSstFileInfo file1_info;
     Status s = sst_file_writer.Finish(&file1_info);
     ASSERT_TRUE(s.ok()) << s.ToString();
-
-    // Current file size should be non-zero after success write.
-    ASSERT_GT(sst_file_writer.FileSize(), 0);
-
     ASSERT_EQ(file1_info.file_path, file1);
     ASSERT_EQ(file1_info.num_entries, 100);
     ASSERT_EQ(file1_info.smallest_key, Key(0));
@@ -191,10 +182,7 @@ TEST_F(ExternalSSTFileTest, Basic) {
     }
     ExternalSstFileInfo file3_info;
     s = sst_file_writer.Finish(&file3_info);
-
     ASSERT_TRUE(s.ok()) << s.ToString();
-    // Current file size should be non-zero after success finish.
-    ASSERT_GT(sst_file_writer.FileSize(), 0);
     ASSERT_EQ(file3_info.file_path, file3);
     ASSERT_EQ(file3_info.num_entries, 105);
     ASSERT_EQ(file3_info.smallest_key, Key(195));
@@ -1760,6 +1748,8 @@ TEST_F(ExternalSSTFileTest, CompactionDeadlock) {
   // `DBImpl::AddFile:Start` will wait until we be here
   TEST_SYNC_POINT("ExternalSSTFileTest::DeadLock:1");
 
+  ASSERT_EQ(running_threads.load(), 2);
+
   // Wait for IngestExternalFile() to start and aquire mutex
   TEST_SYNC_POINT("ExternalSSTFileTest::DeadLock:2");
 
@@ -1945,47 +1935,6 @@ TEST_F(ExternalSSTFileTest, SnapshotInconsistencyBug) {
   db_->ReleaseSnapshot(snap);
 }
 
-TEST_F(ExternalSSTFileTest, FadviseTrigger) {
-  Options options = CurrentOptions();
-  const int kNumKeys = 10000;
-
-  size_t total_fadvised_bytes = 0;
-  rocksdb::SyncPoint::GetInstance()->SetCallBack(
-      "SstFileWriter::InvalidatePageCache", [&](void* arg) {
-        size_t fadvise_size = *(reinterpret_cast<size_t*>(arg));
-        total_fadvised_bytes += fadvise_size;
-      });
-  rocksdb::SyncPoint::GetInstance()->EnableProcessing();
-
-  std::unique_ptr<SstFileWriter> sst_file_writer;
-
-  std::string sst_file_path = sst_files_dir_ + "file_fadvise_disable.sst";
-  sst_file_writer.reset(new SstFileWriter(EnvOptions(), options,
-                                          options.comparator, nullptr, false));
-  ASSERT_OK(sst_file_writer->Open(sst_file_path));
-  for (int i = 0; i < kNumKeys; i++) {
-    ASSERT_OK(sst_file_writer->Add(Key(i), Key(i)));
-  }
-  ASSERT_OK(sst_file_writer->Finish());
-  // fadvise disabled
-  ASSERT_EQ(total_fadvised_bytes, 0);
-
-
-  sst_file_path = sst_files_dir_ + "file_fadvise_enable.sst";
-  sst_file_writer.reset(new SstFileWriter(EnvOptions(), options,
-                                          options.comparator, nullptr, true));
-  ASSERT_OK(sst_file_writer->Open(sst_file_path));
-  for (int i = 0; i < kNumKeys; i++) {
-    ASSERT_OK(sst_file_writer->Add(Key(i), Key(i)));
-  }
-  ASSERT_OK(sst_file_writer->Finish());
-  // fadvise enabled
-  ASSERT_EQ(total_fadvised_bytes, sst_file_writer->FileSize());
-  ASSERT_GT(total_fadvised_bytes, 0);
-
-  rocksdb::SyncPoint::GetInstance()->DisableProcessing();
-}
-
 #endif  // ROCKSDB_LITE
 
 }  // namespace rocksdb

db/file_indexer.cc

@@ -9,9 +9,8 @@
 
 #include "db/file_indexer.h"
 #include <algorithm>
-#include <functional>
-#include "db/version_edit.h"
 #include "rocksdb/comparator.h"
+#include "db/version_edit.h"
 
 namespace rocksdb {
 

db/flush_job.cc

@@ -230,12 +230,6 @@ Status FlushJob::Run(FileMetaData* file_meta) {
   return s;
 }
 
-void FlushJob::Cancel() {
-  db_mutex_->AssertHeld();
-  assert(base_ != nullptr);
-  base_->Unref();
-}
-
 Status FlushJob::WriteLevel0Table() {
   AutoThreadOperationStageUpdater stage_updater(
       ThreadStatus::STAGE_FLUSH_WRITE_L0);

db/flush_job.h

@@ -67,11 +67,9 @@ class FlushJob {
 
   ~FlushJob();
 
-  // Require db_mutex held.
-  // Once PickMemTable() is called, either Run() or Cancel() has to be call.
+  // Require db_mutex held
   void PickMemTable();
   Status Run(FileMetaData* file_meta = nullptr);
-  void Cancel();
   TableProperties GetTableProperties() const { return table_properties_; }
 
  private:

db/forward_iterator_bench.cc

@@ -330,7 +330,7 @@ int main(int argc, char** argv) {
   options.compaction_style = rocksdb::CompactionStyle::kCompactionStyleNone;
   options.level0_slowdown_writes_trigger = 99999;
   options.level0_stop_writes_trigger = 99999;
-  options.use_direct_writes = true;
+  options.allow_os_buffer = false;
   options.write_buffer_size = FLAGS_memtable_size;
   rocksdb::BlockBasedTableOptions table_options;
   table_options.block_cache = rocksdb::NewLRUCache(FLAGS_block_cache_size);

db/internal_stats.cc

@@ -865,7 +865,7 @@ void InternalStats::DumpCFMapStats(std::map<std::string, double>* cf_stats) {
   }
 }
 
-void InternalStats::DumpCFMapStats(
+int InternalStats::DumpCFMapStats(
     std::map<int, std::map<LevelStatType, double>>* levels_stats,
     CompactionStats* compaction_stats_sum) {
   const VersionStorageInfo* vstorage = cfd_->current()->storage_info();
@@ -925,10 +925,11 @@ void InternalStats::DumpCFMapStats(
   PrepareLevelStats(&sum_stats, total_files, total_files_being_compacted,
                     total_file_size, 0, w_amp, *compaction_stats_sum);
   (*levels_stats)[-1] = sum_stats;  //  -1 is for the Sum level
+  return num_levels_to_check;
 }
 
 void InternalStats::DumpCFStats(std::string* value) {
-  char buf[2000];
+  char buf[1000];
   // Per-ColumnFamily stats
   PrintLevelStatsHeader(buf, sizeof(buf), cfd_->GetName());
   value->append(buf);
@@ -936,8 +937,8 @@ void InternalStats::DumpCFStats(std::string* value) {
   // Print stats for each level
   std::map<int, std::map<LevelStatType, double>> levels_stats;
   CompactionStats compaction_stats_sum(0);
-  DumpCFMapStats(&levels_stats, &compaction_stats_sum);
-  for (int l = 0; l < number_levels_; ++l) {
+  int levels = DumpCFMapStats(&levels_stats, &compaction_stats_sum);
+  for (int l = 0; l < levels; ++l) {
     if (levels_stats.find(l) != levels_stats.end()) {
       PrintLevelStats(buf, sizeof(buf), "L" + ToString(l), levels_stats[l]);
       value->append(buf);

db/internal_stats.h

@@ -273,7 +273,7 @@ class InternalStats {
  private:
   void DumpDBStats(std::string* value);
   void DumpCFMapStats(std::map<std::string, double>* cf_stats);
-  void DumpCFMapStats(
+  int DumpCFMapStats(
       std::map<int, std::map<LevelStatType, double>>* level_stats,
       CompactionStats* compaction_stats_sum);
   void DumpCFStats(std::string* value);

db/listener_test.cc

@@ -119,7 +119,7 @@ TEST_F(EventListenerTest, OnSingleDBCompactionTest) {
   options.max_bytes_for_level_base = options.target_file_size_base * 2;
   options.max_bytes_for_level_multiplier = 2;
   options.compression = kNoCompression;
-#ifdef ROCKSDB_USING_THREAD_STATUS
+#if ROCKSDB_USING_THREAD_STATUS
   options.enable_thread_tracking = true;
 #endif  // ROCKSDB_USING_THREAD_STATUS
   options.level0_file_num_compaction_trigger = kNumL0Files;
@@ -176,7 +176,7 @@ class TestFlushListener : public EventListener {
     ASSERT_GT(info.table_properties.num_data_blocks, 0U);
     ASSERT_GT(info.table_properties.num_entries, 0U);
 
-#ifdef ROCKSDB_USING_THREAD_STATUS
+#if ROCKSDB_USING_THREAD_STATUS
     // Verify the id of the current thread that created this table
     // file matches the id of any active flush or compaction thread.
     uint64_t thread_id = env_->GetThreadID();
@@ -232,7 +232,7 @@ class TestFlushListener : public EventListener {
 TEST_F(EventListenerTest, OnSingleDBFlushTest) {
   Options options;
   options.write_buffer_size = k110KB;
-#ifdef ROCKSDB_USING_THREAD_STATUS
+#if ROCKSDB_USING_THREAD_STATUS
   options.enable_thread_tracking = true;
 #endif  // ROCKSDB_USING_THREAD_STATUS
   TestFlushListener* listener = new TestFlushListener(options.env);
@@ -268,7 +268,7 @@ TEST_F(EventListenerTest, OnSingleDBFlushTest) {
 TEST_F(EventListenerTest, MultiCF) {
   Options options;
   options.write_buffer_size = k110KB;
-#ifdef ROCKSDB_USING_THREAD_STATUS
+#if ROCKSDB_USING_THREAD_STATUS
   options.enable_thread_tracking = true;
 #endif  // ROCKSDB_USING_THREAD_STATUS
   TestFlushListener* listener = new TestFlushListener(options.env);
@@ -302,7 +302,7 @@ TEST_F(EventListenerTest, MultiCF) {
 
 TEST_F(EventListenerTest, MultiDBMultiListeners) {
   Options options;
-#ifdef ROCKSDB_USING_THREAD_STATUS
+#if ROCKSDB_USING_THREAD_STATUS
   options.enable_thread_tracking = true;
 #endif  // ROCKSDB_USING_THREAD_STATUS
   options.table_properties_collector_factories.push_back(
@@ -384,7 +384,7 @@ TEST_F(EventListenerTest, MultiDBMultiListeners) {
 
 TEST_F(EventListenerTest, DisableBGCompaction) {
   Options options;
-#ifdef ROCKSDB_USING_THREAD_STATUS
+#if ROCKSDB_USING_THREAD_STATUS
   options.enable_thread_tracking = true;
 #endif  // ROCKSDB_USING_THREAD_STATUS
   TestFlushListener* listener = new TestFlushListener(options.env);

db/memtable.cc

@@ -560,7 +560,7 @@ static bool SaveValue(void* arg, const char* entry) {
     ValueType type;
     UnPackSequenceAndType(tag, &s->seq, &type);
 
-    if ((type == kTypeValue || type == kTypeMerge) &&
+    if ((type == kTypeValue || type == kTypeDeletion) &&
         range_del_agg->ShouldDelete(Slice(key_ptr, key_length))) {
       type = kTypeRangeDeletion;
     }
@@ -717,22 +717,29 @@ void MemTable::Update(SequenceNumber seq,
       ValueType type;
       SequenceNumber unused;
       UnPackSequenceAndType(tag, &unused, &type);
-      if (type == kTypeValue) {
-        Slice prev_value = GetLengthPrefixedSlice(key_ptr + key_length);
-        uint32_t prev_size = static_cast<uint32_t>(prev_value.size());
-        uint32_t new_size = static_cast<uint32_t>(value.size());
+      switch (type) {
+        case kTypeValue: {
+          Slice prev_value = GetLengthPrefixedSlice(key_ptr + key_length);
+          uint32_t prev_size = static_cast<uint32_t>(prev_value.size());
+          uint32_t new_size = static_cast<uint32_t>(value.size());
 
-        // Update value, if new value size  <= previous value size
-        if (new_size <= prev_size ) {
-          char* p = EncodeVarint32(const_cast<char*>(key_ptr) + key_length,
-                                   new_size);
-          WriteLock wl(GetLock(lkey.user_key()));
-          memcpy(p, value.data(), value.size());
-          assert((unsigned)((p + value.size()) - entry) ==
-                 (unsigned)(VarintLength(key_length) + key_length +
-                            VarintLength(value.size()) + value.size()));
-          return;
+          // Update value, if new value size  <= previous value size
+          if (new_size <= prev_size ) {
+            char* p = EncodeVarint32(const_cast<char*>(key_ptr) + key_length,
+                                     new_size);
+            WriteLock wl(GetLock(lkey.user_key()));
+            memcpy(p, value.data(), value.size());
+            assert((unsigned)((p + value.size()) - entry) ==
+                   (unsigned)(VarintLength(key_length) + key_length +
+                              VarintLength(value.size()) + value.size()));
+            return;
+          }
         }
+        default:
+          // If the latest value is kTypeDeletion, kTypeMerge or kTypeLogData
+          // we don't have enough space for update inplace
+            Add(seq, kTypeValue, key, value);
+            return;
       }
     }
   }

db/merge_helper.cc

@@ -104,10 +104,6 @@ Status MergeHelper::MergeUntil(InternalIterator* iter,
   Status s;
   bool hit_the_next_user_key = false;
   for (; iter->Valid(); iter->Next(), original_key_is_iter = false) {
-    if (IsShuttingDown()) {
-      return Status::ShutdownInProgress();
-    }
-
     ParsedInternalKey ikey;
     assert(keys_.size() == merge_context_.GetNumOperands());
 
@@ -196,10 +192,7 @@ Status MergeHelper::MergeUntil(InternalIterator* iter,
               ? CompactionFilter::Decision::kKeep
               : FilterMerge(orig_ikey.user_key, value_slice);
       if (range_del_agg != nullptr &&
-
-          range_del_agg->ShouldDelete(
-              iter->key(),
-              RangeDelAggregator::RangePositioningMode::kForwardTraversal) &&
+          range_del_agg->ShouldDelete(iter->key()) &&
           filter != CompactionFilter::Decision::kRemoveAndSkipUntil) {
         filter = CompactionFilter::Decision::kRemove;
       }
@@ -282,6 +275,10 @@ Status MergeHelper::MergeUntil(InternalIterator* iter,
     // We haven't seen the beginning of the key nor a Put/Delete.
     // Attempt to use the user's associative merge function to
     // merge the stacked merge operands into a single operand.
+    //
+    // TODO(noetzli) The docblock of MergeUntil suggests that a successful
+    // partial merge returns Status::OK(). Should we change the status code
+    // after a successful partial merge?
     s = Status::MergeInProgress();
     if (merge_context_.GetNumOperands() >= 2 &&
         merge_context_.GetNumOperands() >= min_partial_merge_operands_) {

db/merge_helper.h

@@ -34,13 +34,11 @@ class MergeHelper {
               const CompactionFilter* compaction_filter, Logger* logger,
               unsigned min_partial_merge_operands,
               bool assert_valid_internal_key, SequenceNumber latest_snapshot,
-              int level = 0, Statistics* stats = nullptr,
-              const std::atomic<bool>* shutting_down = nullptr)
+              int level = 0, Statistics* stats = nullptr)
       : env_(env),
         user_comparator_(user_comparator),
         user_merge_operator_(user_merge_operator),
         compaction_filter_(compaction_filter),
-        shutting_down_(shutting_down),
         logger_(logger),
         min_partial_merge_operands_(min_partial_merge_operands),
         assert_valid_internal_key_(assert_valid_internal_key),
@@ -83,12 +81,10 @@ class MergeHelper {
   //
   // Returns one of the following statuses:
   // - OK: Entries were successfully merged.
-  // - MergeInProgress: Put/Delete not encountered, and didn't reach the start
-  //   of key's history. Output consists of merge operands only.
+  // - MergeInProgress: Put/Delete not encountered and unable to merge operands.
   // - Corruption: Merge operator reported unsuccessful merge or a corrupted
   //   key has been encountered and not expected (applies only when compiling
   //   with asserts removed).
-  // - ShutdownInProgress: interrupted by shutdown (*shutting_down == true).
   //
   // REQUIRED: The first key in the input is not corrupted.
   Status MergeUntil(InternalIterator* iter,
@@ -154,7 +150,6 @@ class MergeHelper {
   const Comparator* user_comparator_;
   const MergeOperator* user_merge_operator_;
   const CompactionFilter* compaction_filter_;
-  const std::atomic<bool>* shutting_down_;
   Logger* logger_;
   unsigned min_partial_merge_operands_;
   bool assert_valid_internal_key_; // enforce no internal key corruption?
@@ -176,11 +171,6 @@ class MergeHelper {
   bool has_compaction_filter_skip_until_ = false;
   std::string compaction_filter_value_;
   InternalKey compaction_filter_skip_until_;
-
-  bool IsShuttingDown() {
-    // This is a best-effort facility, so memory_order_relaxed is sufficient.
-    return shutting_down_ && shutting_down_->load(std::memory_order_relaxed);
-  }
 };
 
 // MergeOutputIterator can be used to iterate over the result of a merge.

db/pinned_iterators_manager.h

@@ -16,7 +16,7 @@ namespace rocksdb {
 // PinnedIteratorsManager will be notified whenever we need to pin an Iterator
 // and it will be responsible for deleting pinned Iterators when they are
 // not needed anymore.
-class PinnedIteratorsManager : public Cleanable {
+class PinnedIteratorsManager {
  public:
   PinnedIteratorsManager() : pinning_enabled(false) {}
   ~PinnedIteratorsManager() {
@@ -59,16 +59,16 @@ class PinnedIteratorsManager : public Cleanable {
 
     // Remove duplicate pointers
     std::sort(pinned_ptrs_.begin(), pinned_ptrs_.end());
-    auto unique_end = std::unique(pinned_ptrs_.begin(), pinned_ptrs_.end());
+    std::unique(pinned_ptrs_.begin(), pinned_ptrs_.end());
 
-    for (auto i = pinned_ptrs_.begin(); i != unique_end; ++i) {
-      void* ptr = i->first;
-      ReleaseFunction release_func = i->second;
+    for (size_t i = 0; i < pinned_ptrs_.size(); i++) {
+      assert(i == 0 || pinned_ptrs_[i].first != pinned_ptrs_[i - 1].first);
+
+      void* ptr = pinned_ptrs_[i].first;
+      ReleaseFunction release_func = pinned_ptrs_[i].second;
       release_func(ptr);
     }
     pinned_ptrs_.clear();
-    // Also do cleanups from the base Cleanable
-    Cleanable::Reset();
   }
 
  private:

db/range_del_aggregator.cc

@@ -11,40 +11,29 @@ namespace rocksdb {
 
 RangeDelAggregator::RangeDelAggregator(
     const InternalKeyComparator& icmp,
-    const std::vector<SequenceNumber>& snapshots,
-    bool collapse_deletions /* = true */)
-    : upper_bound_(kMaxSequenceNumber),
-      icmp_(icmp),
-      collapse_deletions_(collapse_deletions) {
+    const std::vector<SequenceNumber>& snapshots)
+    : upper_bound_(kMaxSequenceNumber), icmp_(icmp) {
   InitRep(snapshots);
 }
 
 RangeDelAggregator::RangeDelAggregator(const InternalKeyComparator& icmp,
-                                       SequenceNumber snapshot,
-                                       bool collapse_deletions /* = false */)
-    : upper_bound_(snapshot),
-      icmp_(icmp),
-      collapse_deletions_(collapse_deletions) {}
+                                       SequenceNumber snapshot)
+    : upper_bound_(snapshot), icmp_(icmp) {}
 
 void RangeDelAggregator::InitRep(const std::vector<SequenceNumber>& snapshots) {
   assert(rep_ == nullptr);
   rep_.reset(new Rep());
   for (auto snapshot : snapshots) {
     rep_->stripe_map_.emplace(
-        snapshot,
-        PositionalTombstoneMap(TombstoneMap(
-            stl_wrappers::LessOfComparator(icmp_.user_comparator()))));
+        snapshot, TombstoneMap(stl_wrappers::LessOfComparator(&icmp_)));
   }
   // Data newer than any snapshot falls in this catch-all stripe
   rep_->stripe_map_.emplace(
-      kMaxSequenceNumber,
-      PositionalTombstoneMap(TombstoneMap(
-          stl_wrappers::LessOfComparator(icmp_.user_comparator()))));
+      kMaxSequenceNumber, TombstoneMap(stl_wrappers::LessOfComparator(&icmp_)));
   rep_->pinned_iters_mgr_.StartPinning();
 }
 
-bool RangeDelAggregator::ShouldDelete(
-    const Slice& internal_key, RangeDelAggregator::RangePositioningMode mode) {
+bool RangeDelAggregator::ShouldDelete(const Slice& internal_key) {
   if (rep_ == nullptr) {
     return false;
   }
@@ -52,103 +41,32 @@ bool RangeDelAggregator::ShouldDelete(
   if (!ParseInternalKey(internal_key, &parsed)) {
     assert(false);
   }
-  return ShouldDelete(parsed, mode);
+  return ShouldDelete(parsed);
 }
 
-bool RangeDelAggregator::ShouldDelete(
-    const ParsedInternalKey& parsed,
-    RangeDelAggregator::RangePositioningMode mode) {
+bool RangeDelAggregator::ShouldDelete(const ParsedInternalKey& parsed) {
   assert(IsValueType(parsed.type));
   if (rep_ == nullptr) {
     return false;
   }
-  auto& positional_tombstone_map = GetPositionalTombstoneMap(parsed.sequence);
-  const auto& tombstone_map = positional_tombstone_map.raw_map;
-  if (tombstone_map.empty()) {
-    return false;
-  }
-  auto& tombstone_map_iter = positional_tombstone_map.iter;
-  if (tombstone_map_iter == tombstone_map.end() &&
-      (mode == kForwardTraversal || mode == kBackwardTraversal)) {
-    // invalid (e.g., if AddTombstones() changed the deletions), so need to
-    // reseek
-    mode = kBinarySearch;
-  }
-  switch (mode) {
-    case kFullScan:
-      assert(!collapse_deletions_);
-      // The maintained state (PositionalTombstoneMap::iter) isn't useful when
-      // we linear scan from the beginning each time, but we maintain it anyways
-      // for consistency.
-      tombstone_map_iter = tombstone_map.begin();
-      while (tombstone_map_iter != tombstone_map.end()) {
-        const auto& tombstone = tombstone_map_iter->second;
-        if (icmp_.user_comparator()->Compare(parsed.user_key,
-                                             tombstone.start_key_) < 0) {
-          break;
-        }
-        if (parsed.sequence < tombstone.seq_ &&
-            icmp_.user_comparator()->Compare(parsed.user_key,
-                                             tombstone.end_key_) < 0) {
-          return true;
-        }
-        ++tombstone_map_iter;
-      }
-      return false;
-    case kForwardTraversal:
-      assert(collapse_deletions_ && tombstone_map_iter != tombstone_map.end());
-      if (tombstone_map_iter == tombstone_map.begin() &&
-          icmp_.user_comparator()->Compare(parsed.user_key,
-                                           tombstone_map_iter->first) < 0) {
-        // before start of deletion intervals
-        return false;
-      }
-      while (std::next(tombstone_map_iter) != tombstone_map.end() &&
-             icmp_.user_comparator()->Compare(
-                 std::next(tombstone_map_iter)->first, parsed.user_key) <= 0) {
-        ++tombstone_map_iter;
-      }
-      break;
-    case kBackwardTraversal:
-      assert(collapse_deletions_ && tombstone_map_iter != tombstone_map.end());
-      while (tombstone_map_iter != tombstone_map.begin() &&
-             icmp_.user_comparator()->Compare(parsed.user_key,
-                                              tombstone_map_iter->first) < 0) {
-        --tombstone_map_iter;
-      }
-      if (tombstone_map_iter == tombstone_map.begin() &&
-          icmp_.user_comparator()->Compare(parsed.user_key,
-                                           tombstone_map_iter->first) < 0) {
-        // before start of deletion intervals
-        return false;
-      }
-      break;
-    case kBinarySearch:
-      assert(collapse_deletions_);
-      tombstone_map_iter =
-          tombstone_map.upper_bound(parsed.user_key);
-      if (tombstone_map_iter == tombstone_map.begin()) {
-        // before start of deletion intervals
-        return false;
-      }
-      --tombstone_map_iter;
+  const auto& tombstone_map = GetTombstoneMap(parsed.sequence);
+  for (const auto& start_key_and_tombstone : tombstone_map) {
+    const auto& tombstone = start_key_and_tombstone.second;
+    if (icmp_.user_comparator()->Compare(parsed.user_key,
+                                         tombstone.start_key_) < 0) {
       break;
+    }
+    if (parsed.sequence < tombstone.seq_ &&
+        icmp_.user_comparator()->Compare(parsed.user_key, tombstone.end_key_) <
+            0) {
+      return true;
+    }
   }
-  assert(mode != kFullScan);
-  assert(tombstone_map_iter != tombstone_map.end() &&
-         icmp_.user_comparator()->Compare(tombstone_map_iter->first,
-                                          parsed.user_key) <= 0);
-  assert(std::next(tombstone_map_iter) == tombstone_map.end() ||
-         icmp_.user_comparator()->Compare(
-             parsed.user_key, std::next(tombstone_map_iter)->first) < 0);
-  return parsed.sequence < tombstone_map_iter->second.seq_;
+  return false;
 }
 
 bool RangeDelAggregator::ShouldAddTombstones(
     bool bottommost_level /* = false */) {
-  // TODO(andrewkr): can we just open a file and throw it away if it ends up
-  // empty after AddToBuilder()? This function doesn't take into subcompaction
-  // boundaries so isn't completely accurate.
   if (rep_ == nullptr) {
     return false;
   }
@@ -160,7 +78,7 @@ bool RangeDelAggregator::ShouldAddTombstones(
     ++stripe_map_iter;
   }
   while (stripe_map_iter != rep_->stripe_map_.end()) {
-    if (!stripe_map_iter->second.raw_map.empty()) {
+    if (!stripe_map_iter->second.empty()) {
       return true;
     }
     ++stripe_map_iter;
@@ -179,8 +97,6 @@ Status RangeDelAggregator::AddTombstones(
     if (first_iter) {
       if (rep_ == nullptr) {
         InitRep({upper_bound_});
-      } else {
-        InvalidateTombstoneMapPositions();
       }
       first_iter = false;
     }
@@ -189,7 +105,8 @@ Status RangeDelAggregator::AddTombstones(
       return Status::Corruption("Unable to parse range tombstone InternalKey");
     }
     RangeTombstone tombstone(parsed_key, input->value());
-    AddTombstone(std::move(tombstone));
+    auto& tombstone_map = GetTombstoneMap(tombstone.seq_);
+    tombstone_map.emplace(input->key(), std::move(tombstone));
     input->Next();
   }
   if (!first_iter) {
@@ -198,172 +115,8 @@ Status RangeDelAggregator::AddTombstones(
   return Status::OK();
 }
 
-void RangeDelAggregator::InvalidateTombstoneMapPositions() {
-  if (rep_ == nullptr) {
-    return;
-  }
-  for (auto stripe_map_iter = rep_->stripe_map_.begin();
-       stripe_map_iter != rep_->stripe_map_.end(); ++stripe_map_iter) {
-    stripe_map_iter->second.iter = stripe_map_iter->second.raw_map.end();
-  }
-}
-
-Status RangeDelAggregator::AddTombstone(RangeTombstone tombstone) {
-  auto& positional_tombstone_map = GetPositionalTombstoneMap(tombstone.seq_);
-  auto& tombstone_map = positional_tombstone_map.raw_map;
-  if (collapse_deletions_) {
-    // In collapsed mode, we only fill the seq_ field in the TombstoneMap's
-    // values. The end_key is unneeded because we assume the tombstone extends
-    // until the next tombstone starts. For gaps between real tombstones and
-    // for the last real tombstone, we denote end keys by inserting fake
-    // tombstones with sequence number zero.
-    std::vector<RangeTombstone> new_range_dels{
-        tombstone, RangeTombstone(tombstone.end_key_, Slice(), 0)};
-    auto new_range_dels_iter = new_range_dels.begin();
-    // Position at the first overlapping existing tombstone; if none exists,
-    // insert until we find an existing one overlapping a new point
-    const Slice* tombstone_map_begin = nullptr;
-    if (!tombstone_map.empty()) {
-      tombstone_map_begin = &tombstone_map.begin()->first;
-    }
-    auto last_range_dels_iter = new_range_dels_iter;
-    while (new_range_dels_iter != new_range_dels.end() &&
-           (tombstone_map_begin == nullptr ||
-            icmp_.user_comparator()->Compare(new_range_dels_iter->start_key_,
-                                             *tombstone_map_begin) < 0)) {
-      tombstone_map.emplace(
-          new_range_dels_iter->start_key_,
-          RangeTombstone(Slice(), Slice(), new_range_dels_iter->seq_));
-      last_range_dels_iter = new_range_dels_iter;
-      ++new_range_dels_iter;
-    }
-    if (new_range_dels_iter == new_range_dels.end()) {
-      return Status::OK();
-    }
-    // above loop advances one too far
-    new_range_dels_iter = last_range_dels_iter;
-    auto tombstone_map_iter =
-        tombstone_map.upper_bound(new_range_dels_iter->start_key_);
-    // if nothing overlapped we would've already inserted all the new points
-    // and returned early
-    assert(tombstone_map_iter != tombstone_map.begin());
-    tombstone_map_iter--;
-
-    // untermed_seq is non-kMaxSequenceNumber when we covered an existing point
-    // but haven't seen its corresponding endpoint. It's used for (1) deciding
-    // whether to forcibly insert the new interval's endpoint; and (2) possibly
-    // raising the seqnum for the to-be-inserted element (we insert the max
-    // seqnum between the next new interval and the unterminated interval).
-    SequenceNumber untermed_seq = kMaxSequenceNumber;
-    while (tombstone_map_iter != tombstone_map.end() &&
-           new_range_dels_iter != new_range_dels.end()) {
-      const Slice *tombstone_map_iter_end = nullptr,
-                  *new_range_dels_iter_end = nullptr;
-      if (tombstone_map_iter != tombstone_map.end()) {
-        auto next_tombstone_map_iter = std::next(tombstone_map_iter);
-        if (next_tombstone_map_iter != tombstone_map.end()) {
-          tombstone_map_iter_end = &next_tombstone_map_iter->first;
-        }
-      }
-      if (new_range_dels_iter != new_range_dels.end()) {
-        auto next_new_range_dels_iter = std::next(new_range_dels_iter);
-        if (next_new_range_dels_iter != new_range_dels.end()) {
-          new_range_dels_iter_end = &next_new_range_dels_iter->start_key_;
-        }
-      }
-
-      // our positions in existing/new tombstone collections should always
-      // overlap. The non-overlapping cases are handled above and below this
-      // loop.
-      assert(new_range_dels_iter_end == nullptr ||
-             icmp_.user_comparator()->Compare(tombstone_map_iter->first,
-                                              *new_range_dels_iter_end) < 0);
-      assert(tombstone_map_iter_end == nullptr ||
-             icmp_.user_comparator()->Compare(new_range_dels_iter->start_key_,
-                                              *tombstone_map_iter_end) < 0);
-
-      int new_to_old_start_cmp = icmp_.user_comparator()->Compare(
-          new_range_dels_iter->start_key_, tombstone_map_iter->first);
-      // nullptr end means extends infinitely rightwards, set new_to_old_end_cmp
-      // accordingly so we can use common code paths later.
-      int new_to_old_end_cmp;
-      if (new_range_dels_iter_end == nullptr &&
-          tombstone_map_iter_end == nullptr) {
-        new_to_old_end_cmp = 0;
-      } else if (new_range_dels_iter_end == nullptr) {
-        new_to_old_end_cmp = 1;
-      } else if (tombstone_map_iter_end == nullptr) {
-        new_to_old_end_cmp = -1;
-      } else {
-        new_to_old_end_cmp = icmp_.user_comparator()->Compare(
-            *new_range_dels_iter_end, *tombstone_map_iter_end);
-      }
-
-      if (new_to_old_start_cmp < 0) {
-        // the existing one's left endpoint comes after, so raise/delete it if
-        // it's covered.
-        if (tombstone_map_iter->second.seq_ < new_range_dels_iter->seq_) {
-          untermed_seq = tombstone_map_iter->second.seq_;
-          if (tombstone_map_iter != tombstone_map.begin() &&
-              std::prev(tombstone_map_iter)->second.seq_ ==
-                  new_range_dels_iter->seq_) {
-            tombstone_map_iter = tombstone_map.erase(tombstone_map_iter);
-            --tombstone_map_iter;
-          } else {
-            tombstone_map_iter->second.seq_ = new_range_dels_iter->seq_;
-          }
-        }
-      } else if (new_to_old_start_cmp > 0) {
-        if (untermed_seq != kMaxSequenceNumber ||
-            tombstone_map_iter->second.seq_ < new_range_dels_iter->seq_) {
-          auto seq = tombstone_map_iter->second.seq_;
-          // need to adjust this element if not intended to span beyond the new
-          // element (i.e., was_tombstone_map_iter_raised == true), or if it
-          // can be raised
-          tombstone_map_iter = tombstone_map.emplace(
-              new_range_dels_iter->start_key_,
-              RangeTombstone(
-                  Slice(), Slice(),
-                  std::max(
-                      untermed_seq == kMaxSequenceNumber ? 0 : untermed_seq,
-                      new_range_dels_iter->seq_)));
-          untermed_seq = seq;
-        }
-      } else {
-        // their left endpoints coincide, so raise the existing one if needed
-        if (tombstone_map_iter->second.seq_ < new_range_dels_iter->seq_) {
-          untermed_seq = tombstone_map_iter->second.seq_;
-          tombstone_map_iter->second.seq_ = new_range_dels_iter->seq_;
-        }
-      }
-
-      // advance whichever one ends earlier, or both if their right endpoints
-      // coincide
-      if (new_to_old_end_cmp < 0) {
-        ++new_range_dels_iter;
-      } else if (new_to_old_end_cmp > 0) {
-        ++tombstone_map_iter;
-        untermed_seq = kMaxSequenceNumber;
-      } else {
-        ++new_range_dels_iter;
-        ++tombstone_map_iter;
-        untermed_seq = kMaxSequenceNumber;
-      }
-    }
-    while (new_range_dels_iter != new_range_dels.end()) {
-      tombstone_map.emplace(
-          new_range_dels_iter->start_key_,
-          RangeTombstone(Slice(), Slice(), new_range_dels_iter->seq_));
-      ++new_range_dels_iter;
-    }
-  } else {
-    tombstone_map.emplace(tombstone.start_key_, std::move(tombstone));
-  }
-  return Status::OK();
-}
-
-RangeDelAggregator::PositionalTombstoneMap&
-RangeDelAggregator::GetPositionalTombstoneMap(SequenceNumber seq) {
+RangeDelAggregator::TombstoneMap& RangeDelAggregator::GetTombstoneMap(
+    SequenceNumber seq) {
   assert(rep_ != nullptr);
   // The stripe includes seqnum for the snapshot above and excludes seqnum for
   // the snapshot below.
@@ -395,16 +148,10 @@ void RangeDelAggregator::AddToBuilder(
   auto stripe_map_iter = rep_->stripe_map_.begin();
   assert(stripe_map_iter != rep_->stripe_map_.end());
   if (bottommost_level) {
-    // TODO(andrewkr): these are counted for each compaction output file, so
-    // lots of double-counting.
-    if (!stripe_map_iter->second.raw_map.empty()) {
-      range_del_out_stats->num_range_del_drop_obsolete +=
-          static_cast<int64_t>(stripe_map_iter->second.raw_map.size()) -
-          (collapse_deletions_ ? 1 : 0);
-      range_del_out_stats->num_record_drop_obsolete +=
-          static_cast<int64_t>(stripe_map_iter->second.raw_map.size()) -
-          (collapse_deletions_ ? 1 : 0);
-    }
+    range_del_out_stats->num_range_del_drop_obsolete +=
+        static_cast<int64_t>(stripe_map_iter->second.size());
+    range_del_out_stats->num_record_drop_obsolete +=
+        static_cast<int64_t>(stripe_map_iter->second.size());
     // For the bottommost level, keys covered by tombstones in the first
     // (oldest) stripe have been compacted away, so the tombstones are obsolete.
     ++stripe_map_iter;
@@ -414,23 +161,8 @@ void RangeDelAggregator::AddToBuilder(
   // insert them into a std::map on the read path.
   bool first_added = false;
   while (stripe_map_iter != rep_->stripe_map_.end()) {
-    for (auto tombstone_map_iter = stripe_map_iter->second.raw_map.begin();
-         tombstone_map_iter != stripe_map_iter->second.raw_map.end();
-         ++tombstone_map_iter) {
-      RangeTombstone tombstone;
-      if (collapse_deletions_) {
-        auto next_tombstone_map_iter = std::next(tombstone_map_iter);
-        if (next_tombstone_map_iter == stripe_map_iter->second.raw_map.end() ||
-            tombstone_map_iter->second.seq_ == 0) {
-          // it's a sentinel tombstone
-          continue;
-        }
-        tombstone.start_key_ = tombstone_map_iter->first;
-        tombstone.end_key_ = next_tombstone_map_iter->first;
-        tombstone.seq_ = tombstone_map_iter->second.seq_;
-      } else {
-        tombstone = tombstone_map_iter->second;
-      }
+    for (const auto& start_key_and_tombstone : stripe_map_iter->second) {
+      const auto& tombstone = start_key_and_tombstone.second;
       if (upper_bound != nullptr &&
           icmp_.user_comparator()->Compare(*upper_bound,
                                            tombstone.start_key_) <= 0) {
@@ -509,7 +241,7 @@ bool RangeDelAggregator::IsEmpty() {
   }
   for (auto stripe_map_iter = rep_->stripe_map_.begin();
        stripe_map_iter != rep_->stripe_map_.end(); ++stripe_map_iter) {
-    if (!stripe_map_iter->second.raw_map.empty()) {
+    if (!stripe_map_iter->second.empty()) {
       return false;
     }
   }

db/range_del_aggregator.h

@@ -32,51 +32,27 @@ class RangeDelAggregator {
   //    stripes, which is the seqnum range between consecutive snapshots,
   //    including the higher snapshot and excluding the lower one. Currently,
   //    this is used by ShouldDelete() to prevent deletion of keys that are
-  //    covered by range tombstones in other snapshot stripes. This constructor
-  //    is used for writes (flush/compaction). All DB snapshots are provided
-  //    such that no keys are removed that are uncovered according to any DB
-  //    snapshot.
+  //    covered by range tombstones in other snapshot stripes. In case of writes
+  //    (flush/compaction), all DB snapshots are provided such that no keys are
+  //    removed that are uncovered according to any DB snapshot. In case of read
+  //    (get/iterator), only the user snapshot is provided such that the seqnum
+  //    space is divided into two stripes, where only tombstones in the older
+  //    stripe are considered by ShouldDelete().
   // Note this overload does not lazily initialize Rep.
   RangeDelAggregator(const InternalKeyComparator& icmp,
-                     const std::vector<SequenceNumber>& snapshots,
-                     bool collapse_deletions = true);
+                     const std::vector<SequenceNumber>& snapshots);
 
   // @param upper_bound Similar to snapshots above, except with a single
   //    snapshot, which allows us to store the snapshot on the stack and defer
   //    initialization of heap-allocating members (in Rep) until the first range
-  //    deletion is encountered. This constructor is used in case of reads (get/
-  //    iterator), for which only the user snapshot (upper_bound) is provided
-  //    such that the seqnum space is divided into two stripes. Only the older
-  //    stripe will be used by ShouldDelete().
+  //    deletion is encountered.
   RangeDelAggregator(const InternalKeyComparator& icmp,
-                     SequenceNumber upper_bound,
-                     bool collapse_deletions = false);
-
-  // We maintain position in the tombstone map across calls to ShouldDelete. The
-  // caller may wish to specify a mode to optimize positioning the iterator
-  // during the next call to ShouldDelete. The non-kFullScan modes are only
-  // available when deletion collapsing is enabled.
-  //
-  // For example, if we invoke Next() on an iterator, kForwardTraversal should
-  // be specified to advance one-by-one through deletions until one is found
-  // with its interval containing the key. This will typically be faster than
-  // doing a full binary search (kBinarySearch).
-  enum RangePositioningMode {
-    kFullScan,  // used iff collapse_deletions_ == false
-    kForwardTraversal,
-    kBackwardTraversal,
-    kBinarySearch,
-  };
+                     SequenceNumber upper_bound);
 
   // Returns whether the key should be deleted, which is the case when it is
   // covered by a range tombstone residing in the same snapshot stripe.
-  // @param mode If collapse_deletions_ is true, this dictates how we will find
-  //             the deletion whose interval contains this key. Otherwise, its
-  //             value must be kFullScan indicating linear scan from beginning..
-  bool ShouldDelete(const ParsedInternalKey& parsed,
-                    RangePositioningMode mode = kFullScan);
-  bool ShouldDelete(const Slice& internal_key,
-                    RangePositioningMode mode = kFullScan);
+  bool ShouldDelete(const ParsedInternalKey& parsed);
+  bool ShouldDelete(const Slice& internal_key);
   bool ShouldAddTombstones(bool bottommost_level = false);
 
   // Adds tombstones to the tombstone aggregation structure maintained by this
@@ -84,13 +60,6 @@ class RangeDelAggregator {
   // @return non-OK status if any of the tombstone keys are corrupted.
   Status AddTombstones(std::unique_ptr<InternalIterator> input);
 
-  // Resets iterators maintained across calls to ShouldDelete(). This may be
-  // called when the tombstones change, or the owner may call explicitly, e.g.,
-  // if it's an iterator that just seeked to an arbitrary position. The effect
-  // of invalidation is that the following call to ShouldDelete() will binary
-  // search for its tombstone.
-  void InvalidateTombstoneMapPositions();
-
   // Writes tombstones covering a range to a table builder.
   // @param extend_before_min_key If true, the range of tombstones to be added
   //    to the TableBuilder starts from the beginning of the key-range;
@@ -118,26 +87,12 @@ class RangeDelAggregator {
   bool IsEmpty();
 
  private:
-  // Maps tombstone user start key -> tombstone object
-  typedef std::multimap<Slice, RangeTombstone, stl_wrappers::LessOfComparator>
+  // Maps tombstone internal start key -> tombstone object
+  typedef std::map<Slice, RangeTombstone, stl_wrappers::LessOfComparator>
       TombstoneMap;
-  // Also maintains position in TombstoneMap last seen by ShouldDelete(). The
-  // end iterator indicates invalidation (e.g., if AddTombstones() changes the
-  // underlying map). End iterator cannot be invalidated.
-  struct PositionalTombstoneMap {
-    explicit PositionalTombstoneMap(TombstoneMap _raw_map)
-        : raw_map(std::move(_raw_map)), iter(raw_map.end()) {}
-    PositionalTombstoneMap(const PositionalTombstoneMap&) = delete;
-    PositionalTombstoneMap(PositionalTombstoneMap&& other)
-        : raw_map(std::move(other.raw_map)), iter(raw_map.end()) {}
-
-    TombstoneMap raw_map;
-    TombstoneMap::const_iterator iter;
-  };
-
   // Maps snapshot seqnum -> map of tombstones that fall in that stripe, i.e.,
   // their seqnums are greater than the next smaller snapshot's seqnum.
-  typedef std::map<SequenceNumber, PositionalTombstoneMap> StripeMap;
+  typedef std::map<SequenceNumber, TombstoneMap> StripeMap;
 
   struct Rep {
     StripeMap stripe_map_;
@@ -148,14 +103,11 @@ class RangeDelAggregator {
   // once the first range deletion is encountered.
   void InitRep(const std::vector<SequenceNumber>& snapshots);
 
-  PositionalTombstoneMap& GetPositionalTombstoneMap(SequenceNumber seq);
-  Status AddTombstone(RangeTombstone tombstone);
+  TombstoneMap& GetTombstoneMap(SequenceNumber seq);
 
   SequenceNumber upper_bound_;
   std::unique_ptr<Rep> rep_;
   const InternalKeyComparator& icmp_;
-  // collapse range deletions so they're binary searchable
-  const bool collapse_deletions_;
 };
 
 }  // namespace rocksdb

db/range_del_aggregator_test.cc

@@ -1,157 +0,0 @@
-//  Copyright (c) 2016-present, Facebook, Inc.  All rights reserved.
-//  This source code is licensed under the BSD-style license found in the
-//  LICENSE file in the root directory of this source tree. An additional grant
-//  of patent rights can be found in the PATENTS file in the same directory.
-
-#include <algorithm>
-
-#include "db/db_test_util.h"
-#include "db/range_del_aggregator.h"
-#include "rocksdb/comparator.h"
-#include "util/testutil.h"
-
-namespace rocksdb {
-
-class RangeDelAggregatorTest : public testing::Test {};
-
-namespace {
-
-struct ExpectedPoint {
-  Slice begin;
-  SequenceNumber seq;
-};
-
-enum Direction {
-  kForward,
-  kReverse,
-};
-
-void VerifyRangeDels(const std::vector<RangeTombstone>& range_dels,
-                     const std::vector<ExpectedPoint>& expected_points) {
-  // Test same result regardless of which order the range deletions are added.
-  for (Direction dir : {kForward, kReverse}) {
-    auto icmp = InternalKeyComparator(BytewiseComparator());
-    RangeDelAggregator range_del_agg(icmp, {} /* snapshots */, true);
-    std::vector<std::string> keys, values;
-    for (const auto& range_del : range_dels) {
-      auto key_and_value = range_del.Serialize();
-      keys.push_back(key_and_value.first.Encode().ToString());
-      values.push_back(key_and_value.second.ToString());
-    }
-    if (dir == kReverse) {
-      std::reverse(keys.begin(), keys.end());
-      std::reverse(values.begin(), values.end());
-    }
-    std::unique_ptr<test::VectorIterator> range_del_iter(
-        new test::VectorIterator(keys, values));
-    range_del_agg.AddTombstones(std::move(range_del_iter));
-
-    for (const auto expected_point : expected_points) {
-      ParsedInternalKey parsed_key;
-      parsed_key.user_key = expected_point.begin;
-      parsed_key.sequence = expected_point.seq;
-      parsed_key.type = kTypeValue;
-      ASSERT_FALSE(range_del_agg.ShouldDelete(
-          parsed_key,
-          RangeDelAggregator::RangePositioningMode::kForwardTraversal));
-      if (parsed_key.sequence > 0) {
-        --parsed_key.sequence;
-        ASSERT_TRUE(range_del_agg.ShouldDelete(
-            parsed_key,
-            RangeDelAggregator::RangePositioningMode::kForwardTraversal));
-      }
-    }
-  }
-}
-
-}  // anonymous namespace
-
-TEST_F(RangeDelAggregatorTest, Empty) { VerifyRangeDels({}, {{"a", 0}}); }
-
-TEST_F(RangeDelAggregatorTest, SameStartAndEnd) {
-  VerifyRangeDels({{"a", "a", 5}}, {{" ", 0}, {"a", 0}, {"b", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, Single) {
-  VerifyRangeDels({{"a", "b", 10}}, {{" ", 0}, {"a", 10}, {"b", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, OverlapAboveLeft) {
-  VerifyRangeDels({{"a", "c", 10}, {"b", "d", 5}},
-                  {{" ", 0}, {"a", 10}, {"c", 5}, {"d", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, OverlapAboveRight) {
-  VerifyRangeDels({{"a", "c", 5}, {"b", "d", 10}},
-                  {{" ", 0}, {"a", 5}, {"b", 10}, {"d", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, OverlapAboveMiddle) {
-  VerifyRangeDels({{"a", "d", 5}, {"b", "c", 10}},
-                  {{" ", 0}, {"a", 5}, {"b", 10}, {"c", 5}, {"d", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, OverlapFully) {
-  VerifyRangeDels({{"a", "d", 10}, {"b", "c", 5}},
-                  {{" ", 0}, {"a", 10}, {"d", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, OverlapPoint) {
-  VerifyRangeDels({{"a", "b", 5}, {"b", "c", 10}},
-                  {{" ", 0}, {"a", 5}, {"b", 10}, {"c", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, SameStartKey) {
-  VerifyRangeDels({{"a", "c", 5}, {"a", "b", 10}},
-                  {{" ", 0}, {"a", 10}, {"b", 5}, {"c", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, SameEndKey) {
-  VerifyRangeDels({{"a", "d", 5}, {"b", "d", 10}},
-                  {{" ", 0}, {"a", 5}, {"b", 10}, {"d", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, GapsBetweenRanges) {
-  VerifyRangeDels(
-      {{"a", "b", 5}, {"c", "d", 10}, {"e", "f", 15}},
-      {{" ", 0}, {"a", 5}, {"b", 0}, {"c", 10}, {"d", 0}, {"e", 15}, {"f", 0}});
-}
-
-// 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) {
-  VerifyRangeDels(
-      {{"b", "d", 5}, {"c", "f", 10}, {"e", "g", 15}, {"a", "f", 20}},
-      {{" ", 0}, {"a", 20}, {"f", 15}, {"g", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, CoverMultipleFromRight) {
-  VerifyRangeDels(
-      {{"b", "d", 5}, {"c", "f", 10}, {"e", "g", 15}, {"c", "h", 20}},
-      {{" ", 0}, {"b", 5}, {"c", 20}, {"h", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, CoverMultipleFully) {
-  VerifyRangeDels(
-      {{"b", "d", 5}, {"c", "f", 10}, {"e", "g", 15}, {"a", "h", 20}},
-      {{" ", 0}, {"a", 20}, {"h", 0}});
-}
-
-TEST_F(RangeDelAggregatorTest, AlternateMultipleAboveBelow) {
-  VerifyRangeDels(
-      {{"b", "d", 15}, {"c", "f", 10}, {"e", "g", 20}, {"a", "h", 5}},
-      {{" ", 0},
-       {"a", 5},
-       {"b", 15},
-       {"d", 10},
-       {"e", 20},
-       {"g", 5},
-       {"h", 0}});
-}
-
-}  // namespace rocksdb
-
-int main(int argc, char** argv) {
-  ::testing::InitGoogleTest(&argc, argv);
-  return RUN_ALL_TESTS();
-}

db/version_builder.cc

@@ -16,7 +16,6 @@
 #include <inttypes.h>
 #include <algorithm>
 #include <atomic>
-#include <functional>
 #include <set>
 #include <thread>
 #include <unordered_map>

db/version_set.h

@@ -520,8 +520,6 @@ class Version {
     return next_;
   }
 
-  int TEST_refs() const { return refs_; }
-
   VersionStorageInfo* storage_info() { return &storage_info_; }
 
   VersionSet* version_set() { return vset_; }

db/write_callback_test.cc

@@ -5,7 +5,6 @@
 
 #ifndef ROCKSDB_LITE
 
-#include <functional>
 #include <string>
 #include <utility>
 #include <vector>

docs/_posts/2017-01-06-rocksdb-5-0-1-released.markdown

@@ -1,26 +0,0 @@
----
-title: RocksDB 5.0.1 Released!
-layout: post
-author: yiwu
-category: blog
----
-
-### Public API Change
-
-  * Options::max_bytes_for_level_multiplier is now a double along with all getters and setters.
-  * Support dynamically change `delayed_write_rate` and `max_total_wal_size` options via SetDBOptions().
-  * Introduce DB::DeleteRange for optimized deletion of large ranges of contiguous keys.
-  * Support dynamically change `delayed_write_rate` option via SetDBOptions().
-  * Options::allow_concurrent_memtable_write and Options::enable_write_thread_adaptive_yield are now true by default.
-  * Remove Tickers::SEQUENCE_NUMBER to avoid confusion if statistics object is shared among RocksDB instance. Alternatively DB::GetLatestSequenceNumber() can be used to get the same value.
-  * Options.level0_stop_writes_trigger default value changes from 24 to 32.
-  * New compaction filter API: CompactionFilter::FilterV2(). Allows to drop ranges of keys.
-  * Removed flashcache support.
-  * DB::AddFile() is deprecated and is replaced with DB::IngestExternalFile(). DB::IngestExternalFile() remove all the restrictions that existed for DB::AddFile.
-
-### New Features
-
-  * Add avoid_flush_during_shutdown option, which speeds up DB shutdown by not flushing unpersisted data (i.e. with disableWAL = true). Unpersisted data will be lost. The options is dynamically changeable via SetDBOptions().
-  * Add memtable_insert_with_hint_prefix_extractor option. The option is mean to reduce CPU usage for inserting keys into memtable, if keys can be group by prefix and insert for each prefix are sequential or almost sequential. See include/rocksdb/options.h for more details.
-  * Add LuaCompactionFilter in utilities.  This allows developers to write compaction filters in Lua.  To use this feature, LUA_PATH needs to be set to the root directory of Lua.
-  * No longer populate "LATEST_BACKUP" file in backup directory, which formerly contained the number of the latest backup. The latest backup can be determined by finding the highest numbered file in the "meta/" subdirectory.

examples/Makefile

@@ -10,7 +10,7 @@ endif
 	
 .PHONY: clean librocksdb
 
-all: simple_example column_families_example compact_files_example c_simple_example optimistic_transaction_example transaction_example compaction_filter_example options_file_example
+all: simple_example column_families_example compact_files_example c_simple_example optimistic_transaction_example transaction_example compaction_filter_example
 
 simple_example: librocksdb simple_example.cc
 	$(CXX) $(CXXFLAGS) $@.cc -o$@ ../librocksdb.a -I../include -O2 -std=c++11 $(PLATFORM_LDFLAGS) $(PLATFORM_CXXFLAGS) $(EXEC_LDFLAGS)
@@ -40,7 +40,7 @@ options_file_example: librocksdb options_file_example.cc
 	$(CXX) $(CXXFLAGS) $@.cc -o$@ ../librocksdb.a -I../include -O2 -std=c++11 $(PLATFORM_LDFLAGS) $(PLATFORM_CXXFLAGS) $(EXEC_LDFLAGS)
 
 clean:
-	rm -rf ./simple_example ./column_families_example ./compact_files_example ./compaction_filter_example ./c_simple_example c_simple_example.o ./optimistic_transaction_example ./transaction_example ./options_file_example
+	rm -rf ./simple_example ./column_families_example ./compact_files_example ./compaction_filter_example ./c_simple_example c_simple_example.o ./optimistic_transaction_example ./transaction_example
 
 librocksdb:
 	cd .. && $(MAKE) static_lib

examples/c_simple_example.c

@@ -1,8 +1,3 @@
-// Copyright (c) 2011-present, Facebook, Inc.  All rights reserved.
-// This source code is licensed under the BSD-style license found in the
-// LICENSE file in the root directory of this source tree. An additional grant
-// of patent rights can be found in the PATENTS file in the same directory.
-
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>

examples/rocksdb_option_file_example.ini

@@ -73,13 +73,12 @@
   error_if_exists=false
   recycle_log_file_num=0
   skip_log_error_on_recovery=false
+  allow_mmap_reads=false
+  allow_os_buffer=true
   db_log_dir=
   new_table_reader_for_compaction_inputs=true
-  allow_mmap_reads=false
   allow_mmap_writes=false
-  use_direct_reads=false
-  use_direct_writes=false
-
+  
 
 [CFOptions "default"]
   compaction_style=kCompactionStyleLevel
@@ -128,7 +127,7 @@
   write_buffer_size=134217728
   disable_auto_compactions=false
   inplace_update_support=false
-
+  
 [TableOptions/BlockBasedTable "default"]
   format_version=2
   whole_key_filtering=true

examples/simple_example.cc

@@ -2,7 +2,6 @@
 // This source code is licensed under the BSD-style license found in the
 // LICENSE file in the root directory of this source tree. An additional grant
 // of patent rights can be found in the PATENTS file in the same directory.
-
 #include <cstdio>
 #include <string>
 

include/rocksdb/c.h

@@ -90,7 +90,6 @@ typedef struct rocksdb_iterator_t        rocksdb_iterator_t;
 typedef struct rocksdb_logger_t          rocksdb_logger_t;
 typedef struct rocksdb_mergeoperator_t   rocksdb_mergeoperator_t;
 typedef struct rocksdb_options_t         rocksdb_options_t;
-typedef struct rocksdb_compactoptions_t rocksdb_compactoptions_t;
 typedef struct rocksdb_block_based_table_options_t
     rocksdb_block_based_table_options_t;
 typedef struct rocksdb_cuckoo_table_options_t
@@ -316,15 +315,6 @@ extern ROCKSDB_LIBRARY_API void rocksdb_compact_range_cf(
     const char* start_key, size_t start_key_len, const char* limit_key,
     size_t limit_key_len);
 
-extern ROCKSDB_LIBRARY_API void rocksdb_compact_range_opt(
-    rocksdb_t* db, rocksdb_compactoptions_t* opt, const char* start_key,
-    size_t start_key_len, const char* limit_key, size_t limit_key_len);
-
-extern ROCKSDB_LIBRARY_API void rocksdb_compact_range_cf_opt(
-    rocksdb_t* db, rocksdb_column_family_handle_t* column_family,
-    rocksdb_compactoptions_t* opt, const char* start_key, size_t start_key_len,
-    const char* limit_key, size_t limit_key_len);
-
 extern ROCKSDB_LIBRARY_API void rocksdb_delete_file(rocksdb_t* db,
                                                     const char* name);
 
@@ -424,22 +414,6 @@ extern ROCKSDB_LIBRARY_API void rocksdb_writebatch_deletev(
 extern ROCKSDB_LIBRARY_API void rocksdb_writebatch_deletev_cf(
     rocksdb_writebatch_t* b, rocksdb_column_family_handle_t* column_family,
     int num_keys, const char* const* keys_list, const size_t* keys_list_sizes);
-extern ROCKSDB_LIBRARY_API void rocksdb_writebatch_delete_range(
-    rocksdb_writebatch_t* b, const char* start_key, size_t start_key_len,
-    const char* end_key, size_t end_key_len);
-extern ROCKSDB_LIBRARY_API void rocksdb_writebatch_delete_range_cf(
-    rocksdb_writebatch_t* b, rocksdb_column_family_handle_t* column_family,
-    const char* start_key, size_t start_key_len, const char* end_key,
-    size_t end_key_len);
-extern ROCKSDB_LIBRARY_API void rocksdb_writebatch_delete_rangev(
-    rocksdb_writebatch_t* b, int num_keys, const char* const* start_keys_list,
-    const size_t* start_keys_list_sizes, const char* const* end_keys_list,
-    const size_t* end_keys_list_sizes);
-extern ROCKSDB_LIBRARY_API void rocksdb_writebatch_delete_rangev_cf(
-    rocksdb_writebatch_t* b, rocksdb_column_family_handle_t* column_family,
-    int num_keys, const char* const* start_keys_list,
-    const size_t* start_keys_list_sizes, const char* const* end_keys_list,
-    const size_t* end_keys_list_sizes);
 extern ROCKSDB_LIBRARY_API void rocksdb_writebatch_put_log_data(
     rocksdb_writebatch_t*, const char* blob, size_t len);
 extern ROCKSDB_LIBRARY_API void rocksdb_writebatch_iterate(
@@ -657,14 +631,12 @@ extern ROCKSDB_LIBRARY_API void rocksdb_options_set_manifest_preallocation_size(
 extern ROCKSDB_LIBRARY_API void
 rocksdb_options_set_purge_redundant_kvs_while_flush(rocksdb_options_t*,
                                                     unsigned char);
+extern ROCKSDB_LIBRARY_API void rocksdb_options_set_allow_os_buffer(
+    rocksdb_options_t*, unsigned char);
 extern ROCKSDB_LIBRARY_API void rocksdb_options_set_allow_mmap_reads(
     rocksdb_options_t*, unsigned char);
 extern ROCKSDB_LIBRARY_API void rocksdb_options_set_allow_mmap_writes(
     rocksdb_options_t*, unsigned char);
-extern ROCKSDB_LIBRARY_API void rocksdb_options_set_use_direct_reads(
-    rocksdb_options_t*, unsigned char);
-extern ROCKSDB_LIBRARY_API void rocksdb_options_set_use_direct_writes(
-    rocksdb_options_t*, unsigned char);
 extern ROCKSDB_LIBRARY_API void rocksdb_options_set_is_fd_close_on_exec(
     rocksdb_options_t*, unsigned char);
 extern ROCKSDB_LIBRARY_API void rocksdb_options_set_skip_log_error_on_recovery(
@@ -877,10 +849,6 @@ extern ROCKSDB_LIBRARY_API void rocksdb_readoptions_set_tailing(
     rocksdb_readoptions_t*, unsigned char);
 extern ROCKSDB_LIBRARY_API void rocksdb_readoptions_set_readahead_size(
     rocksdb_readoptions_t*, size_t);
-extern ROCKSDB_LIBRARY_API void rocksdb_readoptions_set_pin_data(
-    rocksdb_readoptions_t*, unsigned char);
-extern ROCKSDB_LIBRARY_API void rocksdb_readoptions_set_total_order_seek(
-    rocksdb_readoptions_t*, unsigned char);
 
 /* Write options */
 
@@ -893,20 +861,6 @@ extern ROCKSDB_LIBRARY_API void rocksdb_writeoptions_set_sync(
 extern ROCKSDB_LIBRARY_API void rocksdb_writeoptions_disable_WAL(
     rocksdb_writeoptions_t* opt, int disable);
 
-/* Compact range options */
-
-extern ROCKSDB_LIBRARY_API rocksdb_compactoptions_t*
-rocksdb_compactoptions_create();
-extern ROCKSDB_LIBRARY_API void rocksdb_compactoptions_destroy(
-    rocksdb_compactoptions_t*);
-extern ROCKSDB_LIBRARY_API void
-rocksdb_compactoptions_set_exclusive_manual_compaction(
-    rocksdb_compactoptions_t*, unsigned char);
-extern ROCKSDB_LIBRARY_API void rocksdb_compactoptions_set_change_level(
-    rocksdb_compactoptions_t*, unsigned char);
-extern ROCKSDB_LIBRARY_API void rocksdb_compactoptions_set_target_level(
-    rocksdb_compactoptions_t*, int);
-
 /* Flush options */
 
 extern ROCKSDB_LIBRARY_API rocksdb_flushoptions_t*
@@ -923,10 +877,6 @@ extern ROCKSDB_LIBRARY_API rocksdb_cache_t* rocksdb_cache_create_lru(
 extern ROCKSDB_LIBRARY_API void rocksdb_cache_destroy(rocksdb_cache_t* cache);
 extern ROCKSDB_LIBRARY_API void rocksdb_cache_set_capacity(
     rocksdb_cache_t* cache, size_t capacity);
-extern ROCKSDB_LIBRARY_API size_t
-rocksdb_cache_get_usage(rocksdb_cache_t* cache);
-extern ROCKSDB_LIBRARY_API size_t
-rocksdb_cache_get_pinned_usage(rocksdb_cache_t* cache);
 
 /* Env */
 

include/rocksdb/cache.h

@@ -24,7 +24,6 @@
 
 #include <stdint.h>
 #include <memory>
-#include <string>
 #include "rocksdb/slice.h"
 #include "rocksdb/statistics.h"
 #include "rocksdb/status.h"
@@ -102,12 +101,6 @@ class Cache {
   // function.
   virtual Handle* Lookup(const Slice& key, Statistics* stats = nullptr) = 0;
 
-  // Increments the reference count for the handle if it refers to an entry in
-  // the cache. Returns true if refcount was incremented; otherwise, returns
-  // false.
-  // REQUIRES: handle must have been returned by a method on *this.
-  virtual bool Ref(Handle* handle) = 0;
-
   // Release a mapping returned by a previous Lookup().
   // REQUIRES: handle must not have been released yet.
   // REQUIRES: handle must have been returned by a method on *this.
@@ -174,8 +167,6 @@ class Cache {
   // Prerequisit: no entry is referenced.
   virtual void EraseUnRefEntries() = 0;
 
-  virtual std::string GetPrintableOptions() const { return ""; }
-
  private:
   // No copying allowed
   Cache(const Cache&);

include/rocksdb/compaction_filter.h

@@ -140,12 +140,6 @@ class CompactionFilter {
   //      by kRemoveAndSkipUntil can disappear from a snapshot - beware
   //      if you're using TransactionDB or DB::GetSnapshot().
   //
-  //      Another warning: if value for a key was overwritten or merged into
-  //      (multiple Put()s or Merge()s), and compaction filter skips this key
-  //      with kRemoveAndSkipUntil, it's possible that it will remove only
-  //      the new value, exposing the old value that was supposed to be
-  //      overwritten.
-  //
   //      If you use kRemoveAndSkipUntil, consider also reducing
   //      compaction_readahead_size option.
   //

include/rocksdb/convenience.h

@@ -288,8 +288,6 @@ Status GetStringFromColumnFamilyOptions(std::string* opts_str,
 Status GetStringFromCompressionType(std::string* compression_str,
                                     CompressionType compression_type);
 
-std::vector<CompressionType> GetSupportedCompressions();
-
 Status GetBlockBasedTableOptionsFromString(
     const BlockBasedTableOptions& table_options,
     const std::string& opts_str,

include/rocksdb/env.h

@@ -19,7 +19,6 @@
 
 #include <stdint.h>
 #include <cstdarg>
-#include <functional>
 #include <limits>
 #include <memory>
 #include <string>
@@ -51,7 +50,6 @@ struct ThreadStatus;
 using std::unique_ptr;
 using std::shared_ptr;
 
-const size_t kDefaultPageSize = 4 * 1024;
 
 // Options while opening a file to read/write
 struct EnvOptions {
@@ -62,6 +60,9 @@ struct EnvOptions {
   // construct from Options
   explicit EnvOptions(const DBOptions& options);
 
+  // If true, then allow caching of data in environment buffers
+  bool use_os_buffer = true;
+
    // If true, then use mmap to read data
   bool use_mmap_reads = false;
 
@@ -199,10 +200,6 @@ class Env {
   // Store in *result the names of the children of the specified directory.
   // The names are relative to "dir".
   // Original contents of *results are dropped.
-  // Returns OK if "dir" exists and "*result" contains its children.
-  //         NotFound if "dir" does not exist, the calling process does not have
-  //                  permission to access "dir", or if "dir" is invalid.
-  //         IOError if an IO Error was encountered
   virtual Status GetChildren(const std::string& dir,
                              std::vector<std::string>* result) = 0;
 
@@ -212,10 +209,6 @@ class Env {
   // result.
   // The name attributes are relative to "dir".
   // Original contents of *results are dropped.
-  // Returns OK if "dir" exists and "*result" contains its children.
-  //         NotFound if "dir" does not exist, the calling process does not have
-  //                  permission to access "dir", or if "dir" is invalid.
-  //         IOError if an IO Error was encountered
   virtual Status GetChildrenFileAttributes(const std::string& dir,
                                            std::vector<FileAttributes>* result);
 
@@ -371,8 +364,8 @@ class Env {
   // OptimizeForManifestWrite will create a new EnvOptions object that is a copy
   // of the EnvOptions in the parameters, but is optimized for writing manifest
   // files. Default implementation returns the copy of the same object.
-  virtual EnvOptions OptimizeForManifestWrite(
-      const EnvOptions& env_options) const;
+  virtual EnvOptions OptimizeForManifestWrite(const EnvOptions& env_options)
+      const;
 
   // Returns the status of all threads that belong to the current Env.
   virtual Status GetThreadList(std::vector<ThreadStatus>* thread_list) {
@@ -430,27 +423,12 @@ class SequentialFile {
   // REQUIRES: External synchronization
   virtual Status Skip(uint64_t n) = 0;
 
-  // Indicates the upper layers if the current SequentialFile implementation
-  // uses direct IO.
-  virtual bool use_direct_io() const { return false; }
-
-  // Use the returned alignment value to allocate
-  // aligned buffer for Direct I/O
-  virtual size_t GetRequiredBufferAlignment() const { return kDefaultPageSize; }
-
   // Remove any kind of caching of data from the offset to offset+length
   // of this file. If the length is 0, then it refers to the end of file.
   // If the system is not caching the file contents, then this is a noop.
   virtual Status InvalidateCache(size_t offset, size_t length) {
     return Status::NotSupported("InvalidateCache not supported.");
   }
-
-  // Positioned Read for direct I/O
-  // If Direct I/O enabled, offset, n, and scratch should be properly aligned
-  virtual Status PositionedRead(uint64_t offset, size_t n, Slice* result,
-                                char* scratch) {
-    return Status::NotSupported();
-  }
 };
 
 // A file abstraction for randomly reading the contents of a file.
@@ -468,7 +446,6 @@ class RandomAccessFile {
   // status.
   //
   // Safe for concurrent use by multiple threads.
-  // If Direct I/O enabled, offset, n, and scratch should be aligned properly.
   virtual Status Read(uint64_t offset, size_t n, Slice* result,
                       char* scratch) const = 0;
 
@@ -506,14 +483,6 @@ class RandomAccessFile {
 
   virtual void Hint(AccessPattern pattern) {}
 
-  // Indicates the upper layers if the current RandomAccessFile implementation
-  // uses direct IO.
-  virtual bool use_direct_io() const { return false; }
-
-  // Use the returned alignment value to allocate
-  // aligned buffer for Direct I/O
-  virtual size_t GetRequiredBufferAlignment() const { return kDefaultPageSize; }
-
   // Remove any kind of caching of data from the offset to offset+length
   // of this file. If the length is 0, then it refers to the end of file.
   // If the system is not caching the file contents, then this is a noop.
@@ -534,6 +503,21 @@ class WritableFile {
   }
   virtual ~WritableFile();
 
+  // Indicates if the class makes use of unbuffered I/O
+  // If false you must pass aligned buffer to Write()
+  virtual bool UseOSBuffer() const {
+    return true;
+  }
+
+  const size_t c_DefaultPageSize = 4 * 1024;
+
+  // Use the returned alignment value to allocate
+  // aligned buffer for Write() when UseOSBuffer()
+  // returns false
+  virtual size_t GetRequiredBufferAlignment() const {
+    return c_DefaultPageSize;
+  }
+
   // Append data to the end of the file
   // Note: A WriteabelFile object must support either Append or
   // PositionedAppend, so the users cannot mix the two.
@@ -545,7 +529,7 @@ class WritableFile {
   // the sector. The implementation thus needs to also rewrite the last
   // partial sector.
   // Note: PositionAppend does not guarantee moving the file offset after the
-  // write. A WritableFile object must support either Append or
+  // write. A WriteabelFile object must support either Append or
   // PositionedAppend, so the users cannot mix the two.
   //
   // PositionedAppend() can only happen on the page/sector boundaries. For that
@@ -592,11 +576,8 @@ class WritableFile {
 
   // Indicates the upper layers if the current WritableFile implementation
   // uses direct IO.
-  virtual bool use_direct_io() const { return false; }
+  virtual bool UseDirectIO() const { return false; }
 
-  // Use the returned alignment value to allocate
-  // aligned buffer for Direct I/O
-  virtual size_t GetRequiredBufferAlignment() const { return kDefaultPageSize; }
   /*
    * Change the priority in rate limiter if rate limiting is enabled.
    * If rate limiting is not enabled, this call has no effect.
@@ -705,13 +686,20 @@ class RandomRWFile {
   RandomRWFile() {}
   virtual ~RandomRWFile() {}
 
-  // Indicates if the class makes use of direct I/O
+  // Indicates if the class makes use of unbuffered I/O
   // If false you must pass aligned buffer to Write()
-  virtual bool use_direct_io() const { return false; }
+  virtual bool UseOSBuffer() const {
+    return true;
+  }
+
+  const size_t c_DefaultPageSize = 4 * 1024;
 
   // Use the returned alignment value to allocate
-  // aligned buffer for Direct I/O
-  virtual size_t GetRequiredBufferAlignment() const { return kDefaultPageSize; }
+  // aligned buffer for Write() when UseOSBuffer()
+  // returns false
+  virtual size_t GetRequiredBufferAlignment() const {
+    return c_DefaultPageSize;
+  }
 
   // Used by the file_reader_writer to decide if the ReadAhead wrapper
   // should simply forward the call and do not enact read_ahead buffering or locking.
@@ -725,7 +713,7 @@ class RandomRWFile {
   virtual void EnableReadAhead() {}
 
   // Write bytes in `data` at  offset `offset`, Returns Status::OK() on success.
-  // Pass aligned buffer when use_direct_io() returns true.
+  // Pass aligned buffer when UseOSBuffer() returns false.
   virtual Status Write(uint64_t offset, const Slice& data) = 0;
 
   // Read up to `n` bytes starting from offset `offset` and store them in

include/rocksdb/iterator.h

@@ -36,9 +36,6 @@ class Cleanable {
   // not abstract and therefore clients should not override it.
   typedef void (*CleanupFunction)(void* arg1, void* arg2);
   void RegisterCleanup(CleanupFunction function, void* arg1, void* arg2);
-  void DelegateCleanupsTo(Cleanable* other);
-  // DoCleanup and also resets the pointers for reuse
-  void Reset();
 
  protected:
   struct Cleanup {
@@ -48,13 +45,6 @@ class Cleanable {
     Cleanup* next;
   };
   Cleanup cleanup_;
-  // It also becomes the owner of c
-  void RegisterCleanup(Cleanup* c);
-
- private:
-  // Performs all the cleanups. It does not reset the pointers. Making it
-  // private to prevent misuse
-  inline void DoCleanup();
 };
 
 class Iterator : public Cleanable {

include/rocksdb/options.h

@@ -1100,6 +1100,26 @@ struct DBOptions {
   // large amounts of data (such as xfs's allocsize option).
   size_t manifest_preallocation_size;
 
+  // Hint the OS that it should not buffer disk I/O. Enabling this
+  // parameter may improve performance but increases pressure on the
+  // system cache.
+  //
+  // The exact behavior of this parameter is platform dependent.
+  //
+  // On POSIX systems, after RocksDB reads data from disk it will
+  // mark the pages as "unneeded". The operating system may - or may not
+  // - evict these pages from memory, reducing pressure on the system
+  // cache. If the disk block is requested again this can result in
+  // additional disk I/O.
+  //
+  // On WINDOWS system, files will be opened in "unbuffered I/O" mode
+  // which means that data read from the disk will not be cached or
+  // bufferized. The hardware buffer of the devices may however still
+  // be used. Memory mapped files are not impacted by this parameter.
+  //
+  // Default: true
+  bool allow_os_buffer;
+
   // Allow the OS to mmap file for reading sst tables. Default: false
   bool allow_mmap_reads;
 
@@ -1108,22 +1128,10 @@ struct DBOptions {
   // Default: false
   bool allow_mmap_writes;
 
-  // Enable direct I/O mode for read/write
-  // they may or may not improve performance depending on the use case
-  //
-  // Files will be opened in "direct I/O" mode
-  // which means that data r/w from the disk will not be cached or
-  // bufferized. The hardware buffer of the devices may however still
-  // be used. Memory mapped files are not impacted by these parameters.
-
   // Use O_DIRECT for reading file
   // Default: false
   bool use_direct_reads;
 
-  // Use O_DIRECT for writing file
-  // Default: false
-  bool use_direct_writes;
-
   // If false, fallocate() calls are bypassed
   bool allow_fallocate;
 

include/rocksdb/persistent_cache.h

@@ -54,8 +54,6 @@ class PersistentCache {
   // Persistent cache can be initialized as a tier of caches. The stats are per
   // tire top-down
   virtual StatsType Stats() = 0;
-
-  virtual std::string GetPrintableOptions() const = 0;
 };
 
 // Factor method to create a new persistent cache

include/rocksdb/sst_file_writer.h

@@ -47,12 +47,9 @@ class SstFileWriter {
   // User can pass `column_family` to specify that the the generated file will
   // be ingested into this column_family, note that passing nullptr means that
   // the column_family is unknown.
-  // If invalidate_page_cache is set to true, SstFileWriter will give the OS a
-  // hint that this file pages is not needed everytime we write 1MB to the file
   SstFileWriter(const EnvOptions& env_options, const Options& options,
                 const Comparator* user_comparator,
-                ColumnFamilyHandle* column_family = nullptr,
-                bool invalidate_page_cache = true);
+                ColumnFamilyHandle* column_family = nullptr);
 
   ~SstFileWriter();
 
@@ -69,12 +66,7 @@ class SstFileWriter {
   // which will be populated with information about the created sst file
   Status Finish(ExternalSstFileInfo* file_info = nullptr);
 
-  // Return the current file size.
-  uint64_t FileSize();
-
  private:
-  void InvalidatePageCache(bool closing);
-
   struct Rep;
   Rep* rep_;
 };

include/rocksdb/status.h

@@ -75,7 +75,6 @@ class Status {
 
   SubCode subcode() const { return subcode_; }
 
-  // Returns a C style string indicating the message of the Status
   const char* getState() const { return state_; }
 
   // Return a success status.

include/rocksdb/table.h

@@ -157,9 +157,9 @@ struct BlockBasedTableOptions {
   // a SstTable. Instead, buffer in WritableFileWriter will take
   // care of the flushing when it is full.
   //
-  // This option helps a lot when direct I/O writes
-  // (use_direct_writes = true) is used, since it avoids small
-  // direct disk write.
+  // On Windows, this option helps a lot when unbuffered I/O
+  // (allow_os_buffer = false) is used, since it avoids small
+  // unbuffered disk write.
   //
   // User may also adjust writable_file_max_buffer_size to optimize disk I/O
   // size.

include/rocksdb/thread_status.h

@@ -20,11 +20,12 @@
 #include <utility>
 #include <vector>
 
-#if !defined(ROCKSDB_LITE) && \
+#ifndef ROCKSDB_USING_THREAD_STATUS
+#define ROCKSDB_USING_THREAD_STATUS \
+    !defined(ROCKSDB_LITE) && \
     !defined(NROCKSDB_THREAD_STATUS) && \
     !defined(OS_MACOSX) && \
     !defined(IOS_CROSS_COMPILE)
-#define ROCKSDB_USING_THREAD_STATUS
 #endif
 
 namespace rocksdb {

include/rocksdb/utilities/env_librados.h

@@ -1,7 +1,5 @@
-// Copyright (c) 2011-present, Facebook, Inc.  All rights reserved.
-// This source code is licensed under the BSD-style license found in the
-// LICENSE file in the root directory of this source tree. An additional grant
-// of patent rights can be found in the PATENTS file in the same directory.
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
 #ifndef ROCKSDB_UTILITIES_ENV_LIBRADOS_H
 #define ROCKSDB_UTILITIES_ENV_LIBRADOS_H
 
@@ -17,16 +15,17 @@ namespace rocksdb {
 class LibradosWritableFile;
 
 class EnvLibrados : public EnvWrapper {
- public:
+public:
   // Create a brand new sequentially-readable file with the specified name.
   // On success, stores a pointer to the new file in *result and returns OK.
   // On failure stores nullptr in *result and returns non-OK.  If the file does
   // not exist, returns a non-OK status.
   //
   // The returned file will only be accessed by one thread at a time.
-  Status NewSequentialFile(const std::string& fname,
-                           std::unique_ptr<SequentialFile>* result,
-                           const EnvOptions& options) override;
+  Status NewSequentialFile(
+    const std::string& fname,
+    std::unique_ptr<SequentialFile>* result,
+    const EnvOptions& options);
 
   // Create a brand new random access read-only file with the
   // specified name.  On success, stores a pointer to the new file in
@@ -35,9 +34,10 @@ class EnvLibrados : public EnvWrapper {
   // status.
   //
   // The returned file may be concurrently accessed by multiple threads.
-  Status NewRandomAccessFile(const std::string& fname,
-                             std::unique_ptr<RandomAccessFile>* result,
-                             const EnvOptions& options) override;
+  Status NewRandomAccessFile(
+    const std::string& fname,
+    std::unique_ptr<RandomAccessFile>* result,
+    const EnvOptions& options);
 
   // Create an object that writes to a new file with the specified
   // name.  Deletes any existing file with the same name and creates a
@@ -46,15 +46,17 @@ class EnvLibrados : public EnvWrapper {
   // returns non-OK.
   //
   // The returned file will only be accessed by one thread at a time.
-  Status NewWritableFile(const std::string& fname,
-                         std::unique_ptr<WritableFile>* result,
-                         const EnvOptions& options) override;
+  Status NewWritableFile(
+    const std::string& fname,
+    std::unique_ptr<WritableFile>* result,
+    const EnvOptions& options);
 
   // Reuse an existing file by renaming it and opening it as writable.
-  Status ReuseWritableFile(const std::string& fname,
-                           const std::string& old_fname,
-                           std::unique_ptr<WritableFile>* result,
-                           const EnvOptions& options) override;
+  Status ReuseWritableFile(
+    const std::string& fname,
+    const std::string& old_fname,
+    std::unique_ptr<WritableFile>* result,
+    const EnvOptions& options);
 
   // Create an object that represents a directory. Will fail if directory
   // doesn't exist. If the directory exists, it will open the directory
@@ -63,44 +65,47 @@ class EnvLibrados : public EnvWrapper {
   // On success, stores a pointer to the new Directory in
   // *result and returns OK. On failure stores nullptr in *result and
   // returns non-OK.
-  Status NewDirectory(const std::string& name,
-                      std::unique_ptr<Directory>* result) override;
+  Status NewDirectory(
+    const std::string& name,
+    std::unique_ptr<Directory>* result);
 
   // Returns OK if the named file exists.
   //         NotFound if the named file does not exist,
   //                  the calling process does not have permission to determine
   //                  whether this file exists, or if the path is invalid.
   //         IOError if an IO Error was encountered
-  Status FileExists(const std::string& fname) override;
+  Status FileExists(const std::string& fname);
 
   // Store in *result the names of the children of the specified directory.
   // The names are relative to "dir".
   // Original contents of *results are dropped.
-  Status GetChildren(const std::string& dir, std::vector<std::string>* result);
+  Status GetChildren(const std::string& dir,
+                     std::vector<std::string>* result);
 
   // Delete the named file.
-  Status DeleteFile(const std::string& fname) override;
+  Status DeleteFile(const std::string& fname);
 
   // Create the specified directory. Returns error if directory exists.
-  Status CreateDir(const std::string& dirname) override;
+  Status CreateDir(const std::string& dirname);
 
   // Creates directory if missing. Return Ok if it exists, or successful in
   // Creating.
-  Status CreateDirIfMissing(const std::string& dirname) override;
+  Status CreateDirIfMissing(const std::string& dirname);
 
   // Delete the specified directory.
-  Status DeleteDir(const std::string& dirname) override;
+  Status DeleteDir(const std::string& dirname);
 
   // Store the size of fname in *file_size.
-  Status GetFileSize(const std::string& fname, uint64_t* file_size) override;
+  Status GetFileSize(const std::string& fname, uint64_t* file_size);
 
   // Store the last modification time of fname in *file_mtime.
   Status GetFileModificationTime(const std::string& fname,
-                                 uint64_t* file_mtime) override;
+                                 uint64_t* file_mtime);
   // Rename file src to target.
-  Status RenameFile(const std::string& src, const std::string& target) override;
+  Status RenameFile(const std::string& src,
+                    const std::string& target);
   // Hard Link file src to target.
-  Status LinkFile(const std::string& src, const std::string& target) override;
+  Status LinkFile(const std::string& src, const std::string& target);
 
   // Lock the specified file.  Used to prevent concurrent access to
   // the same db by multiple processes.  On failure, stores nullptr in
@@ -124,7 +129,8 @@ class EnvLibrados : public EnvWrapper {
   Status UnlockFile(FileLock* lock);
 
   // Get full directory name for this db.
-  Status GetAbsolutePath(const std::string& db_path, std::string* output_path);
+  Status GetAbsolutePath(const std::string& db_path,
+                         std::string* output_path);
 
   // Generate unique id
   std::string GenerateUniqueId();
@@ -136,29 +142,31 @@ class EnvLibrados : public EnvWrapper {
                        const std::string& config_path,
                        const std::string& db_pool);
 
-  explicit EnvLibrados(
-      const std::string& client_name,  // first 3 parameters are
-                                       // for RADOS client init
-      const std::string& cluster_name, const uint64_t flags,
-      const std::string& db_name, const std::string& config_path,
-      const std::string& db_pool, const std::string& wal_dir,
-      const std::string& wal_pool, const uint64_t write_buffer_size);
-  ~EnvLibrados() { _rados.shutdown(); }
-
- private:
+  explicit EnvLibrados(const std::string& client_name,      // first 3 parameters are for RADOS client init
+                       const std::string& cluster_name,
+                       const uint64_t flags,
+                       const std::string& db_name,
+                       const std::string& config_path,
+                       const std::string& db_pool,
+                       const std::string& wal_dir,
+                       const std::string& wal_pool,
+                       const uint64_t write_buffer_size);
+  ~EnvLibrados() {
+    _rados.shutdown();
+  }
+private:
   std::string _client_name;
   std::string _cluster_name;
   uint64_t _flags;
-  std::string _db_name;  // get from user, readable string; Also used as db_id
-                         // for db metadata
+  std::string _db_name;                                     // get from user, readable string; Also used as db_id for db metadata
   std::string _config_path;
-  librados::Rados _rados;  // RADOS client
+  librados::Rados _rados;                                   // RADOS client
   std::string _db_pool_name;
-  librados::IoCtx _db_pool_ioctx;  // IoCtx for connecting db_pool
-  std::string _wal_dir;            // WAL dir path
+  librados::IoCtx _db_pool_ioctx;                           // IoCtx for connecting db_pool
+  std::string _wal_dir;                                     // WAL dir path
   std::string _wal_pool_name;
-  librados::IoCtx _wal_pool_ioctx;  // IoCtx for connecting wal_pool
-  uint64_t _write_buffer_size;      // WritableFile buffer max size
+  librados::IoCtx _wal_pool_ioctx;                          // IoCtx for connecting wal_pool
+  uint64_t _write_buffer_size;                              // WritableFile buffer max size
 
   /* private function to communicate with rados */
   std::string _CreateFid();
@@ -167,8 +175,10 @@ class EnvLibrados : public EnvWrapper {
   Status _RenameFid(const std::string& old_fname, const std::string& new_fname);
   Status _AddFid(const std::string& fname, const std::string& fid);
   Status _DelFid(const std::string& fname);
-  Status _GetSubFnames(const std::string& dirname,
-                       std::vector<std::string>* result);
+  Status _GetSubFnames(
+    const std::string& dirname,
+    std::vector<std::string> * result
+  );
   librados::IoCtx* _GetIoctx(const std::string& prefix);
   friend class LibradosWritableFile;
 };

include/rocksdb/utilities/env_mirror.h

@@ -15,10 +15,11 @@
 // semantics and behavior are correct (in that they match that of an
 // existing, stable Env, like the default POSIX one).
 
-#pragma once
-
 #ifndef ROCKSDB_LITE
 
+#ifndef STORAGE_ROCKSDB_INCLUDE_UTILITIES_ENVMIRROR_H_
+#define STORAGE_ROCKSDB_INCLUDE_UTLIITIES_ENVMIRROR_H_
+
 #include <iostream>
 #include <algorithm>
 #include <vector>
@@ -173,4 +174,6 @@ class EnvMirror : public EnvWrapper {
 
 }  // namespace rocksdb
 
+#endif  // STORAGE_ROCKSDB_INCLUDE_UTILITIES_ENVMIRROR_H_
+
 #endif  // ROCKSDB_LITE

include/rocksdb/utilities/transaction.h

@@ -209,11 +209,10 @@ class Transaction {
   // or other errors if this key could not be read.
   virtual Status GetForUpdate(const ReadOptions& options,
                               ColumnFamilyHandle* column_family,
-                              const Slice& key, std::string* value,
-                              bool exclusive = true) = 0;
+                              const Slice& key, std::string* value) = 0;
 
   virtual Status GetForUpdate(const ReadOptions& options, const Slice& key,
-                              std::string* value, bool exclusive = true) = 0;
+                              std::string* value) = 0;
 
   virtual std::vector<Status> MultiGetForUpdate(
       const ReadOptions& options,
@@ -402,10 +401,10 @@ class Transaction {
 
   virtual bool IsDeadlockDetect() const { return false; }
 
-  virtual std::vector<TransactionID> GetWaitingTxns(uint32_t* column_family_id,
-                                                    std::string* key) const {
+  virtual TransactionID GetWaitingTxn(uint32_t* column_family_id,
+                                      const std::string** key) const {
     assert(false);
-    return std::vector<TransactionID>();
+    return 0;
   }
 
   enum TransactionState {

include/rocksdb/utilities/transaction_db.h

@@ -104,8 +104,7 @@ struct TransactionOptions {
 
 struct KeyLockInfo {
   std::string key;
-  std::vector<TransactionID> ids;
-  bool exclusive;
+  TransactionID id;
 };
 
 class TransactionDB : public StackableDB {

include/rocksdb/version.h

@@ -5,8 +5,8 @@
 #pragma once
 
 #define ROCKSDB_MAJOR 5
-#define ROCKSDB_MINOR 1
-#define ROCKSDB_PATCH 5
+#define ROCKSDB_MINOR 0
+#define ROCKSDB_PATCH 2
 
 // Do not use these. We made the mistake of declaring macros starting with
 // double underscore. Now we have to live with our choice. We'll deprecate these

java/benchmark/src/main/java/org/rocksdb/benchmark/DbBenchmark.java

@@ -1417,18 +1417,12 @@ public class DbBenchmark {
       }
     },
     /* TODO(yhchiang): enable the following
-    direct_reads(rocksdb::EnvOptions().use_direct_reads,
-        "Allow direct I/O reads.") {
+    bufferedio(rocksdb::EnvOptions().use_os_buffer,
+        "Allow buffered io using OS buffers.") {
       @Override public Object parseValue(String value) {
         return parseBoolean(value);
       }
-      },
-    direct_writes(rocksdb::EnvOptions().use_direct_reads,
-      "Allow direct I/O reads.") {
-      @Override public Object parseValue(String value) {
-      return parseBoolean(value);
-      }
-      },
+    },
     */
     mmap_read(false,
         "Allow reads to occur via mmap-ing files.") {

java/rocksjni/env_options.cc

@@ -49,43 +49,23 @@ void Java_org_rocksdb_EnvOptions_disposeInternal(JNIEnv *env, jobject jobj,
 
 /*
  * Class:     org_rocksdb_EnvOptions
- * Method:    setUseDirectReads
+ * Method:    setUseOsBuffer
  * Signature: (JZ)V
  */
-void Java_org_rocksdb_EnvOptions_setUseDirectReads(JNIEnv *env, jobject jobj,
-                                                   jlong jhandle,
-                                                   jboolean use_direct_reads) {
-  ENV_OPTIONS_SET_BOOL(jhandle, use_direct_reads);
+void Java_org_rocksdb_EnvOptions_setUseOsBuffer(JNIEnv *env, jobject jobj,
+                                                jlong jhandle,
+                                                jboolean use_os_buffer) {
+  ENV_OPTIONS_SET_BOOL(jhandle, use_os_buffer);
 }
 
 /*
  * Class:     org_rocksdb_EnvOptions
- * Method:    useDirectReads
+ * Method:    useOsBuffer
  * Signature: (J)Z
  */
-jboolean Java_org_rocksdb_EnvOptions_useDirectReads(JNIEnv *env, jobject jobj,
-                                                    jlong jhandle) {
-  return ENV_OPTIONS_GET(jhandle, use_direct_reads);
-}
-
-/*
- * Class:     org_rocksdb_EnvOptions
- * Method:    setUseDirectWrites
- * Signature: (JZ)V
- */
-void Java_org_rocksdb_EnvOptions_setUseDirectWrites(
-    JNIEnv *env, jobject jobj, jlong jhandle, jboolean use_direct_writes) {
-  ENV_OPTIONS_SET_BOOL(jhandle, use_direct_writes);
-}
-
-/*
- * Class:     org_rocksdb_EnvOptions
- * Method:    useDirectWrites
- * Signature: (J)Z
- */
-jboolean Java_org_rocksdb_EnvOptions_useDirectWrites(JNIEnv *env, jobject jobj,
-                                                     jlong jhandle) {
-  return ENV_OPTIONS_GET(jhandle, use_direct_writes);
+jboolean Java_org_rocksdb_EnvOptions_useOsBuffer(JNIEnv *env, jobject jobj,
+                                                 jlong jhandle) {
+  return ENV_OPTIONS_GET(jhandle, use_os_buffer);
 }
 
 /*
@@ -130,6 +110,47 @@ jboolean Java_org_rocksdb_EnvOptions_useMmapWrites(JNIEnv *env, jobject jobj,
   return ENV_OPTIONS_GET(jhandle, use_mmap_writes);
 }
 
+/*
+ * Class:     org_rocksdb_EnvOptions
+ * Method:    setUseDirectReads
+ * Signature: (JZ)V
+ */
+void Java_org_rocksdb_EnvOptions_setUseDirectReads(JNIEnv *env, jobject jobj,
+                                                   jlong jhandle,
+                                                   jboolean use_direct_reads) {
+  ENV_OPTIONS_SET_BOOL(jhandle, use_direct_reads);
+}
+
+/*
+ * Class:     org_rocksdb_EnvOptions
+ * Method:    useDirectReads
+ * Signature: (J)Z
+ */
+jboolean Java_org_rocksdb_EnvOptions_useDirectReads(JNIEnv *env, jobject jobj,
+                                                    jlong jhandle) {
+  return ENV_OPTIONS_GET(jhandle, use_direct_reads);
+}
+
+/*
+ * Class:     org_rocksdb_EnvOptions
+ * Method:    setUseDirectWrites
+ * Signature: (JZ)V
+ */
+void Java_org_rocksdb_EnvOptions_setUseDirectWrites(
+    JNIEnv *env, jobject jobj, jlong jhandle, jboolean use_direct_writes) {
+  ENV_OPTIONS_SET_BOOL(jhandle, use_direct_writes);
+}
+
+/*
+ * Class:     org_rocksdb_EnvOptions
+ * Method:    useDirectWrites
+ * Signature: (J)Z
+ */
+jboolean Java_org_rocksdb_EnvOptions_useDirectWrites(JNIEnv *env, jobject jobj,
+                                                     jlong jhandle) {
+  return ENV_OPTIONS_GET(jhandle, use_direct_writes);
+}
+
 /*
  * Class:     org_rocksdb_EnvOptions
  * Method:    setAllowFallocate

java/rocksjni/options.cc

@@ -870,6 +870,27 @@ void Java_org_rocksdb_Options_setManifestPreallocationSize(
   }
 }
 
+/*
+ * Class:     org_rocksdb_Options
+ * Method:    allowOsBuffer
+ * Signature: (J)Z
+ */
+jboolean Java_org_rocksdb_Options_allowOsBuffer(
+    JNIEnv* env, jobject jobj, jlong jhandle) {
+  return reinterpret_cast<rocksdb::Options*>(jhandle)->allow_os_buffer;
+}
+
+/*
+ * Class:     org_rocksdb_Options
+ * Method:    setAllowOsBuffer
+ * Signature: (JZ)V
+ */
+void Java_org_rocksdb_Options_setAllowOsBuffer(
+    JNIEnv* env, jobject jobj, jlong jhandle, jboolean allow_os_buffer) {
+  reinterpret_cast<rocksdb::Options*>(jhandle)->allow_os_buffer =
+      static_cast<bool>(allow_os_buffer);
+}
+
 /*
  * Method:    setTableFactory
  * Signature: (JJ)V
@@ -922,50 +943,6 @@ void Java_org_rocksdb_Options_setAllowMmapWrites(
       static_cast<bool>(allow_mmap_writes);
 }
 
-/*
- * Class:     org_rocksdb_Options
- * Method:    useDirectReads
- * Signature: (J)Z
- */
-jboolean Java_org_rocksdb_Options_useDirectReads(JNIEnv* env, jobject jobj,
-                                                 jlong jhandle) {
-  return reinterpret_cast<rocksdb::Options*>(jhandle)->use_direct_reads;
-}
-
-/*
- * Class:     org_rocksdb_Options
- * Method:    setUseDirectReads
- * Signature: (JZ)V
- */
-void Java_org_rocksdb_Options_setUseDirectReads(JNIEnv* env, jobject jobj,
-                                                jlong jhandle,
-                                                jboolean use_direct_reads) {
-  reinterpret_cast<rocksdb::Options*>(jhandle)->use_direct_reads =
-      static_cast<bool>(use_direct_reads);
-}
-
-/*
- * Class:     org_rocksdb_Options
- * Method:    useDirectWrites
- * Signature: (J)Z
- */
-jboolean Java_org_rocksdb_Options_useDirectWrites(JNIEnv* env, jobject jobj,
-                                                  jlong jhandle) {
-  return reinterpret_cast<rocksdb::Options*>(jhandle)->use_direct_writes;
-}
-
-/*
- * Class:     org_rocksdb_Options
- * Method:    setUseDirectReads
- * Signature: (JZ)V
- */
-void Java_org_rocksdb_Options_setUseDirectWrites(JNIEnv* env, jobject jobj,
-                                                 jlong jhandle,
-                                                 jboolean use_direct_writes) {
-  reinterpret_cast<rocksdb::Options*>(jhandle)->use_direct_writes =
-      static_cast<bool>(use_direct_writes);
-}
-
 /*
  * Class:     org_rocksdb_Options
  * Method:    isFdCloseOnExec
@@ -4167,46 +4144,23 @@ jlong Java_org_rocksdb_DBOptions_manifestPreallocationSize(
 
 /*
  * Class:     org_rocksdb_DBOptions
- * Method:    useDirectReads
- * Signature: (J)Z
- */
-jboolean Java_org_rocksdb_DBOptions_useDirectReads(JNIEnv* env, jobject jobj,
-                                                   jlong jhandle) {
-  return reinterpret_cast<rocksdb::DBOptions*>(jhandle)->use_direct_reads;
-}
-
-/*
- * Class:     org_rocksdb_DBOptions
- * Method:    setUseDirectReads
+ * Method:    setAllowOsBuffer
  * Signature: (JZ)V
  */
-void Java_org_rocksdb_DBOptions_setUseDirectReads(JNIEnv* env, jobject jobj,
-                                                  jlong jhandle,
-                                                  jboolean use_direct_reads) {
-  reinterpret_cast<rocksdb::DBOptions*>(jhandle)->use_direct_reads =
-      static_cast<bool>(use_direct_reads);
+void Java_org_rocksdb_DBOptions_setAllowOsBuffer(
+    JNIEnv* env, jobject jobj, jlong jhandle, jboolean allow_os_buffer) {
+  reinterpret_cast<rocksdb::DBOptions*>(jhandle)->allow_os_buffer =
+      static_cast<bool>(allow_os_buffer);
 }
 
 /*
  * Class:     org_rocksdb_DBOptions
- * Method:    useDirectWrites
+ * Method:    allowOsBuffer
  * Signature: (J)Z
  */
-jboolean Java_org_rocksdb_DBOptions_useDirectWrites(JNIEnv* env, jobject jobj,
-                                                    jlong jhandle) {
-  return reinterpret_cast<rocksdb::DBOptions*>(jhandle)->use_direct_writes;
-}
-
-/*
- * Class:     org_rocksdb_DBOptions
- * Method:    setUseDirectReads
- * Signature: (JZ)V
- */
-void Java_org_rocksdb_DBOptions_setUseDirectWrites(JNIEnv* env, jobject jobj,
-                                                   jlong jhandle,
-                                                   jboolean use_direct_writes) {
-  reinterpret_cast<rocksdb::DBOptions*>(jhandle)->use_direct_writes =
-      static_cast<bool>(use_direct_writes);
+jboolean Java_org_rocksdb_DBOptions_allowOsBuffer(
+    JNIEnv* env, jobject jobj, jlong jhandle) {
+  return reinterpret_cast<rocksdb::DBOptions*>(jhandle)->allow_os_buffer;
 }
 
 /*

java/rocksjni/portal.h

@@ -11,7 +11,6 @@
 #define JAVA_ROCKSJNI_PORTAL_H_
 
 #include <jni.h>
-#include <functional>
 #include <limits>
 #include <string>
 #include <vector>
@@ -120,8 +119,8 @@ class StatusJni : public RocksDBNativeClass<rocksdb::Status*, StatusJni> {
 
     jstring jstate = nullptr;
     if (status.getState() != nullptr) {
-      const char* const state = status.getState();
-      jstate = env->NewStringUTF(state);
+      std::string s(status.getState());
+      jstate = env->NewStringUTF(s.c_str());
     }
     return env->NewObject(getJClass(env), mid, toJavaStatusCode(status.code()),
                           toJavaStatusSubCode(status.subcode()), jstate);
@@ -160,7 +159,7 @@ class StatusJni : public RocksDBNativeClass<rocksdb::Status*, StatusJni> {
       case rocksdb::Status::Code::kTryAgain:
         return 0xD;
       default:
-        return 0x7F;  // undefined
+        return 0xFF;  // undefined
     }
   }
 
@@ -177,9 +176,9 @@ class StatusJni : public RocksDBNativeClass<rocksdb::Status*, StatusJni> {
       case rocksdb::Status::SubCode::kLockLimit:
         return 0x3;
       case rocksdb::Status::SubCode::kMaxSubCode:
-        return 0x7E;
+        return 0xFE;
       default:
-        return 0x7F;  // undefined
+        return 0xFF;  // undefined
     }
   }
 };

java/rocksjni/rocksjni.cc

@@ -9,12 +9,11 @@
 #include <jni.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <algorithm>
-#include <functional>
 #include <memory>
 #include <string>
 #include <tuple>
 #include <vector>
+#include <algorithm>
 
 #include "include/org_rocksdb_RocksDB.h"
 #include "rocksdb/cache.h"

java/src/main/java/org/rocksdb/DBOptions.java

@@ -457,31 +457,17 @@ public class DBOptions extends RocksObject implements DBOptionsInterface {
   }
 
   @Override
-  public DBOptions setUseDirectReads(
-      final boolean useDirectReads) {
+  public DBOptions setAllowOsBuffer(
+      final boolean allowOsBuffer) {
     assert(isOwningHandle());
-    setUseDirectReads(nativeHandle_, useDirectReads);
+    setAllowOsBuffer(nativeHandle_, allowOsBuffer);
     return this;
   }
 
   @Override
-  public boolean useDirectReads() {
+  public boolean allowOsBuffer() {
     assert(isOwningHandle());
-    return useDirectReads(nativeHandle_);
-  }
-
-  @Override
-  public DBOptions setUseDirectWrites(
-      final boolean useDirectWrites) {
-    assert(isOwningHandle());
-    setUseDirectWrites(nativeHandle_, useDirectWrites);
-    return this;
-  }
-
-  @Override
-  public boolean useDirectWrites() {
-    assert(isOwningHandle());
-    return useDirectWrites(nativeHandle_);
+    return allowOsBuffer(nativeHandle_);
   }
 
   @Override
@@ -724,10 +710,9 @@ public long delayedWriteRate(){
   private native void setManifestPreallocationSize(
       long handle, long size) throws IllegalArgumentException;
   private native long manifestPreallocationSize(long handle);
-  private native void setUseDirectReads(long handle, boolean useDirectReads);
-  private native boolean useDirectReads(long handle);
-  private native void setUseDirectWrites(long handle, boolean useDirectWrites);
-  private native boolean useDirectWrites(long handle);
+  private native void setAllowOsBuffer(
+      long handle, boolean allowOsBuffer);
+  private native boolean allowOsBuffer(long handle);
   private native void setAllowMmapReads(
       long handle, boolean allowMmapReads);
   private native boolean allowMmapReads(long handle);

java/src/main/java/org/rocksdb/DBOptionsInterface.java

@@ -673,38 +673,21 @@ public interface DBOptionsInterface {
   long manifestPreallocationSize();
 
   /**
-   * Enable the OS to use direct I/O for reading sst tables.
-   * Default: false
+   * Data being read from file storage may be buffered in the OS
+   * Default: true
    *
-   * @param useDirectReads if true, then direct read is enabled
+   * @param allowOsBuffer if true, then OS buffering is allowed.
    * @return the instance of the current Object.
    */
-  Object setUseDirectReads(boolean useDirectReads);
+  Object setAllowOsBuffer(boolean allowOsBuffer);
 
   /**
-   * Enable the OS to use direct I/O for reading sst tables.
-   * Default: false
+   * Data being read from file storage may be buffered in the OS
+   * Default: true
    *
-   * @return if true, then direct reads are enabled
+   * @return if true, then OS buffering is allowed.
    */
-  boolean useDirectReads();
-
-  /**
-   * Enable the OS to use direct I/O for writing sst tables.
-   * Default: false
-   *
-   * @param useDirectWrites if true, then direct write is enabled
-   * @return the instance of the current Object.
-   */
-  Object setUseDirectWrites(boolean useDirectWrites);
-
-  /**
-   * Enable the OS to use direct I/O for writing sst tables.
-   * Default: false
-   *
-   * @return if true, then direct writes are enabled
-   */
-  boolean useDirectWrites();
+  boolean allowOsBuffer();
 
   /**
    * Allow the OS to mmap file for reading sst tables.

java/src/main/java/org/rocksdb/Options.java

@@ -530,32 +530,18 @@ public class Options extends RocksObject
   }
 
   @Override
-  public Options setUseDirectReads(final boolean useDirectReads) {
+  public boolean allowOsBuffer() {
     assert(isOwningHandle());
-    setUseDirectReads(nativeHandle_, useDirectReads);
+    return allowOsBuffer(nativeHandle_);
+  }
+
+  @Override
+  public Options setAllowOsBuffer(final boolean allowOsBuffer) {
+    assert(isOwningHandle());
+    setAllowOsBuffer(nativeHandle_, allowOsBuffer);
     return this;
   }
 
-  @Override
-  public boolean useDirectReads() {
-    assert(isOwningHandle());
-    return useDirectReads(nativeHandle_);
-  }
-
-  @Override
-  public Options setUseDirectWrites(final boolean useDirectWrites) {
-    assert(isOwningHandle());
-    setUseDirectWrites(nativeHandle_, useDirectWrites);
-    return this;
-  }
-
-  @Override
-  public boolean useDirectWrites() {
-    assert(isOwningHandle());
-    return useDirectWrites(nativeHandle_);
-  }
-
-
   @Override
   public boolean allowMmapReads() {
     assert(isOwningHandle());
@@ -1303,10 +1289,9 @@ public class Options extends RocksObject
   private native void setManifestPreallocationSize(
       long handle, long size) throws IllegalArgumentException;
   private native long manifestPreallocationSize(long handle);
-  private native void setUseDirectReads(long handle, boolean useDirectReads);
-  private native boolean useDirectReads(long handle);
-  private native void setUseDirectWrites(long handle, boolean useDirectWrites);
-  private native boolean useDirectWrites(long handle);
+  private native void setAllowOsBuffer(
+      long handle, boolean allowOsBuffer);
+  private native boolean allowOsBuffer(long handle);
   private native void setAllowMmapReads(
       long handle, boolean allowMmapReads);
   private native boolean allowMmapReads(long handle);

java/src/main/java/org/rocksdb/Status.java

@@ -92,7 +92,7 @@ public class Status {
     MutexTimeout( (byte)0x1),
     LockTimeout(  (byte)0x2),
     LockLimit(    (byte)0x3),
-    MaxSubCode(   (byte)0x7E);
+    MaxSubCode(   (byte)0xFE);
 
     private final byte value;
 

java/src/test/java/org/rocksdb/DBOptionsTest.java

@@ -281,20 +281,11 @@ public class DBOptionsTest {
   }
 
   @Test
-  public void useDirectReads() {
+  public void allowOsBuffer() {
     try(final DBOptions opt = new DBOptions()) {
       final boolean boolValue = rand.nextBoolean();
-      opt.setUseDirectReads(boolValue);
-      assertThat(opt.useDirectReads()).isEqualTo(boolValue);
-    }
-  }
-
-  @Test
-  public void useDirectWrites() {
-    try(final DBOptions opt = new DBOptions()) {
-      final boolean boolValue = rand.nextBoolean();
-      opt.setUseDirectWrites(boolValue);
-      assertThat(opt.useDirectWrites()).isEqualTo(boolValue);
+      opt.setAllowOsBuffer(boolValue);
+      assertThat(opt.allowOsBuffer()).isEqualTo(boolValue);
     }
   }
 

java/src/test/java/org/rocksdb/EnvOptionsTest.java

@@ -18,6 +18,15 @@ public class EnvOptionsTest {
 
   public static final Random rand = PlatformRandomHelper.getPlatformSpecificRandomFactory();
 
+  @Test
+  public void useOsBuffer() {
+    try (final EnvOptions envOptions = new EnvOptions()) {
+      final boolean boolValue = rand.nextBoolean();
+      envOptions.setUseOsBuffer(boolValue);
+      assertThat(envOptions.useOsBuffer()).isEqualTo(boolValue);
+    }
+  }
+
   @Test
   public void useMmapReads() {
     try (final EnvOptions envOptions = new EnvOptions()) {

java/src/test/java/org/rocksdb/OptionsTest.java

@@ -572,20 +572,11 @@ public class OptionsTest {
   }
 
   @Test
-  public void useDirectReads() {
-    try(final Options opt = new Options()) {
+  public void allowOsBuffer() {
+    try (final Options opt = new Options()) {
       final boolean boolValue = rand.nextBoolean();
-      opt.setUseDirectReads(boolValue);
-      assertThat(opt.useDirectReads()).isEqualTo(boolValue);
-    }
-  }
-
-  @Test
-  public void useDirectWrites() {
-    try(final Options opt = new Options()) {
-      final boolean boolValue = rand.nextBoolean();
-      opt.setUseDirectWrites(boolValue);
-      assertThat(opt.useDirectWrites()).isEqualTo(boolValue);
+      opt.setAllowOsBuffer(boolValue);
+      assertThat(opt.allowOsBuffer()).isEqualTo(boolValue);
     }
   }
 

java/src/test/java/org/rocksdb/RocksDBTest.java

@@ -12,7 +12,6 @@ import org.junit.rules.TemporaryFolder;
 import java.util.*;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.Assert.fail;
 
 public class RocksDBTest {
 
@@ -43,21 +42,6 @@ public class RocksDBTest {
     }
   }
 
-  @Test
-  public void openWhenOpen() throws RocksDBException {
-    final String dbPath = dbFolder.getRoot().getAbsolutePath();
-
-    try (final RocksDB db1 = RocksDB.open(dbPath)) {
-      try (final RocksDB db2 = RocksDB.open(dbPath)) {
-        fail("Should have thrown an exception when opening the same db twice");
-      } catch (final RocksDBException e) {
-        assertThat(e.getStatus().getCode()).isEqualTo(Status.Code.IOError);
-        assertThat(e.getStatus().getSubCode()).isEqualTo(Status.SubCode.None);
-        assertThat(e.getStatus().getState()).startsWith("lock ");
-      }
-    }
-  }
-
   @Test
   public void put() throws RocksDBException {
     try (final RocksDB db = RocksDB.open(dbFolder.getRoot().getAbsolutePath());

port/port_posix.h

@@ -90,19 +90,7 @@ class CondVar;
 
 class Mutex {
  public:
-// We want to give users opportunity to default all the mutexes to adaptive if
-// not specified otherwise. This enables a quick way to conduct various
-// performance related experiements.
-//
-// NB! Support for adaptive mutexes is turned on by definining
-// ROCKSDB_PTHREAD_ADAPTIVE_MUTEX during the compilation. If you use RocksDB
-// build environment then this happens automatically; otherwise it's up to the
-// consumer to define the identifier.
-#ifdef ROCKSDB_DEFAULT_TO_ADAPTIVE_MUTEX
-  explicit Mutex(bool adaptive = true);
-#else
-  explicit Mutex(bool adaptive = false);
-#endif
+  /* implicit */ Mutex(bool adaptive = false);
   ~Mutex();
 
   void Lock();

port/stack_trace.cc

@@ -110,7 +110,6 @@ void PrintStack(int first_frames_to_skip) {
     fprintf(stderr, "#%-2d  ", i - first_frames_to_skip);
     PrintStackTraceLine((symbols != nullptr) ? symbols[i] : nullptr, frames[i]);
   }
-  free(symbols);
 }
 
 static void StackTraceHandler(int sig) {

port/win/env_win.cc

@@ -7,10 +7,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 
-#include "port/win/env_win.h"
 #include <algorithm>
-#include <ctime>
 #include <thread>
+#include <ctime>
 
 #include <errno.h>
 #include <process.h> // _getpid
@@ -26,6 +25,7 @@
 #include "port/dirent.h"
 #include "port/win/win_logger.h"
 #include "port/win/io_win.h"
+#include "port/win/env_win.h"
 
 #include "util/iostats_context_imp.h"
 
@@ -148,7 +148,7 @@ Status WinEnvIO::NewRandomAccessFile(const std::string& fname,
   // Random access is to disable read-ahead as the system reads too much data
   DWORD fileFlags = FILE_ATTRIBUTE_READONLY;
 
-  if (options.use_direct_reads && !options.use_mmap_reads) {
+  if (!options.use_os_buffer && !options.use_mmap_reads) {
     fileFlags |= FILE_FLAG_NO_BUFFERING;
   } else {
     fileFlags |= FILE_FLAG_RANDOM_ACCESS;
@@ -229,8 +229,8 @@ Status WinEnvIO::NewRandomAccessFile(const std::string& fname,
 }
 
 Status WinEnvIO::NewWritableFile(const std::string& fname,
-                                 std::unique_ptr<WritableFile>* result,
-                                 const EnvOptions& options) {
+  std::unique_ptr<WritableFile>* result,
+  const EnvOptions& options) {
   const size_t c_BufferCapacity = 64 * 1024;
 
   EnvOptions local_options(options);
@@ -240,7 +240,7 @@ Status WinEnvIO::NewWritableFile(const std::string& fname,
 
   DWORD fileFlags = FILE_ATTRIBUTE_NORMAL;
 
-  if (local_options.use_direct_writes && !local_options.use_mmap_writes) {
+  if (!local_options.use_os_buffer && !local_options.use_mmap_writes) {
     fileFlags = FILE_FLAG_NO_BUFFERING;
   }
 
@@ -305,7 +305,7 @@ Status WinEnvIO::NewRandomRWFile(const std::string & fname,
   DWORD creation_disposition = OPEN_ALWAYS; // Create if necessary or open existing
   DWORD file_flags = FILE_FLAG_RANDOM_ACCESS;
 
-  if (options.use_direct_reads && options.use_direct_writes) {
+  if (!options.use_os_buffer) {
     file_flags |= FILE_FLAG_NO_BUFFERING;
   }
 
@@ -361,8 +361,6 @@ Status WinEnvIO::FileExists(const std::string& fname) {
 
 Status WinEnvIO::GetChildren(const std::string& dir,
   std::vector<std::string>* result) {
-
-  result->clear();
   std::vector<std::string> output;
 
   Status status;
@@ -372,14 +370,7 @@ Status WinEnvIO::GetChildren(const std::string& dir,
     CloseDir);
 
   if (!dirp) {
-    switch (errno) {
-      case EACCES:
-      case ENOENT:
-      case ENOTDIR:
-        return Status::NotFound();
-      default:
-        return IOError(dir, errno);
-    }
+    status = IOError(dir, errno);
   } else {
     if (result->capacity() > 0) {
       output.reserve(result->capacity());
@@ -746,11 +737,11 @@ std::string WinEnvIO::TimeToString(uint64_t secondsSince1970) {
 EnvOptions WinEnvIO::OptimizeForLogWrite(const EnvOptions& env_options,
   const DBOptions& db_options) const {
   EnvOptions optimized = env_options;
-  optimized.bytes_per_sync = db_options.wal_bytes_per_sync;
   optimized.use_mmap_writes = false;
-  // This is because we flush only whole pages on unbuffered io and
+  optimized.bytes_per_sync = db_options.wal_bytes_per_sync;
+  optimized.use_os_buffer =
+    true;  // This is because we flush only whole pages on unbuffered io and
   // the last records are not guaranteed to be flushed.
-  optimized.use_direct_writes = false;
   // TODO(icanadi) it's faster if fallocate_with_keep_size is false, but it
   // breaks TransactionLogIteratorStallAtLastRecord unit test. Fix the unit
   // test and make this false
@@ -762,7 +753,7 @@ EnvOptions WinEnvIO::OptimizeForManifestWrite(
   const EnvOptions& env_options) const {
   EnvOptions optimized = env_options;
   optimized.use_mmap_writes = false;
-  optimized.use_direct_writes = false;
+  optimized.use_os_buffer = true;
   optimized.fallocate_with_keep_size = true;
   return optimized;
 }
@@ -916,8 +907,8 @@ Status WinEnv::NewRandomAccessFile(const std::string& fname,
 }
 
 Status WinEnv::NewWritableFile(const std::string& fname,
-                               std::unique_ptr<WritableFile>* result,
-                               const EnvOptions& options) {
+  std::unique_ptr<WritableFile>* result,
+  const EnvOptions& options) {
   return winenv_io_.NewWritableFile(fname, result, options);
 }