Fix some errors in async prefetching in FilePrefetchBuffer (#9734)

Summary:
In ReadOption `async_io` which prefetches the data asynchronously, db_bench and db_stress runs were failing  because wrong data was prefetched which resulted in Error: Checksum mismatched. Wrong data was copied because capacity was less than actual size needed. It has been fixed in this PR.

Since there are two separate methods for async and sync prefetching, these changes are in async prefetching methods and any changes would not effect normal prefetching. I ran the regressions to make sure normal prefetching is fine.

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

Test Plan:
1. CircleCI jobs

2.  Ran db_bench
```
. /db_bench -use_existing_db=true
-db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32
-value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680
-duration=120 -ops_between_duration_checks=1 -async_io=1 -adaptive_readahead=1

```
3. Ran db_stress test
```
export CRASH_TEST_EXT_ARGS=" --async_io=1 --adaptive_readahead=1"
make crash_test -j
```

4. Run regressions for async_io disabled.

Old flow without any async changes:
```
./db_bench -use_existing_db=true -db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32 -value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680 -duration=120 -ops_between_duration_checks=1
Initializing RocksDB Options from the specified file
Initializing RocksDB Options from command-line flags
RocksDB:    version 7.0
Date:       Thu Mar 17 13:11:34 2022
CPU:        24 * Intel Core Processor (Broadwell)
CPUCache:   16384 KB
Keys:       32 bytes each (+ 0 bytes user-defined timestamp)
Values:     512 bytes each (256 bytes after compression)
Entries:    5000000
Prefix:    0 bytes
Keys per prefix:    0
RawSize:    2594.0 MB (estimated)
FileSize:   1373.3 MB (estimated)
Write rate: 0 bytes/second
Read rate: 0 ops/second
Compression: Snappy
Compression sampling rate: 0
Memtablerep: SkipListFactory
Perf Level: 1
------------------------------------------------
DB path: [/tmp/prefix_scan_prefetch_main]
seekrandom   :  483618.390 micros/op 2 ops/sec;  338.9 MB/s (249 of 249 found)
```

With async prefetching changes and async_io disabled to make sure in normal prefetching there is no regression.
 ```
 ./db_bench -use_existing_db=true -db=/tmp/prefix_scan_prefetch_main -benchmarks="seekrandom" -key_size=32 -value_size=512 -num=5000000 -use_direct_reads=true -seek_nexts=327680 -duration=120 -ops_between_duration_checks=1 --async_io=0
Initializing RocksDB Options from the specified file
Initializing RocksDB Options from command-line flags
RocksDB:    version 7.1
Date:       Wed Mar 23 15:56:37 2022
CPU:        24 * Intel Core Processor (Broadwell)
CPUCache:   16384 KB
Keys:       32 bytes each (+ 0 bytes user-defined timestamp)
Values:     512 bytes each (256 bytes after compression)
Entries:    5000000
Prefix:    0 bytes
Keys per prefix:    0
RawSize:    2594.0 MB (estimated)
FileSize:   1373.3 MB (estimated)
Write rate: 0 bytes/second
Read rate: 0 ops/second
Compression: Snappy
Compression sampling rate: 0
Memtablerep: SkipListFactory
Perf Level: 1
------------------------------------------------
DB path: [/tmp/prefix_scan_prefetch_main]
seekrandom   :  481819.816 micros/op 2 ops/sec;  340.2 MB/s (250 of 250 found)
```

Reviewed By: riversand963

Differential Revision: D35058471

Pulled By: akankshamahajan15

fbshipit-source-id: 9233a1e6d97cea0c7a8111bfb9e8ac3251c341ce

.circleci/config.yml

@@ -102,16 +102,10 @@ commands:
 
   install-benchmark:
     steps:
-      - run:
-          name: Install ninja build
-          command: sudo apt-get update -y && sudo apt-get install -y ninja-build
       - run:
           name: Install benchmark
           command: |
-            git clone --depth 1 --branch v1.6.1 https://github.com/google/benchmark.git ~/benchmark
-            cd ~/benchmark && mkdir build && cd build
-            cmake .. -GNinja -DCMAKE_BUILD_TYPE=Release -DBENCHMARK_ENABLE_GTEST_TESTS=0
-            ninja && sudo ninja install
+            sudo apt-get update -y && sudo apt-get install -y libbenchmark-dev
 
   upgrade-cmake:
     steps:
@@ -437,8 +431,8 @@ jobs:
     resource_class: xlarge
     steps:
       - pre-steps
-      - run: sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test && sudo apt-get update -y && sudo apt-get install gcc-11 g++-11 libgflags-dev
       - install-benchmark
+      - run: sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test && sudo apt-get update -y && sudo apt-get install gcc-11 g++-11 libgflags-dev
       - run: CC=gcc-11 CXX=g++-11 V=1 make -j16 all microbench
       - post-steps
 
@@ -559,6 +553,9 @@ jobs:
             echo 'export PATH=$JAVA_HOME/bin:$PATH' >> $BASH_ENV
             which java && java -version
             which javac && javac -version
+      - run:
+          name: "Build RocksDBJava Shared Library"
+          command: make V=1 J=8 -j8 rocksdbjava
       - run:
           name: "Test RocksDBJava"
           command: make V=1 J=8 -j8 jtest
@@ -604,6 +601,9 @@ jobs:
             echo 'export PATH=$JAVA_HOME/bin:$PATH' >> $BASH_ENV
             which java && java -version
             which javac && javac -version
+      - run:
+          name: "Build RocksDBJava Shared Library"
+          command: make V=1 J=16 -j16 rocksdbjava
       - run:
           name: "Test RocksDBJava"
           command: make V=1 J=16 -j16 jtest

.travis.yml

@@ -239,10 +239,10 @@ script:
       OPT=-DTRAVIS LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=only make -j$MK_PARALLEL all_but_some_tests check_some
       ;;
     1)
-      OPT=-DTRAVIS LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=exclude ROCKSDBTESTS_END=backup_engine_test make -j$MK_PARALLEL check_some
+      OPT=-DTRAVIS LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=exclude ROCKSDBTESTS_END=backupable_db_test make -j$MK_PARALLEL check_some
       ;;
     2)
-      OPT="-DTRAVIS -DROCKSDB_NAMESPACE=alternative_rocksdb_ns" LIB_MODE=shared V=1 make -j$MK_PARALLEL tools && OPT="-DTRAVIS -DROCKSDB_NAMESPACE=alternative_rocksdb_ns" LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=exclude ROCKSDBTESTS_START=backup_engine_test ROCKSDBTESTS_END=db_universal_compaction_test make -j$MK_PARALLEL check_some
+      OPT="-DTRAVIS -DROCKSDB_NAMESPACE=alternative_rocksdb_ns" LIB_MODE=shared V=1 make -j$MK_PARALLEL tools && OPT="-DTRAVIS -DROCKSDB_NAMESPACE=alternative_rocksdb_ns" LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=exclude ROCKSDBTESTS_START=backupable_db_test ROCKSDBTESTS_END=db_universal_compaction_test make -j$MK_PARALLEL check_some
       ;;
     3)
       OPT=-DTRAVIS LIB_MODE=shared V=1 ROCKSDBTESTS_PLATFORM_DEPENDENT=exclude ROCKSDBTESTS_START=db_universal_compaction_test ROCKSDBTESTS_END=table_properties_collector_test make -j$MK_PARALLEL check_some

CMakeLists.txt

@@ -2,7 +2,7 @@
 #     This cmake build is for Windows 64-bit only.
 #
 # Prerequisites:
-#     You must have at least Visual Studio 2019. Start the Developer Command Prompt window that is a part of Visual Studio installation.
+#     You must have at least Visual Studio 2015 Update 3. Start the Developer Command Prompt window that is a part of Visual Studio installation.
 #     Run the build commands from within the Developer Command Prompt window to have paths to the compiler and runtime libraries set.
 #     You must have git.exe in your %PATH% environment variable.
 #
@@ -849,7 +849,7 @@ set(SOURCES
         util/thread_local.cc
         util/threadpool_imp.cc
         util/xxhash.cc
-        utilities/backup/backup_engine.cc
+        utilities/backupable/backupable_db.cc
         utilities/blob_db/blob_compaction_filter.cc
         utilities/blob_db/blob_db.cc
         utilities/blob_db/blob_db_impl.cc
@@ -1315,7 +1315,7 @@ if(WITH_TESTS)
         util/thread_list_test.cc
         util/thread_local_test.cc
         util/work_queue_test.cc
-        utilities/backup/backup_engine_test.cc
+        utilities/backupable/backupable_db_test.cc
         utilities/blob_db/blob_db_test.cc
         utilities/cassandra/cassandra_functional_test.cc
         utilities/cassandra/cassandra_format_test.cc

HISTORY.md

@@ -1,24 +1,4 @@
 # Rocksdb Change Log
-## Unreleased
-### Bug Fixes
-* Fixed bug which caused rocksdb failure in the situation when rocksdb was accessible using UNC path
-* Fixed a race condition when 2PC is disabled and WAL tracking in the MANIFEST is enabled. The race condition is between two background flush threads trying to install flush results, causing a WAL deletion not tracked in the MANIFEST. A future DB open may fail.
-* Fixed a heap use-after-free race with DropColumnFamily.
-* Fixed a bug that `rocksdb.read.block.compaction.micros` cannot track compaction stats (#9722).
-* Fixed `file_type`, `relative_filename` and `directory` fields returned by `GetLiveFilesMetaData()`, which were added in inheriting from `FileStorageInfo`.
-* Fixed a bug affecting `track_and_verify_wals_in_manifest`. Without the fix, application may see "open error: Corruption: Missing WAL with log number" while trying to open the db. The corruption is a false alarm but prevents DB open (#9766).
-* Fix segfault in FilePrefetchBuffer with async_io as it doesn't wait for pending jobs to complete on destruction.
-* Fix ERROR_HANDLER_AUTORESUME_RETRY_COUNT stat whose value was set wrong in portal.h
-
-### New Features
-* For db_bench when --seed=0 or --seed is not set then it uses the current time as the seed value. Previously it used the value 1000.
-* For db_bench when --benchmark lists multiple tests and each test uses a seed for a RNG then the seeds across tests will no longer be repeated.
-* Added an option to dynamically charge an updating estimated memory usage of block-based table reader to block cache if block cache available. To enable this feature, set `BlockBasedTableOptions::reserve_table_reader_memory = true`.
-* Add new stat ASYNC_READ_BYTES that calculates number of bytes read during async read call and users can check if async code path is being called by RocksDB internal automatic prefetching for sequential reads.
-
-### Behavior changes
-* Disallow usage of commit-time-write-batch for write-prepared/write-unprepared transactions if TransactionOptions::use_only_the_last_commit_time_batch_for_recovery is false to prevent two (or more) uncommitted versions of the same key in the database. Otherwise, bottommost compaction may violate the internal key uniqueness invariant of SSTs if the sequence numbers of both internal keys are zeroed out (#9794).
-
 ## 7.1.0 (03/23/2022)
 ### New Features
 * Allow WriteBatchWithIndex to index a WriteBatch that includes keys with user-defined timestamps. The index itself does not have timestamp.
@@ -1678,7 +1658,7 @@ if set to something > 0 user will see 2 changes in iterators behavior 1) only ke
 * Added a new way to report QPS from db_bench (check out --report_file and --report_interval_seconds)
 * Added a cache for individual rows. See DBOptions::row_cache for more info.
 * Several new features on EventListener (see include/rocksdb/listener.h):
- - OnCompactionCompleted() now returns per-compaction job statistics, defined in include/rocksdb/compaction_job_stats.h.
+ - OnCompationCompleted() now returns per-compaction job statistics, defined in include/rocksdb/compaction_job_stats.h.
  - Added OnTableFileCreated() and OnTableFileDeleted().
 * Add compaction_options_universal.enable_trivial_move to true, to allow trivial move while performing universal compaction. Trivial move will happen only when all the input files are non overlapping.
 

INSTALL.md

@@ -47,7 +47,7 @@ to build a portable binary, add `PORTABLE=1` before your make commands, like thi
 
 * If you wish to build the RocksJava static target, then cmake is required for building Snappy.
 
-* If you wish to run microbench (e.g, `make microbench`, `make ribbon_bench` or `cmake -DWITH_BENCHMARK=1`), Google benchmark >= 1.6.0 is needed.
+* If you wish to run microbench (e.g, `make microbench`, `make ribbon_bench` or `cmake -DWITH_BENCHMARK=1`), Google benchmark < 1.6.0 is needed.
 
 ## Supported platforms
 
@@ -180,7 +180,8 @@ to build a portable binary, add `PORTABLE=1` before your make commands, like thi
 * **iOS**:
   * Run: `TARGET_OS=IOS make static_lib`. When building the project which uses rocksdb iOS library, make sure to define two important pre-processing macros: `ROCKSDB_LITE` and `IOS_CROSS_COMPILE`.
 
-* **Windows** (Visual Studio 2017 to up):
+* **Windows**:
+  * For building with MS Visual Studio 13 you will need Update 4 installed.
   * Read and follow the instructions at CMakeLists.txt
   * Or install via [vcpkg](https://github.com/microsoft/vcpkg)
        * run `vcpkg install rocksdb:x64-windows`

Makefile

@@ -826,8 +826,8 @@ release: clean
 coverage: clean
 	COVERAGEFLAGS="-fprofile-arcs -ftest-coverage" LDFLAGS+="-lgcov" $(MAKE) J=1 all check
 	cd coverage && ./coverage_test.sh
-	# Delete intermediate files
-	$(FIND) . -type f \( -name "*.gcda" -o -name "*.gcno" \) -exec rm -f {} \;
+        # Delete intermediate files
+	$(FIND) . -type f -regex ".*\.\(\(gcda\)\|\(gcno\)\)" -exec rm -f {} \;
 
 ifneq (,$(filter check parallel_check,$(MAKECMDGOALS)),)
 # Use /dev/shm if it has the sticky bit set (otherwise, /tmp),
@@ -1214,9 +1214,9 @@ clean-rocks:
 	rm -f $(BENCHMARKS) $(TOOLS) $(TESTS) $(PARALLEL_TEST) $(ALL_STATIC_LIBS) $(ALL_SHARED_LIBS) $(MICROBENCHS)
 	rm -rf $(CLEAN_FILES) ios-x86 ios-arm scan_build_report
 	$(FIND) . -name "*.[oda]" -exec rm -f {} \;
-	$(FIND) . -type f \( -name "*.gcda" -o -name "*.gcno" \) -exec rm -f {} \;
+	$(FIND) . -type f -regex ".*\.\(\(gcda\)\|\(gcno\)\)" -exec rm -f {} \;
 
-clean-rocksjava: clean-rocks
+clean-rocksjava:
 	rm -rf jl jls
 	cd java && $(MAKE) clean
 
@@ -1551,7 +1551,7 @@ perf_context_test: $(OBJ_DIR)/db/perf_context_test.o $(TEST_LIBRARY) $(LIBRARY)
 prefix_test: $(OBJ_DIR)/db/prefix_test.o $(TEST_LIBRARY) $(LIBRARY)
 	$(AM_LINK)
 
-backup_engine_test: $(OBJ_DIR)/utilities/backup/backup_engine_test.o $(TEST_LIBRARY) $(LIBRARY)
+backupable_db_test: $(OBJ_DIR)/utilities/backupable/backupable_db_test.o $(TEST_LIBRARY) $(LIBRARY)
 	$(AM_LINK)
 
 checkpoint_test: $(OBJ_DIR)/utilities/checkpoint/checkpoint_test.o $(TEST_LIBRARY) $(LIBRARY)
@@ -2044,8 +2044,8 @@ ROCKSDB_JAVADOCS_JAR = rocksdbjni-$(ROCKSDB_JAVA_VERSION)-javadoc.jar
 ROCKSDB_SOURCES_JAR = rocksdbjni-$(ROCKSDB_JAVA_VERSION)-sources.jar
 SHA256_CMD = sha256sum
 
-ZLIB_VER ?= 1.2.12
-ZLIB_SHA256 ?= 91844808532e5ce316b3c010929493c0244f3d37593afd6de04f71821d5136d9
+ZLIB_VER ?= 1.2.11
+ZLIB_SHA256 ?= c3e5e9fdd5004dcb542feda5ee4f0ff0744628baf8ed2dd5d66f8ca1197cb1a1
 ZLIB_DOWNLOAD_BASE ?= http://zlib.net
 BZIP2_VER ?= 1.0.8
 BZIP2_SHA256 ?= ab5a03176ee106d3f0fa90e381da478ddae405918153cca248e682cd0c4a2269
@@ -2372,10 +2372,10 @@ jtest: rocksdbjava
 jdb_bench:
 	cd java;$(MAKE) db_bench;
 
-commit_prereq:
-	echo "TODO: bring this back using parts of old precommit_checker.py and rocksdb-lego-determinator"
-	false # J=$(J) build_tools/precommit_checker.py unit clang_unit release clang_release tsan asan ubsan lite unit_non_shm
-	# $(MAKE) clean && $(MAKE) jclean && $(MAKE) rocksdbjava;
+commit_prereq: build_tools/rocksdb-lego-determinator \
+               build_tools/precommit_checker.py
+	J=$(J) build_tools/precommit_checker.py unit unit_481 clang_unit release release_481 clang_release tsan asan ubsan lite unit_non_shm
+	$(MAKE) clean && $(MAKE) jclean && $(MAKE) rocksdbjava;
 
 # ---------------------------------------------------------------------------
 #  	Platform-specific compilation

ROCKSDB_LITE.md

@@ -4,7 +4,7 @@ RocksDBLite is a project focused on mobile use cases, which don't need a lot of
 
 Some examples of the features disabled by ROCKSDB_LITE:
 * compiled-in support for LDB tool
-* No backup engine
+* No backupable DB
 * No support for replication (which we provide in form of TransactionalIterator)
 * No advanced monitoring tools
 * No special-purpose memtables that are highly optimized for specific use cases

buckifier/bench.json

@@ -3,23 +3,27 @@
         "benchmarks": {
             "db_basic_bench": {
                 "DBGet/comp_style:1/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:0/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 510.387506
                     }
                 ],
                 "DBGet/comp_style:2/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:0/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 497.077261
                     }
@@ -131,29 +135,33 @@
         "expected_runtime_one_iter": 2438,
         "name": "rocksdb_microbench_suite_0",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
             "db_basic_bench": {
                 "DBGet/comp_style:1/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:1/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 510.88842
                     }
                 ],
                 "DBGet/comp_style:2/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:1/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 497.546918
                     }
@@ -258,29 +266,33 @@
         "expected_runtime_one_iter": 2437,
         "name": "rocksdb_microbench_suite_1",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
             "db_basic_bench": {
                 "DBGet/comp_style:0/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:0/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 479.941992
                     }
                 ],
                 "DBGet/comp_style:1/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:0/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 513.865636
                     }
@@ -319,7 +331,7 @@
         "expected_runtime_one_iter": 2446,
         "name": "rocksdb_microbench_suite_2",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
@@ -345,23 +357,27 @@
                     }
                 ],
                 "DBGet/comp_style:1/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:0/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 471.459789
                     }
                 ],
                 "DBGet/comp_style:1/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:1/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 514.201628
                     }
@@ -445,7 +461,7 @@
         "expected_runtime_one_iter": 2437,
         "name": "rocksdb_microbench_suite_3",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
@@ -471,23 +487,27 @@
                     }
                 ],
                 "DBGet/comp_style:2/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:0/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 694.628355
                     }
                 ],
                 "DBGet/comp_style:2/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:1/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 517.245591
                     }
@@ -570,7 +590,7 @@
         "expected_runtime_one_iter": 2437,
         "name": "rocksdb_microbench_suite_4",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
@@ -700,18 +720,20 @@
         "expected_runtime_one_iter": 2437,
         "name": "rocksdb_microbench_suite_5",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
             "db_basic_bench": {
                 "DBGet/comp_style:0/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:0/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 658.890773
                     }
@@ -800,18 +822,20 @@
         "expected_runtime_one_iter": 2437,
         "name": "rocksdb_microbench_suite_6",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
             "db_basic_bench": {
                 "DBGet/comp_style:0/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:1/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 505.270116
                     }
@@ -827,12 +851,14 @@
                     }
                 ],
                 "DBGet/comp_style:2/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:1/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 532.702176
                     }
@@ -913,7 +939,7 @@
         "expected_runtime_one_iter": 2438,
         "name": "rocksdb_microbench_suite_7",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
@@ -1030,7 +1056,7 @@
         "expected_runtime_one_iter": 2437,
         "name": "rocksdb_microbench_suite_8",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
@@ -1056,12 +1082,14 @@
                     }
                 ],
                 "DBGet/comp_style:2/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:1/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 497.710941
                     }
@@ -1127,40 +1155,46 @@
         "expected_runtime_one_iter": 2437,
         "name": "rocksdb_microbench_suite_9",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
             "db_basic_bench": {
                 "DBGet/comp_style:0/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:1/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 624.132525
                     }
                 ],
                 "DBGet/comp_style:0/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:0/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 510.156004
                     }
                 ],
                 "DBGet/comp_style:0/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:1/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 551.935026
                     }
@@ -1212,40 +1246,46 @@
         "expected_runtime_one_iter": 2437,
         "name": "rocksdb_microbench_suite_10",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
             "db_basic_bench": {
                 "DBGet/comp_style:0/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:1/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 507.805668
                     }
                 ],
                 "DBGet/comp_style:1/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:0/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 557.749335
                     }
                 ],
                 "DBGet/comp_style:1/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:1/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 623.018994
                     }
@@ -1280,7 +1320,7 @@
         "expected_runtime_one_iter": 2446,
         "name": "rocksdb_microbench_suite_11",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
@@ -1296,12 +1336,14 @@
                     }
                 ],
                 "DBGet/comp_style:0/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:0/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 609.749844
                     }
@@ -1317,12 +1359,14 @@
                     }
                 ],
                 "DBGet/comp_style:1/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:1/enable_filter:1/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 557.886298
                     }
@@ -1414,7 +1458,7 @@
         "expected_runtime_one_iter": 2437,
         "name": "rocksdb_microbench_suite_12",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
@@ -1440,12 +1484,14 @@
                     }
                 ],
                 "DBGet/comp_style:2/max_data:134217728/per_key_size:256/enable_statistics:1/negative_query:0/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 578.538571
                     }
@@ -1513,7 +1559,7 @@
         "expected_runtime_one_iter": 2437,
         "name": "rocksdb_microbench_suite_13",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     },
     {
         "benchmarks": {
@@ -1539,12 +1585,14 @@
                     }
                 ],
                 "DBGet/comp_style:2/max_data:134217728/per_key_size:1024/enable_statistics:1/negative_query:0/enable_filter:0/iterations:10240/threads:1": [
+                    "get_p95",
                     "db_size",
                     "get_mean",
                     "threads",
                     "real_time",
                     "cpu_time",
                     "neg_qu_pct",
+                    "get_p99",
                     {
                         "est_runtime": 501.900122
                     }
@@ -1613,6 +1661,6 @@
         "expected_runtime_one_iter": 2437,
         "name": "rocksdb_microbench_suite_14",
         "regression_threshold": 10,
-        "sl_iterations": 3
+        "sl_iterations": 2
     }
-]
+]

build_tools/build_detect_platform

@@ -63,7 +63,18 @@ if [ -z "$ROCKSDB_NO_FBCODE" -a -d /mnt/gvfs/third-party ]; then
     if [ "$LIB_MODE" == "shared" ]; then
       PIC_BUILD=1
     fi
-    source "$PWD/build_tools/fbcode_config_platform009.sh"
+    if [ -n "$ROCKSDB_FBCODE_BUILD_WITH_481" ]; then
+      # we need this to build with MySQL. Don't use for other purposes.
+      source "$PWD/build_tools/fbcode_config4.8.1.sh"
+    elif [ -n "$ROCKSDB_FBCODE_BUILD_WITH_5xx" ]; then
+      source "$PWD/build_tools/fbcode_config.sh"
+    elif [ -n "$ROCKSDB_FBCODE_BUILD_WITH_PLATFORM007" ]; then
+      source "$PWD/build_tools/fbcode_config_platform007.sh"
+    elif [ -n "$ROCKSDB_FBCODE_BUILD_WITH_PLATFORM009" ]; then
+      source "$PWD/build_tools/fbcode_config_platform009.sh"
+    else
+      source "$PWD/build_tools/fbcode_config_platform009.sh"
+    fi
 fi
 
 # Delete existing output, if it exists
@@ -587,7 +598,7 @@ EOF
     fi
     if ! test $ROCKSDB_DISABLE_BENCHMARK; then
         # Test whether google benchmark is available
-        $CXX $PLATFORM_CXXFLAGS -x c++ - -o /dev/null -lbenchmark -lpthread 2>/dev/null  <<EOF
+        $CXX $PLATFORM_CXXFLAGS -x c++ - -o /dev/null -lbenchmark 2>/dev/null  <<EOF
           #include <benchmark/benchmark.h>
           int main() {}
 EOF

build_tools/dependencies.sh

@@ -0,0 +1,19 @@
+# Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+GCC_BASE=/mnt/gvfs/third-party2/gcc/7331085db891a2ef4a88a48a751d834e8d68f4cb/5.x/centos7-native/c447969
+CLANG_BASE=/mnt/gvfs/third-party2/llvm-fb/1bd23f9917738974ad0ff305aa23eb5f93f18305/9.0.0/centos7-native/c9f9104
+LIBGCC_BASE=/mnt/gvfs/third-party2/libgcc/6ace84e956873d53638c738b6f65f3f469cca74c/5.x/gcc-5-glibc-2.23/339d858
+GLIBC_BASE=/mnt/gvfs/third-party2/glibc/192b0f42d63dcf6210d6ceae387b49af049e6e0c/2.23/gcc-5-glibc-2.23/ca1d1c0
+SNAPPY_BASE=/mnt/gvfs/third-party2/snappy/7f9bdaada18f59bc27ec2b0871eb8a6144343aef/1.1.3/gcc-5-glibc-2.23/9bc6787
+ZLIB_BASE=/mnt/gvfs/third-party2/zlib/2d9f0b9a4274cc21f61272a9e89bdb859bce8f1f/1.2.8/gcc-5-glibc-2.23/9bc6787
+BZIP2_BASE=/mnt/gvfs/third-party2/bzip2/dc49a21c5fceec6456a7a28a94dcd16690af1337/1.0.6/gcc-5-glibc-2.23/9bc6787
+LZ4_BASE=/mnt/gvfs/third-party2/lz4/0f607f8fc442ea7d6b876931b1898bb573d5e5da/1.9.1/gcc-5-glibc-2.23/9bc6787
+ZSTD_BASE=/mnt/gvfs/third-party2/zstd/ca22bc441a4eb709e9e0b1f9fec9750fed7b31c5/1.4.x/gcc-5-glibc-2.23/03859b5
+GFLAGS_BASE=/mnt/gvfs/third-party2/gflags/0b9929d2588991c65a57168bf88aff2db87c5d48/2.2.0/gcc-5-glibc-2.23/9bc6787
+JEMALLOC_BASE=/mnt/gvfs/third-party2/jemalloc/c26f08f47ac35fc31da2633b7da92d6b863246eb/master/gcc-5-glibc-2.23/0c8f76d
+NUMA_BASE=/mnt/gvfs/third-party2/numa/3f3fb57a5ccc5fd21c66416c0b83e0aa76a05376/2.0.11/gcc-5-glibc-2.23/9bc6787
+LIBUNWIND_BASE=/mnt/gvfs/third-party2/libunwind/40c73d874898b386a71847f1b99115d93822d11f/1.4/gcc-5-glibc-2.23/b443de1
+TBB_BASE=/mnt/gvfs/third-party2/tbb/4ce8e8dba77cdbd81b75d6f0c32fd7a1b76a11ec/2018_U5/gcc-5-glibc-2.23/9bc6787
+KERNEL_HEADERS_BASE=/mnt/gvfs/third-party2/kernel-headers/fb251ecd2f5ae16f8671f7014c246e52a748fe0b/4.0.9-36_fbk5_2933_gd092e3f/gcc-5-glibc-2.23/da39a3e
+BINUTILS_BASE=/mnt/gvfs/third-party2/binutils/2e3cb7d119b3cea5f1e738cc13a1ac69f49eb875/2.29.1/centos7-native/da39a3e
+VALGRIND_BASE=/mnt/gvfs/third-party2/valgrind/d42d152a15636529b0861ec493927200ebebca8e/3.15.0/gcc-5-glibc-2.23/9bc6787
+LUA_BASE=/mnt/gvfs/third-party2/lua/f0cd714433206d5139df61659eb7b28b1dea6683/5.2.3/gcc-5-glibc-2.23/65372bd

build_tools/dependencies_4.8.1.sh

@@ -0,0 +1,20 @@
+# shellcheck disable=SC2148
+# Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+GCC_BASE=/mnt/gvfs/third-party2/gcc/cf7d14c625ce30bae1a4661c2319c5a283e4dd22/4.8.1/centos6-native/cc6c9dc
+CLANG_BASE=/mnt/gvfs/third-party2/llvm-fb/8598c375b0e94e1448182eb3df034704144a838d/stable/centos6-native/3f16ddd
+LIBGCC_BASE=/mnt/gvfs/third-party2/libgcc/d6e0a7da6faba45f5e5b1638f9edd7afc2f34e7d/4.8.1/gcc-4.8.1-glibc-2.17/8aac7fc
+GLIBC_BASE=/mnt/gvfs/third-party2/glibc/d282e6e8f3d20f4e40a516834847bdc038e07973/2.17/gcc-4.8.1-glibc-2.17/99df8fc
+SNAPPY_BASE=/mnt/gvfs/third-party2/snappy/8c38a4c1e52b4c2cc8a9cdc31b9c947ed7dbfcb4/1.1.3/gcc-4.8.1-glibc-2.17/c3f970a
+ZLIB_BASE=/mnt/gvfs/third-party2/zlib/0882df3713c7a84f15abe368dc004581f20b39d7/1.2.8/gcc-4.8.1-glibc-2.17/c3f970a
+BZIP2_BASE=/mnt/gvfs/third-party2/bzip2/740325875f6729f42d28deaa2147b0854f3a347e/1.0.6/gcc-4.8.1-glibc-2.17/c3f970a
+LZ4_BASE=/mnt/gvfs/third-party2/lz4/0e790b441e2d9acd68d51e1d2e028f88c6a79ddf/r131/gcc-4.8.1-glibc-2.17/c3f970a
+ZSTD_BASE=/mnt/gvfs/third-party2/zstd/9455f75ff7f4831dc9fda02a6a0f8c68922fad8f/1.0.0/gcc-4.8.1-glibc-2.17/c3f970a
+GFLAGS_BASE=/mnt/gvfs/third-party2/gflags/f001a51b2854957676d07306ef3abf67186b5c8b/2.1.1/gcc-4.8.1-glibc-2.17/c3f970a
+JEMALLOC_BASE=/mnt/gvfs/third-party2/jemalloc/fc8a13ca1fffa4d0765c716c5a0b49f0c107518f/master/gcc-4.8.1-glibc-2.17/8d31e51
+NUMA_BASE=/mnt/gvfs/third-party2/numa/17c514c4d102a25ca15f4558be564eeed76f4b6a/2.0.8/gcc-4.8.1-glibc-2.17/c3f970a
+LIBUNWIND_BASE=/mnt/gvfs/third-party2/libunwind/ad576de2a1ea560c4d3434304f0fc4e079bede42/trunk/gcc-4.8.1-glibc-2.17/675d945
+TBB_BASE=/mnt/gvfs/third-party2/tbb/9d9a554877d0c5bef330fe818ab7178806dd316a/4.0_update2/gcc-4.8.1-glibc-2.17/c3f970a
+KERNEL_HEADERS_BASE=/mnt/gvfs/third-party2/kernel-headers/7c111ff27e0c466235163f00f280a9d617c3d2ec/4.0.9-36_fbk5_2933_gd092e3f/gcc-4.8.1-glibc-2.17/da39a3e
+BINUTILS_BASE=/mnt/gvfs/third-party2/binutils/b7fd454c4b10c6a81015d4524ed06cdeab558490/2.26/centos6-native/da39a3e
+VALGRIND_BASE=/mnt/gvfs/third-party2/valgrind/d7f4d4d86674a57668e3a96f76f0e17dd0eb8765/3.8.1/gcc-4.8.1-glibc-2.17/c3f970a
+LUA_BASE=/mnt/gvfs/third-party2/lua/61e4abf5813bbc39bc4f548757ccfcadde175a48/5.2.3/centos6-native/730f94e

build_tools/dependencies_platform007.sh

@@ -0,0 +1,20 @@
+# Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+GCC_BASE=/mnt/gvfs/third-party2/gcc/7331085db891a2ef4a88a48a751d834e8d68f4cb/7.x/centos7-native/b2ef2b6
+CLANG_BASE=/mnt/gvfs/third-party2/llvm-fb/963d9aeda70cc4779885b1277484fe7544a04e3e/9.0.0/platform007/9e92d53/
+LIBGCC_BASE=/mnt/gvfs/third-party2/libgcc/6ace84e956873d53638c738b6f65f3f469cca74c/7.x/platform007/5620abc
+GLIBC_BASE=/mnt/gvfs/third-party2/glibc/192b0f42d63dcf6210d6ceae387b49af049e6e0c/2.26/platform007/f259413
+SNAPPY_BASE=/mnt/gvfs/third-party2/snappy/7f9bdaada18f59bc27ec2b0871eb8a6144343aef/1.1.3/platform007/ca4da3d
+ZLIB_BASE=/mnt/gvfs/third-party2/zlib/2d9f0b9a4274cc21f61272a9e89bdb859bce8f1f/1.2.8/platform007/ca4da3d
+BZIP2_BASE=/mnt/gvfs/third-party2/bzip2/dc49a21c5fceec6456a7a28a94dcd16690af1337/1.0.6/platform007/ca4da3d
+LZ4_BASE=/mnt/gvfs/third-party2/lz4/0f607f8fc442ea7d6b876931b1898bb573d5e5da/1.9.1/platform007/ca4da3d
+ZSTD_BASE=/mnt/gvfs/third-party2/zstd/ca22bc441a4eb709e9e0b1f9fec9750fed7b31c5/1.4.x/platform007/15a3614
+GFLAGS_BASE=/mnt/gvfs/third-party2/gflags/0b9929d2588991c65a57168bf88aff2db87c5d48/2.2.0/platform007/ca4da3d
+JEMALLOC_BASE=/mnt/gvfs/third-party2/jemalloc/c26f08f47ac35fc31da2633b7da92d6b863246eb/master/platform007/c26c002
+NUMA_BASE=/mnt/gvfs/third-party2/numa/3f3fb57a5ccc5fd21c66416c0b83e0aa76a05376/2.0.11/platform007/ca4da3d
+LIBUNWIND_BASE=/mnt/gvfs/third-party2/libunwind/40c73d874898b386a71847f1b99115d93822d11f/1.4/platform007/6f3e0a9
+TBB_BASE=/mnt/gvfs/third-party2/tbb/4ce8e8dba77cdbd81b75d6f0c32fd7a1b76a11ec/2018_U5/platform007/ca4da3d
+LIBURING_BASE=/mnt/gvfs/third-party2/liburing/79427253fd0d42677255aacfe6d13bfe63f752eb/20190828/platform007/ca4da3d
+KERNEL_HEADERS_BASE=/mnt/gvfs/third-party2/kernel-headers/fb251ecd2f5ae16f8671f7014c246e52a748fe0b/fb/platform007/da39a3e
+BINUTILS_BASE=/mnt/gvfs/third-party2/binutils/ab9f09bba370e7066cafd4eb59752db93f2e8312/2.29.1/platform007/15a3614
+VALGRIND_BASE=/mnt/gvfs/third-party2/valgrind/d42d152a15636529b0861ec493927200ebebca8e/3.15.0/platform007/ca4da3d
+LUA_BASE=/mnt/gvfs/third-party2/lua/f0cd714433206d5139df61659eb7b28b1dea6683/5.3.4/platform007/5007832

build_tools/dependencies_platform009.sh

@@ -18,4 +18,4 @@ KERNEL_HEADERS_BASE=/mnt/gvfs/third-party2/kernel-headers/32b8a2407b634df3f8f948
 BINUTILS_BASE=/mnt/gvfs/third-party2/binutils/08634589372fa5f237bfd374e8c644a8364e78c1/2.32/platform009/ba86d1f/
 VALGRIND_BASE=/mnt/gvfs/third-party2/valgrind/6ae525939ad02e5e676855082fbbc7828dbafeac/3.15.0/platform009/7f3b187
 LUA_BASE=/mnt/gvfs/third-party2/lua/162efd9561a3d21f6869f4814011e9cf1b3ff4dc/5.3.4/platform009/a6271c4
-BENCHMARK_BASE=/mnt/gvfs/third-party2/benchmark/30bf49ad6414325e17f3425b0edcb64239427ae3/1.6.1/platform009/7f3b187
+BENCHMARK_BASE=/mnt/gvfs/third-party2/benchmark/bce8d9564eaf161700aa3a20b1051564acf555fb/1.5.5/platform009/7f3b187

build_tools/fbcode_config4.8.1.sh

@@ -0,0 +1,120 @@
+#!/bin/sh
+# Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+#
+# Set environment variables so that we can compile rocksdb using
+# fbcode settings.  It uses the latest g++ compiler and also
+# uses jemalloc
+
+BASEDIR=`dirname $BASH_SOURCE`
+source "$BASEDIR/dependencies_4.8.1.sh"
+
+# location of libgcc
+LIBGCC_INCLUDE="$LIBGCC_BASE/include"
+LIBGCC_LIBS=" -L $LIBGCC_BASE/lib"
+
+# location of glibc
+GLIBC_INCLUDE="$GLIBC_BASE/include"
+GLIBC_LIBS=" -L $GLIBC_BASE/lib"
+
+# location of snappy headers and libraries
+SNAPPY_INCLUDE=" -I $SNAPPY_BASE/include"
+SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy.a"
+
+# location of zlib headers and libraries
+ZLIB_INCLUDE=" -I $ZLIB_BASE/include"
+ZLIB_LIBS=" $ZLIB_BASE/lib/libz.a"
+
+# location of bzip headers and libraries
+BZIP2_INCLUDE=" -I $BZIP2_BASE/include/"
+BZIP2_LIBS=" $BZIP2_BASE/lib/libbz2.a"
+
+LZ4_INCLUDE=" -I $LZ4_BASE/include"
+LZ4_LIBS=" $LZ4_BASE/lib/liblz4.a"
+
+ZSTD_INCLUDE=" -I $ZSTD_BASE/include"
+ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd.a"
+
+# location of gflags headers and libraries
+GFLAGS_INCLUDE=" -I $GFLAGS_BASE/include/"
+GFLAGS_LIBS=" $GFLAGS_BASE/lib/libgflags.a"
+
+# location of jemalloc
+JEMALLOC_INCLUDE=" -I $JEMALLOC_BASE/include"
+JEMALLOC_LIB="$JEMALLOC_BASE/lib/libjemalloc.a"
+
+# location of numa
+NUMA_INCLUDE=" -I $NUMA_BASE/include/"
+NUMA_LIB=" $NUMA_BASE/lib/libnuma.a"
+
+# location of libunwind
+LIBUNWIND="$LIBUNWIND_BASE/lib/libunwind.a"
+
+# location of tbb
+TBB_INCLUDE=" -isystem $TBB_BASE/include/"
+TBB_LIBS="$TBB_BASE/lib/libtbb.a"
+
+test "$USE_SSE" || USE_SSE=1
+export USE_SSE
+test "$PORTABLE" || PORTABLE=1
+export PORTABLE
+
+BINUTILS="$BINUTILS_BASE/bin"
+AR="$BINUTILS/ar"
+
+DEPS_INCLUDE="$SNAPPY_INCLUDE $ZLIB_INCLUDE $BZIP2_INCLUDE $LZ4_INCLUDE $ZSTD_INCLUDE $GFLAGS_INCLUDE $NUMA_INCLUDE $TBB_INCLUDE"
+
+STDLIBS="-L $GCC_BASE/lib64"
+
+if [ -z "$USE_CLANG" ]; then
+  # gcc
+  CC="$GCC_BASE/bin/gcc"
+  CXX="$GCC_BASE/bin/g++"
+  CXX="$GCC_BASE/bin/gcc-ar"
+
+  CFLAGS="-B$BINUTILS/gold -m64 -mtune=generic"
+  CFLAGS+=" -isystem $GLIBC_INCLUDE"
+  CFLAGS+=" -isystem $LIBGCC_INCLUDE"
+  JEMALLOC=1
+else
+  # clang
+  CLANG_BIN="$CLANG_BASE/bin"
+  CLANG_LIB="$CLANG_BASE/lib"
+  CLANG_INCLUDE="$CLANG_LIB/clang/*/include"
+  CC="$CLANG_BIN/clang"
+  CXX="$CLANG_BIN/clang++"
+  AR="$CLANG_BIN/llvm-ar"
+
+  KERNEL_HEADERS_INCLUDE="$KERNEL_HEADERS_BASE/include/"
+
+  CFLAGS="-B$BINUTILS/gold -nostdinc -nostdlib"
+  CFLAGS+=" -isystem $LIBGCC_BASE/include/c++/4.8.1 "
+  CFLAGS+=" -isystem $LIBGCC_BASE/include/c++/4.8.1/x86_64-facebook-linux "
+  CFLAGS+=" -isystem $GLIBC_INCLUDE"
+  CFLAGS+=" -isystem $LIBGCC_INCLUDE"
+  CFLAGS+=" -isystem $CLANG_INCLUDE"
+  CFLAGS+=" -isystem $KERNEL_HEADERS_INCLUDE/linux "
+  CFLAGS+=" -isystem $KERNEL_HEADERS_INCLUDE "
+  CXXFLAGS="-nostdinc++"
+fi
+
+CFLAGS+=" $DEPS_INCLUDE"
+CFLAGS+=" -DROCKSDB_PLATFORM_POSIX -DROCKSDB_LIB_IO_POSIX -DROCKSDB_FALLOCATE_PRESENT -DROCKSDB_MALLOC_USABLE_SIZE -DROCKSDB_RANGESYNC_PRESENT -DROCKSDB_SCHED_GETCPU_PRESENT -DROCKSDB_SUPPORT_THREAD_LOCAL -DHAVE_SSE42"
+CFLAGS+=" -DSNAPPY -DGFLAGS=google -DZLIB -DBZIP2 -DLZ4 -DZSTD -DNUMA -DTBB"
+CXXFLAGS+=" $CFLAGS"
+
+EXEC_LDFLAGS=" $SNAPPY_LIBS $ZLIB_LIBS $BZIP2_LIBS $LZ4_LIBS $ZSTD_LIBS $GFLAGS_LIBS $NUMA_LIB $TBB_LIBS"
+EXEC_LDFLAGS+=" -Wl,--dynamic-linker,/usr/local/fbcode/gcc-4.8.1-glibc-2.17/lib/ld.so"
+EXEC_LDFLAGS+=" $LIBUNWIND"
+EXEC_LDFLAGS+=" -Wl,-rpath=/usr/local/fbcode/gcc-4.8.1-glibc-2.17/lib"
+# required by libtbb
+EXEC_LDFLAGS+=" -ldl"
+
+PLATFORM_LDFLAGS="$LIBGCC_LIBS $GLIBC_LIBS $STDLIBS -lgcc -lstdc++"
+
+EXEC_LDFLAGS_SHARED="$SNAPPY_LIBS $ZLIB_LIBS $BZIP2_LIBS $LZ4_LIBS $ZSTD_LIBS $GFLAGS_LIBS"
+
+VALGRIND_VER="$VALGRIND_BASE/bin/"
+
+LUA_PATH="$LUA_BASE"
+
+export CC CXX AR CFLAGS CXXFLAGS EXEC_LDFLAGS EXEC_LDFLAGS_SHARED VALGRIND_VER JEMALLOC_LIB JEMALLOC_INCLUDE LUA_PATH

build_tools/fbcode_config_platform007.sh

@@ -0,0 +1,170 @@
+#!/bin/sh
+# Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+#
+# Set environment variables so that we can compile rocksdb using
+# fbcode settings.  It uses the latest g++ and clang compilers and also
+# uses jemalloc
+# Environment variables that change the behavior of this script:
+# PIC_BUILD -- if true, it will only take pic versions of libraries from fbcode. libraries that don't have pic variant will not be included
+
+
+BASEDIR=`dirname $BASH_SOURCE`
+source "$BASEDIR/dependencies_platform007.sh"
+
+CFLAGS=""
+
+# libgcc
+LIBGCC_INCLUDE="$LIBGCC_BASE/include/c++/7.3.0"
+LIBGCC_LIBS=" -L $LIBGCC_BASE/lib"
+
+# glibc
+GLIBC_INCLUDE="$GLIBC_BASE/include"
+GLIBC_LIBS=" -L $GLIBC_BASE/lib"
+
+# snappy
+SNAPPY_INCLUDE=" -I $SNAPPY_BASE/include/"
+if test -z $PIC_BUILD; then
+  SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy.a"
+else
+  SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy_pic.a"
+fi
+CFLAGS+=" -DSNAPPY"
+
+if test -z $PIC_BUILD; then
+  # location of zlib headers and libraries
+  ZLIB_INCLUDE=" -I $ZLIB_BASE/include/"
+  ZLIB_LIBS=" $ZLIB_BASE/lib/libz.a"
+  CFLAGS+=" -DZLIB"
+
+  # location of bzip headers and libraries
+  BZIP_INCLUDE=" -I $BZIP2_BASE/include/"
+  BZIP_LIBS=" $BZIP2_BASE/lib/libbz2.a"
+  CFLAGS+=" -DBZIP2"
+
+  LZ4_INCLUDE=" -I $LZ4_BASE/include/"
+  LZ4_LIBS=" $LZ4_BASE/lib/liblz4.a"
+  CFLAGS+=" -DLZ4"
+fi
+
+ZSTD_INCLUDE=" -I $ZSTD_BASE/include/"
+if test -z $PIC_BUILD; then
+  ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd.a"
+else
+  ZSTD_LIBS=" $ZSTD_BASE/lib/libzstd_pic.a"
+fi
+CFLAGS+=" -DZSTD"
+
+# location of gflags headers and libraries
+GFLAGS_INCLUDE=" -I $GFLAGS_BASE/include/"
+if test -z $PIC_BUILD; then
+  GFLAGS_LIBS=" $GFLAGS_BASE/lib/libgflags.a"
+else
+  GFLAGS_LIBS=" $GFLAGS_BASE/lib/libgflags_pic.a"
+fi
+CFLAGS+=" -DGFLAGS=gflags"
+
+# location of jemalloc
+JEMALLOC_INCLUDE=" -I $JEMALLOC_BASE/include/"
+JEMALLOC_LIB=" $JEMALLOC_BASE/lib/libjemalloc.a"
+
+if test -z $PIC_BUILD; then
+  # location of numa
+  NUMA_INCLUDE=" -I $NUMA_BASE/include/"
+  NUMA_LIB=" $NUMA_BASE/lib/libnuma.a"
+  CFLAGS+=" -DNUMA"
+
+  # location of libunwind
+  LIBUNWIND="$LIBUNWIND_BASE/lib/libunwind.a"
+fi
+
+# location of TBB
+TBB_INCLUDE=" -isystem $TBB_BASE/include/"
+if test -z $PIC_BUILD; then
+  TBB_LIBS="$TBB_BASE/lib/libtbb.a"
+else
+  TBB_LIBS="$TBB_BASE/lib/libtbb_pic.a"
+fi
+CFLAGS+=" -DTBB"
+
+# location of LIBURING
+LIBURING_INCLUDE=" -isystem $LIBURING_BASE/include/"
+if test -z $PIC_BUILD; then
+  LIBURING_LIBS="$LIBURING_BASE/lib/liburing.a"
+else
+  LIBURING_LIBS="$LIBURING_BASE/lib/liburing_pic.a"
+fi
+CFLAGS+=" -DLIBURING"
+
+test "$USE_SSE" || USE_SSE=1
+export USE_SSE
+test "$PORTABLE" || PORTABLE=1
+export PORTABLE
+
+BINUTILS="$BINUTILS_BASE/bin"
+AR="$BINUTILS/ar"
+
+DEPS_INCLUDE="$SNAPPY_INCLUDE $ZLIB_INCLUDE $BZIP_INCLUDE $LZ4_INCLUDE $ZSTD_INCLUDE $GFLAGS_INCLUDE $NUMA_INCLUDE $TBB_INCLUDE $LIBURING_INCLUDE"
+
+STDLIBS="-L $GCC_BASE/lib64"
+
+CLANG_BIN="$CLANG_BASE/bin"
+CLANG_LIB="$CLANG_BASE/lib"
+CLANG_SRC="$CLANG_BASE/../../src"
+
+CLANG_ANALYZER="$CLANG_BIN/clang++"
+CLANG_SCAN_BUILD="$CLANG_SRC/llvm/tools/clang/tools/scan-build/bin/scan-build"
+
+if [ -z "$USE_CLANG" ]; then
+  # gcc
+  CC="$GCC_BASE/bin/gcc"
+  CXX="$GCC_BASE/bin/g++"
+  AR="$GCC_BASE/bin/gcc-ar"
+
+  CFLAGS+=" -B$BINUTILS/gold"
+  CFLAGS+=" -isystem $LIBGCC_INCLUDE"
+  CFLAGS+=" -isystem $GLIBC_INCLUDE"
+  JEMALLOC=1
+else
+  # clang
+  CLANG_INCLUDE="$CLANG_LIB/clang/stable/include"
+  CC="$CLANG_BIN/clang"
+  CXX="$CLANG_BIN/clang++"
+  AR="$CLANG_BIN/llvm-ar"
+
+  KERNEL_HEADERS_INCLUDE="$KERNEL_HEADERS_BASE/include"
+
+  CFLAGS+=" -B$BINUTILS/gold -nostdinc -nostdlib"
+  CFLAGS+=" -isystem $LIBGCC_BASE/include/c++/7.x "
+  CFLAGS+=" -isystem $LIBGCC_BASE/include/c++/7.x/x86_64-facebook-linux "
+  CFLAGS+=" -isystem $GLIBC_INCLUDE"
+  CFLAGS+=" -isystem $LIBGCC_INCLUDE"
+  CFLAGS+=" -isystem $CLANG_INCLUDE"
+  CFLAGS+=" -isystem $KERNEL_HEADERS_INCLUDE/linux "
+  CFLAGS+=" -isystem $KERNEL_HEADERS_INCLUDE "
+  CFLAGS+=" -Wno-expansion-to-defined "
+  CXXFLAGS="-nostdinc++"
+fi
+
+CFLAGS+=" $DEPS_INCLUDE"
+CFLAGS+=" -DROCKSDB_PLATFORM_POSIX -DROCKSDB_LIB_IO_POSIX -DROCKSDB_FALLOCATE_PRESENT -DROCKSDB_MALLOC_USABLE_SIZE -DROCKSDB_RANGESYNC_PRESENT -DROCKSDB_SCHED_GETCPU_PRESENT -DROCKSDB_SUPPORT_THREAD_LOCAL -DHAVE_SSE42 -DROCKSDB_IOURING_PRESENT"
+CXXFLAGS+=" $CFLAGS"
+
+EXEC_LDFLAGS=" $SNAPPY_LIBS $ZLIB_LIBS $BZIP_LIBS $LZ4_LIBS $ZSTD_LIBS $GFLAGS_LIBS $NUMA_LIB $TBB_LIBS $LIBURING_LIBS"
+EXEC_LDFLAGS+=" -B$BINUTILS/gold"
+EXEC_LDFLAGS+=" -Wl,--dynamic-linker,/usr/local/fbcode/platform007/lib/ld.so"
+EXEC_LDFLAGS+=" $LIBUNWIND"
+EXEC_LDFLAGS+=" -Wl,-rpath=/usr/local/fbcode/platform007/lib"
+# required by libtbb
+EXEC_LDFLAGS+=" -ldl"
+
+PLATFORM_LDFLAGS="$LIBGCC_LIBS $GLIBC_LIBS $STDLIBS -lgcc -lstdc++"
+
+EXEC_LDFLAGS_SHARED="$SNAPPY_LIBS $ZLIB_LIBS $BZIP_LIBS $LZ4_LIBS $ZSTD_LIBS $GFLAGS_LIBS $TBB_LIBS $LIBURING_LIBS"
+
+VALGRIND_VER="$VALGRIND_BASE/bin/"
+
+# lua not supported because it's on track for deprecation, I think
+LUA_PATH=
+LUA_LIB=
+
+export CC CXX AR CFLAGS CXXFLAGS EXEC_LDFLAGS EXEC_LDFLAGS_SHARED VALGRIND_VER JEMALLOC_LIB JEMALLOC_INCLUDE CLANG_ANALYZER CLANG_SCAN_BUILD LUA_PATH LUA_LIB

build_tools/precommit_checker.py

@@ -0,0 +1,209 @@
+#!/usr/bin/env python2.7
+# Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+import argparse
+import commands
+import subprocess
+import sys
+import re
+import os
+import time
+
+
+#
+# Simple logger
+#
+
+class Log:
+
+    def __init__(self, filename):
+        self.filename = filename
+        self.f = open(self.filename, 'w+', 0)
+
+    def caption(self, str):
+        line = "\n##### %s #####\n" % str
+        if self.f:
+            self.f.write("%s \n" % line)
+        else:
+            print(line)
+
+    def error(self, str):
+        data = "\n\n##### ERROR ##### %s" % str
+        if self.f:
+            self.f.write("%s \n" % data)
+        else:
+            print(data)
+
+    def log(self, str):
+        if self.f:
+            self.f.write("%s \n" % str)
+        else:
+            print(str)
+
+#
+# Shell Environment
+#
+
+
+class Env(object):
+
+    def __init__(self, logfile, tests):
+        self.tests = tests
+        self.log = Log(logfile)
+
+    def shell(self, cmd, path=os.getcwd()):
+        if path:
+            os.chdir(path)
+
+        self.log.log("==== shell session ===========================")
+        self.log.log("%s> %s" % (path, cmd))
+        status = subprocess.call("cd %s; %s" % (path, cmd), shell=True,
+                                 stdout=self.log.f, stderr=self.log.f)
+        self.log.log("status = %s" % status)
+        self.log.log("============================================== \n\n")
+        return status
+
+    def GetOutput(self, cmd, path=os.getcwd()):
+        if path:
+            os.chdir(path)
+
+        self.log.log("==== shell session ===========================")
+        self.log.log("%s> %s" % (path, cmd))
+        status, out = commands.getstatusoutput(cmd)
+        self.log.log("status = %s" % status)
+        self.log.log("out = %s" % out)
+        self.log.log("============================================== \n\n")
+        return status, out
+
+#
+# Pre-commit checker
+#
+
+
+class PreCommitChecker(Env):
+
+    def __init__(self, args):
+        Env.__init__(self, args.logfile, args.tests)
+        self.ignore_failure = args.ignore_failure
+
+    #
+    #   Get commands for a given job from the determinator file
+    #
+    def get_commands(self, test):
+        status, out = self.GetOutput(
+            "RATIO=1 build_tools/rocksdb-lego-determinator %s" % test, ".")
+        return status, out
+
+    #
+    # Run a specific CI job
+    #
+    def run_test(self, test):
+        self.log.caption("Running test %s locally" % test)
+
+        # get commands for the CI job determinator
+        status, cmds = self.get_commands(test)
+        if status != 0:
+            self.log.error("Error getting commands for test %s" % test)
+            return False
+
+        # Parse the JSON to extract the commands to run
+        cmds = re.findall("'shell':'([^\']*)'", cmds)
+
+        if len(cmds) == 0:
+            self.log.log("No commands found")
+            return False
+
+        # Run commands
+        for cmd in cmds:
+            # Replace J=<..> with the local environment variable
+            if "J" in os.environ:
+                cmd = cmd.replace("J=1", "J=%s" % os.environ["J"])
+                cmd = cmd.replace("make ", "make -j%s " % os.environ["J"])
+            # Run the command
+            status = self.shell(cmd, ".")
+            if status != 0:
+                self.log.error("Error running command %s for test %s"
+                               % (cmd, test))
+                return False
+
+        return True
+
+    #
+    # Run specified CI jobs
+    #
+    def run_tests(self):
+        if not self.tests:
+            self.log.error("Invalid args. Please provide tests")
+            return False
+
+        self.print_separator()
+        self.print_row("TEST", "RESULT")
+        self.print_separator()
+
+        result = True
+        for test in self.tests:
+            start_time = time.time()
+            self.print_test(test)
+            result = self.run_test(test)
+            elapsed_min = (time.time() - start_time) / 60
+            if not result:
+                self.log.error("Error running test %s" % test)
+                self.print_result("FAIL (%dm)" % elapsed_min)
+                if not self.ignore_failure:
+                    return False
+                result = False
+            else:
+                self.print_result("PASS (%dm)" % elapsed_min)
+
+        self.print_separator()
+        return result
+
+    #
+    # Print a line
+    #
+    def print_separator(self):
+        print("".ljust(60, "-"))
+
+    #
+    # Print two colums
+    #
+    def print_row(self, c0, c1):
+        print("%s%s" % (c0.ljust(40), c1.ljust(20)))
+
+    def print_test(self, test):
+        print(test.ljust(40), end="")
+        sys.stdout.flush()
+
+    def print_result(self, result):
+        print(result.ljust(20))
+
+#
+# Main
+#
+parser = argparse.ArgumentParser(description='RocksDB pre-commit checker.')
+
+# --log <logfile>
+parser.add_argument('--logfile', default='/tmp/precommit-check.log',
+                    help='Log file. Default is /tmp/precommit-check.log')
+# --ignore_failure
+parser.add_argument('--ignore_failure', action='store_true', default=False,
+                    help='Stop when an error occurs')
+# <test ....>
+parser.add_argument('tests', nargs='+',
+                    help='CI test(s) to run. e.g: unit punit asan tsan ubsan')
+
+args = parser.parse_args()
+checker = PreCommitChecker(args)
+
+print("Please follow log %s" % checker.log.filename)
+
+if not checker.run_tests():
+    print("Error running tests. Please check log file %s"
+          % checker.log.filename)
+    sys.exit(1)
+
+sys.exit(0)

build_tools/run_ci_db_test.ps1

@@ -42,7 +42,7 @@ $RunOnly.Add("c_test") | Out-Null
 $RunOnly.Add("compact_on_deletion_collector_test") | Out-Null
 $RunOnly.Add("merge_test") | Out-Null
 $RunOnly.Add("stringappend_test") | Out-Null # Apparently incorrectly written
-$RunOnly.Add("backup_engine_test") | Out-Null # Disabled
+$RunOnly.Add("backupable_db_test") | Out-Null # Disabled
 $RunOnly.Add("timer_queue_test") | Out-Null # Not a gtest
 
 if($RunAll -and $SuiteRun -ne "") {
@@ -491,3 +491,5 @@ if(!$script:success) {
  }
 
  exit 0
+
+

cache/cache_entry_roles.cc

@@ -21,7 +21,6 @@ std::array<const char*, kNumCacheEntryRoles> kCacheEntryRoleToCamelString{{
     "WriteBuffer",
     "CompressionDictionaryBuildingBuffer",
     "FilterConstruction",
-    "BlockBasedTableReader",
     "Misc",
 }};
 
@@ -35,7 +34,6 @@ std::array<const char*, kNumCacheEntryRoles> kCacheEntryRoleToHyphenString{{
     "write-buffer",
     "compression-dictionary-building-buffer",
     "filter-construction",
-    "block-based-table-reader",
     "misc",
 }};
 

cache/cache_entry_roles.h

@@ -39,9 +39,6 @@ enum class CacheEntryRole {
   // Filter reservations to account for
   // (new) bloom and ribbon filter construction's memory usage
   kFilterConstruction,
-  // BlockBasedTableReader reservations to account for
-  // its memory usage
-  kBlockBasedTableReader,
   // Default bucket, for miscellaneous cache entries. Do not use for
   // entries that could potentially add up to large usage.
   kMisc,

cache/cache_reservation_manager.cc

@@ -17,30 +17,12 @@
 #include "rocksdb/cache.h"
 #include "rocksdb/slice.h"
 #include "rocksdb/status.h"
-#include "table/block_based/reader_common.h"
+#include "table/block_based/block_based_table_reader.h"
 #include "util/coding.h"
 
 namespace ROCKSDB_NAMESPACE {
-
-template <CacheEntryRole R>
-CacheReservationManagerImpl<R>::CacheReservationHandle::CacheReservationHandle(
-    std::size_t incremental_memory_used,
-    std::shared_ptr<CacheReservationManagerImpl> cache_res_mgr)
-    : incremental_memory_used_(incremental_memory_used) {
-  assert(cache_res_mgr);
-  cache_res_mgr_ = cache_res_mgr;
-}
-
-template <CacheEntryRole R>
-CacheReservationManagerImpl<
-    R>::CacheReservationHandle::~CacheReservationHandle() {
-  Status s = cache_res_mgr_->ReleaseCacheReservation(incremental_memory_used_);
-  s.PermitUncheckedError();
-}
-
-template <CacheEntryRole R>
-CacheReservationManagerImpl<R>::CacheReservationManagerImpl(
-    std::shared_ptr<Cache> cache, bool delayed_decrease)
+CacheReservationManager::CacheReservationManager(std::shared_ptr<Cache> cache,
+                                                 bool delayed_decrease)
     : delayed_decrease_(delayed_decrease),
       cache_allocated_size_(0),
       memory_used_(0) {
@@ -48,15 +30,14 @@ CacheReservationManagerImpl<R>::CacheReservationManagerImpl(
   cache_ = cache;
 }
 
-template <CacheEntryRole R>
-CacheReservationManagerImpl<R>::~CacheReservationManagerImpl() {
+CacheReservationManager::~CacheReservationManager() {
   for (auto* handle : dummy_handles_) {
     cache_->Release(handle, true);
   }
 }
 
 template <CacheEntryRole R>
-Status CacheReservationManagerImpl<R>::UpdateCacheReservation(
+Status CacheReservationManager::UpdateCacheReservation(
     std::size_t new_mem_used) {
   memory_used_ = new_mem_used;
   std::size_t cur_cache_allocated_size =
@@ -64,7 +45,7 @@ Status CacheReservationManagerImpl<R>::UpdateCacheReservation(
   if (new_mem_used == cur_cache_allocated_size) {
     return Status::OK();
   } else if (new_mem_used > cur_cache_allocated_size) {
-    Status s = IncreaseCacheReservation(new_mem_used);
+    Status s = IncreaseCacheReservation<R>(new_mem_used);
     return s;
   } else {
     // In delayed decrease mode, we don't decrease cache reservation
@@ -85,32 +66,41 @@ Status CacheReservationManagerImpl<R>::UpdateCacheReservation(
   }
 }
 
+// Explicitly instantiate templates for "CacheEntryRole" values we use.
+// This makes it possible to keep the template definitions in the .cc file.
+template Status CacheReservationManager::UpdateCacheReservation<
+    CacheEntryRole::kWriteBuffer>(std::size_t new_mem_used);
+template Status CacheReservationManager::UpdateCacheReservation<
+    CacheEntryRole::kCompressionDictionaryBuildingBuffer>(
+    std::size_t new_mem_used);
+// For cache reservation manager unit tests
+template Status CacheReservationManager::UpdateCacheReservation<
+    CacheEntryRole::kMisc>(std::size_t new_mem_used);
+
 template <CacheEntryRole R>
-Status CacheReservationManagerImpl<R>::MakeCacheReservation(
+Status CacheReservationManager::MakeCacheReservation(
     std::size_t incremental_memory_used,
-    std::unique_ptr<CacheReservationManager::CacheReservationHandle>* handle) {
-  assert(handle);
+    std::unique_ptr<CacheReservationHandle<R>>* handle) {
+  assert(handle != nullptr);
   Status s =
-      UpdateCacheReservation(GetTotalMemoryUsed() + incremental_memory_used);
-  (*handle).reset(new CacheReservationManagerImpl::CacheReservationHandle(
-      incremental_memory_used,
-      std::enable_shared_from_this<
-          CacheReservationManagerImpl<R>>::shared_from_this()));
+      UpdateCacheReservation<R>(GetTotalMemoryUsed() + incremental_memory_used);
+  (*handle).reset(new CacheReservationHandle<R>(incremental_memory_used,
+                                                shared_from_this()));
   return s;
 }
 
-template <CacheEntryRole R>
-Status CacheReservationManagerImpl<R>::ReleaseCacheReservation(
-    std::size_t incremental_memory_used) {
-  assert(GetTotalMemoryUsed() >= incremental_memory_used);
-  std::size_t updated_total_mem_used =
-      GetTotalMemoryUsed() - incremental_memory_used;
-  Status s = UpdateCacheReservation(updated_total_mem_used);
-  return s;
-}
+template Status
+CacheReservationManager::MakeCacheReservation<CacheEntryRole::kMisc>(
+    std::size_t incremental_memory_used,
+    std::unique_ptr<CacheReservationHandle<CacheEntryRole::kMisc>>* handle);
+template Status CacheReservationManager::MakeCacheReservation<
+    CacheEntryRole::kFilterConstruction>(
+    std::size_t incremental_memory_used,
+    std::unique_ptr<
+        CacheReservationHandle<CacheEntryRole::kFilterConstruction>>* handle);
 
 template <CacheEntryRole R>
-Status CacheReservationManagerImpl<R>::IncreaseCacheReservation(
+Status CacheReservationManager::IncreaseCacheReservation(
     std::size_t new_mem_used) {
   Status return_status = Status::OK();
   while (new_mem_used > cache_allocated_size_.load(std::memory_order_relaxed)) {
@@ -128,8 +118,7 @@ Status CacheReservationManagerImpl<R>::IncreaseCacheReservation(
   return return_status;
 }
 
-template <CacheEntryRole R>
-Status CacheReservationManagerImpl<R>::DecreaseCacheReservation(
+Status CacheReservationManager::DecreaseCacheReservation(
     std::size_t new_mem_used) {
   Status return_status = Status::OK();
 
@@ -148,18 +137,15 @@ Status CacheReservationManagerImpl<R>::DecreaseCacheReservation(
   return return_status;
 }
 
-template <CacheEntryRole R>
-std::size_t CacheReservationManagerImpl<R>::GetTotalReservedCacheSize() {
+std::size_t CacheReservationManager::GetTotalReservedCacheSize() {
   return cache_allocated_size_.load(std::memory_order_relaxed);
 }
 
-template <CacheEntryRole R>
-std::size_t CacheReservationManagerImpl<R>::GetTotalMemoryUsed() {
+std::size_t CacheReservationManager::GetTotalMemoryUsed() {
   return memory_used_;
 }
 
-template <CacheEntryRole R>
-Slice CacheReservationManagerImpl<R>::GetNextCacheKey() {
+Slice CacheReservationManager::GetNextCacheKey() {
   // Calling this function will have the side-effect of changing the
   // underlying cache_key_ that is shared among other keys generated from this
   // fucntion. Therefore please make sure the previous keys are saved/copied
@@ -169,15 +155,34 @@ Slice CacheReservationManagerImpl<R>::GetNextCacheKey() {
 }
 
 template <CacheEntryRole R>
-Cache::DeleterFn CacheReservationManagerImpl<R>::TEST_GetNoopDeleterForRole() {
+Cache::DeleterFn CacheReservationManager::TEST_GetNoopDeleterForRole() {
   return GetNoopDeleterForRole<R>();
 }
 
-template class CacheReservationManagerImpl<
-    CacheEntryRole::kBlockBasedTableReader>;
-template class CacheReservationManagerImpl<
-    CacheEntryRole::kCompressionDictionaryBuildingBuffer>;
-template class CacheReservationManagerImpl<CacheEntryRole::kFilterConstruction>;
-template class CacheReservationManagerImpl<CacheEntryRole::kMisc>;
-template class CacheReservationManagerImpl<CacheEntryRole::kWriteBuffer>;
+template Cache::DeleterFn CacheReservationManager::TEST_GetNoopDeleterForRole<
+    CacheEntryRole::kFilterConstruction>();
+
+template <CacheEntryRole R>
+CacheReservationHandle<R>::CacheReservationHandle(
+    std::size_t incremental_memory_used,
+    std::shared_ptr<CacheReservationManager> cache_res_mgr)
+    : incremental_memory_used_(incremental_memory_used) {
+  assert(cache_res_mgr != nullptr);
+  cache_res_mgr_ = cache_res_mgr;
+}
+
+template <CacheEntryRole R>
+CacheReservationHandle<R>::~CacheReservationHandle() {
+  assert(cache_res_mgr_ != nullptr);
+  assert(cache_res_mgr_->GetTotalMemoryUsed() >= incremental_memory_used_);
+
+  Status s = cache_res_mgr_->UpdateCacheReservation<R>(
+      cache_res_mgr_->GetTotalMemoryUsed() - incremental_memory_used_);
+  s.PermitUncheckedError();
+}
+
+// Explicitly instantiate templates for "CacheEntryRole" values we use.
+// This makes it possible to keep the template definitions in the .cc file.
+template class CacheReservationHandle<CacheEntryRole::kMisc>;
+template class CacheReservationHandle<CacheEntryRole::kFilterConstruction>;
 }  // namespace ROCKSDB_NAMESPACE

cache/cache_reservation_manager.h

@@ -13,90 +13,54 @@
 #include <cstddef>
 #include <cstdint>
 #include <memory>
-#include <mutex>
 #include <vector>
 
 #include "cache/cache_entry_roles.h"
-#include "cache/cache_key.h"
 #include "rocksdb/cache.h"
 #include "rocksdb/slice.h"
 #include "rocksdb/status.h"
+#include "table/block_based/block_based_table_reader.h"
 #include "util/coding.h"
 
 namespace ROCKSDB_NAMESPACE {
-// CacheReservationManager is an interface for reserving cache space for the
-// memory used
-class CacheReservationManager {
- public:
-  // CacheReservationHandle is for managing the lifetime of a cache reservation
-  // for an incremental amount of memory used (i.e, incremental_memory_used)
-  class CacheReservationHandle {
-   public:
-    virtual ~CacheReservationHandle() {}
-  };
-  virtual ~CacheReservationManager() {}
-  virtual Status UpdateCacheReservation(std::size_t new_memory_used) = 0;
-  virtual Status MakeCacheReservation(
-      std::size_t incremental_memory_used,
-      std::unique_ptr<CacheReservationManager::CacheReservationHandle>
-          *handle) = 0;
-  virtual std::size_t GetTotalReservedCacheSize() = 0;
-  virtual std::size_t GetTotalMemoryUsed() = 0;
-};
 
-// CacheReservationManagerImpl implements interface CacheReservationManager
-// for reserving cache space for the memory used by inserting/releasing dummy
-// entries in the cache.
+template <CacheEntryRole R>
+class CacheReservationHandle;
+
+// CacheReservationManager is for reserving cache space for the memory used
+// through inserting/releasing dummy entries in the cache.
 //
 // This class is NOT thread-safe, except that GetTotalReservedCacheSize()
 // can be called without external synchronization.
-template <CacheEntryRole R>
-class CacheReservationManagerImpl
-    : public CacheReservationManager,
-      public std::enable_shared_from_this<CacheReservationManagerImpl<R>> {
+class CacheReservationManager
+    : public std::enable_shared_from_this<CacheReservationManager> {
  public:
-  class CacheReservationHandle
-      : public CacheReservationManager::CacheReservationHandle {
-   public:
-    CacheReservationHandle(
-        std::size_t incremental_memory_used,
-        std::shared_ptr<CacheReservationManagerImpl> cache_res_mgr);
-    ~CacheReservationHandle() override;
-
-   private:
-    std::size_t incremental_memory_used_;
-    std::shared_ptr<CacheReservationManagerImpl> cache_res_mgr_;
-  };
-
-  // Construct a CacheReservationManagerImpl
+  // Construct a CacheReservationManager
   // @param cache The cache where dummy entries are inserted and released for
   // reserving cache space
   // @param delayed_decrease If set true, then dummy entries won't be released
-  //                         immediately when memory usage decreases.
+  // immediately when memory usage decreases.
   //                         Instead, it will be released when the memory usage
   //                         decreases to 3/4 of what we have reserved so far.
   //                         This is for saving some future dummy entry
   //                         insertion when memory usage increases are likely to
   //                         happen in the near future.
-  //
-  // REQUIRED: cache is not nullptr
-  explicit CacheReservationManagerImpl(std::shared_ptr<Cache> cache,
-                                       bool delayed_decrease = false);
+  explicit CacheReservationManager(std::shared_ptr<Cache> cache,
+                                   bool delayed_decrease = false);
 
   // no copy constructor, copy assignment, move constructor, move assignment
-  CacheReservationManagerImpl(const CacheReservationManagerImpl &) = delete;
-  CacheReservationManagerImpl &operator=(const CacheReservationManagerImpl &) =
-      delete;
-  CacheReservationManagerImpl(CacheReservationManagerImpl &&) = delete;
-  CacheReservationManagerImpl &operator=(CacheReservationManagerImpl &&) =
-      delete;
+  CacheReservationManager(const CacheReservationManager &) = delete;
+  CacheReservationManager &operator=(const CacheReservationManager &) = delete;
+  CacheReservationManager(CacheReservationManager &&) = delete;
+  CacheReservationManager &operator=(CacheReservationManager &&) = delete;
 
-  ~CacheReservationManagerImpl() override;
+  ~CacheReservationManager();
 
-  // One of the two ways of reserving/releasing cache space,
-  // see MakeCacheReservation() for the other.
-  //
-  // Use ONLY one of these two ways to prevent unexpected behavior.
+  template <CacheEntryRole R>
+
+  // One of the two ways of reserving/releasing cache,
+  // see CacheReservationManager::MakeCacheReservation() for the other.
+  // Use ONLY one of them to prevent unexpected behavior.
   //
   // Insert and release dummy entries in the cache to
   // match the size of total dummy entries with the least multiple of
@@ -126,13 +90,11 @@ class CacheReservationManagerImpl
   //         Otherwise, it returns the first non-ok status;
   //         On releasing dummy entries, it always returns Status::OK().
   //         On keeping dummy entries the same, it always returns Status::OK().
-  Status UpdateCacheReservation(std::size_t new_memory_used) override;
+  Status UpdateCacheReservation(std::size_t new_memory_used);
 
-  // One of the two ways of reserving cache space and releasing is done through
-  // destruction of CacheReservationHandle.
-  // See UpdateCacheReservation() for the other way.
-  //
-  // Use ONLY one of these two ways to prevent unexpected behavior.
+  // One of the two ways of reserving/releasing cache,
+  // see CacheReservationManager::UpdateCacheReservation() for the other.
+  // Use ONLY one of them to prevent unexpected behavior.
   //
   // Insert dummy entries in the cache for the incremental memory usage
   // to match the size of total dummy entries with the least multiple of
@@ -156,19 +118,21 @@ class CacheReservationManagerImpl
   //        calling MakeCacheReservation() is needed if you want
   //        GetTotalMemoryUsed() indeed returns the latest memory used.
   //
-  // @param handle An pointer to std::unique_ptr<CacheReservationHandle> that
-  //        manages the lifetime of the cache reservation represented by the
-  //        handle.
+  // @param handle An pointer to std::unique_ptr<CacheReservationHandle<R>> that
+  //        manages the lifetime of the handle and its cache reservation.
   //
   // @return It returns Status::OK() if all dummy
   //         entry insertions succeed.
   //         Otherwise, it returns the first non-ok status;
   //
   // REQUIRES: handle != nullptr
+  // REQUIRES: The CacheReservationManager object is NOT managed by
+  //           std::unique_ptr as CacheReservationHandle needs to
+  //           shares ownership to the CacheReservationManager object.
+  template <CacheEntryRole R>
   Status MakeCacheReservation(
       std::size_t incremental_memory_used,
-      std::unique_ptr<CacheReservationManager::CacheReservationHandle> *handle)
-      override;
+      std::unique_ptr<CacheReservationHandle<R>> *handle);
 
   // Return the size of the cache (which is a multiple of kSizeDummyEntry)
   // successfully reserved by calling UpdateCacheReservation().
@@ -178,25 +142,25 @@ class CacheReservationManagerImpl
   // smaller number than the actual reserved cache size due to
   // the returned number will always be a multiple of kSizeDummyEntry
   // and cache full might happen in the middle of inserting a dummy entry.
-  std::size_t GetTotalReservedCacheSize() override;
+  std::size_t GetTotalReservedCacheSize();
 
   // Return the latest total memory used indicated by the most recent call of
   // UpdateCacheReservation(std::size_t new_memory_used);
-  std::size_t GetTotalMemoryUsed() override;
+  std::size_t GetTotalMemoryUsed();
 
   static constexpr std::size_t GetDummyEntrySize() { return kSizeDummyEntry; }
 
   // For testing only - it is to help ensure the NoopDeleterForRole<R>
-  // accessed from CacheReservationManagerImpl and the one accessed from the
-  // test are from the same translation units
+  // accessed from CacheReservationManager and the one accessed from the test
+  // are from the same translation units
+  template <CacheEntryRole R>
   static Cache::DeleterFn TEST_GetNoopDeleterForRole();
 
  private:
   static constexpr std::size_t kSizeDummyEntry = 256 * 1024;
 
   Slice GetNextCacheKey();
-
-  Status ReleaseCacheReservation(std::size_t incremental_memory_used);
+  template <CacheEntryRole R>
   Status IncreaseCacheReservation(std::size_t new_mem_used);
   Status DecreaseCacheReservation(std::size_t new_mem_used);
 
@@ -208,81 +172,20 @@ class CacheReservationManagerImpl
   CacheKey cache_key_;
 };
 
-class ConcurrentCacheReservationManager
-    : public CacheReservationManager,
-      public std::enable_shared_from_this<ConcurrentCacheReservationManager> {
+// CacheReservationHandle is for managing the lifetime of a cache reservation
+// This class is NOT thread-safe
+template <CacheEntryRole R>
+class CacheReservationHandle {
  public:
-  class CacheReservationHandle
-      : public CacheReservationManager::CacheReservationHandle {
-   public:
-    CacheReservationHandle(
-        std::shared_ptr<ConcurrentCacheReservationManager> cache_res_mgr,
-        std::unique_ptr<CacheReservationManager::CacheReservationHandle>
-            cache_res_handle) {
-      assert(cache_res_mgr && cache_res_handle);
-      cache_res_mgr_ = cache_res_mgr;
-      cache_res_handle_ = std::move(cache_res_handle);
-    }
-
-    ~CacheReservationHandle() override {
-      std::lock_guard<std::mutex> lock(cache_res_mgr_->cache_res_mgr_mu_);
-      cache_res_handle_.reset();
-    }
-
-   private:
-    std::shared_ptr<ConcurrentCacheReservationManager> cache_res_mgr_;
-    std::unique_ptr<CacheReservationManager::CacheReservationHandle>
-        cache_res_handle_;
-  };
-
-  explicit ConcurrentCacheReservationManager(
-      std::shared_ptr<CacheReservationManager> cache_res_mgr) {
-    cache_res_mgr_ = std::move(cache_res_mgr);
-  }
-  ConcurrentCacheReservationManager(const ConcurrentCacheReservationManager &) =
-      delete;
-  ConcurrentCacheReservationManager &operator=(
-      const ConcurrentCacheReservationManager &) = delete;
-  ConcurrentCacheReservationManager(ConcurrentCacheReservationManager &&) =
-      delete;
-  ConcurrentCacheReservationManager &operator=(
-      ConcurrentCacheReservationManager &&) = delete;
-
-  ~ConcurrentCacheReservationManager() override {}
-
-  inline Status UpdateCacheReservation(std::size_t new_memory_used) override {
-    std::lock_guard<std::mutex> lock(cache_res_mgr_mu_);
-    return cache_res_mgr_->UpdateCacheReservation(new_memory_used);
-  }
-  inline Status MakeCacheReservation(
+  // REQUIRES: cache_res_mgr != nullptr
+  explicit CacheReservationHandle(
       std::size_t incremental_memory_used,
-      std::unique_ptr<CacheReservationManager::CacheReservationHandle> *handle)
-      override {
-    std::unique_ptr<CacheReservationManager::CacheReservationHandle>
-        wrapped_handle;
-    Status s;
-    {
-      std::lock_guard<std::mutex> lock(cache_res_mgr_mu_);
-      s = cache_res_mgr_->MakeCacheReservation(incremental_memory_used,
-                                               &wrapped_handle);
-    }
-    (*handle).reset(
-        new ConcurrentCacheReservationManager::CacheReservationHandle(
-            std::enable_shared_from_this<
-                ConcurrentCacheReservationManager>::shared_from_this(),
-            std::move(wrapped_handle)));
-    return s;
-  }
-  inline std::size_t GetTotalReservedCacheSize() override {
-    return cache_res_mgr_->GetTotalReservedCacheSize();
-  }
-  inline std::size_t GetTotalMemoryUsed() override {
-    std::lock_guard<std::mutex> lock(cache_res_mgr_mu_);
-    return cache_res_mgr_->GetTotalMemoryUsed();
-  }
+      std::shared_ptr<CacheReservationManager> cache_res_mgr);
+
+  ~CacheReservationHandle();
 
  private:
-  std::mutex cache_res_mgr_mu_;
+  std::size_t incremental_memory_used_;
   std::shared_ptr<CacheReservationManager> cache_res_mgr_;
 };
 }  // namespace ROCKSDB_NAMESPACE

cache/cache_reservation_manager_test.cc

@@ -15,6 +15,7 @@
 #include "cache/cache_entry_roles.h"
 #include "rocksdb/cache.h"
 #include "rocksdb/slice.h"
+#include "table/block_based/block_based_table_reader.h"
 #include "test_util/testharness.h"
 #include "util/coding.h"
 
@@ -22,24 +23,25 @@ namespace ROCKSDB_NAMESPACE {
 class CacheReservationManagerTest : public ::testing::Test {
  protected:
   static constexpr std::size_t kSizeDummyEntry =
-      CacheReservationManagerImpl<CacheEntryRole::kMisc>::GetDummyEntrySize();
+      CacheReservationManager::GetDummyEntrySize();
   static constexpr std::size_t kCacheCapacity = 4096 * kSizeDummyEntry;
   static constexpr int kNumShardBits = 0;  // 2^0 shard
   static constexpr std::size_t kMetaDataChargeOverhead = 10000;
 
   std::shared_ptr<Cache> cache = NewLRUCache(kCacheCapacity, kNumShardBits);
-  std::shared_ptr<CacheReservationManager> test_cache_rev_mng;
+  std::unique_ptr<CacheReservationManager> test_cache_rev_mng;
 
   CacheReservationManagerTest() {
-    test_cache_rev_mng =
-        std::make_shared<CacheReservationManagerImpl<CacheEntryRole::kMisc>>(
-            cache);
+    test_cache_rev_mng.reset(new CacheReservationManager(cache));
   }
 };
 
 TEST_F(CacheReservationManagerTest, GenerateCacheKey) {
   std::size_t new_mem_used = 1 * kSizeDummyEntry;
-  Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  Status s =
+      test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   ASSERT_EQ(s, Status::OK());
   ASSERT_GE(cache->GetPinnedUsage(), 1 * kSizeDummyEntry);
   ASSERT_LT(cache->GetPinnedUsage(),
@@ -64,7 +66,10 @@ TEST_F(CacheReservationManagerTest, GenerateCacheKey) {
 
 TEST_F(CacheReservationManagerTest, KeepCacheReservationTheSame) {
   std::size_t new_mem_used = 1 * kSizeDummyEntry;
-  Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  Status s =
+      test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   ASSERT_EQ(s, Status::OK());
   ASSERT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
             1 * kSizeDummyEntry);
@@ -74,7 +79,9 @@ TEST_F(CacheReservationManagerTest, KeepCacheReservationTheSame) {
   ASSERT_LT(initial_pinned_usage,
             1 * kSizeDummyEntry + kMetaDataChargeOverhead);
 
-  s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  s = test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::OK())
       << "Failed to keep cache reservation the same when new_mem_used equals "
          "to current cache reservation";
@@ -93,7 +100,10 @@ TEST_F(CacheReservationManagerTest, KeepCacheReservationTheSame) {
 TEST_F(CacheReservationManagerTest,
        IncreaseCacheReservationByMultiplesOfDummyEntrySize) {
   std::size_t new_mem_used = 2 * kSizeDummyEntry;
-  Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  Status s =
+      test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::OK())
       << "Failed to increase cache reservation correctly";
   EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
@@ -111,7 +121,10 @@ TEST_F(CacheReservationManagerTest,
 TEST_F(CacheReservationManagerTest,
        IncreaseCacheReservationNotByMultiplesOfDummyEntrySize) {
   std::size_t new_mem_used = 2 * kSizeDummyEntry + kSizeDummyEntry / 2;
-  Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  Status s =
+      test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::OK())
       << "Failed to increase cache reservation correctly";
   EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
@@ -130,7 +143,7 @@ TEST(CacheReservationManagerIncreaseReservcationOnFullCacheTest,
      IncreaseCacheReservationOnFullCache) {
   ;
   constexpr std::size_t kSizeDummyEntry =
-      CacheReservationManagerImpl<CacheEntryRole::kMisc>::GetDummyEntrySize();
+      CacheReservationManager::GetDummyEntrySize();
   constexpr std::size_t kSmallCacheCapacity = 4 * kSizeDummyEntry;
   constexpr std::size_t kBigCacheCapacity = 4096 * kSizeDummyEntry;
   constexpr std::size_t kMetaDataChargeOverhead = 10000;
@@ -140,12 +153,14 @@ TEST(CacheReservationManagerIncreaseReservcationOnFullCacheTest,
   lo.num_shard_bits = 0;  // 2^0 shard
   lo.strict_capacity_limit = true;
   std::shared_ptr<Cache> cache = NewLRUCache(lo);
-  std::shared_ptr<CacheReservationManager> test_cache_rev_mng =
-      std::make_shared<CacheReservationManagerImpl<CacheEntryRole::kMisc>>(
-          cache);
+  std::unique_ptr<CacheReservationManager> test_cache_rev_mng(
+      new CacheReservationManager(cache));
 
   std::size_t new_mem_used = kSmallCacheCapacity + 1;
-  Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  Status s =
+      test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::Incomplete())
       << "Failed to return status to indicate failure of dummy entry insertion "
          "during cache reservation on full cache";
@@ -168,7 +183,9 @@ TEST(CacheReservationManagerIncreaseReservcationOnFullCacheTest,
          "encountering cache resevation failure due to full cache";
 
   new_mem_used = kSmallCacheCapacity / 2;  // 2 dummy entries
-  s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  s = test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::OK())
       << "Failed to decrease cache reservation after encountering cache "
          "reservation failure due to full cache";
@@ -190,7 +207,9 @@ TEST(CacheReservationManagerIncreaseReservcationOnFullCacheTest,
 
   // Create cache full again for subsequent tests
   new_mem_used = kSmallCacheCapacity + 1;
-  s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  s = test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::Incomplete())
       << "Failed to return status to indicate failure of dummy entry insertion "
          "during cache reservation on full cache";
@@ -216,7 +235,9 @@ TEST(CacheReservationManagerIncreaseReservcationOnFullCacheTest,
   // succeed
   cache->SetCapacity(kBigCacheCapacity);
   new_mem_used = kSmallCacheCapacity + 1;
-  s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  s = test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::OK())
       << "Failed to increase cache reservation after increasing cache capacity "
          "and mitigating cache full error";
@@ -238,7 +259,10 @@ TEST(CacheReservationManagerIncreaseReservcationOnFullCacheTest,
 TEST_F(CacheReservationManagerTest,
        DecreaseCacheReservationByMultiplesOfDummyEntrySize) {
   std::size_t new_mem_used = 2 * kSizeDummyEntry;
-  Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  Status s =
+      test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   ASSERT_EQ(s, Status::OK());
   ASSERT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
             2 * kSizeDummyEntry);
@@ -248,7 +272,9 @@ TEST_F(CacheReservationManagerTest,
             2 * kSizeDummyEntry + kMetaDataChargeOverhead);
 
   new_mem_used = 1 * kSizeDummyEntry;
-  s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  s = test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::OK())
       << "Failed to decrease cache reservation correctly";
   EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
@@ -266,7 +292,10 @@ TEST_F(CacheReservationManagerTest,
 TEST_F(CacheReservationManagerTest,
        DecreaseCacheReservationNotByMultiplesOfDummyEntrySize) {
   std::size_t new_mem_used = 2 * kSizeDummyEntry;
-  Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  Status s =
+      test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   ASSERT_EQ(s, Status::OK());
   ASSERT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
             2 * kSizeDummyEntry);
@@ -276,7 +305,9 @@ TEST_F(CacheReservationManagerTest,
             2 * kSizeDummyEntry + kMetaDataChargeOverhead);
 
   new_mem_used = kSizeDummyEntry / 2;
-  s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  s = test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::OK())
       << "Failed to decrease cache reservation correctly";
   EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
@@ -294,7 +325,7 @@ TEST_F(CacheReservationManagerTest,
 TEST(CacheReservationManagerWithDelayedDecreaseTest,
      DecreaseCacheReservationWithDelayedDecrease) {
   constexpr std::size_t kSizeDummyEntry =
-      CacheReservationManagerImpl<CacheEntryRole::kMisc>::GetDummyEntrySize();
+      CacheReservationManager::GetDummyEntrySize();
   constexpr std::size_t kCacheCapacity = 4096 * kSizeDummyEntry;
   constexpr std::size_t kMetaDataChargeOverhead = 10000;
 
@@ -302,12 +333,14 @@ TEST(CacheReservationManagerWithDelayedDecreaseTest,
   lo.capacity = kCacheCapacity;
   lo.num_shard_bits = 0;
   std::shared_ptr<Cache> cache = NewLRUCache(lo);
-  std::shared_ptr<CacheReservationManager> test_cache_rev_mng =
-      std::make_shared<CacheReservationManagerImpl<CacheEntryRole::kMisc>>(
-          cache, true /* delayed_decrease */);
+  std::unique_ptr<CacheReservationManager> test_cache_rev_mng(
+      new CacheReservationManager(cache, true /* delayed_decrease */));
 
   std::size_t new_mem_used = 8 * kSizeDummyEntry;
-  Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  Status s =
+      test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   ASSERT_EQ(s, Status::OK());
   ASSERT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
             8 * kSizeDummyEntry);
@@ -318,7 +351,9 @@ TEST(CacheReservationManagerWithDelayedDecreaseTest,
             8 * kSizeDummyEntry + kMetaDataChargeOverhead);
 
   new_mem_used = 6 * kSizeDummyEntry;
-  s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  s = test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::OK()) << "Failed to delay decreasing cache reservation";
   EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
             8 * kSizeDummyEntry)
@@ -330,7 +365,9 @@ TEST(CacheReservationManagerWithDelayedDecreaseTest,
       << "Failed to delay decreasing underlying dummy entries in cache";
 
   new_mem_used = 7 * kSizeDummyEntry;
-  s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  s = test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::OK()) << "Failed to delay decreasing cache reservation";
   EXPECT_EQ(test_cache_rev_mng->GetTotalReservedCacheSize(),
             8 * kSizeDummyEntry)
@@ -342,7 +379,9 @@ TEST(CacheReservationManagerWithDelayedDecreaseTest,
       << "Failed to delay decreasing underlying dummy entries in cache";
 
   new_mem_used = 6 * kSizeDummyEntry - 1;
-  s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+  s = test_cache_rev_mng
+          ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+              new_mem_used);
   EXPECT_EQ(s, Status::OK())
       << "Failed to decrease cache reservation correctly when new_mem_used < "
          "GetTotalReservedCacheSize() * 3 / 4 on delayed decrease mode";
@@ -366,7 +405,7 @@ TEST(CacheReservationManagerWithDelayedDecreaseTest,
 TEST(CacheReservationManagerDestructorTest,
      ReleaseRemainingDummyEntriesOnDestruction) {
   constexpr std::size_t kSizeDummyEntry =
-      CacheReservationManagerImpl<CacheEntryRole::kMisc>::GetDummyEntrySize();
+      CacheReservationManager::GetDummyEntrySize();
   constexpr std::size_t kCacheCapacity = 4096 * kSizeDummyEntry;
   constexpr std::size_t kMetaDataChargeOverhead = 10000;
 
@@ -375,11 +414,13 @@ TEST(CacheReservationManagerDestructorTest,
   lo.num_shard_bits = 0;
   std::shared_ptr<Cache> cache = NewLRUCache(lo);
   {
-    std::shared_ptr<CacheReservationManager> test_cache_rev_mng =
-        std::make_shared<CacheReservationManagerImpl<CacheEntryRole::kMisc>>(
-            cache);
+    std::unique_ptr<CacheReservationManager> test_cache_rev_mng(
+        new CacheReservationManager(cache));
     std::size_t new_mem_used = 1 * kSizeDummyEntry;
-    Status s = test_cache_rev_mng->UpdateCacheReservation(new_mem_used);
+    Status s =
+        test_cache_rev_mng
+            ->UpdateCacheReservation<ROCKSDB_NAMESPACE::CacheEntryRole::kMisc>(
+                new_mem_used);
     ASSERT_EQ(s, Status::OK());
     ASSERT_GE(cache->GetPinnedUsage(), 1 * kSizeDummyEntry);
     ASSERT_LT(cache->GetPinnedUsage(),
@@ -401,19 +442,18 @@ TEST(CacheReservationHandleTest, HandleTest) {
   std::shared_ptr<Cache> cache = NewLRUCache(lo);
 
   std::shared_ptr<CacheReservationManager> test_cache_rev_mng(
-      std::make_shared<CacheReservationManagerImpl<CacheEntryRole::kMisc>>(
-          cache));
+      std::make_shared<CacheReservationManager>(cache));
 
   std::size_t mem_used = 0;
   const std::size_t incremental_mem_used_handle_1 = 1 * kSizeDummyEntry;
   const std::size_t incremental_mem_used_handle_2 = 2 * kSizeDummyEntry;
-  std::unique_ptr<CacheReservationManager::CacheReservationHandle> handle_1,
+  std::unique_ptr<CacheReservationHandle<CacheEntryRole::kMisc>> handle_1,
       handle_2;
 
   // To test consecutive CacheReservationManager::MakeCacheReservation works
   // correctly in terms of returning the handle as well as updating cache
   // reservation and the latest total memory used
-  Status s = test_cache_rev_mng->MakeCacheReservation(
+  Status s = test_cache_rev_mng->MakeCacheReservation<CacheEntryRole::kMisc>(
       incremental_mem_used_handle_1, &handle_1);
   mem_used = mem_used + incremental_mem_used_handle_1;
   ASSERT_EQ(s, Status::OK());
@@ -423,8 +463,8 @@ TEST(CacheReservationHandleTest, HandleTest) {
   EXPECT_GE(cache->GetPinnedUsage(), mem_used);
   EXPECT_LT(cache->GetPinnedUsage(), mem_used + kMetaDataChargeOverhead);
 
-  s = test_cache_rev_mng->MakeCacheReservation(incremental_mem_used_handle_2,
-                                               &handle_2);
+  s = test_cache_rev_mng->MakeCacheReservation<CacheEntryRole::kMisc>(
+      incremental_mem_used_handle_2, &handle_2);
   mem_used = mem_used + incremental_mem_used_handle_2;
   ASSERT_EQ(s, Status::OK());
   EXPECT_TRUE(handle_2 != nullptr);
@@ -433,9 +473,8 @@ TEST(CacheReservationHandleTest, HandleTest) {
   EXPECT_GE(cache->GetPinnedUsage(), mem_used);
   EXPECT_LT(cache->GetPinnedUsage(), mem_used + kMetaDataChargeOverhead);
 
-  // To test
-  // CacheReservationManager::CacheReservationHandle::~CacheReservationHandle()
-  // works correctly in releasing the cache reserved for the handle
+  // To test CacheReservationHandle::~CacheReservationHandle() works correctly
+  // in releasing the cache reserved for the handle
   handle_1.reset();
   EXPECT_TRUE(handle_1 == nullptr);
   mem_used = mem_used - incremental_mem_used_handle_1;

cache/clock_cache.cc

@@ -286,8 +286,8 @@ class ClockCacheShard final : public CacheShard {
     return Lookup(key, hash);
   }
   bool Release(Cache::Handle* handle, bool /*useful*/,
-               bool erase_if_last_ref) override {
-    return Release(handle, erase_if_last_ref);
+               bool force_erase) override {
+    return Release(handle, force_erase);
   }
   bool IsReady(Cache::Handle* /*handle*/) override { return true; }
   void Wait(Cache::Handle* /*handle*/) override {}
@@ -297,7 +297,7 @@ class ClockCacheShard final : public CacheShard {
   //
   // Not necessary to hold mutex_ before being called.
   bool Ref(Cache::Handle* handle) override;
-  bool Release(Cache::Handle* handle, bool erase_if_last_ref = false) override;
+  bool Release(Cache::Handle* handle, bool force_erase = false) override;
   void Erase(const Slice& key, uint32_t hash) override;
   bool EraseAndConfirm(const Slice& key, uint32_t hash,
                        CleanupContext* context);
@@ -725,11 +725,11 @@ Cache::Handle* ClockCacheShard::Lookup(const Slice& key, uint32_t hash) {
   return reinterpret_cast<Cache::Handle*>(handle);
 }
 
-bool ClockCacheShard::Release(Cache::Handle* h, bool erase_if_last_ref) {
+bool ClockCacheShard::Release(Cache::Handle* h, bool force_erase) {
   CleanupContext context;
   CacheHandle* handle = reinterpret_cast<CacheHandle*>(h);
   bool erased = Unref(handle, true, &context);
-  if (erase_if_last_ref && !erased) {
+  if (force_erase && !erased) {
     erased = EraseAndConfirm(handle->key, handle->hash, &context);
   }
   Cleanup(context);

cache/lru_cache.cc

@@ -76,12 +76,13 @@ LRUHandle** LRUHandleTable::FindPointer(const Slice& key, uint32_t hash) {
 
 void LRUHandleTable::Resize() {
   if (length_bits_ >= max_length_bits_) {
-    // Due to reaching limit of hash information, if we made the table bigger,
-    // we would allocate more addresses but only the same number would be used.
+    // Due to reaching limit of hash information, if we made the table
+    // bigger, we would allocate more addresses but only the same
+    // number would be used.
     return;
   }
   if (length_bits_ >= 31) {
-    // Avoid undefined behavior shifting uint32_t by 32.
+    // Avoid undefined behavior shifting uint32_t by 32
     return;
   }
 
@@ -124,7 +125,7 @@ LRUCacheShard::LRUCacheShard(
       mutex_(use_adaptive_mutex),
       secondary_cache_(secondary_cache) {
   set_metadata_charge_policy(metadata_charge_policy);
-  // Make empty circular linked list.
+  // Make empty circular linked list
   lru_.next = &lru_;
   lru_.prev = &lru_;
   lru_low_pri_ = &lru_;
@@ -137,7 +138,7 @@ void LRUCacheShard::EraseUnRefEntries() {
     MutexLock l(&mutex_);
     while (lru_.next != &lru_) {
       LRUHandle* old = lru_.next;
-      // LRU list contains only elements which can be evicted.
+      // LRU list contains only elements which can be evicted
       assert(old->InCache() && !old->HasRefs());
       LRU_Remove(old);
       table_.Remove(old->key(), old->hash);
@@ -167,7 +168,7 @@ void LRUCacheShard::ApplyToSomeEntries(
 
   assert(average_entries_per_lock > 0);
   // Assuming we are called with same average_entries_per_lock repeatedly,
-  // this simplifies some logic (index_end will not overflow).
+  // this simplifies some logic (index_end will not overflow)
   assert(average_entries_per_lock < length || *state == 0);
 
   uint32_t index_begin = *state >> (32 - length_bits);
@@ -273,7 +274,7 @@ void LRUCacheShard::EvictFromLRU(size_t charge,
                                  autovector<LRUHandle*>* deleted) {
   while ((usage_ + charge) > capacity_ && lru_.next != &lru_) {
     LRUHandle* old = lru_.next;
-    // LRU list contains only elements which can be evicted.
+    // LRU list contains only elements which can be evicted
     assert(old->InCache() && !old->HasRefs());
     LRU_Remove(old);
     table_.Remove(old->key(), old->hash);
@@ -294,8 +295,8 @@ void LRUCacheShard::SetCapacity(size_t capacity) {
     EvictFromLRU(0, &last_reference_list);
   }
 
-  // Try to insert the evicted entries into tiered cache.
-  // Free the entries outside of mutex for performance reasons.
+  // Try to insert the evicted entries into tiered cache
+  // Free the entries outside of mutex for performance reasons
   for (auto entry : last_reference_list) {
     if (secondary_cache_ && entry->IsSecondaryCacheCompatible() &&
         !entry->IsPromoted()) {
@@ -321,7 +322,7 @@ Status LRUCacheShard::InsertItem(LRUHandle* e, Cache::Handle** handle,
     MutexLock l(&mutex_);
 
     // Free the space following strict LRU policy until enough space
-    // is freed or the lru list is empty.
+    // is freed or the lru list is empty
     EvictFromLRU(total_charge, &last_reference_list);
 
     if ((usage_ + total_charge) > capacity_ &&
@@ -348,7 +349,7 @@ Status LRUCacheShard::InsertItem(LRUHandle* e, Cache::Handle** handle,
         assert(old->InCache());
         old->SetInCache(false);
         if (!old->HasRefs()) {
-          // old is on LRU because it's in cache and its reference count is 0.
+          // old is on LRU because it's in cache and its reference count is 0
           LRU_Remove(old);
           size_t old_total_charge =
               old->CalcTotalCharge(metadata_charge_policy_);
@@ -360,7 +361,7 @@ Status LRUCacheShard::InsertItem(LRUHandle* e, Cache::Handle** handle,
       if (handle == nullptr) {
         LRU_Insert(e);
       } else {
-        // If caller already holds a ref, no need to take one here.
+        // If caller already holds a ref, no need to take one here
         if (!e->HasRefs()) {
           e->Ref();
         }
@@ -369,8 +370,8 @@ Status LRUCacheShard::InsertItem(LRUHandle* e, Cache::Handle** handle,
     }
   }
 
-  // Try to insert the evicted entries into the secondary cache.
-  // Free the entries here outside of mutex for performance reasons.
+  // Try to insert the evicted entries into the secondary cache
+  // Free the entries here outside of mutex for performance reasons
   for (auto entry : last_reference_list) {
     if (secondary_cache_ && entry->IsSecondaryCacheCompatible() &&
         !entry->IsPromoted()) {
@@ -403,7 +404,7 @@ void LRUCacheShard::Promote(LRUHandle* e) {
     Status s = InsertItem(e, &handle, /*free_handle_on_fail=*/false);
     if (!s.ok()) {
       // Item is in memory, but not accounted against the cache capacity.
-      // When the handle is released, the item should get deleted.
+      // When the handle is released, the item should get deleted
       assert(!e->InCache());
     }
   } else {
@@ -436,8 +437,8 @@ Cache::Handle* LRUCacheShard::Lookup(
   }
 
   // If handle table lookup failed, then allocate a handle outside the
-  // mutex if we're going to lookup in the secondary cache.
-  // Only support synchronous for now.
+  // mutex if we're going to lookup in the secondary cache
+  // Only support synchronous for now
   // TODO: Support asynchronous lookup in secondary cache
   if (!e && secondary_cache_ && helper && helper->saveto_cb) {
     // For objects from the secondary cache, we expect the caller to provide
@@ -468,7 +469,7 @@ Cache::Handle* LRUCacheShard::Lookup(
       if (wait) {
         Promote(e);
         if (!e->value) {
-          // The secondary cache returned a handle, but the lookup failed.
+          // The secondary cache returned a handle, but the lookup failed
           e->Unref();
           e->Free();
           e = nullptr;
@@ -478,7 +479,7 @@ Cache::Handle* LRUCacheShard::Lookup(
         }
       } else {
         // If wait is false, we always return a handle and let the caller
-        // release the handle after checking for success or failure.
+        // release the handle after checking for success or failure
         e->SetIncomplete(true);
         // This may be slightly inaccurate, if the lookup eventually fails.
         // But the probability is very low.
@@ -493,7 +494,7 @@ Cache::Handle* LRUCacheShard::Lookup(
 bool LRUCacheShard::Ref(Cache::Handle* h) {
   LRUHandle* e = reinterpret_cast<LRUHandle*>(h);
   MutexLock l(&mutex_);
-  // To create another reference - entry must be already externally referenced.
+  // To create another reference - entry must be already externally referenced
   assert(e->HasRefs());
   e->Ref();
   return true;
@@ -506,7 +507,7 @@ void LRUCacheShard::SetHighPriorityPoolRatio(double high_pri_pool_ratio) {
   MaintainPoolSize();
 }
 
-bool LRUCacheShard::Release(Cache::Handle* handle, bool erase_if_last_ref) {
+bool LRUCacheShard::Release(Cache::Handle* handle, bool force_erase) {
   if (handle == nullptr) {
     return false;
   }
@@ -516,15 +517,15 @@ bool LRUCacheShard::Release(Cache::Handle* handle, bool erase_if_last_ref) {
     MutexLock l(&mutex_);
     last_reference = e->Unref();
     if (last_reference && e->InCache()) {
-      // The item is still in cache, and nobody else holds a reference to it.
-      if (usage_ > capacity_ || erase_if_last_ref) {
-        // The LRU list must be empty since the cache is full.
-        assert(lru_.next == &lru_ || erase_if_last_ref);
-        // Take this opportunity and remove the item.
+      // The item is still in cache, and nobody else holds a reference to it
+      if (usage_ > capacity_ || force_erase) {
+        // The LRU list must be empty since the cache is full
+        assert(lru_.next == &lru_ || force_erase);
+        // Take this opportunity and remove the item
         table_.Remove(e->key(), e->hash);
         e->SetInCache(false);
       } else {
-        // Put the item back on the LRU list, and don't free it.
+        // Put the item back on the LRU list, and don't free it
         LRU_Insert(e);
         last_reference = false;
       }
@@ -541,7 +542,7 @@ bool LRUCacheShard::Release(Cache::Handle* handle, bool erase_if_last_ref) {
     }
   }
 
-  // Free the entry here outside of mutex for performance reasons.
+  // Free the entry here outside of mutex for performance reasons
   if (last_reference) {
     e->Free();
   }
@@ -553,8 +554,8 @@ Status LRUCacheShard::Insert(const Slice& key, uint32_t hash, void* value,
                              void (*deleter)(const Slice& key, void* value),
                              const Cache::CacheItemHelper* helper,
                              Cache::Handle** handle, Cache::Priority priority) {
-  // Allocate the memory here outside of the mutex.
-  // If the cache is full, we'll have to release it.
+  // Allocate the memory here outside of the mutex
+  // If the cache is full, we'll have to release it
   // It shouldn't happen very often though.
   LRUHandle* e = reinterpret_cast<LRUHandle*>(
       new char[sizeof(LRUHandle) - 1 + key.size()]);
@@ -602,8 +603,8 @@ void LRUCacheShard::Erase(const Slice& key, uint32_t hash) {
     }
   }
 
-  // Free the entry here outside of mutex for performance reasons.
-  // last_reference will only be true if e != nullptr.
+  // Free the entry here outside of mutex for performance reasons
+  // last_reference will only be true if e != nullptr
   if (last_reference) {
     e->Free();
   }
@@ -704,7 +705,7 @@ uint32_t LRUCache::GetHash(Handle* handle) const {
 }
 
 void LRUCache::DisownData() {
-  // Leak data only if that won't generate an ASAN/valgrind warning.
+  // Leak data only if that won't generate an ASAN/valgrind warning
   if (!kMustFreeHeapAllocations) {
     shards_ = nullptr;
     num_shards_ = 0;
@@ -764,10 +765,10 @@ std::shared_ptr<Cache> NewLRUCache(
     CacheMetadataChargePolicy metadata_charge_policy,
     const std::shared_ptr<SecondaryCache>& secondary_cache) {
   if (num_shard_bits >= 20) {
-    return nullptr;  // The cache cannot be sharded into too many fine pieces.
+    return nullptr;  // the cache cannot be sharded into too many fine pieces
   }
   if (high_pri_pool_ratio < 0.0 || high_pri_pool_ratio > 1.0) {
-    // Invalid high_pri_pool_ratio
+    // invalid high_pri_pool_ratio
     return nullptr;
   }
   if (num_shard_bits < 0) {

cache/lru_cache.h

@@ -81,11 +81,11 @@ struct LRUHandle {
     IN_HIGH_PRI_POOL = (1 << 2),
     // Whether this entry has had any lookups (hits).
     HAS_HIT = (1 << 3),
-    // Can this be inserted into the secondary cache.
+    // Can this be inserted into the secondary cache
     IS_SECONDARY_CACHE_COMPATIBLE = (1 << 4),
-    // Is the handle still being read from a lower tier.
+    // Is the handle still being read from a lower tier
     IS_PENDING = (1 << 5),
-    // Has the item been promoted from a lower tier.
+    // Has the item been promoted from a lower tier
     IS_PROMOTED = (1 << 6),
   };
 
@@ -208,7 +208,7 @@ struct LRUHandle {
     delete[] reinterpret_cast<char*>(this);
   }
 
-  // Calculate the memory usage by metadata.
+  // Calculate the memory usage by metadata
   inline size_t CalcTotalCharge(
       CacheMetadataChargePolicy metadata_charge_policy) {
     size_t meta_charge = 0;
@@ -216,7 +216,7 @@ struct LRUHandle {
 #ifdef ROCKSDB_MALLOC_USABLE_SIZE
       meta_charge += malloc_usable_size(static_cast<void*>(this));
 #else
-      // This is the size that is used when a new handle is created.
+      // This is the size that is used when a new handle is created
       meta_charge += sizeof(LRUHandle) - 1 + key_length;
 #endif
     }
@@ -272,10 +272,10 @@ class LRUHandleTable {
   // a linked list of cache entries that hash into the bucket.
   std::unique_ptr<LRUHandle*[]> list_;
 
-  // Number of elements currently in the table.
+  // Number of elements currently in the table
   uint32_t elems_;
 
-  // Set from max_upper_hash_bits (see constructor).
+  // Set from max_upper_hash_bits (see constructor)
   const int max_length_bits_;
 };
 
@@ -291,7 +291,7 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard {
 
   // Separate from constructor so caller can easily make an array of LRUCache
   // if current usage is more than new capacity, the function will attempt to
-  // free the needed space.
+  // free the needed space
   virtual void SetCapacity(size_t capacity) override;
 
   // Set the flag to reject insertion if cache if full.
@@ -314,7 +314,8 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard {
     assert(helper);
     return Insert(key, hash, value, charge, nullptr, helper, handle, priority);
   }
-  // If helper_cb is null, the values of the following arguments don't matter.
+  // If helper_cb is null, the values of the following arguments don't
+  // matter
   virtual Cache::Handle* Lookup(const Slice& key, uint32_t hash,
                                 const ShardedCache::CacheItemHelper* helper,
                                 const ShardedCache::CreateCallback& create_cb,
@@ -325,14 +326,14 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard {
                   nullptr);
   }
   virtual bool Release(Cache::Handle* handle, bool /*useful*/,
-                       bool erase_if_last_ref) override {
-    return Release(handle, erase_if_last_ref);
+                       bool force_erase) override {
+    return Release(handle, force_erase);
   }
   virtual bool IsReady(Cache::Handle* /*handle*/) override;
   virtual void Wait(Cache::Handle* /*handle*/) override {}
   virtual bool Ref(Cache::Handle* handle) override;
   virtual bool Release(Cache::Handle* handle,
-                       bool erase_if_last_ref = false) override;
+                       bool force_erase = false) override;
   virtual void Erase(const Slice& key, uint32_t hash) override;
 
   // Although in some platforms the update of size_t is atomic, to make sure
@@ -353,8 +354,8 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard {
 
   void TEST_GetLRUList(LRUHandle** lru, LRUHandle** lru_low_pri);
 
-  //  Retrieves number of elements in LRU, for unit test purpose only.
-  //  Not threadsafe.
+  //  Retrieves number of elements in LRU, for unit test purpose only
+  //  not threadsafe
   size_t TEST_GetLRUSize();
 
   //  Retrieves high pri pool ratio
@@ -364,8 +365,7 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard {
   friend class LRUCache;
   // Insert an item into the hash table and, if handle is null, insert into
   // the LRU list. Older items are evicted as necessary. If the cache is full
-  // and free_handle_on_fail is true, the item is deleted and handle is set to
-  // nullptr.
+  // and free_handle_on_fail is true, the item is deleted and handle is set to.
   Status InsertItem(LRUHandle* item, Cache::Handle** handle,
                     bool free_handle_on_fail);
   Status Insert(const Slice& key, uint32_t hash, void* value, size_t charge,
@@ -389,7 +389,7 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard {
   // Free some space following strict LRU policy until enough space
   // to hold (usage_ + charge) is freed or the lru list is empty
   // This function is not thread safe - it needs to be executed while
-  // holding the mutex_.
+  // holding the mutex_
   void EvictFromLRU(size_t charge, autovector<LRUHandle*>* deleted);
 
   // Initialized before use.
@@ -429,10 +429,10 @@ class ALIGN_AS(CACHE_LINE_SIZE) LRUCacheShard final : public CacheShard {
   // ------------vvvvvvvvvvvvv-----------
   LRUHandleTable table_;
 
-  // Memory size for entries residing in the cache.
+  // Memory size for entries residing in the cache
   size_t usage_;
 
-  // Memory size for entries residing only in the LRU list.
+  // Memory size for entries residing only in the LRU list
   size_t lru_usage_;
 
   // mutex_ protects the following state.
@@ -467,9 +467,9 @@ class LRUCache
   virtual void DisownData() override;
   virtual void WaitAll(std::vector<Handle*>& handles) override;
 
-  //  Retrieves number of elements in LRU, for unit test purpose only.
+  //  Retrieves number of elements in LRU, for unit test purpose only
   size_t TEST_GetLRUSize();
-  //  Retrieves high pri pool ratio.
+  //  Retrieves high pri pool ratio
   double GetHighPriPoolRatio();
 
  private:

cache/sharded_cache.cc

@@ -104,15 +104,14 @@ bool ShardedCache::Ref(Handle* handle) {
   return GetShard(Shard(hash))->Ref(handle);
 }
 
-bool ShardedCache::Release(Handle* handle, bool erase_if_last_ref) {
+bool ShardedCache::Release(Handle* handle, bool force_erase) {
   uint32_t hash = GetHash(handle);
-  return GetShard(Shard(hash))->Release(handle, erase_if_last_ref);
+  return GetShard(Shard(hash))->Release(handle, force_erase);
 }
 
-bool ShardedCache::Release(Handle* handle, bool useful,
-                           bool erase_if_last_ref) {
+bool ShardedCache::Release(Handle* handle, bool useful, bool force_erase) {
   uint32_t hash = GetHash(handle);
-  return GetShard(Shard(hash))->Release(handle, useful, erase_if_last_ref);
+  return GetShard(Shard(hash))->Release(handle, useful, force_erase);
 }
 
 void ShardedCache::Erase(const Slice& key) {

cache/sharded_cache.h

@@ -37,11 +37,11 @@ class CacheShard {
                                 Cache::Priority priority, bool wait,
                                 Statistics* stats) = 0;
   virtual bool Release(Cache::Handle* handle, bool useful,
-                       bool erase_if_last_ref) = 0;
+                       bool force_erase) = 0;
   virtual bool IsReady(Cache::Handle* handle) = 0;
   virtual void Wait(Cache::Handle* handle) = 0;
   virtual bool Ref(Cache::Handle* handle) = 0;
-  virtual bool Release(Cache::Handle* handle, bool erase_if_last_ref) = 0;
+  virtual bool Release(Cache::Handle* handle, bool force_erase) = 0;
   virtual void Erase(const Slice& key, uint32_t hash) = 0;
   virtual void SetCapacity(size_t capacity) = 0;
   virtual void SetStrictCapacityLimit(bool strict_capacity_limit) = 0;
@@ -94,11 +94,11 @@ class ShardedCache : public Cache {
                          const CreateCallback& create_cb, Priority priority,
                          bool wait, Statistics* stats = nullptr) override;
   virtual bool Release(Handle* handle, bool useful,
-                       bool erase_if_last_ref = false) override;
+                       bool force_erase = false) override;
   virtual bool IsReady(Handle* handle) override;
   virtual void Wait(Handle* handle) override;
   virtual bool Ref(Handle* handle) override;
-  virtual bool Release(Handle* handle, bool erase_if_last_ref = false) override;
+  virtual bool Release(Handle* handle, bool force_erase = false) override;
   virtual void Erase(const Slice& key) override;
   virtual uint64_t NewId() override;
   virtual size_t GetCapacity() const override;

coverage/coverage_test.sh

@@ -12,7 +12,7 @@ fi
 ROOT=".."
 # Fetch right version of gcov
 if [ -d /mnt/gvfs/third-party -a -z "$CXX" ]; then
-  source $ROOT/build_tools/fbcode_config_platform009.sh
+  source $ROOT/build_tools/fbcode_config_platform007.sh
   GCOV=$GCC_BASE/bin/gcov
 else
   GCOV=$(which gcov)

crash_test.mk

@@ -42,12 +42,6 @@ crash_test_with_ts: $(DB_STRESS_CMD)
 	$(CRASHTEST_MAKE) whitebox_crash_test_with_ts
 	$(CRASHTEST_MAKE) blackbox_crash_test_with_ts
 
-crash_test_with_multiops_wc_txn: $(DB_STRESS_CMD)
-	$(CRASHTEST_MAKE) blackbox_crash_test_with_multiops_wc_txn
-
-crash_test_with_multiops_wp_txn: $(DB_STRESS_CMD)
-	$(CRASHTEST_MAKE) blackbox_crash_test_with_multiops_wp_txn
-
 blackbox_crash_test: $(DB_STRESS_CMD)
 	$(CRASHTEST_PY) --simple blackbox $(CRASH_TEST_EXT_ARGS)
 	$(CRASHTEST_PY) blackbox $(CRASH_TEST_EXT_ARGS)

db/c_test.c

@@ -2448,7 +2448,7 @@ int main(int argc, char** argv) {
     rocksdb_fifo_compaction_options_destroy(fco);
   }
 
-  StartPhase("backup_engine_option");
+  StartPhase("backupable_db_option");
   {
     rocksdb_backup_engine_options_t* bdo;
     bdo = rocksdb_backup_engine_options_create("path");

db/column_family.cc

@@ -1562,6 +1562,20 @@ ColumnFamilyData* ColumnFamilySet::CreateColumnFamily(
   return new_cfd;
 }
 
+// REQUIRES: DB mutex held
+void ColumnFamilySet::FreeDeadColumnFamilies() {
+  autovector<ColumnFamilyData*> to_delete;
+  for (auto cfd = dummy_cfd_->next_; cfd != dummy_cfd_; cfd = cfd->next_) {
+    if (cfd->refs_.load(std::memory_order_relaxed) == 0) {
+      to_delete.push_back(cfd);
+    }
+  }
+  for (auto cfd : to_delete) {
+    // this is very rare, so it's not a problem that we do it under a mutex
+    delete cfd;
+  }
+}
+
 // under a DB mutex AND from a write thread
 void ColumnFamilySet::RemoveColumnFamily(ColumnFamilyData* cfd) {
   auto cfd_iter = column_family_data_.find(cfd->GetID());

db/column_family.h

@@ -520,10 +520,9 @@ class ColumnFamilyData {
   ThreadLocalPtr* TEST_GetLocalSV() { return local_sv_.get(); }
   WriteBufferManager* write_buffer_mgr() { return write_buffer_manager_; }
 
-  static const uint32_t kDummyColumnFamilyDataId;
-
  private:
   friend class ColumnFamilySet;
+  static const uint32_t kDummyColumnFamilyDataId;
   ColumnFamilyData(uint32_t id, const std::string& name,
                    Version* dummy_versions, Cache* table_cache,
                    WriteBufferManager* write_buffer_manager,
@@ -629,8 +628,10 @@ class ColumnFamilyData {
 // held and it needs to be executed from the write thread. SetDropped() also
 // guarantees that it will be called only from single-threaded LogAndApply(),
 // but this condition is not that important.
-// * Iteration -- hold DB mutex. If you want to release the DB mutex in the
-// body of the iteration, wrap in a RefedColumnFamilySet.
+// * Iteration -- hold DB mutex, but you can release it in the body of
+// iteration. If you release DB mutex in body, reference the column
+// family before the mutex and unreference after you unlock, since the column
+// family might get dropped when the DB mutex is released
 // * GetDefault() -- thread safe
 // * GetColumnFamily() -- either inside of DB mutex or from a write thread
 // * GetNextColumnFamilyID(), GetMaxColumnFamily(), UpdateMaxColumnFamily(),
@@ -642,12 +643,17 @@ class ColumnFamilySet {
    public:
     explicit iterator(ColumnFamilyData* cfd)
         : current_(cfd) {}
-    // NOTE: minimum operators for for-loop iteration
     iterator& operator++() {
-      current_ = current_->next_;
+      // dropped column families might still be included in this iteration
+      // (we're only removing them when client drops the last reference to the
+      // column family).
+      // dummy is never dead, so this will never be infinite
+      do {
+        current_ = current_->next_;
+      } while (current_->refs_.load(std::memory_order_relaxed) == 0);
       return *this;
     }
-    bool operator!=(const iterator& other) const {
+    bool operator!=(const iterator& other) {
       return this->current_ != other.current_;
     }
     ColumnFamilyData* operator*() { return current_; }
@@ -686,6 +692,10 @@ class ColumnFamilySet {
   iterator begin() { return iterator(dummy_cfd_->next_); }
   iterator end() { return iterator(dummy_cfd_); }
 
+  // REQUIRES: DB mutex held
+  // Don't call while iterating over ColumnFamilySet
+  void FreeDeadColumnFamilies();
+
   Cache* get_table_cache() { return table_cache_; }
 
   WriteBufferManager* write_buffer_manager() { return write_buffer_manager_; }
@@ -728,55 +738,6 @@ class ColumnFamilySet {
   std::string db_session_id_;
 };
 
-// A wrapper for ColumnFamilySet that supports releasing DB mutex during each
-// iteration over the iterator, because the cfd is Refed and Unrefed during
-// each iteration to prevent concurrent CF drop from destroying it (until
-// Unref).
-class RefedColumnFamilySet {
- public:
-  explicit RefedColumnFamilySet(ColumnFamilySet* cfs) : wrapped_(cfs) {}
-
-  class iterator {
-   public:
-    explicit iterator(ColumnFamilySet::iterator wrapped) : wrapped_(wrapped) {
-      MaybeRef(*wrapped_);
-    }
-    ~iterator() { MaybeUnref(*wrapped_); }
-    inline void MaybeRef(ColumnFamilyData* cfd) {
-      if (cfd->GetID() != ColumnFamilyData::kDummyColumnFamilyDataId) {
-        cfd->Ref();
-      }
-    }
-    inline void MaybeUnref(ColumnFamilyData* cfd) {
-      if (cfd->GetID() != ColumnFamilyData::kDummyColumnFamilyDataId) {
-        cfd->UnrefAndTryDelete();
-      }
-    }
-    // NOTE: minimum operators for for-loop iteration
-    inline iterator& operator++() {
-      ColumnFamilyData* old = *wrapped_;
-      ++wrapped_;
-      // Can only unref & potentially free cfd after accessing its next_
-      MaybeUnref(old);
-      MaybeRef(*wrapped_);
-      return *this;
-    }
-    inline bool operator!=(const iterator& other) const {
-      return this->wrapped_ != other.wrapped_;
-    }
-    inline ColumnFamilyData* operator*() { return *wrapped_; }
-
-   private:
-    ColumnFamilySet::iterator wrapped_;
-  };
-
-  iterator begin() { return iterator(wrapped_->begin()); }
-  iterator end() { return iterator(wrapped_->end()); }
-
- private:
-  ColumnFamilySet* wrapped_;
-};
-
 // We use ColumnFamilyMemTablesImpl to provide WriteBatch a way to access
 // memtables of different column families (specified by ID in the write batch)
 class ColumnFamilyMemTablesImpl : public ColumnFamilyMemTables {

db/compaction/compaction_job.cc

@@ -1341,8 +1341,6 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
   CompactionRangeDelAggregator range_del_agg(&cfd->internal_comparator(),
                                              existing_snapshots_);
 
-  // TODO: since we already use C++17, should use
-  // std::optional<const Slice> instead.
   const Slice* const start = sub_compact->start;
   const Slice* const end = sub_compact->end;
 
@@ -1364,13 +1362,9 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
 
   // Although the v2 aggregator is what the level iterator(s) know about,
   // the AddTombstones calls will be propagated down to the v1 aggregator.
-  std::unique_ptr<InternalIterator> raw_input(versions_->MakeInputIterator(
-      read_options, sub_compact->compaction, &range_del_agg,
-      file_options_for_read_,
-      (start == nullptr) ? std::optional<const Slice>{}
-                         : std::optional<const Slice>{*start},
-      (end == nullptr) ? std::optional<const Slice>{}
-                       : std::optional<const Slice>{*end}));
+  std::unique_ptr<InternalIterator> raw_input(
+      versions_->MakeInputIterator(read_options, sub_compact->compaction,
+                                   &range_del_agg, file_options_for_read_));
   InternalIterator* input = raw_input.get();
 
   IterKey start_ikey;

db/db_bloom_filter_test.cc

@@ -17,11 +17,8 @@
 #include "db/db_test_util.h"
 #include "options/options_helper.h"
 #include "port/stack_trace.h"
-#include "rocksdb/advanced_options.h"
 #include "rocksdb/convenience.h"
-#include "rocksdb/filter_policy.h"
 #include "rocksdb/perf_context.h"
-#include "rocksdb/statistics.h"
 #include "rocksdb/table.h"
 #include "table/block_based/block_based_table_reader.h"
 #include "table/block_based/filter_policy_internal.h"
@@ -801,87 +798,83 @@ TEST_F(DBBloomFilterTest, BloomFilterRate) {
   }
 }
 
-namespace {
-struct CompatibilityConfig {
-  std::shared_ptr<const FilterPolicy> policy;
-  bool partitioned;
-  uint32_t format_version;
-
-  void SetInTableOptions(BlockBasedTableOptions* table_options) {
-    table_options->filter_policy = policy;
-    table_options->partition_filters = partitioned;
-    if (partitioned) {
-      table_options->index_type =
-          BlockBasedTableOptions::IndexType::kTwoLevelIndexSearch;
-    } else {
-      table_options->index_type =
-          BlockBasedTableOptions::IndexType::kBinarySearch;
-    }
-    table_options->format_version = format_version;
-  }
-};
-// High bits per key -> almost no FPs
-std::shared_ptr<const FilterPolicy> kCompatibilityBloomPolicy{
-    NewBloomFilterPolicy(20)};
-// bloom_before_level=-1 -> always use Ribbon
-std::shared_ptr<const FilterPolicy> kCompatibilityRibbonPolicy{
-    NewRibbonFilterPolicy(20, -1)};
-
-std::vector<CompatibilityConfig> kCompatibilityConfigs = {
-    {Create(20, kDeprecatedBlock), false,
-     BlockBasedTableOptions().format_version},
-    {kCompatibilityBloomPolicy, false, BlockBasedTableOptions().format_version},
-    {kCompatibilityBloomPolicy, true, BlockBasedTableOptions().format_version},
-    {kCompatibilityBloomPolicy, false, /* legacy Bloom */ 4U},
-    {kCompatibilityRibbonPolicy, false,
-     BlockBasedTableOptions().format_version},
-    {kCompatibilityRibbonPolicy, true, BlockBasedTableOptions().format_version},
-};
-}  // namespace
-
 TEST_F(DBBloomFilterTest, BloomFilterCompatibility) {
   Options options = CurrentOptions();
   options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
-  options.level0_file_num_compaction_trigger =
-      static_cast<int>(kCompatibilityConfigs.size()) + 1;
-  options.max_open_files = -1;
+  BlockBasedTableOptions table_options;
+  table_options.filter_policy.reset(NewBloomFilterPolicy(10, true));
+  options.table_factory.reset(NewBlockBasedTableFactory(table_options));
 
-  Close();
+  // Create with block based filter
+  CreateAndReopenWithCF({"pikachu"}, options);
 
-  // Create one file for each kind of filter. Each file covers a distinct key
-  // range.
-  for (size_t i = 0; i < kCompatibilityConfigs.size(); ++i) {
-    BlockBasedTableOptions table_options;
-    kCompatibilityConfigs[i].SetInTableOptions(&table_options);
-    ASSERT_TRUE(table_options.filter_policy != nullptr);
-    options.table_factory.reset(NewBlockBasedTableFactory(table_options));
-    Reopen(options);
-
-    std::string prefix = ToString(i) + "_";
-    ASSERT_OK(Put(prefix + "A", "val"));
-    ASSERT_OK(Put(prefix + "Z", "val"));
-    ASSERT_OK(Flush());
+  const int maxKey = 10000;
+  for (int i = 0; i < maxKey; i++) {
+    ASSERT_OK(Put(1, Key(i), Key(i)));
   }
+  ASSERT_OK(Put(1, Key(maxKey + 55555), Key(maxKey + 55555)));
+  Flush(1);
 
-  // Test filter is used between each pair of {reader,writer} configurations,
-  // because any built-in FilterPolicy should be able to read filters from any
-  // other built-in FilterPolicy
-  for (size_t i = 0; i < kCompatibilityConfigs.size(); ++i) {
+  // Check db with full filter
+  table_options.filter_policy.reset(NewBloomFilterPolicy(10, false));
+  options.table_factory.reset(NewBlockBasedTableFactory(table_options));
+  ReopenWithColumnFamilies({"default", "pikachu"}, options);
+
+  // Check if they can be found
+  for (int i = 0; i < maxKey; i++) {
+    ASSERT_EQ(Key(i), Get(1, Key(i)));
+  }
+  ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 0);
+
+  // Check db with partitioned full filter
+  table_options.partition_filters = true;
+  table_options.index_type =
+      BlockBasedTableOptions::IndexType::kTwoLevelIndexSearch;
+  table_options.filter_policy.reset(NewBloomFilterPolicy(10, false));
+  options.table_factory.reset(NewBlockBasedTableFactory(table_options));
+  ReopenWithColumnFamilies({"default", "pikachu"}, options);
+
+  // Check if they can be found
+  for (int i = 0; i < maxKey; i++) {
+    ASSERT_EQ(Key(i), Get(1, Key(i)));
+  }
+  ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 0);
+}
+
+TEST_F(DBBloomFilterTest, BloomFilterReverseCompatibility) {
+  for (bool partition_filters : {true, false}) {
+    Options options = CurrentOptions();
+    options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
     BlockBasedTableOptions table_options;
-    kCompatibilityConfigs[i].SetInTableOptions(&table_options);
-    options.table_factory.reset(NewBlockBasedTableFactory(table_options));
-    Reopen(options);
-    for (size_t j = 0; j < kCompatibilityConfigs.size(); ++j) {
-      std::string prefix = ToString(j) + "_";
-      ASSERT_EQ("val", Get(prefix + "A"));  // Filter positive
-      ASSERT_EQ("val", Get(prefix + "Z"));  // Filter positive
-      // Filter negative, with high probability
-      ASSERT_EQ("NOT_FOUND", Get(prefix + "Q"));
-      // FULL_POSITIVE does not include block-based filter case (j == 0)
-      EXPECT_EQ(TestGetAndResetTickerCount(options, BLOOM_FILTER_FULL_POSITIVE),
-                j == 0 ? 0 : 2);
-      EXPECT_EQ(TestGetAndResetTickerCount(options, BLOOM_FILTER_USEFUL), 1);
+    if (partition_filters) {
+      table_options.partition_filters = true;
+      table_options.index_type =
+          BlockBasedTableOptions::IndexType::kTwoLevelIndexSearch;
     }
+    table_options.filter_policy.reset(NewBloomFilterPolicy(10, false));
+    options.table_factory.reset(NewBlockBasedTableFactory(table_options));
+    DestroyAndReopen(options);
+
+    // Create with full filter
+    CreateAndReopenWithCF({"pikachu"}, options);
+
+    const int maxKey = 10000;
+    for (int i = 0; i < maxKey; i++) {
+      ASSERT_OK(Put(1, Key(i), Key(i)));
+    }
+    ASSERT_OK(Put(1, Key(maxKey + 55555), Key(maxKey + 55555)));
+    Flush(1);
+
+    // Check db with block_based filter
+    table_options.filter_policy.reset(NewBloomFilterPolicy(10, true));
+    options.table_factory.reset(NewBlockBasedTableFactory(table_options));
+    ReopenWithColumnFamilies({"default", "pikachu"}, options);
+
+    // Check if they can be found
+    for (int i = 0; i < maxKey; i++) {
+      ASSERT_EQ(Key(i), Get(1, Key(i)));
+    }
+    ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 0);
   }
 }
 
@@ -926,7 +919,7 @@ class FilterConstructResPeakTrackingCache : public CacheWrapper {
   }
 
   using Cache::Release;
-  bool Release(Handle* handle, bool erase_if_last_ref = false) override {
+  bool Release(Handle* handle, bool force_erase = false) override {
     auto deleter = GetDeleter(handle);
     if (deleter == kNoopDeleterForFilterConstruction) {
       if (!last_peak_tracked_) {
@@ -936,7 +929,7 @@ class FilterConstructResPeakTrackingCache : public CacheWrapper {
       }
       cur_cache_res_ -= GetCharge(handle);
     }
-    bool is_successful = target_->Release(handle, erase_if_last_ref);
+    bool is_successful = target_->Release(handle, force_erase);
     return is_successful;
   }
 
@@ -959,8 +952,8 @@ class FilterConstructResPeakTrackingCache : public CacheWrapper {
 
 const Cache::DeleterFn
     FilterConstructResPeakTrackingCache::kNoopDeleterForFilterConstruction =
-        CacheReservationManagerImpl<
-            CacheEntryRole::kFilterConstruction>::TEST_GetNoopDeleterForRole();
+        CacheReservationManager::TEST_GetNoopDeleterForRole<
+            CacheEntryRole::kFilterConstruction>();
 
 // To align with the type of hash entry being reserved in implementation.
 using FilterConstructionReserveMemoryHash = uint64_t;
@@ -990,9 +983,7 @@ class DBFilterConstructionReserveMemoryTestWithParam
       // trigger at least 1 dummy entry reservation each for hash entries and
       // final filter, we need a large number of keys to ensure we have at least
       // two partitions.
-      num_key_ = 18 *
-                 CacheReservationManagerImpl<
-                     CacheEntryRole::kFilterConstruction>::GetDummyEntrySize() /
+      num_key_ = 18 * CacheReservationManager::GetDummyEntrySize() /
                  sizeof(FilterConstructionReserveMemoryHash);
     } else if (policy_ == kFastLocalBloom) {
       // For Bloom Filter + FullFilter case, since we design the num_key_ to
@@ -1002,9 +993,7 @@ class DBFilterConstructionReserveMemoryTestWithParam
       // behavior and we don't need a large number of keys to verify we
       // indeed charge the final filter for cache reservation, even though final
       // filter is a lot smaller than hash entries.
-      num_key_ = 1 *
-                 CacheReservationManagerImpl<
-                     CacheEntryRole::kFilterConstruction>::GetDummyEntrySize() /
+      num_key_ = 1 * CacheReservationManager::GetDummyEntrySize() /
                  sizeof(FilterConstructionReserveMemoryHash);
     } else {
       // For Ribbon Filter + FullFilter case, we need a large enough number of
@@ -1012,9 +1001,7 @@ class DBFilterConstructionReserveMemoryTestWithParam
       // reservation will trigger at least another dummy entry (or equivalently
       // to saying, causing another peak in cache reservation) as banding
       // reservation might not be a multiple of dummy entry.
-      num_key_ = 12 *
-                 CacheReservationManagerImpl<
-                     CacheEntryRole::kFilterConstruction>::GetDummyEntrySize() /
+      num_key_ = 12 * CacheReservationManager::GetDummyEntrySize() /
                  sizeof(FilterConstructionReserveMemoryHash);
     }
   }
@@ -1169,8 +1156,8 @@ TEST_P(DBFilterConstructionReserveMemoryTestWithParam, ReserveMemory) {
     return;
   }
 
-  const std::size_t kDummyEntrySize = CacheReservationManagerImpl<
-      CacheEntryRole::kFilterConstruction>::GetDummyEntrySize();
+  const std::size_t kDummyEntrySize =
+      CacheReservationManager::GetDummyEntrySize();
 
   const std::size_t predicted_hash_entries_cache_res =
       num_key * sizeof(FilterConstructionReserveMemoryHash);
@@ -1358,12 +1345,9 @@ TEST_P(DBFilterConstructionReserveMemoryTestWithParam, ReserveMemory) {
      *
      */
     if (!partition_filters) {
-      ASSERT_GE(
-          std::floor(
-              1.0 * predicted_final_filter_cache_res /
-              CacheReservationManagerImpl<
-                  CacheEntryRole::kFilterConstruction>::GetDummyEntrySize()),
-          1)
+      ASSERT_GE(std::floor(1.0 * predicted_final_filter_cache_res /
+                           CacheReservationManager::GetDummyEntrySize()),
+                1)
           << "Final filter cache reservation too small for this test - please "
              "increase the number of keys";
       if (!detect_filter_construct_corruption) {

db/db_compaction_test.cc

@@ -1354,74 +1354,6 @@ TEST_P(DBCompactionTestWithParam, TrivialMoveTargetLevel) {
   }
 }
 
-TEST_P(DBCompactionTestWithParam, PartialOverlappingL0) {
-  class SubCompactionEventListener : public EventListener {
-   public:
-    void OnSubcompactionCompleted(const SubcompactionJobInfo&) override {
-      sub_compaction_finished_++;
-    }
-    std::atomic<int> sub_compaction_finished_{0};
-  };
-
-  Options options = CurrentOptions();
-  options.disable_auto_compactions = true;
-  options.write_buffer_size = 10 * 1024 * 1024;
-  options.max_subcompactions = max_subcompactions_;
-  SubCompactionEventListener* listener = new SubCompactionEventListener();
-  options.listeners.emplace_back(listener);
-
-  DestroyAndReopen(options);
-
-  // For subcompactino to trigger, output level needs to be non-empty.
-  ASSERT_OK(Put("key", ""));
-  ASSERT_OK(Put("kez", ""));
-  ASSERT_OK(Flush());
-  ASSERT_OK(Put("key", ""));
-  ASSERT_OK(Put("kez", ""));
-  ASSERT_OK(Flush());
-  ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
-
-  // Ranges that are only briefly overlapping so that they won't be trivially
-  // moved but subcompaction ranges would only contain a subset of files.
-  std::vector<std::pair<int32_t, int32_t>> ranges = {
-      {100, 199}, {198, 399}, {397, 600}, {598, 800}, {799, 900}, {895, 999},
-  };
-  int32_t value_size = 10 * 1024;  // 10 KB
-
-  Random rnd(301);
-  std::map<int32_t, std::string> values;
-  for (size_t i = 0; i < ranges.size(); i++) {
-    for (int32_t j = ranges[i].first; j <= ranges[i].second; j++) {
-      values[j] = rnd.RandomString(value_size);
-      ASSERT_OK(Put(Key(j), values[j]));
-    }
-    ASSERT_OK(Flush());
-  }
-
-  int32_t level0_files = NumTableFilesAtLevel(0, 0);
-  ASSERT_EQ(level0_files, ranges.size());    // Multiple files in L0
-  ASSERT_EQ(NumTableFilesAtLevel(1, 0), 1);  // One file in L1
-
-  listener->sub_compaction_finished_ = 0;
-  ASSERT_OK(db_->EnableAutoCompaction({db_->DefaultColumnFamily()}));
-  ASSERT_OK(dbfull()->TEST_WaitForCompact());
-  if (max_subcompactions_ > 3) {
-    // RocksDB might not generate the exact number of sub compactions.
-    // Here we validate that at least subcompaction happened.
-    ASSERT_GT(listener->sub_compaction_finished_.load(), 2);
-  }
-
-  // We expect that all the files were compacted to L1
-  ASSERT_EQ(NumTableFilesAtLevel(0, 0), 0);
-  ASSERT_GT(NumTableFilesAtLevel(1, 0), 1);
-
-  for (size_t i = 0; i < ranges.size(); i++) {
-    for (int32_t j = ranges[i].first; j <= ranges[i].second; j++) {
-      ASSERT_EQ(Get(Key(j)), values[j]);
-    }
-  }
-}
-
 TEST_P(DBCompactionTestWithParam, ManualCompactionPartial) {
   int32_t trivial_move = 0;
   int32_t non_trivial_move = 0;

db/db_filesnapshot.cc

@@ -45,15 +45,17 @@ Status DBImpl::FlushForGetLiveFiles() {
     }
     mutex_.Lock();
   } else {
-    for (auto cfd : versions_->GetRefedColumnFamilySet()) {
+    for (auto cfd : *versions_->GetColumnFamilySet()) {
       if (cfd->IsDropped()) {
         continue;
       }
+      cfd->Ref();
       mutex_.Unlock();
       status = FlushMemTable(cfd, FlushOptions(), FlushReason::kGetLiveFiles);
       TEST_SYNC_POINT("DBImpl::GetLiveFiles:1");
       TEST_SYNC_POINT("DBImpl::GetLiveFiles:2");
       mutex_.Lock();
+      cfd->UnrefAndTryDelete();
       if (!status.ok() && !status.IsColumnFamilyDropped()) {
         break;
       } else if (status.IsColumnFamilyDropped()) {
@@ -61,6 +63,7 @@ Status DBImpl::FlushForGetLiveFiles() {
       }
     }
   }
+  versions_->GetColumnFamilySet()->FreeDeadColumnFamilies();
   return status;
 }
 
@@ -96,7 +99,7 @@ Status DBImpl::GetLiveFiles(std::vector<std::string>& ret,
               3);  // for CURRENT + MANIFEST + OPTIONS
 
   // create names of the live files. The names are not absolute
-  // paths, instead they are relative to dbname_.
+  // paths, instead they are relative to dbname_;
   for (const auto& table_file_number : live_table_files) {
     ret.emplace_back(MakeTableFileName("", table_file_number));
   }
@@ -166,7 +169,7 @@ Status DBImpl::GetCurrentWalFile(std::unique_ptr<LogFile>* current_log_file) {
 Status DBImpl::GetLiveFilesStorageInfo(
     const LiveFilesStorageInfoOptions& opts,
     std::vector<LiveFileStorageInfo>* files) {
-  // To avoid returning partial results, only move results to files on success.
+  // To avoid returning partial results, only move to ouput on success
   assert(files);
   files->clear();
   std::vector<LiveFileStorageInfo> results;
@@ -180,7 +183,7 @@ Status DBImpl::GetLiveFilesStorageInfo(
     if (opts.wal_size_for_flush == port::kMaxUint64) {
       flush_memtable = false;
     } else if (opts.wal_size_for_flush > 0) {
-      // If the outstanding log files are small, we skip the flush.
+      // If out standing log files are small, we skip the flush.
       s = GetSortedWalFiles(live_wal_files);
 
       if (!s.ok()) {
@@ -313,7 +316,8 @@ Status DBImpl::GetLiveFilesStorageInfo(
     info.relative_filename = kCurrentFileName;
     info.directory = GetName();
     info.file_type = kCurrentFile;
-    // CURRENT could be replaced so we have to record the contents as needed.
+    // CURRENT could be replaced so we have to record the contents we want
+    // for it
     info.replacement_contents = manifest_fname + "\n";
     info.size = manifest_fname.size() + 1;
     if (opts.include_checksum_info) {
@@ -355,7 +359,7 @@ Status DBImpl::GetLiveFilesStorageInfo(
   TEST_SYNC_POINT("CheckpointImpl::CreateCustomCheckpoint:AfterGetLive1");
   TEST_SYNC_POINT("CheckpointImpl::CreateCustomCheckpoint:AfterGetLive2");
 
-  // If we have more than one column family, we also need to get WAL files.
+  // if we have more than one column family, we need to also get WAL files
   if (s.ok()) {
     s = GetSortedWalFiles(live_wal_files);
   }
@@ -393,7 +397,7 @@ Status DBImpl::GetLiveFilesStorageInfo(
   }
 
   if (s.ok()) {
-    // Only move results to output on success.
+    // Only move output on success
     *files = std::move(results);
   }
   return s;

db/db_flush_test.cc

@@ -2271,7 +2271,7 @@ TEST_P(DBAtomicFlushTest, ManualFlushUnder2PC) {
 
   // The recovered min log number with prepared data should be non-zero.
   // In 2pc mode, MinLogNumberToKeep returns the
-  // VersionSet::min_log_number_to_keep recovered from MANIFEST, if it's 0,
+  // VersionSet::min_log_number_to_keep_2pc recovered from MANIFEST, if it's 0,
   // it means atomic flush didn't write the min_log_number_to_keep to MANIFEST.
   cfs.push_back(kDefaultColumnFamilyName);
   ASSERT_OK(TryReopenWithColumnFamilies(cfs, options));

db/db_impl/db_impl.cc

@@ -375,12 +375,15 @@ Status DBImpl::ResumeImpl(DBRecoverContext context) {
       s = AtomicFlushMemTables(cfds, flush_opts, context.flush_reason);
       mutex_.Lock();
     } else {
-      for (auto cfd : versions_->GetRefedColumnFamilySet()) {
+      for (auto cfd : *versions_->GetColumnFamilySet()) {
         if (cfd->IsDropped()) {
           continue;
         }
-        InstrumentedMutexUnlock u(&mutex_);
+        cfd->Ref();
+        mutex_.Unlock();
         s = FlushMemTable(cfd, flush_opts, context.flush_reason);
+        mutex_.Lock();
+        cfd->UnrefAndTryDelete();
         if (!s.ok()) {
           break;
         }
@@ -492,14 +495,18 @@ void DBImpl::CancelAllBackgroundWork(bool wait) {
       s.PermitUncheckedError();  //**TODO: What to do on error?
       mutex_.Lock();
     } else {
-      for (auto cfd : versions_->GetRefedColumnFamilySet()) {
+      for (auto cfd : *versions_->GetColumnFamilySet()) {
         if (!cfd->IsDropped() && cfd->initialized() && !cfd->mem()->IsEmpty()) {
-          InstrumentedMutexUnlock u(&mutex_);
+          cfd->Ref();
+          mutex_.Unlock();
           Status s = FlushMemTable(cfd, FlushOptions(), FlushReason::kShutDown);
           s.PermitUncheckedError();  //**TODO: What to do on error?
+          mutex_.Lock();
+          cfd->UnrefAndTryDelete();
         }
       }
     }
+    versions_->GetColumnFamilySet()->FreeDeadColumnFamilies();
   }
 
   shutting_down_.store(true, std::memory_order_release);
@@ -962,13 +969,18 @@ void DBImpl::DumpStats() {
   TEST_SYNC_POINT("DBImpl::DumpStats:StartRunning");
   {
     InstrumentedMutexLock l(&mutex_);
-    for (auto cfd : versions_->GetRefedColumnFamilySet()) {
+    for (auto cfd : *versions_->GetColumnFamilySet()) {
       if (cfd->initialized()) {
         // Release DB mutex for gathering cache entry stats. Pass over all
         // column families for this first so that other stats are dumped
         // near-atomically.
-        InstrumentedMutexUnlock u(&mutex_);
-        cfd->internal_stats()->CollectCacheEntryStats(/*foreground=*/false);
+        // Get a ref before unlocking
+        cfd->Ref();
+        {
+          InstrumentedMutexUnlock u(&mutex_);
+          cfd->internal_stats()->CollectCacheEntryStats(/*foreground=*/false);
+        }
+        cfd->UnrefAndTryDelete();
       }
     }
 
@@ -1081,6 +1093,7 @@ Status DBImpl::SetOptions(
   MutableCFOptions new_options;
   Status s;
   Status persist_options_status;
+  persist_options_status.PermitUncheckedError();  // Allow uninitialized access
   SuperVersionContext sv_context(/* create_superversion */ true);
   {
     auto db_options = GetDBOptions();
@@ -1116,11 +1129,9 @@ Status DBImpl::SetOptions(
                    "[%s] SetOptions() succeeded", cfd->GetName().c_str());
     new_options.Dump(immutable_db_options_.info_log.get());
     if (!persist_options_status.ok()) {
-      // NOTE: WriteOptionsFile already logs on failure
       s = persist_options_status;
     }
   } else {
-    persist_options_status.PermitUncheckedError();  // less important
     ROCKS_LOG_WARN(immutable_db_options_.info_log, "[%s] SetOptions() failed",
                    cfd->GetName().c_str());
   }
@@ -1889,8 +1900,6 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, const Slice& key,
       return s;
     }
   }
-  TEST_SYNC_POINT("DBImpl::GetImpl:PostMemTableGet:0");
-  TEST_SYNC_POINT("DBImpl::GetImpl:PostMemTableGet:1");
   PinnedIteratorsManager pinned_iters_mgr;
   if (!done) {
     PERF_TIMER_GUARD(get_from_output_files_time);
@@ -1908,6 +1917,8 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, const Slice& key,
   {
     PERF_TIMER_GUARD(get_post_process_time);
 
+    ReturnAndCleanupSuperVersion(cfd, sv);
+
     RecordTick(stats_, NUMBER_KEYS_READ);
     size_t size = 0;
     if (s.ok()) {
@@ -1934,8 +1945,6 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, const Slice& key,
       PERF_COUNTER_ADD(get_read_bytes, size);
     }
     RecordInHistogram(stats_, BYTES_PER_READ, size);
-
-    ReturnAndCleanupSuperVersion(cfd, sv);
   }
   return s;
 }
@@ -3481,13 +3490,15 @@ bool DBImpl::GetAggregatedIntProperty(const Slice& property,
     // Needs mutex to protect the list of column families.
     InstrumentedMutexLock l(&mutex_);
     uint64_t value;
-    for (auto* cfd : versions_->GetRefedColumnFamilySet()) {
+    for (auto* cfd : *versions_->GetColumnFamilySet()) {
       if (!cfd->initialized()) {
         continue;
       }
+      cfd->Ref();
       ret = GetIntPropertyInternal(cfd, *property_info, true, &value);
       // GetIntPropertyInternal may release db mutex and re-acquire it.
       mutex_.AssertHeld();
+      cfd->UnrefAndTryDelete();
       if (ret) {
         sum += value;
       } else {
@@ -5078,7 +5089,6 @@ Status DBImpl::VerifyChecksumInternal(const ReadOptions& read_options,
     }
   }
 
-  // TODO: simplify using GetRefedColumnFamilySet?
   std::vector<ColumnFamilyData*> cfd_list;
   {
     InstrumentedMutexLock l(&mutex_);

db/db_impl/db_impl.h

@@ -2144,7 +2144,7 @@ class DBImpl : public DB {
   // only during recovery. Using this feature enables not writing the state to
   // memtable on normal writes and hence improving the throughput. Each new
   // write of the state will replace the previous state entirely even if the
-  // keys in the two consecutive states do not overlap.
+  // keys in the two consecuitive states do not overlap.
   // It is protected by log_write_mutex_ when two_write_queues_ is enabled.
   // Otherwise only the heaad of write_thread_ can access it.
   WriteBatch cached_recoverable_state_;

db/db_impl/db_impl_compaction_flush.cc

@@ -2973,6 +2973,8 @@ void DBImpl::BackgroundCallCompaction(PrepickedCompaction* prepicked_compaction,
       bg_bottom_compaction_scheduled_--;
     }
 
+    versions_->GetColumnFamilySet()->FreeDeadColumnFamilies();
+
     // See if there's more work to be done
     MaybeScheduleFlushOrCompaction();
 

db/db_impl/db_impl_files.cc

@@ -23,7 +23,11 @@
 namespace ROCKSDB_NAMESPACE {
 
 uint64_t DBImpl::MinLogNumberToKeep() {
-  return versions_->min_log_number_to_keep();
+  if (allow_2pc()) {
+    return versions_->min_log_number_to_keep_2pc();
+  } else {
+    return versions_->MinLogNumberWithUnflushedData();
+  }
 }
 
 uint64_t DBImpl::MinObsoleteSstNumberToKeep() {
@@ -220,6 +224,7 @@ void DBImpl::FindObsoleteFiles(JobContext* job_context, bool force,
     }
 
     // Add log files in wal_dir
+
     if (!immutable_db_options_.IsWalDirSameAsDBPath(dbname_)) {
       std::vector<std::string> log_files;
       Status s = env_->GetChildren(immutable_db_options_.wal_dir, &log_files);
@@ -229,7 +234,6 @@ void DBImpl::FindObsoleteFiles(JobContext* job_context, bool force,
             log_file, immutable_db_options_.wal_dir);
       }
     }
-
     // Add info log files in db_log_dir
     if (!immutable_db_options_.db_log_dir.empty() &&
         immutable_db_options_.db_log_dir != dbname_) {

db/db_impl/db_impl_open.cc

@@ -866,11 +866,6 @@ Status DBImpl::RecoverLogFiles(const std::vector<uint64_t>& wal_numbers,
   bool flushed = false;
   uint64_t corrupted_wal_number = kMaxSequenceNumber;
   uint64_t min_wal_number = MinLogNumberToKeep();
-  if (!allow_2pc()) {
-    // In non-2pc mode, we skip WALs that do not back unflushed data.
-    min_wal_number =
-        std::max(min_wal_number, versions_->MinLogNumberWithUnflushedData());
-  }
   for (auto wal_number : wal_numbers) {
     if (wal_number < min_wal_number) {
       ROCKS_LOG_INFO(immutable_db_options_.info_log,
@@ -1275,16 +1270,9 @@ Status DBImpl::RecoverLogFiles(const std::vector<uint64_t>& wal_numbers,
       }
 
       std::unique_ptr<VersionEdit> wal_deletion;
-      if (flushed) {
-        wal_deletion = std::make_unique<VersionEdit>();
-        if (immutable_db_options_.track_and_verify_wals_in_manifest) {
-          wal_deletion->DeleteWalsBefore(max_wal_number + 1);
-        }
-        if (!allow_2pc()) {
-          // In non-2pc mode, flushing the memtables of the column families
-          // means we can advance min_log_number_to_keep.
-          wal_deletion->SetMinLogNumberToKeep(max_wal_number + 1);
-        }
+      if (immutable_db_options_.track_and_verify_wals_in_manifest) {
+        wal_deletion.reset(new VersionEdit);
+        wal_deletion->DeleteWalsBefore(max_wal_number + 1);
         edit_lists.back().push_back(wal_deletion.get());
       }
 
@@ -1363,14 +1351,7 @@ Status DBImpl::RestoreAliveLogFiles(const std::vector<uint64_t>& wal_numbers) {
   // FindObsoleteFiles()
   total_log_size_ = 0;
   log_empty_ = false;
-  uint64_t min_wal_with_unflushed_data =
-      versions_->MinLogNumberWithUnflushedData();
   for (auto wal_number : wal_numbers) {
-    if (!allow_2pc() && wal_number < min_wal_with_unflushed_data) {
-      // In non-2pc mode, the WAL files not backing unflushed data are not
-      // alive, thus should not be added to the alive_log_files_.
-      continue;
-    }
     // We preallocate space for wals, but then after a crash and restart, those
     // preallocated space are not needed anymore. It is likely only the last
     // log has such preallocated space, so we only truncate for the last log.
@@ -1971,10 +1952,10 @@ Status DBImpl::Open(const DBOptions& db_options, const std::string& dbname,
           "DB::Open() failed --- Unable to persist Options file",
           persist_options_status.ToString());
     }
-  }
-  if (!s.ok()) {
+  } else {
     ROCKS_LOG_WARN(impl->immutable_db_options_.info_log,
-                   "DB::Open() failed: %s", s.ToString().c_str());
+                   "Persisting Option File error: %s",
+                   persist_options_status.ToString().c_str());
   }
   if (s.ok()) {
     s = impl->StartPeriodicWorkScheduler();

db/db_impl/db_impl_write.cc

@@ -129,7 +129,7 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options,
                          PreReleaseCallback* pre_release_callback) {
   assert(!seq_per_batch_ || batch_cnt != 0);
   if (my_batch == nullptr) {
-    return Status::InvalidArgument("Batch is nullptr!");
+    return Status::Corruption("Batch is nullptr!");
   } else if (!disable_memtable &&
              WriteBatchInternal::TimestampsUpdateNeeded(*my_batch)) {
     // If writing to memtable, then we require the caller to set/update the
@@ -1882,11 +1882,9 @@ void DBImpl::NotifyOnMemTableSealed(ColumnFamilyData* /*cfd*/,
     return;
   }
 
-  mutex_.Unlock();
   for (auto listener : immutable_db_options_.listeners) {
     listener->OnMemTableSealed(mem_table_info);
   }
-  mutex_.Lock();
 }
 #endif  // ROCKSDB_LITE
 
@@ -2087,9 +2085,11 @@ Status DBImpl::SwitchMemtable(ColumnFamilyData* cfd, WriteContext* context) {
                                      mutable_cf_options);
 
 #ifndef ROCKSDB_LITE
+  mutex_.Unlock();
   // Notify client that memtable is sealed, now that we have successfully
   // installed a new memtable
   NotifyOnMemTableSealed(cfd, memtable_info);
+  mutex_.Lock();
 #endif  // ROCKSDB_LITE
   // It is possible that we got here without checking the value of i_os, but
   // that is okay.  If we did, it most likely means that s was already an error.

db/db_merge_operand_test.cc

@@ -47,7 +47,7 @@ class DBMergeOperandTest : public DBTestBase {
       : DBTestBase("db_merge_operand_test", /*env_do_fsync=*/true) {}
 };
 
-TEST_F(DBMergeOperandTest, CacheEvictedMergeOperandReadAfterFreeBug) {
+TEST_F(DBMergeOperandTest, MergeOperandReadAfterFreeBug) {
   // There was a bug of reading merge operands after they are mistakely freed
   // in DB::GetMergeOperands, which is surfaced by cache full.
   // See PR#9507 for more.
@@ -86,42 +86,6 @@ TEST_F(DBMergeOperandTest, CacheEvictedMergeOperandReadAfterFreeBug) {
   ASSERT_EQ(values[3].ToString(), "v4");
 }
 
-TEST_F(DBMergeOperandTest, FlushedMergeOperandReadAfterFreeBug) {
-  // Repro for a bug where a memtable containing a merge operand could be
-  // deleted before the merge operand was saved to the result.
-  auto options = CurrentOptions();
-  options.merge_operator = MergeOperators::CreateStringAppendOperator();
-  Reopen(options);
-
-  ASSERT_OK(Merge("key", "value"));
-
-  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependency(
-      {{"DBImpl::GetImpl:PostMemTableGet:0",
-        "DBMergeOperandTest::FlushedMergeOperandReadAfterFreeBug:PreFlush"},
-       {"DBMergeOperandTest::FlushedMergeOperandReadAfterFreeBug:PostFlush",
-        "DBImpl::GetImpl:PostMemTableGet:1"}});
-  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
-
-  auto flush_thread = port::Thread([&]() {
-    TEST_SYNC_POINT(
-        "DBMergeOperandTest::FlushedMergeOperandReadAfterFreeBug:PreFlush");
-    ASSERT_OK(Flush());
-    TEST_SYNC_POINT(
-        "DBMergeOperandTest::FlushedMergeOperandReadAfterFreeBug:PostFlush");
-  });
-
-  PinnableSlice value;
-  GetMergeOperandsOptions merge_operands_info;
-  merge_operands_info.expected_max_number_of_operands = 1;
-  int number_of_operands;
-  ASSERT_OK(db_->GetMergeOperands(ReadOptions(), db_->DefaultColumnFamily(),
-                                  "key", &value, &merge_operands_info,
-                                  &number_of_operands));
-  ASSERT_EQ(1, number_of_operands);
-
-  flush_thread.join();
-}
-
 TEST_F(DBMergeOperandTest, GetMergeOperandsBasic) {
   Options options;
   options.create_if_missing = true;

db/db_properties_test.cc

@@ -13,17 +13,12 @@
 #include <string>
 
 #include "db/db_test_util.h"
-#include "options/cf_options.h"
 #include "port/stack_trace.h"
 #include "rocksdb/listener.h"
 #include "rocksdb/options.h"
 #include "rocksdb/perf_context.h"
 #include "rocksdb/perf_level.h"
 #include "rocksdb/table.h"
-#include "table/block_based/block.h"
-#include "table/format.h"
-#include "table/meta_blocks.h"
-#include "table/table_builder.h"
 #include "test_util/mock_time_env.h"
 #include "util/random.h"
 #include "util/string_util.h"
@@ -1997,90 +1992,6 @@ TEST_F(DBPropertiesTest, GetMapPropertyDbStats) {
   Close();
 }
 
-namespace {
-std::string PopMetaIndexKey(InternalIterator* meta_iter) {
-  Status s = meta_iter->status();
-  if (!s.ok()) {
-    return s.ToString();
-  } else if (meta_iter->Valid()) {
-    std::string rv = meta_iter->key().ToString();
-    meta_iter->Next();
-    return rv;
-  } else {
-    return "NOT_FOUND";
-  }
-}
-
-}  // namespace
-
-TEST_F(DBPropertiesTest, TableMetaIndexKeys) {
-  // This is to detect unexpected churn in metaindex block keys. This is more
-  // of a "table test" but table_test.cc doesn't depend on db_test_util.h and
-  // we need ChangeOptions() for broad coverage.
-  constexpr int kKeyCount = 100;
-  do {
-    Options options;
-    options = CurrentOptions(options);
-    DestroyAndReopen(options);
-
-    // Create an SST file
-    for (int key = 0; key < kKeyCount; key++) {
-      ASSERT_OK(Put(Key(key), "val"));
-    }
-    ASSERT_OK(Flush());
-
-    // Find its file number
-    std::vector<LiveFileMetaData> files;
-    db_->GetLiveFilesMetaData(&files);
-    // 1 SST file
-    ASSERT_EQ(1, files.size());
-
-    // Open it for inspection
-    std::string sst_file =
-        files[0].directory + "/" + files[0].relative_filename;
-    std::unique_ptr<FSRandomAccessFile> f;
-    ASSERT_OK(env_->GetFileSystem()->NewRandomAccessFile(
-        sst_file, FileOptions(), &f, nullptr));
-    std::unique_ptr<RandomAccessFileReader> r;
-    r.reset(new RandomAccessFileReader(std::move(f), sst_file));
-    uint64_t file_size = 0;
-    ASSERT_OK(env_->GetFileSize(sst_file, &file_size));
-
-    // Read metaindex
-    BlockContents bc;
-    ASSERT_OK(ReadMetaIndexBlockInFile(r.get(), file_size, 0U,
-                                       ImmutableOptions(options), &bc));
-    Block metaindex_block(std::move(bc));
-    std::unique_ptr<InternalIterator> meta_iter;
-    meta_iter.reset(metaindex_block.NewMetaIterator());
-    meta_iter->SeekToFirst();
-
-    if (strcmp(options.table_factory->Name(),
-               TableFactory::kBlockBasedTableName()) == 0) {
-      auto bbto = options.table_factory->GetOptions<BlockBasedTableOptions>();
-      if (bbto->filter_policy) {
-        if (bbto->partition_filters) {
-          // The key names are intentionally hard-coded here to detect
-          // accidental regression on compatibility.
-          EXPECT_EQ("partitionedfilter.rocksdb.BuiltinBloomFilter",
-                    PopMetaIndexKey(meta_iter.get()));
-        } else {
-          EXPECT_EQ("fullfilter.rocksdb.BuiltinBloomFilter",
-                    PopMetaIndexKey(meta_iter.get()));
-        }
-      }
-      if (bbto->index_type == BlockBasedTableOptions::kHashSearch) {
-        EXPECT_EQ("rocksdb.hashindex.metadata",
-                  PopMetaIndexKey(meta_iter.get()));
-        EXPECT_EQ("rocksdb.hashindex.prefixes",
-                  PopMetaIndexKey(meta_iter.get()));
-      }
-    }
-    EXPECT_EQ("rocksdb.properties", PopMetaIndexKey(meta_iter.get()));
-    EXPECT_EQ("NOT_FOUND", PopMetaIndexKey(meta_iter.get()));
-  } while (ChangeOptions());
-}
-
 #endif  // ROCKSDB_LITE
 
 }  // namespace ROCKSDB_NAMESPACE

db/db_sst_test.cc

@@ -11,9 +11,7 @@
 #include "file/sst_file_manager_impl.h"
 #include "port/port.h"
 #include "port/stack_trace.h"
-#include "rocksdb/cache.h"
 #include "rocksdb/sst_file_manager.h"
-#include "rocksdb/table.h"
 #include "util/random.h"
 
 namespace ROCKSDB_NAMESPACE {
@@ -1395,69 +1393,6 @@ TEST_F(DBSSTTest, OpenDBWithInfiniteMaxOpenFiles) {
   }
 }
 
-TEST_F(DBSSTTest, OpenDBWithInfiniteMaxOpenFilesSubjectToMemoryLimit) {
-  for (bool reserve_table_builder_memory : {true, false}) {
-    // Open DB with infinite max open files
-    //  - First iteration use 1 thread to open files
-    //  - Second iteration use 5 threads to open files
-    for (int iter = 0; iter < 2; iter++) {
-      Options options;
-      options.create_if_missing = true;
-      options.write_buffer_size = 100000;
-      options.disable_auto_compactions = true;
-      options.max_open_files = -1;
-
-      BlockBasedTableOptions table_options;
-      options.table_factory.reset(NewBlockBasedTableFactory(table_options));
-      if (iter == 0) {
-        options.max_file_opening_threads = 1;
-      } else {
-        options.max_file_opening_threads = 5;
-      }
-
-      DestroyAndReopen(options);
-
-      // Create 5 Files in L0 (then move then to L2)
-      for (int i = 0; i < 5; i++) {
-        std::string k = "L2_" + Key(i);
-        ASSERT_OK(Put(k, k + std::string(1000, 'a')));
-        ASSERT_OK(Flush()) << i;
-      }
-      CompactRangeOptions compact_options;
-      compact_options.change_level = true;
-      compact_options.target_level = 2;
-      ASSERT_OK(db_->CompactRange(compact_options, nullptr, nullptr));
-
-      // Create 5 Files in L0
-      for (int i = 0; i < 5; i++) {
-        std::string k = "L0_" + Key(i);
-        ASSERT_OK(Put(k, k + std::string(1000, 'a')));
-        ASSERT_OK(Flush());
-      }
-      Close();
-
-      table_options.reserve_table_reader_memory = reserve_table_builder_memory;
-      table_options.block_cache =
-          NewLRUCache(1024 /* capacity */, 0 /* num_shard_bits */,
-                      true /* strict_capacity_limit */);
-      options.table_factory.reset(NewBlockBasedTableFactory(table_options));
-
-      // Reopening the DB will try to load all existing files, conditionally
-      // subject to memory limit
-      Status s = TryReopen(options);
-      if (table_options.reserve_table_reader_memory) {
-        EXPECT_TRUE(s.IsMemoryLimit());
-        EXPECT_TRUE(s.ToString().find("memory limit based on cache capacity") !=
-                    std::string::npos);
-
-      } else {
-        EXPECT_TRUE(s.ok());
-        ASSERT_EQ("5,0,5", FilesPerLevel(0));
-      }
-    }
-  }
-}
-
 TEST_F(DBSSTTest, GetTotalSstFilesSize) {
   // We don't propagate oldest-key-time table property on compaction and
   // just write 0 as default value. This affect the exact table size, since

db/db_table_properties_test.cc

@@ -7,7 +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 <memory>
 #include <unordered_set>
 #include <vector>
 
@@ -19,7 +18,6 @@
 #include "rocksdb/utilities/table_properties_collectors.h"
 #include "table/format.h"
 #include "table/meta_blocks.h"
-#include "table/table_properties_internal.h"
 #include "test_util/testharness.h"
 #include "test_util/testutil.h"
 #include "util/random.h"

db/db_test.cc

@@ -1078,11 +1078,6 @@ void CheckColumnFamilyMeta(
       ASSERT_LE(file_meta_from_cf.file_creation_time, end_time);
       ASSERT_GE(file_meta_from_cf.oldest_ancester_time, start_time);
       ASSERT_LE(file_meta_from_cf.oldest_ancester_time, end_time);
-      // More from FileStorageInfo
-      ASSERT_EQ(file_meta_from_cf.file_type, kTableFile);
-      ASSERT_EQ(file_meta_from_cf.name,
-                "/" + file_meta_from_cf.relative_filename);
-      ASSERT_EQ(file_meta_from_cf.directory, file_meta_from_cf.db_path);
     }
 
     ASSERT_EQ(level_meta_from_cf.size, level_size);
@@ -1127,11 +1122,6 @@ void CheckLiveFilesMeta(
     ASSERT_EQ(meta.oldest_blob_file_number,
               expected_meta.oldest_blob_file_number);
 
-    // More from FileStorageInfo
-    ASSERT_EQ(meta.file_type, kTableFile);
-    ASSERT_EQ(meta.name, "/" + meta.relative_filename);
-    ASSERT_EQ(meta.directory, meta.db_path);
-
     ++i;
   }
 }

db/db_test_util.h

@@ -894,8 +894,8 @@ class CacheWrapper : public Cache {
   bool Ref(Handle* handle) override { return target_->Ref(handle); }
 
   using Cache::Release;
-  bool Release(Handle* handle, bool erase_if_last_ref = false) override {
-    return target_->Release(handle, erase_if_last_ref);
+  bool Release(Handle* handle, bool force_erase = false) override {
+    return target_->Release(handle, force_erase);
   }
 
   void* Value(Handle* handle) override { return target_->Value(handle); }

db/db_wal_test.cc

@@ -1491,93 +1491,6 @@ TEST_F(DBWALTest, kPointInTimeRecoveryCFConsistency) {
   ASSERT_NOK(TryReopenWithColumnFamilies({"default", "one", "two"}, options));
 }
 
-TEST_F(DBWALTest, RaceInstallFlushResultsWithWalObsoletion) {
-  Options options = CurrentOptions();
-  options.env = env_;
-  options.track_and_verify_wals_in_manifest = true;
-  // The following make sure there are two bg flush threads.
-  options.max_background_jobs = 8;
-
-  const std::string cf1_name("cf1");
-  CreateAndReopenWithCF({cf1_name}, options);
-  assert(handles_.size() == 2);
-
-  {
-    dbfull()->TEST_LockMutex();
-    ASSERT_LE(2, dbfull()->GetBGJobLimits().max_flushes);
-    dbfull()->TEST_UnlockMutex();
-  }
-
-  ASSERT_OK(dbfull()->PauseBackgroundWork());
-
-  ASSERT_OK(db_->Put(WriteOptions(), handles_[1], "foo", "value"));
-  ASSERT_OK(db_->Put(WriteOptions(), "foo", "value"));
-
-  ASSERT_OK(dbfull()->TEST_FlushMemTable(false, true, handles_[1]));
-
-  ASSERT_OK(db_->Put(WriteOptions(), "foo", "value"));
-  ASSERT_OK(dbfull()->TEST_FlushMemTable(false, true, handles_[0]));
-
-  bool called = false;
-  SyncPoint::GetInstance()->DisableProcessing();
-  SyncPoint::GetInstance()->ClearAllCallBacks();
-  // This callback will be called when the first bg flush thread reaches the
-  // point before entering the MANIFEST write queue after flushing the SST
-  // file.
-  // The purpose of the sync points here is to ensure both bg flush threads
-  // finish computing `min_wal_number_to_keep` before any of them updates the
-  // `log_number` for the column family that's being flushed.
-  SyncPoint::GetInstance()->SetCallBack(
-      "MemTableList::TryInstallMemtableFlushResults:AfterComputeMinWalToKeep",
-      [&](void* /*arg*/) {
-        dbfull()->mutex()->AssertHeld();
-        if (!called) {
-          // We are the first bg flush thread in the MANIFEST write queue.
-          // We set up the dependency between sync points for two threads that
-          // will be executing the same code.
-          // For the interleaving of events, see
-          // https://github.com/facebook/rocksdb/pull/9715.
-          // bg flush thread1 will release the db mutex while in the MANIFEST
-          // write queue. In the meantime, bg flush thread2 locks db mutex and
-          // computes the min_wal_number_to_keep (before thread1 writes to
-          // MANIFEST thus before cf1->log_number is updated). Bg thread2 joins
-          // the MANIFEST write queue afterwards and bg flush thread1 proceeds
-          // with writing to MANIFEST.
-          called = true;
-          SyncPoint::GetInstance()->LoadDependency({
-              {"VersionSet::LogAndApply:WriteManifestStart",
-               "DBWALTest::RaceInstallFlushResultsWithWalObsoletion:BgFlush2"},
-              {"DBWALTest::RaceInstallFlushResultsWithWalObsoletion:BgFlush2",
-               "VersionSet::LogAndApply:WriteManifest"},
-          });
-        } else {
-          // The other bg flush thread has already been in the MANIFEST write
-          // queue, and we are after.
-          TEST_SYNC_POINT(
-              "DBWALTest::RaceInstallFlushResultsWithWalObsoletion:BgFlush2");
-        }
-      });
-  SyncPoint::GetInstance()->EnableProcessing();
-
-  ASSERT_OK(dbfull()->ContinueBackgroundWork());
-
-  ASSERT_OK(dbfull()->TEST_WaitForFlushMemTable(handles_[0]));
-  ASSERT_OK(dbfull()->TEST_WaitForFlushMemTable(handles_[1]));
-
-  ASSERT_TRUE(called);
-
-  Close();
-
-  SyncPoint::GetInstance()->DisableProcessing();
-  SyncPoint::GetInstance()->ClearAllCallBacks();
-
-  DB* db1 = nullptr;
-  Status s = DB::OpenForReadOnly(options, dbname_, &db1);
-  ASSERT_OK(s);
-  assert(db1);
-  delete db1;
-}
-
 // Test scope:
 // - We expect to open data store under all circumstances
 // - We expect only data upto the point where the first error was encountered

db/event_helpers.cc

@@ -95,9 +95,8 @@ void EventHelpers::LogAndNotifyTableFileCreationFinished(
     jwriter << "cf_name" << cf_name << "job" << job_id << "event"
             << "table_file_creation"
             << "file_number" << fd.GetNumber() << "file_size"
-            << fd.GetFileSize() << "file_checksum"
-            << Slice(file_checksum).ToString(true) << "file_checksum_func_name"
-            << file_checksum_func_name;
+            << fd.GetFileSize() << "file_checksum" << file_checksum
+            << "file_checksum_func_name" << file_checksum_func_name;
 
     // table_properties
     {

db/filename_test.cc

@@ -171,69 +171,6 @@ TEST_F(FileNameTest, Construction) {
   ASSERT_EQ(kMetaDatabase, type);
 }
 
-TEST_F(FileNameTest, NormalizePath) {
-  // No leading slash
-  const std::string sep = std::string(1, kFilePathSeparator);
-
-  std::string expected = "FOLDER" + sep + "filename.ext";
-  std::string given = "FOLDER" + sep + "filename.ext";
-
-  ASSERT_EQ(expected, NormalizePath(given));
-
-  // Two chars /a
-
-  expected = sep + "a";
-  given = expected;
-  ASSERT_EQ(expected, NormalizePath(given));
-
-  // Two chars a/
-  expected = "a" + sep;
-  given = expected;
-  ASSERT_EQ(expected, NormalizePath(given));
-
-  // Server only
-  expected = sep + sep + "a";
-  given = expected;
-  ASSERT_EQ(expected, NormalizePath(given));
-
-  // Two slashes after character
-  expected = "a" + sep;
-  given = "a" + sep + sep;
-
-  ASSERT_EQ(expected, NormalizePath(given));
-
-  // slash only   /
-  expected = sep;
-  given = expected;
-  ASSERT_EQ(expected, NormalizePath(given));
-
-  // UNC only   //
-  expected = sep;
-  given = sep + sep;
-
-  ASSERT_EQ(expected, NormalizePath(given));
-
-  // 3 slashesy   //
-  expected = sep + sep;
-  given = sep + sep + sep;
-  ASSERT_EQ(expected, NormalizePath(given));
-
-  // 3 slashes   //
-  expected = sep + sep + "a" + sep;
-  given = sep + sep + sep + "a" + sep;
-  ASSERT_EQ(expected, NormalizePath(given));
-
-  // 2 separators in the middle
-  expected = "a" + sep + "b";
-  given = "a" + sep + sep + "b";
-  ASSERT_EQ(expected, NormalizePath(given));
-
-  // UNC with duplicate slashes
-  expected = sep + sep + "SERVER" + sep + "a" + sep + "b" + sep + "c";
-  given = sep + sep + "SERVER" + sep + "a" + sep + sep + "b" + sep + "c";
-  ASSERT_EQ(expected, NormalizePath(given));
-}
-
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

db/memtable_list.cc

@@ -489,8 +489,8 @@ Status MemTableList::TryInstallMemtableFlushResults(
     // TODO(myabandeh): Not sure how batch_count could be 0 here.
     if (batch_count > 0) {
       uint64_t min_wal_number_to_keep = 0;
-      assert(edit_list.size() > 0);
       if (vset->db_options()->allow_2pc) {
+        assert(edit_list.size() > 0);
         // Note that if mempurge is successful, the edit_list will
         // not be applicable (contains info of new min_log number to keep,
         // and level 0 file path of SST file created during normal flush,
@@ -499,26 +499,24 @@ Status MemTableList::TryInstallMemtableFlushResults(
         min_wal_number_to_keep = PrecomputeMinLogNumberToKeep2PC(
             vset, *cfd, edit_list, memtables_to_flush, prep_tracker);
 
-        // We piggyback the information of earliest log file to keep in the
+        // We piggyback the information of  earliest log file to keep in the
         // manifest entry for the last file flushed.
-      } else {
-        min_wal_number_to_keep =
-            PrecomputeMinLogNumberToKeepNon2PC(vset, *cfd, edit_list);
+        edit_list.back()->SetMinLogNumberToKeep(min_wal_number_to_keep);
       }
 
-      VersionEdit wal_deletion;
-      wal_deletion.SetMinLogNumberToKeep(min_wal_number_to_keep);
+      std::unique_ptr<VersionEdit> wal_deletion;
       if (vset->db_options()->track_and_verify_wals_in_manifest) {
+        if (!vset->db_options()->allow_2pc) {
+          min_wal_number_to_keep =
+              PrecomputeMinLogNumberToKeepNon2PC(vset, *cfd, edit_list);
+        }
         if (min_wal_number_to_keep >
             vset->GetWalSet().GetMinWalNumberToKeep()) {
-          wal_deletion.DeleteWalsBefore(min_wal_number_to_keep);
+          wal_deletion.reset(new VersionEdit);
+          wal_deletion->DeleteWalsBefore(min_wal_number_to_keep);
+          edit_list.push_back(wal_deletion.get());
         }
-        TEST_SYNC_POINT_CALLBACK(
-            "MemTableList::TryInstallMemtableFlushResults:"
-            "AfterComputeMinWalToKeep",
-            nullptr);
       }
-      edit_list.push_back(&wal_deletion);
 
       const auto manifest_write_cb = [this, cfd, batch_count, log_buffer,
                                       to_delete, mu](const Status& status) {
@@ -800,19 +798,22 @@ Status InstallMemtableAtomicFlushResults(
   if (vset->db_options()->allow_2pc) {
     min_wal_number_to_keep = PrecomputeMinLogNumberToKeep2PC(
         vset, cfds, edit_lists, mems_list, prep_tracker);
-  } else {
-    min_wal_number_to_keep =
-        PrecomputeMinLogNumberToKeepNon2PC(vset, cfds, edit_lists);
+    edit_lists.back().back()->SetMinLogNumberToKeep(min_wal_number_to_keep);
   }
 
-  VersionEdit wal_deletion;
-  wal_deletion.SetMinLogNumberToKeep(min_wal_number_to_keep);
-  if (vset->db_options()->track_and_verify_wals_in_manifest &&
-      min_wal_number_to_keep > vset->GetWalSet().GetMinWalNumberToKeep()) {
-    wal_deletion.DeleteWalsBefore(min_wal_number_to_keep);
+  std::unique_ptr<VersionEdit> wal_deletion;
+  if (vset->db_options()->track_and_verify_wals_in_manifest) {
+    if (!vset->db_options()->allow_2pc) {
+      min_wal_number_to_keep =
+          PrecomputeMinLogNumberToKeepNon2PC(vset, cfds, edit_lists);
+    }
+    if (min_wal_number_to_keep > vset->GetWalSet().GetMinWalNumberToKeep()) {
+      wal_deletion.reset(new VersionEdit);
+      wal_deletion->DeleteWalsBefore(min_wal_number_to_keep);
+      edit_lists.back().push_back(wal_deletion.get());
+      ++num_entries;
+    }
   }
-  edit_lists.back().push_back(&wal_deletion);
-  ++num_entries;
 
   // Mark the version edits as an atomic group if the number of version edits
   // exceeds 1.

db/version_edit.cc

@@ -120,9 +120,6 @@ bool VersionEdit::EncodeTo(std::string* dst) const {
   if (has_max_column_family_) {
     PutVarint32Varint32(dst, kMaxColumnFamily, max_column_family_);
   }
-  if (has_min_log_number_to_keep_) {
-    PutVarint32Varint64(dst, kMinLogNumberToKeep, min_log_number_to_keep_);
-  }
   if (has_last_sequence_) {
     PutVarint32Varint64(dst, kLastSequence, last_sequence_);
   }

db/version_edit_handler.cc

@@ -394,7 +394,7 @@ void VersionEditHandler::CheckIterationResult(const log::Reader& reader,
   if (s->ok()) {
     version_set_->GetColumnFamilySet()->UpdateMaxColumnFamily(
         version_edit_params_.max_column_family_);
-    version_set_->MarkMinLogNumberToKeep(
+    version_set_->MarkMinLogNumberToKeep2PC(
         version_edit_params_.min_log_number_to_keep_);
     version_set_->MarkFileNumberUsed(version_edit_params_.prev_log_number_);
     version_set_->MarkFileNumberUsed(version_edit_params_.log_number_);
@@ -970,11 +970,12 @@ void DumpManifestHandler::CheckIterationResult(const log::Reader& reader,
   fprintf(stdout,
           "next_file_number %" PRIu64 " last_sequence %" PRIu64
           "  prev_log_number %" PRIu64 " max_column_family %" PRIu32
-          " min_log_number_to_keep %" PRIu64 "\n",
+          " min_log_number_to_keep "
+          "%" PRIu64 "\n",
           version_set_->current_next_file_number(),
           version_set_->LastSequence(), version_set_->prev_log_number(),
           version_set_->column_family_set_->GetMaxColumnFamily(),
-          version_set_->min_log_number_to_keep());
+          version_set_->min_log_number_to_keep_2pc());
 }
 
 }  // namespace ROCKSDB_NAMESPACE

db/version_set.cc

@@ -27,7 +27,6 @@
 #include "db/blob/blob_log_format.h"
 #include "db/compaction/compaction.h"
 #include "db/compaction/file_pri.h"
-#include "db/dbformat.h"
 #include "db/internal_stats.h"
 #include "db/log_reader.h"
 #include "db/log_writer.h"
@@ -1476,7 +1475,7 @@ void Version::GetColumnFamilyMetaData(ColumnFamilyMetaData* cf_meta) {
       const uint64_t file_number = file->fd.GetNumber();
       files.emplace_back(
           MakeTableFileName("", file_number), file_number, file_path,
-          file->fd.GetFileSize(), file->fd.smallest_seqno,
+          static_cast<size_t>(file->fd.GetFileSize()), file->fd.smallest_seqno,
           file->fd.largest_seqno, file->smallest.user_key().ToString(),
           file->largest.user_key().ToString(),
           file->stats.num_reads_sampled.load(std::memory_order_relaxed),
@@ -4142,7 +4141,7 @@ void VersionSet::Reset() {
   }
   db_id_.clear();
   next_file_number_.store(2);
-  min_log_number_to_keep_.store(0);
+  min_log_number_to_keep_2pc_.store(0);
   manifest_file_number_ = 0;
   options_file_number_ = 0;
   pending_manifest_file_number_ = 0;
@@ -4611,7 +4610,8 @@ Status VersionSet::ProcessManifestWrites(
       }
 
       if (last_min_log_number_to_keep != 0) {
-        MarkMinLogNumberToKeep(last_min_log_number_to_keep);
+        // Should only be set in 2PC mode.
+        MarkMinLogNumberToKeep2PC(last_min_log_number_to_keep);
       }
 
       for (int i = 0; i < static_cast<int>(versions.size()); ++i) {
@@ -4965,7 +4965,7 @@ Status VersionSet::Recover(
         ",min_log_number_to_keep is %" PRIu64 "\n",
         manifest_path.c_str(), manifest_file_number_, next_file_number_.load(),
         last_sequence_.load(), log_number, prev_log_number_,
-        column_family_set_->GetMaxColumnFamily(), min_log_number_to_keep());
+        column_family_set_->GetMaxColumnFamily(), min_log_number_to_keep_2pc());
 
     for (auto cfd : *column_family_set_) {
       if (cfd->IsDropped()) {
@@ -5126,6 +5126,9 @@ Status VersionSet::TryRecoverFromOneManifest(
 Status VersionSet::ListColumnFamilies(std::vector<std::string>* column_families,
                                       const std::string& dbname,
                                       FileSystem* fs) {
+  // these are just for performance reasons, not correctness,
+  // so we're fine using the defaults
+  FileOptions soptions;
   // Read "CURRENT" file, which contains a pointer to the current manifest file
   std::string manifest_path;
   uint64_t manifest_file_number;
@@ -5134,24 +5137,16 @@ Status VersionSet::ListColumnFamilies(std::vector<std::string>* column_families,
   if (!s.ok()) {
     return s;
   }
-  return ListColumnFamiliesFromManifest(manifest_path, fs, column_families);
-}
 
-Status VersionSet::ListColumnFamiliesFromManifest(
-    const std::string& manifest_path, FileSystem* fs,
-    std::vector<std::string>* column_families) {
   std::unique_ptr<SequentialFileReader> file_reader;
-  Status s;
   {
     std::unique_ptr<FSSequentialFile> file;
-    // these are just for performance reasons, not correctness,
-    // so we're fine using the defaults
-    s = fs->NewSequentialFile(manifest_path, FileOptions(), &file, nullptr);
+    s = fs->NewSequentialFile(manifest_path, soptions, &file, nullptr);
     if (!s.ok()) {
       return s;
-    }
-    file_reader = std::make_unique<SequentialFileReader>(
-        std::move(file), manifest_path, /*io_tracer=*/nullptr);
+  }
+  file_reader.reset(new SequentialFileReader(std::move(file), manifest_path,
+                                             nullptr /*IOTracer*/));
   }
 
   VersionSet::LogReporter reporter;
@@ -5341,16 +5336,9 @@ Status VersionSet::GetLiveFilesChecksumInfo(FileChecksumList* checksum_list) {
 
 Status VersionSet::DumpManifest(Options& options, std::string& dscname,
                                 bool verbose, bool hex, bool json) {
-  assert(options.env);
-  std::vector<std::string> column_families;
-  Status s = ListColumnFamiliesFromManifest(
-      dscname, options.env->GetFileSystem().get(), &column_families);
-  if (!s.ok()) {
-    return s;
-  }
-
   // Open the specified manifest file.
   std::unique_ptr<SequentialFileReader> file_reader;
+  Status s;
   {
     std::unique_ptr<FSSequentialFile> file;
     const std::shared_ptr<FileSystem>& fs = options.env->GetFileSystem();
@@ -5361,16 +5349,14 @@ Status VersionSet::DumpManifest(Options& options, std::string& dscname,
     if (!s.ok()) {
       return s;
     }
-    file_reader = std::make_unique<SequentialFileReader>(
-        std::move(file), dscname, db_options_->log_readahead_size, io_tracer_);
+    file_reader.reset(new SequentialFileReader(
+        std::move(file), dscname, db_options_->log_readahead_size, io_tracer_));
   }
 
-  std::vector<ColumnFamilyDescriptor> cf_descs;
-  for (const auto& cf : column_families) {
-    cf_descs.emplace_back(cf, options);
-  }
-
-  DumpManifestHandler handler(cf_descs, this, io_tracer_, verbose, hex, json);
+  std::vector<ColumnFamilyDescriptor> column_families(
+      1, ColumnFamilyDescriptor(kDefaultColumnFamilyName, options));
+  DumpManifestHandler handler(column_families, this, io_tracer_, verbose, hex,
+                              json);
   {
     VersionSet::LogReporter reporter;
     reporter.status = &s;
@@ -5392,9 +5378,9 @@ void VersionSet::MarkFileNumberUsed(uint64_t number) {
 }
 // Called only either from ::LogAndApply which is protected by mutex or during
 // recovery which is single-threaded.
-void VersionSet::MarkMinLogNumberToKeep(uint64_t number) {
-  if (min_log_number_to_keep_.load(std::memory_order_relaxed) < number) {
-    min_log_number_to_keep_.store(number, std::memory_order_relaxed);
+void VersionSet::MarkMinLogNumberToKeep2PC(uint64_t number) {
+  if (min_log_number_to_keep_2pc_.load(std::memory_order_relaxed) < number) {
+    min_log_number_to_keep_2pc_.store(number, std::memory_order_relaxed);
   }
 }
 
@@ -5520,7 +5506,7 @@ Status VersionSet::WriteCurrentStateToManifest(
         // min_log_number_to_keep is for the whole db, not for specific column family.
         // So it does not need to be set for every column family, just need to be set once.
         // Since default CF can never be dropped, we set the min_log to the default CF here.
-        uint64_t min_log = min_log_number_to_keep();
+        uint64_t min_log = min_log_number_to_keep_2pc();
         if (min_log != 0) {
           edit.SetMinLogNumberToKeep(min_log);
         }
@@ -5817,9 +5803,7 @@ void VersionSet::AddLiveFiles(std::vector<uint64_t>* live_table_files,
 InternalIterator* VersionSet::MakeInputIterator(
     const ReadOptions& read_options, const Compaction* c,
     RangeDelAggregator* range_del_agg,
-    const FileOptions& file_options_compactions,
-    const std::optional<const Slice>& start,
-    const std::optional<const Slice>& end) {
+    const FileOptions& file_options_compactions) {
   auto cfd = c->column_family_data();
   // Level-0 files have to be merged together.  For other levels,
   // we will make a concatenating iterator per level.
@@ -5834,25 +5818,10 @@ InternalIterator* VersionSet::MakeInputIterator(
       if (c->level(which) == 0) {
         const LevelFilesBrief* flevel = c->input_levels(which);
         for (size_t i = 0; i < flevel->num_files; i++) {
-          const FileMetaData& fmd = *flevel->files[i].file_metadata;
-          if (start.has_value() &&
-              cfd->user_comparator()->Compare(start.value(),
-                                              fmd.largest.user_key()) > 0) {
-            continue;
-          }
-          // We should be able to filter out the case where the end key
-          // equals to the end boundary, since the end key is exclusive.
-          // We try to be extra safe here.
-          if (end.has_value() &&
-              cfd->user_comparator()->Compare(end.value(),
-                                              fmd.smallest.user_key()) < 0) {
-            continue;
-          }
-
           list[num++] = cfd->table_cache()->NewIterator(
               read_options, file_options_compactions,
-              cfd->internal_comparator(), fmd, range_del_agg,
-              c->mutable_cf_options()->prefix_extractor,
+              cfd->internal_comparator(), *flevel->files[i].file_metadata,
+              range_del_agg, c->mutable_cf_options()->prefix_extractor,
               /*table_reader_ptr=*/nullptr,
               /*file_read_hist=*/nullptr, TableReaderCaller::kCompaction,
               /*arena=*/nullptr,
@@ -5925,13 +5894,11 @@ void VersionSet::GetLiveFilesMetaData(std::vector<LiveFileMetaData>* metadata) {
           assert(!cfd->ioptions()->cf_paths.empty());
           filemetadata.db_path = cfd->ioptions()->cf_paths.back().path;
         }
-        filemetadata.directory = filemetadata.db_path;
         const uint64_t file_number = file->fd.GetNumber();
         filemetadata.name = MakeTableFileName("", file_number);
-        filemetadata.relative_filename = filemetadata.name.substr(1);
         filemetadata.file_number = file_number;
         filemetadata.level = level;
-        filemetadata.size = file->fd.GetFileSize();
+        filemetadata.size = static_cast<size_t>(file->fd.GetFileSize());
         filemetadata.smallestkey = file->smallest.user_key().ToString();
         filemetadata.largestkey = file->largest.user_key().ToString();
         filemetadata.smallest_seqno = file->fd.smallest_seqno;

db/version_set.h

@@ -24,7 +24,6 @@
 #include <limits>
 #include <map>
 #include <memory>
-#include <optional>
 #include <set>
 #include <string>
 #include <unordered_set>
@@ -1091,9 +1090,6 @@ class VersionSet {
   // column_families.
   static Status ListColumnFamilies(std::vector<std::string>* column_families,
                                    const std::string& dbname, FileSystem* fs);
-  static Status ListColumnFamiliesFromManifest(
-      const std::string& manifest_path, FileSystem* fs,
-      std::vector<std::string>* column_families);
 
 #ifndef ROCKSDB_LITE
   // Try to reduce the number of levels. This call is valid when
@@ -1132,8 +1128,8 @@ class VersionSet {
 
   uint64_t current_next_file_number() const { return next_file_number_.load(); }
 
-  uint64_t min_log_number_to_keep() const {
-    return min_log_number_to_keep_.load();
+  uint64_t min_log_number_to_keep_2pc() const {
+    return min_log_number_to_keep_2pc_.load();
   }
 
   // Allocate and return a new file number
@@ -1191,7 +1187,7 @@ class VersionSet {
   // Mark the specified log number as deleted
   // REQUIRED: this is only called during single-threaded recovery or repair, or
   // from ::LogAndApply where the global mutex is held.
-  void MarkMinLogNumberToKeep(uint64_t number);
+  void MarkMinLogNumberToKeep2PC(uint64_t number);
 
   // Return the log file number for the log file that is currently
   // being compacted, or zero if there is no such log file.
@@ -1200,12 +1196,10 @@ class VersionSet {
   // Returns the minimum log number which still has data not flushed to any SST
   // file.
   // In non-2PC mode, all the log numbers smaller than this number can be safely
-  // deleted, although we still use `min_log_number_to_keep_` to determine when
-  // to delete a WAL file.
+  // deleted.
   uint64_t MinLogNumberWithUnflushedData() const {
     return PreComputeMinLogNumberWithUnflushedData(nullptr);
   }
-
   // Returns the minimum log number which still has data not flushed to any SST
   // file.
   // Empty column families' log number is considered to be
@@ -1262,13 +1256,10 @@ class VersionSet {
   // Create an iterator that reads over the compaction inputs for "*c".
   // The caller should delete the iterator when no longer needed.
   // @param read_options Must outlive the returned iterator.
-  // @param start, end indicates compaction range
   InternalIterator* MakeInputIterator(
       const ReadOptions& read_options, const Compaction* c,
       RangeDelAggregator* range_del_agg,
-      const FileOptions& file_options_compactions,
-      const std::optional<const Slice>& start,
-      const std::optional<const Slice>& end);
+      const FileOptions& file_options_compactions);
 
   // Add all files listed in any live version to *live_table_files and
   // *live_blob_files. Note that these lists may contain duplicates.
@@ -1306,10 +1297,6 @@ class VersionSet {
                         uint64_t min_pending_output);
 
   ColumnFamilySet* GetColumnFamilySet() { return column_family_set_.get(); }
-  RefedColumnFamilySet GetRefedColumnFamilySet() {
-    return RefedColumnFamilySet(GetColumnFamilySet());
-  }
-
   const FileOptions& file_options() { return file_options_; }
   void ChangeFileOptions(const MutableDBOptions& new_options) {
     file_options_.writable_file_max_buffer_size =
@@ -1412,8 +1399,9 @@ class VersionSet {
   const ImmutableDBOptions* const db_options_;
   std::atomic<uint64_t> next_file_number_;
   // Any WAL number smaller than this should be ignored during recovery,
-  // and is qualified for being deleted.
-  std::atomic<uint64_t> min_log_number_to_keep_ = {0};
+  // and is qualified for being deleted in 2PC mode. In non-2PC mode, this
+  // number is ignored.
+  std::atomic<uint64_t> min_log_number_to_keep_2pc_ = {0};
   uint64_t manifest_file_number_;
   uint64_t options_file_number_;
   uint64_t options_file_size_;

db/version_set_test.cc

@@ -3424,7 +3424,6 @@ TEST_F(VersionSetTestMissingFiles, NoFileMissing) {
 }
 
 TEST_F(VersionSetTestMissingFiles, MinLogNumberToKeep2PC) {
-  db_options_.allow_2pc = true;
   NewDB();
 
   SstInfo sst(100, kDefaultColumnFamilyName, "a");
@@ -3436,12 +3435,12 @@ TEST_F(VersionSetTestMissingFiles, MinLogNumberToKeep2PC) {
   edit.AddFile(0, file_metas[0]);
   edit.SetMinLogNumberToKeep(kMinWalNumberToKeep2PC);
   ASSERT_OK(LogAndApplyToDefaultCF(edit));
-  ASSERT_EQ(versions_->min_log_number_to_keep(), kMinWalNumberToKeep2PC);
+  ASSERT_EQ(versions_->min_log_number_to_keep_2pc(), kMinWalNumberToKeep2PC);
 
   for (int i = 0; i < 3; i++) {
     CreateNewManifest();
     ReopenDB();
-    ASSERT_EQ(versions_->min_log_number_to_keep(), kMinWalNumberToKeep2PC);
+    ASSERT_EQ(versions_->min_log_number_to_keep_2pc(), kMinWalNumberToKeep2PC);
   }
 }
 

db/write_batch.cc

@@ -22,13 +22,12 @@
 //    kTypeColumnFamilySingleDeletion varint32 varstring
 //    kTypeColumnFamilyRangeDeletion varint32 varstring varstring
 //    kTypeColumnFamilyMerge varint32 varstring varstring
-//    kTypeBeginPrepareXID
-//    kTypeEndPrepareXID varstring
+//    kTypeBeginPrepareXID varstring
+//    kTypeEndPrepareXID
 //    kTypeCommitXID varstring
-//    kTypeCommitXIDAndTimestamp varstring varstring
 //    kTypeRollbackXID varstring
-//    kTypeBeginPersistedPrepareXID
-//    kTypeBeginUnprepareXID
+//    kTypeBeginPersistedPrepareXID varstring
+//    kTypeBeginUnprepareXID varstring
 //    kTypeNoop
 // varstring :=
 //    len: varint32

db_stress_tool/db_stress_common.h

@@ -134,7 +134,6 @@ DECLARE_int32(set_in_place_one_in);
 DECLARE_int64(cache_size);
 DECLARE_int32(cache_numshardbits);
 DECLARE_bool(cache_index_and_filter_blocks);
-DECLARE_bool(reserve_table_reader_memory);
 DECLARE_int32(top_level_index_pinning);
 DECLARE_int32(partition_pinning);
 DECLARE_int32(unpartitioned_pinning);
@@ -283,10 +282,6 @@ DECLARE_uint64(wp_snapshot_cache_bits);
 DECLARE_uint64(wp_commit_cache_bits);
 #endif  // !ROCKSDB_LITE
 
-DECLARE_bool(adaptive_readahead);
-DECLARE_bool(async_io);
-DECLARE_string(wal_compression);
-
 constexpr long KB = 1024;
 constexpr int kRandomValueMaxFactor = 3;
 constexpr int kValueMaxLen = 100;

db_stress_tool/db_stress_gflags.cc

@@ -304,11 +304,6 @@ DEFINE_int32(cache_numshardbits, 6,
 DEFINE_bool(cache_index_and_filter_blocks, false,
             "True if indexes/filters should be cached in block cache.");
 
-DEFINE_bool(reserve_table_reader_memory, false,
-            "A dynamically updating charge to block cache, loosely based on "
-            "the actual memory usage of table reader, will occur to account "
-            "the memory, if block cache available.");
-
 DEFINE_int32(
     top_level_index_pinning,
     static_cast<int32_t>(ROCKSDB_NAMESPACE::PinningTier::kFallback),
@@ -917,14 +912,4 @@ DEFINE_uint64(wp_commit_cache_bits, 23ull,
               "commit cache. Default: 23 (8M entries)");
 #endif  // !ROCKSDB_LITE
 
-DEFINE_bool(adaptive_readahead, false,
-            "Carry forward internal auto readahead size from one file to next "
-            "file at each level during iteration");
-DEFINE_bool(
-    async_io, false,
-    "Does asynchronous prefetching when internal auto readahead is enabled");
-
-DEFINE_string(wal_compression, "none",
-              "Algorithm to use for WAL compression. none to disable.");
-
 #endif  // GFLAGS

db_stress_tool/db_stress_test_base.cc

@@ -21,7 +21,7 @@
 #include "rocksdb/utilities/object_registry.h"
 #include "test_util/testutil.h"
 #include "util/cast_util.h"
-#include "utilities/backup/backup_engine_impl.h"
+#include "utilities/backupable/backupable_db_impl.h"
 #include "utilities/fault_injection_fs.h"
 #include "utilities/fault_injection_secondary_cache.h"
 
@@ -645,8 +645,6 @@ void StressTest::OperateDb(ThreadState* thread) {
   ReadOptions read_opts(FLAGS_verify_checksum, true);
   read_opts.rate_limiter_priority =
       FLAGS_rate_limit_user_ops ? Env::IO_USER : Env::IO_TOTAL;
-  read_opts.async_io = FLAGS_async_io;
-  read_opts.adaptive_readahead = FLAGS_adaptive_readahead;
   WriteOptions write_opts;
   if (FLAGS_rate_limit_auto_wal_flush) {
     write_opts.rate_limiter_priority = Env::IO_USER;
@@ -2296,8 +2294,6 @@ void StressTest::PrintEnv() const {
           static_cast<int>(FLAGS_fail_if_options_file_error));
   fprintf(stdout, "User timestamp size bytes : %d\n",
           static_cast<int>(FLAGS_user_timestamp_size));
-  fprintf(stdout, "WAL compression           : %s\n",
-          FLAGS_wal_compression.c_str());
 
   fprintf(stdout, "------------------------------------------------\n");
 }
@@ -2321,8 +2317,6 @@ void StressTest::Open() {
     block_based_options.block_cache_compressed = compressed_cache_;
     block_based_options.checksum = checksum_type_e;
     block_based_options.block_size = FLAGS_block_size;
-    block_based_options.reserve_table_reader_memory =
-        FLAGS_reserve_table_reader_memory;
     block_based_options.format_version =
         static_cast<uint32_t>(FLAGS_format_version);
     block_based_options.index_block_restart_interval =
@@ -2440,9 +2434,6 @@ void StressTest::Open() {
         FLAGS_blob_garbage_collection_force_threshold;
     options_.blob_compaction_readahead_size =
         FLAGS_blob_compaction_readahead_size;
-
-    options_.wal_compression =
-        StringToCompressionType(FLAGS_wal_compression.c_str());
   } else {
 #ifdef ROCKSDB_LITE
     fprintf(stderr, "--options_file not supported in lite mode\n");

db_stress_tool/no_batched_ops_stress.cc

@@ -45,8 +45,8 @@ class NonBatchedOpsStressTest : public StressTest {
       if (thread->shared->HasVerificationFailedYet()) {
         break;
       }
-      if (thread->rand.OneIn(4)) {
-        // 1/4 chance use iterator to verify this range
+      if (thread->rand.OneIn(3)) {
+        // 1/3 chance use iterator to verify this range
         Slice prefix;
         std::string seek_key = Key(start);
         std::unique_ptr<Iterator> iter(
@@ -91,8 +91,8 @@ class NonBatchedOpsStressTest : public StressTest {
                           from_db.data(), from_db.length());
           }
         }
-      } else if (thread->rand.OneIn(3)) {
-        // 1/4 chance use Get to verify this range
+      } else if (thread->rand.OneIn(2)) {
+        // 1/3 chance use Get to verify this range
         for (auto i = start; i < end; i++) {
           if (thread->shared->HasVerificationFailedYet()) {
             break;
@@ -108,8 +108,8 @@ class NonBatchedOpsStressTest : public StressTest {
                           from_db.data(), from_db.length());
           }
         }
-      } else if (thread->rand.OneIn(2)) {
-        // 1/4 chance use MultiGet to verify this range
+      } else {
+        // 1/3 chance use MultiGet to verify this range
         for (auto i = start; i < end;) {
           if (thread->shared->HasVerificationFailedYet()) {
             break;
@@ -140,47 +140,6 @@ class NonBatchedOpsStressTest : public StressTest {
 
           i += batch_size;
         }
-      } else {
-        // 1/4 chance use GetMergeOperand to verify this range
-        // Start off with small size that will be increased later if necessary
-        std::vector<PinnableSlice> values(4);
-        GetMergeOperandsOptions merge_operands_info;
-        merge_operands_info.expected_max_number_of_operands =
-            static_cast<int>(values.size());
-        for (auto i = start; i < end; i++) {
-          if (thread->shared->HasVerificationFailedYet()) {
-            break;
-          }
-          std::string from_db;
-          std::string keystr = Key(i);
-          Slice k = keystr;
-          int number_of_operands = 0;
-          Status s = db_->GetMergeOperands(options, column_families_[cf], k,
-                                           values.data(), &merge_operands_info,
-                                           &number_of_operands);
-          if (s.IsIncomplete()) {
-            // Need to resize values as there are more than values.size() merge
-            // operands on this key. Should only happen a few times when we
-            // encounter a key that had more merge operands than any key seen so
-            // far
-            values.resize(number_of_operands);
-            merge_operands_info.expected_max_number_of_operands =
-                static_cast<int>(number_of_operands);
-            s = db_->GetMergeOperands(options, column_families_[cf], k,
-                                      values.data(), &merge_operands_info,
-                                      &number_of_operands);
-          }
-          // Assumed here that GetMergeOperands always sets number_of_operand
-          if (number_of_operands) {
-            from_db = values[number_of_operands - 1].ToString();
-          }
-          VerifyValue(static_cast<int>(cf), i, options, shared, from_db, s,
-                      true);
-          if (from_db.length()) {
-            PrintKeyValue(static_cast<int>(cf), static_cast<uint32_t>(i),
-                          from_db.data(), from_db.length());
-          }
-        }
       }
     }
   }

env/env_test.cc

@@ -85,8 +85,6 @@ struct Deleter {
   void (*fn_)(void*);
 };
 
-extern "C" bool RocksDbIOUringEnable() { return true; }
-
 std::unique_ptr<char, Deleter> NewAligned(const size_t size, const char ch) {
   char* ptr = nullptr;
 #ifdef OS_WIN
@@ -1258,7 +1256,7 @@ TEST_P(EnvPosixTestWithParam, MultiRead) {
     // Random Read
     Random rnd(301 + attempt);
     ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
-        "UpdateResults::io_uring_result", [&](void* arg) {
+        "UpdateResults:io_uring_result", [&](void* arg) {
           if (attempt > 0) {
             // No failure in the first attempt.
             size_t& bytes_read = *static_cast<size_t*>(arg);
@@ -1328,7 +1326,7 @@ TEST_F(EnvPosixTest, MultiReadNonAlignedLargeNum) {
     const int num_reads = rnd.Uniform(512) + 1;
 
     ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
-        "UpdateResults::io_uring_result", [&](void* arg) {
+        "UpdateResults:io_uring_result", [&](void* arg) {
           if (attempt > 5) {
             // Improve partial result rates in second half of the run to
             // cover the case of repeated partial results.
@@ -3310,6 +3308,7 @@ TEST_F(TestAsyncRead, ReadAsync) {
     }
   }
 }
+
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

env/fs_posix.cc

@@ -1045,12 +1045,9 @@ class PosixFileSystem : public FileSystem {
 
   // EXPERIMENTAL
   //
-  // TODO akankshamahajan:
-  // 1. Update Poll API to take into account min_completions
+  // TODO akankshamahajan: Update Poll API to take into account min_completions
   // and returns if number of handles in io_handles (any order) completed is
   // equal to atleast min_completions.
-  // 2. Currently in case of direct_io, Read API is called because of which call
-  // to Poll API fails as it expects IOHandle to be populated.
   virtual IOStatus Poll(std::vector<void*>& io_handles,
                         size_t /*min_completions*/) override {
 #if defined(ROCKSDB_IOURING_PRESENT)
@@ -1097,14 +1094,12 @@ class PosixFileSystem : public FileSystem {
         req.offset = posix_handle->offset;
         req.len = posix_handle->len;
         size_t finished_len = 0;
-        size_t bytes_read = 0;
         UpdateResult(cqe, "", req.len, posix_handle->iov.iov_len,
-                     true /*async_read*/, finished_len, &req, bytes_read);
+                     true /*async_read*/, finished_len, &req);
         posix_handle->is_finished = true;
         io_uring_cqe_seen(iu, cqe);
         posix_handle->cb(req, posix_handle->cb_arg);
         (void)finished_len;
-        (void)bytes_read;
 
         if (static_cast<Posix_IOHandle*>(io_handles[i]) == posix_handle) {
           break;

env/io_posix.cc

@@ -744,36 +744,31 @@ IOStatus PosixRandomAccessFile::MultiRead(FSReadRequest* reqs,
       wrap_cache.erase(wrap_check);
 
       FSReadRequest* req = req_wrap->req;
-      size_t bytes_read = 0;
       UpdateResult(cqe, filename_, req->len, req_wrap->iov.iov_len,
-                   false /*async_read*/, req_wrap->finished_len, req,
-                   bytes_read);
+                   false /*async_read*/, req_wrap->finished_len, req);
       int32_t res = cqe->res;
-      if (res >= 0) {
-        if (bytes_read == 0) {
-          /// cqe->res == 0 can means EOF, or can mean partial results. See
-          // comment
-          // https://github.com/facebook/rocksdb/pull/6441#issuecomment-589843435
-          // Fall back to pread in this case.
-          if (use_direct_io() &&
-              !IsSectorAligned(req_wrap->finished_len,
-                               GetRequiredBufferAlignment())) {
-            // Bytes reads don't fill sectors. Should only happen at the end
-            // of the file.
-            req->result = Slice(req->scratch, req_wrap->finished_len);
-            req->status = IOStatus::OK();
-          } else {
-            Slice tmp_slice;
-            req->status =
-                Read(req->offset + req_wrap->finished_len,
-                     req->len - req_wrap->finished_len, options, &tmp_slice,
-                     req->scratch + req_wrap->finished_len, dbg);
-            req->result =
-                Slice(req->scratch, req_wrap->finished_len + tmp_slice.size());
-          }
-        } else if (bytes_read < req_wrap->iov.iov_len) {
-          incomplete_rq_list.push_back(req_wrap);
+      if (res == 0) {
+        /// cqe->res == 0 can means EOF, or can mean partial results. See
+        // comment
+        // https://github.com/facebook/rocksdb/pull/6441#issuecomment-589843435
+        // Fall back to pread in this case.
+        if (use_direct_io() && !IsSectorAligned(req_wrap->finished_len,
+                                                GetRequiredBufferAlignment())) {
+          // Bytes reads don't fill sectors. Should only happen at the end
+          // of the file.
+          req->result = Slice(req->scratch, req_wrap->finished_len);
+          req->status = IOStatus::OK();
+        } else {
+          Slice tmp_slice;
+          req->status =
+              Read(req->offset + req_wrap->finished_len,
+                   req->len - req_wrap->finished_len, options, &tmp_slice,
+                   req->scratch + req_wrap->finished_len, dbg);
+          req->result =
+              Slice(req->scratch, req_wrap->finished_len + tmp_slice.size());
         }
+      } else if (res > 0 && res < static_cast<int32_t>(req_wrap->iov.iov_len)) {
+        incomplete_rq_list.push_back(req_wrap);
       }
       io_uring_cqe_seen(iu, cqe);
     }
@@ -901,8 +896,8 @@ IOStatus PosixRandomAccessFile::ReadAsync(
 
   // Initialize Posix_IOHandle.
   posix_handle->iu = iu;
-  posix_handle->iov.iov_base = req.scratch;
-  posix_handle->iov.iov_len = req.len;
+  posix_handle->iov.iov_base = posix_handle->scratch;
+  posix_handle->iov.iov_len = posix_handle->len;
   posix_handle->cb = cb;
   posix_handle->cb_arg = cb_arg;
   posix_handle->offset = req.offset;

env/io_posix.h

@@ -66,13 +66,12 @@ struct Posix_IOHandle {
 
 inline void UpdateResult(struct io_uring_cqe* cqe, const std::string& file_name,
                          size_t len, size_t iov_len, bool async_read,
-                         size_t& finished_len, FSReadRequest* req,
-                         size_t& bytes_read) {
+                         size_t& finished_len, FSReadRequest* req) {
   if (cqe->res < 0) {
     req->result = Slice(req->scratch, 0);
     req->status = IOError("Req failed", file_name, cqe->res);
   } else {
-    bytes_read = static_cast<size_t>(cqe->res);
+    size_t bytes_read = static_cast<size_t>(cqe->res);
     TEST_SYNC_POINT_CALLBACK("UpdateResults::io_uring_result", &bytes_read);
     if (bytes_read == iov_len) {
       req->result = Slice(req->scratch, req->len);

file/file_prefetch_buffer.cc

@@ -111,6 +111,13 @@ Status FilePrefetchBuffer::ReadAsync(const IOOptions& opts,
                                      Env::IOPriority rate_limiter_priority,
                                      uint64_t read_len, uint64_t chunk_len,
                                      uint64_t rounddown_start, uint32_t index) {
+  // Reset io_handle.
+  if (io_handle_ != nullptr && del_fn_ != nullptr) {
+    del_fn_(io_handle_);
+    io_handle_ = nullptr;
+    del_fn_ = nullptr;
+  }
+
   // callback for async read request.
   auto fp = std::bind(&FilePrefetchBuffer::PrefetchAsyncCallback, this,
                       std::placeholders::_1, std::placeholders::_2);
@@ -122,7 +129,6 @@ Status FilePrefetchBuffer::ReadAsync(const IOOptions& opts,
   req.scratch = bufs_[index].buffer_.BufferStart() + chunk_len;
   Status s = reader->ReadAsync(req, opts, fp, nullptr /*cb_arg*/, &io_handle_,
                                &del_fn_, rate_limiter_priority);
-  req.status.PermitUncheckedError();
   if (s.ok()) {
     async_read_in_progress_ = true;
   }
@@ -215,34 +221,24 @@ void FilePrefetchBuffer::CopyDataToBuffer(uint32_t src, uint64_t& offset,
 //        the asynchronous prefetching in second buffer.
 Status FilePrefetchBuffer::PrefetchAsync(const IOOptions& opts,
                                          RandomAccessFileReader* reader,
-                                         uint64_t offset, size_t length,
-                                         size_t readahead_size,
+                                         FileSystem* fs, uint64_t offset,
+                                         size_t length, size_t readahead_size,
                                          Env::IOPriority rate_limiter_priority,
                                          bool& copy_to_third_buffer) {
   if (!enable_) {
     return Status::OK();
   }
-  if (async_read_in_progress_ && fs_ != nullptr) {
+  if (async_read_in_progress_ && fs != nullptr) {
     // Wait for prefetch data to complete.
     // No mutex is needed as PrefetchAsyncCallback updates the result in second
     // buffer and FilePrefetchBuffer should wait for Poll before accessing the
     // second buffer.
     std::vector<void*> handles;
     handles.emplace_back(io_handle_);
-    fs_->Poll(handles, 1).PermitUncheckedError();
+    fs->Poll(handles, 1).PermitUncheckedError();
   }
 
-  // Reset and Release io_handle_ after the Poll API as request has been
-  // completed.
-  async_read_in_progress_ = false;
-  if (io_handle_ != nullptr && del_fn_ != nullptr) {
-    del_fn_(io_handle_);
-    io_handle_ = nullptr;
-    del_fn_ = nullptr;
-  }
-
-  // TODO akanksha: Update TEST_SYNC_POINT after Async APIs are merged with
-  // normal prefetching.
+  // TODO akanksha: Update TEST_SYNC_POINT after new tests are added.
   TEST_SYNC_POINT("FilePrefetchBuffer::Prefetch:Start");
   Status s;
   size_t prefetch_size = length + readahead_size;
@@ -442,8 +438,8 @@ bool FilePrefetchBuffer::TryReadFromCache(const IOOptions& opts,
 bool FilePrefetchBuffer::TryReadFromCacheAsync(
     const IOOptions& opts, RandomAccessFileReader* reader, uint64_t offset,
     size_t n, Slice* result, Status* status,
-    Env::IOPriority rate_limiter_priority, bool for_compaction /* = false */
-) {
+    Env::IOPriority rate_limiter_priority, bool for_compaction /* = false */,
+    FileSystem* fs) {
   if (track_min_offset_ && offset < min_offset_read_) {
     min_offset_read_ = static_cast<size_t>(offset);
   }
@@ -478,7 +474,7 @@ bool FilePrefetchBuffer::TryReadFromCacheAsync(
         if (implicit_auto_readahead_ && async_io_) {
           // Prefetch n + readahead_size_/2 synchronously as remaining
           // readahead_size_/2 will be prefetched asynchronously.
-          s = PrefetchAsync(opts, reader, offset, n, readahead_size_ / 2,
+          s = PrefetchAsync(opts, reader, fs, offset, n, readahead_size_ / 2,
                             rate_limiter_priority, copy_to_third_buffer);
         } else {
           s = Prefetch(opts, reader, offset, n + readahead_size_,
@@ -515,6 +511,7 @@ bool FilePrefetchBuffer::TryReadFromCacheAsync(
 
 void FilePrefetchBuffer::PrefetchAsyncCallback(const FSReadRequest& req,
                                                void* /*cb_arg*/) {
+  async_read_in_progress_ = false;
   uint32_t index = curr_ ^ 1;
   if (req.status.ok()) {
     if (req.offset + req.result.size() <=
@@ -530,5 +527,12 @@ void FilePrefetchBuffer::PrefetchAsyncCallback(const FSReadRequest& req,
     size_t current_size = bufs_[index].buffer_.CurrentSize();
     bufs_[index].buffer_.Size(current_size + req.result.size());
   }
+
+  // Release io_handle_.
+  if (io_handle_ != nullptr && del_fn_ != nullptr) {
+    del_fn_(io_handle_);
+    io_handle_ = nullptr;
+    del_fn_ = nullptr;
+  }
 }
 }  // namespace ROCKSDB_NAMESPACE

file/file_prefetch_buffer.h

@@ -65,7 +65,7 @@ class FilePrefetchBuffer {
   FilePrefetchBuffer(size_t readahead_size = 0, size_t max_readahead_size = 0,
                      bool enable = true, bool track_min_offset = false,
                      bool implicit_auto_readahead = false,
-                     bool async_io = false, FileSystem* fs = nullptr)
+                     bool async_io = false)
       : curr_(0),
         readahead_size_(readahead_size),
         max_readahead_size_(max_readahead_size),
@@ -79,29 +79,13 @@ class FilePrefetchBuffer {
         io_handle_(nullptr),
         del_fn_(nullptr),
         async_read_in_progress_(false),
-        async_io_(async_io),
-        fs_(fs) {
+        async_io_(async_io) {
     // If async_io_ is enabled, data is asynchronously filled in second buffer
     // while curr_ is being consumed. If data is overlapping in two buffers,
     // data is copied to third buffer to return continuous buffer.
     bufs_.resize(3);
   }
 
-  ~FilePrefetchBuffer() {
-    // Wait for any pending async job before destroying the class object.
-    if (async_read_in_progress_ && fs_ != nullptr) {
-      std::vector<void*> handles;
-      handles.emplace_back(io_handle_);
-      fs_->Poll(handles, 1).PermitUncheckedError();
-    }
-    // Release io_handle_.
-    if (io_handle_ != nullptr && del_fn_ != nullptr) {
-      del_fn_(io_handle_);
-      io_handle_ = nullptr;
-      del_fn_ = nullptr;
-    }
-  }
-
   // Load data into the buffer from a file.
   // reader                : the file reader.
   // offset                : the file offset to start reading from.
@@ -116,7 +100,8 @@ class FilePrefetchBuffer {
                   Env::IOPriority rate_limiter_priority);
 
   Status PrefetchAsync(const IOOptions& opts, RandomAccessFileReader* reader,
-                       uint64_t offset, size_t length, size_t readahead_size,
+                       FileSystem* fs, uint64_t offset, size_t length,
+                       size_t readahead_size,
                        Env::IOPriority rate_limiter_priority,
                        bool& copy_to_third_buffer);
 
@@ -144,7 +129,7 @@ class FilePrefetchBuffer {
                              RandomAccessFileReader* reader, uint64_t offset,
                              size_t n, Slice* result, Status* status,
                              Env::IOPriority rate_limiter_priority,
-                             bool for_compaction /* = false */);
+                             bool for_compaction /* = false */, FileSystem* fs);
 
   // The minimum `offset` ever passed to TryReadFromCache(). This will nly be
   // tracked if track_min_offset = true.
@@ -271,6 +256,5 @@ class FilePrefetchBuffer {
   IOHandleDeleter del_fn_;
   bool async_read_in_progress_;
   bool async_io_;
-  FileSystem* fs_;
 };
 }  // namespace ROCKSDB_NAMESPACE

file/filename.cc

@@ -491,12 +491,6 @@ Status GetInfoLogFiles(const std::shared_ptr<FileSystem>& fs,
 
 std::string NormalizePath(const std::string& path) {
   std::string dst;
-
-  if (path.length() > 2 && path[0] == kFilePathSeparator &&
-      path[1] == kFilePathSeparator) {  // Handle UNC names
-    dst.append(2, kFilePathSeparator);
-  }
-
   for (auto c : path) {
     if (!dst.empty() && (c == kFilePathSeparator || c == '/') &&
         (dst.back() == kFilePathSeparator || dst.back() == '/')) {

file/prefetch_test.cc

@@ -694,10 +694,8 @@ TEST_P(PrefetchTest1, DBIterLevelReadAhead) {
   options.write_buffer_size = 1024;
   options.create_if_missing = true;
   options.compression = kNoCompression;
-  options.statistics = CreateDBStatistics();
   options.env = env.get();
-  bool use_direct_io = std::get<0>(GetParam());
-  if (use_direct_io) {
+  if (std::get<0>(GetParam())) {
     options.use_direct_reads = true;
     options.use_direct_io_for_flush_and_compaction = true;
   }
@@ -710,7 +708,8 @@ TEST_P(PrefetchTest1, DBIterLevelReadAhead) {
   options.table_factory.reset(NewBlockBasedTableFactory(table_options));
 
   Status s = TryReopen(options);
-  if (use_direct_io && (s.IsNotSupported() || s.IsInvalidArgument())) {
+  if (std::get<0>(GetParam()) &&
+      (s.IsNotSupported() || s.IsInvalidArgument())) {
     // If direct IO is not supported, skip the test
     return;
   } else {
@@ -767,8 +766,6 @@ TEST_P(PrefetchTest1, DBIterLevelReadAhead) {
       // TODO akanksha: Remove after adding new units.
       ro.async_io = true;
     }
-
-    ASSERT_OK(options.statistics->Reset());
     auto iter = std::unique_ptr<Iterator>(db_->NewIterator(ro));
     int num_keys = 0;
     for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
@@ -776,25 +773,15 @@ TEST_P(PrefetchTest1, DBIterLevelReadAhead) {
       num_keys++;
     }
     ASSERT_EQ(num_keys, total_keys);
+
     ASSERT_GT(buff_prefetch_count, 0);
+    buff_prefetch_count = 0;
     // For index and data blocks.
     if (is_adaptive_readahead) {
       ASSERT_EQ(readahead_carry_over_count, 2 * (num_sst_files - 1));
     } else {
       ASSERT_EQ(readahead_carry_over_count, 0);
     }
-
-    // Check stats to make sure async prefetch is done.
-    {
-      HistogramData async_read_bytes;
-      options.statistics->histogramData(ASYNC_READ_BYTES, &async_read_bytes);
-      if (ro.async_io && !use_direct_io) {
-        ASSERT_GT(async_read_bytes.count, 0);
-      } else {
-        ASSERT_EQ(async_read_bytes.count, 0);
-      }
-    }
-
     SyncPoint::GetInstance()->DisableProcessing();
     SyncPoint::GetInstance()->ClearAllCallBacks();
   }
@@ -915,8 +902,6 @@ TEST_P(PrefetchTest2, DecreaseReadAheadIfInCache) {
     options.use_direct_reads = true;
     options.use_direct_io_for_flush_and_compaction = true;
   }
-
-  options.statistics = CreateDBStatistics();
   BlockBasedTableOptions table_options;
   std::shared_ptr<Cache> cache = NewLRUCache(4 * 1024 * 1024, 2);  // 8MB
   table_options.block_cache = cache;
@@ -963,6 +948,7 @@ TEST_P(PrefetchTest2, DecreaseReadAheadIfInCache) {
   SyncPoint::GetInstance()->EnableProcessing();
   ReadOptions ro;
   ro.adaptive_readahead = true;
+  // TODO akanksha: Remove after adding new units.
   ro.async_io = true;
   {
     /*
@@ -978,9 +964,7 @@ TEST_P(PrefetchTest2, DecreaseReadAheadIfInCache) {
     iter->Seek(BuildKey(1019));
     buff_prefetch_count = 0;
   }
-
   {
-    ASSERT_OK(options.statistics->Reset());
     // After caching, blocks will be read from cache (Sequential blocks)
     auto iter = std::unique_ptr<Iterator>(db_->NewIterator(ro));
     iter->Seek(BuildKey(0));
@@ -1024,124 +1008,11 @@ TEST_P(PrefetchTest2, DecreaseReadAheadIfInCache) {
     ASSERT_TRUE(iter->Valid());
     ASSERT_EQ(current_readahead_size, expected_current_readahead_size);
     ASSERT_EQ(buff_prefetch_count, 2);
-
-    // Check stats to make sure async prefetch is done.
-    {
-      HistogramData async_read_bytes;
-      options.statistics->histogramData(ASYNC_READ_BYTES, &async_read_bytes);
-      if (GetParam()) {
-        ASSERT_EQ(async_read_bytes.count, 0);
-      } else {
-        ASSERT_GT(async_read_bytes.count, 0);
-      }
-    }
-
     buff_prefetch_count = 0;
   }
   Close();
 }
 
-extern "C" bool RocksDbIOUringEnable() { return true; }
-
-// Tests the default implementation of ReadAsync API with PosixFileSystem.
-TEST_F(PrefetchTest2, ReadAsyncWithPosixFS) {
-  if (mem_env_ || encrypted_env_) {
-    ROCKSDB_GTEST_SKIP("Test requires non-mem or non-encrypted environment");
-    return;
-  }
-
-  const int kNumKeys = 1000;
-  std::shared_ptr<MockFS> fs = std::make_shared<MockFS>(
-      FileSystem::Default(), /*support_prefetch=*/false);
-  std::unique_ptr<Env> env(new CompositeEnvWrapper(env_, fs));
-
-  bool use_direct_io = false;
-  Options options = CurrentOptions();
-  options.write_buffer_size = 1024;
-  options.create_if_missing = true;
-  options.compression = kNoCompression;
-  options.env = env.get();
-  options.statistics = CreateDBStatistics();
-  if (use_direct_io) {
-    options.use_direct_reads = true;
-    options.use_direct_io_for_flush_and_compaction = true;
-  }
-  BlockBasedTableOptions table_options;
-  table_options.no_block_cache = true;
-  table_options.cache_index_and_filter_blocks = false;
-  table_options.metadata_block_size = 1024;
-  table_options.index_type =
-      BlockBasedTableOptions::IndexType::kTwoLevelIndexSearch;
-  options.table_factory.reset(NewBlockBasedTableFactory(table_options));
-
-  Status s = TryReopen(options);
-  if (use_direct_io && (s.IsNotSupported() || s.IsInvalidArgument())) {
-    // If direct IO is not supported, skip the test
-    return;
-  } else {
-    ASSERT_OK(s);
-  }
-
-  int total_keys = 0;
-  // Write the keys.
-  {
-    WriteBatch batch;
-    Random rnd(309);
-    for (int j = 0; j < 5; j++) {
-      for (int i = j * kNumKeys; i < (j + 1) * kNumKeys; i++) {
-        ASSERT_OK(batch.Put(BuildKey(i), rnd.RandomString(1000)));
-        total_keys++;
-      }
-      ASSERT_OK(db_->Write(WriteOptions(), &batch));
-      ASSERT_OK(Flush());
-    }
-    MoveFilesToLevel(2);
-  }
-
-  int buff_prefetch_count = 0;
-  bool read_async_called = false;
-  SyncPoint::GetInstance()->SetCallBack("FilePrefetchBuffer::Prefetch:Start",
-                                        [&](void*) { buff_prefetch_count++; });
-  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
-      "UpdateResults::io_uring_result",
-      [&](void* /*arg*/) { read_async_called = true; });
-  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
-
-  // Read the keys.
-  {
-    ReadOptions ro;
-    ro.adaptive_readahead = true;
-    ro.async_io = true;
-
-    ASSERT_OK(options.statistics->Reset());
-    auto iter = std::unique_ptr<Iterator>(db_->NewIterator(ro));
-    int num_keys = 0;
-    for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
-      ASSERT_OK(iter->status());
-      num_keys++;
-    }
-    ASSERT_EQ(num_keys, total_keys);
-    ASSERT_GT(buff_prefetch_count, 0);
-
-    // Check stats to make sure async prefetch is done.
-    {
-      HistogramData async_read_bytes;
-      options.statistics->histogramData(ASYNC_READ_BYTES, &async_read_bytes);
-      // Not all platforms support iouring. In that case, ReadAsync in posix
-      // won't submit async requests.
-      if (read_async_called) {
-        ASSERT_GT(async_read_bytes.count, 0);
-      } else {
-        ASSERT_EQ(async_read_bytes.count, 0);
-      }
-    }
-  }
-
-  SyncPoint::GetInstance()->DisableProcessing();
-  SyncPoint::GetInstance()->ClearAllCallBacks();
-
-  Close();
-}
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

file/random_access_file_reader.cc

@@ -425,75 +425,19 @@ IOStatus RandomAccessFileReader::PrepareIOOptions(const ReadOptions& ro,
   }
 }
 
-// TODO akanksha:
-// 1. Handle use_direct_io case which currently calls Read API.
+// TODO akanksha: Add perf_times etc.
 IOStatus RandomAccessFileReader::ReadAsync(
     FSReadRequest& req, const IOOptions& opts,
     std::function<void(const FSReadRequest&, void*)> cb, void* cb_arg,
     void** io_handle, IOHandleDeleter* del_fn,
     Env::IOPriority rate_limiter_priority) {
   if (use_direct_io()) {
-    // For direct_io, it calls Read API.
     req.status = Read(opts, req.offset, req.len, &(req.result), req.scratch,
                       nullptr /*dbg*/, rate_limiter_priority);
     cb(req, cb_arg);
     return IOStatus::OK();
   }
-
-  // Create a callback and populate info.
-  auto read_async_callback =
-      std::bind(&RandomAccessFileReader::ReadAsyncCallback, this,
-                std::placeholders::_1, std::placeholders::_2);
-  ReadAsyncInfo* read_async_info = new ReadAsyncInfo;
-  read_async_info->cb_ = cb;
-  read_async_info->cb_arg_ = cb_arg;
-  read_async_info->start_time_ = clock_->NowMicros();
-
-#ifndef ROCKSDB_LITE
-  if (ShouldNotifyListeners()) {
-    read_async_info->fs_start_ts_ = FileOperationInfo::StartNow();
-  }
-#endif
-
-  IOStatus s = file_->ReadAsync(req, opts, read_async_callback, read_async_info,
-                                io_handle, del_fn, nullptr /*dbg*/);
-  if (!s.ok()) {
-    delete read_async_info;
-  }
-  return s;
-}
-
-void RandomAccessFileReader::ReadAsyncCallback(const FSReadRequest& req,
-                                               void* cb_arg) {
-  ReadAsyncInfo* read_async_info = static_cast<ReadAsyncInfo*>(cb_arg);
-  assert(read_async_info);
-  assert(read_async_info->cb_);
-
-  read_async_info->cb_(req, read_async_info->cb_arg_);
-
-  // Update stats and notify listeners.
-  if (stats_ != nullptr && file_read_hist_ != nullptr) {
-    // elapsed doesn't take into account delay and overwrite as StopWatch does
-    // in Read.
-    uint64_t elapsed = clock_->NowMicros() - read_async_info->start_time_;
-    file_read_hist_->Add(elapsed);
-  }
-  if (req.status.ok()) {
-    RecordInHistogram(stats_, ASYNC_READ_BYTES, req.result.size());
-  }
-#ifndef ROCKSDB_LITE
-  if (ShouldNotifyListeners()) {
-    auto finish_ts = FileOperationInfo::FinishNow();
-    NotifyOnFileReadFinish(req.offset, req.result.size(),
-                           read_async_info->fs_start_ts_, finish_ts,
-                           req.status);
-  }
-  if (!req.status.ok()) {
-    NotifyOnIOError(req.status, FileOperationType::kRead, file_name(),
-                    req.result.size(), req.offset);
-  }
-#endif
-  RecordIOStats(stats_, file_temperature_, is_last_level_, req.result.size());
-  delete read_async_info;
+  return file_->ReadAsync(req, opts, cb, cb_arg, io_handle, del_fn,
+                          nullptr /*dbg*/);
 }
 }  // namespace ROCKSDB_NAMESPACE

file/random_access_file_reader.h

@@ -92,15 +92,6 @@ class RandomAccessFileReader {
   const Temperature file_temperature_;
   const bool is_last_level_;
 
-  struct ReadAsyncInfo {
-#ifndef ROCKSDB_LITE
-    FileOperationInfo::StartTimePoint fs_start_ts_;
-#endif
-    uint64_t start_time_;
-    std::function<void(const FSReadRequest&, void*)> cb_;
-    void* cb_arg_;
-  };
-
  public:
   explicit RandomAccessFileReader(
       std::unique_ptr<FSRandomAccessFile>&& raf, const std::string& _file_name,
@@ -188,7 +179,5 @@ class RandomAccessFileReader {
                      std::function<void(const FSReadRequest&, void*)> cb,
                      void* cb_arg, void** io_handle, IOHandleDeleter* del_fn,
                      Env::IOPriority rate_limiter_priority);
-
-  void ReadAsyncCallback(const FSReadRequest& req, void* cb_arg);
 };
 }  // namespace ROCKSDB_NAMESPACE

file/sst_file_manager_impl.h

@@ -166,7 +166,7 @@ class SstFileManagerImpl : public SstFileManager {
   std::unordered_map<std::string, uint64_t> tracked_files_;
   // The maximum allowed space (in bytes) for sst and blob files.
   uint64_t max_allowed_space_;
-  // DeleteScheduler used to throttle file deletion.
+  // DeleteScheduler used to throttle file deletition.
   DeleteScheduler delete_scheduler_;
   port::CondVar cv_;
   // Flag to force error recovery thread to exit

include/rocksdb/cache.h

@@ -52,8 +52,9 @@ struct LRUCacheOptions {
   // Capacity of the cache.
   size_t capacity = 0;
 
-  // Cache is sharded into 2^num_shard_bits shards, by hash of key.
-  // Refer to NewLRUCache for further information.
+  // Cache is sharded into 2^num_shard_bits shards,
+  // by hash of key. Refer to NewLRUCache for further
+  // information.
   int num_shard_bits = -1;
 
   // If strict_capacity_limit is set,
@@ -90,7 +91,7 @@ struct LRUCacheOptions {
   CacheMetadataChargePolicy metadata_charge_policy =
       kDefaultCacheMetadataChargePolicy;
 
-  // A SecondaryCache instance to use a the non-volatile tier.
+  // A SecondaryCache instance to use a the non-volatile tier
   std::shared_ptr<SecondaryCache> secondary_cache;
 
   LRUCacheOptions() {}
@@ -327,16 +328,17 @@ class Cache {
 
   /**
    * Release a mapping returned by a previous Lookup(). A released entry might
-   * still remain in cache in case it is later looked up by others. If
-   * erase_if_last_ref is set then it also erases it from the cache if there is
-   * no other reference to  it. Erasing it should call the deleter function that
-   * was provided when the entry was inserted.
+   * still  remain in cache in case it is later looked up by others. If
+   * force_erase is set then it also erase it from the cache if there is no
+   * other reference to  it. Erasing it should call the deleter function that
+   * was provided when the
+   * entry was inserted.
    *
    * Returns true if the entry was also erased.
    */
   // REQUIRES: handle must not have been released yet.
   // REQUIRES: handle must have been returned by a method on *this.
-  virtual bool Release(Handle* handle, bool erase_if_last_ref = false) = 0;
+  virtual bool Release(Handle* handle, bool force_erase = false) = 0;
 
   // Return the value encapsulated in a handle returned by a
   // successful Lookup().
@@ -344,7 +346,7 @@ class Cache {
   // REQUIRES: handle must have been returned by a method on *this.
   virtual void* Value(Handle* handle) = 0;
 
-  // If the cache contains the entry for the key, erase it.  Note that the
+  // If the cache contains entry for key, erase it.  Note that the
   // underlying entry will be kept around until all existing handles
   // to it have been released.
   virtual void Erase(const Slice& key) = 0;
@@ -368,19 +370,19 @@ class Cache {
   // full capacity.
   virtual bool HasStrictCapacityLimit() const = 0;
 
-  // Returns the maximum configured capacity of the cache
+  // returns the maximum configured capacity of the cache
   virtual size_t GetCapacity() const = 0;
 
-  // Returns the memory size for the entries residing in the cache.
+  // returns the memory size for the entries residing in the cache.
   virtual size_t GetUsage() const = 0;
 
-  // Returns the memory size for a specific entry in the cache.
+  // returns the memory size for a specific entry in the cache.
   virtual size_t GetUsage(Handle* handle) const = 0;
 
-  // Returns the memory size for the entries in use by the system
+  // returns the memory size for the entries in use by the system
   virtual size_t GetPinnedUsage() const = 0;
 
-  // Returns the charge for the specific entry in the cache.
+  // returns the charge for the specific entry in the cache.
   virtual size_t GetCharge(Handle* handle) const = 0;
 
   // Returns the deleter for the specified entry. This might seem useless
@@ -439,8 +441,9 @@ class Cache {
   // The Insert and Lookup APIs below are intended to allow cached objects
   // to be demoted/promoted between the primary block cache and a secondary
   // cache. The secondary cache could be a non-volatile cache, and will
-  // likely store the object in a different representation. They rely on a
-  // per object CacheItemHelper to do the conversions.
+  // likely store the object in a different representation more suitable
+  // for on disk storage. They rely on a per object CacheItemHelper to do
+  // the conversions.
   // The secondary cache may persist across process and system restarts,
   // and may even be moved between hosts. Therefore, the cache key must
   // be repeatable across restarts/reboots, and globally unique if
@@ -514,9 +517,8 @@ class Cache {
   // parameter specifies whether the data was actually used or not,
   // which may be used by the cache implementation to decide whether
   // to consider it as a hit for retention purposes.
-  virtual bool Release(Handle* handle, bool /*useful*/,
-                       bool erase_if_last_ref) {
-    return Release(handle, erase_if_last_ref);
+  virtual bool Release(Handle* handle, bool /*useful*/, bool force_erase) {
+    return Release(handle, force_erase);
   }
 
   // Determines if the handle returned by Lookup() has a valid value yet. The

include/rocksdb/db.h

@@ -161,13 +161,98 @@ class DB {
   static Status Open(const Options& options, const std::string& name,
                      DB** dbptr);
 
+  // Open the database for read only. All DB interfaces
+  // that modify data, like put/delete, will return error.
+  // If the db is opened in read only mode, then no compactions
+  // will happen.
+  //
+  // While a given DB can be simultaneously open via OpenForReadOnly
+  // by any number of readers, if a DB is simultaneously open by Open
+  // and OpenForReadOnly, the read-only instance has undefined behavior
+  // (though can often succeed if quickly closed) and the read-write
+  // instance is unaffected. See also OpenAsSecondary.
+  //
+  // Not supported in ROCKSDB_LITE, in which case the function will
+  // return Status::NotSupported.
+  static Status OpenForReadOnly(const Options& options, const std::string& name,
+                                DB** dbptr,
+                                bool error_if_wal_file_exists = false);
+
+  // Open the database for read only with column families. When opening DB with
+  // read only, you can specify only a subset of column families in the
+  // database that should be opened. However, you always need to specify default
+  // column family. The default column family name is 'default' and it's stored
+  // in ROCKSDB_NAMESPACE::kDefaultColumnFamilyName
+  //
+  // While a given DB can be simultaneously open via OpenForReadOnly
+  // by any number of readers, if a DB is simultaneously open by Open
+  // and OpenForReadOnly, the read-only instance has undefined behavior
+  // (though can often succeed if quickly closed) and the read-write
+  // instance is unaffected. See also OpenAsSecondary.
+  //
+  // Not supported in ROCKSDB_LITE, in which case the function will
+  // return Status::NotSupported.
+  static Status OpenForReadOnly(
+      const DBOptions& db_options, const std::string& name,
+      const std::vector<ColumnFamilyDescriptor>& column_families,
+      std::vector<ColumnFamilyHandle*>* handles, DB** dbptr,
+      bool error_if_wal_file_exists = false);
+
+  // The following OpenAsSecondary functions create a secondary instance that
+  // can dynamically tail the MANIFEST of a primary that must have already been
+  // created. User can call TryCatchUpWithPrimary to make the secondary
+  // instance catch up with primary (WAL tailing is NOT supported now) whenever
+  // the user feels necessary. Column families created by the primary after the
+  // secondary instance starts are currently ignored by the secondary instance.
+  // Column families opened by secondary and dropped by the primary will be
+  // dropped by secondary as well. However the user of the secondary instance
+  // can still access the data of such dropped column family as long as they
+  // do not destroy the corresponding column family handle.
+  // WAL tailing is not supported at present, but will arrive soon.
+  //
+  // The options argument specifies the options to open the secondary instance.
+  // The name argument specifies the name of the primary db that you have used
+  // to open the primary instance.
+  // The secondary_path argument points to a directory where the secondary
+  // instance stores its info log.
+  // The dbptr is an out-arg corresponding to the opened secondary instance.
+  // The pointer points to a heap-allocated database, and the user should
+  // delete it after use.
+  // Open DB as secondary instance with only the default column family.
+  // Return OK on success, non-OK on failures.
+  static Status OpenAsSecondary(const Options& options, const std::string& name,
+                                const std::string& secondary_path, DB** dbptr);
+
+  // Open DB as secondary instance with column families. You can open a subset
+  // of column families in secondary mode.
+  // The db_options specify the database specific options.
+  // The name argument specifies the name of the primary db that you have used
+  // to open the primary instance.
+  // The secondary_path argument points to a directory where the secondary
+  // instance stores its info log.
+  // The column_families argument specifies a list of column families to open.
+  // If any of the column families does not exist, the function returns non-OK
+  // status.
+  // The handles is an out-arg corresponding to the opened database column
+  // family handles.
+  // The dbptr is an out-arg corresponding to the opened secondary instance.
+  // The pointer points to a heap-allocated database, and the caller should
+  // delete it after use. Before deleting the dbptr, the user should also
+  // delete the pointers stored in handles vector.
+  // Return OK on success, on-OK on failures.
+  static Status OpenAsSecondary(
+      const DBOptions& db_options, const std::string& name,
+      const std::string& secondary_path,
+      const std::vector<ColumnFamilyDescriptor>& column_families,
+      std::vector<ColumnFamilyHandle*>* handles, DB** dbptr);
+
   // Open DB with column families.
   // db_options specify database specific options
   // column_families is the vector of all column families in the database,
   // containing column family name and options. You need to open ALL column
   // families in the database. To get the list of column families, you can use
-  // ListColumnFamilies().
-  //
+  // ListColumnFamilies(). Also, you can open only a subset of column families
+  // for read-only access.
   // The default column family name is 'default' and it's stored
   // in ROCKSDB_NAMESPACE::kDefaultColumnFamilyName.
   // If everything is OK, handles will on return be the same size
@@ -179,107 +264,6 @@ class DB {
                      const std::vector<ColumnFamilyDescriptor>& column_families,
                      std::vector<ColumnFamilyHandle*>* handles, DB** dbptr);
 
-  // OpenForReadOnly() creates a Read-only instance that supports reads alone.
-  //
-  // All DB interfaces that modify data, like put/delete, will return error.
-  // Automatic Flush and Compactions are disabled and any manual calls
-  // to Flush/Compaction will return error.
-  //
-  // While a given DB can be simultaneously opened via OpenForReadOnly
-  // by any number of readers, if a DB is simultaneously opened by Open
-  // and OpenForReadOnly, the read-only instance has undefined behavior
-  // (though can often succeed if quickly closed) and the read-write
-  // instance is unaffected. See also OpenAsSecondary.
-
-  // Open the database for read only.
-  //
-  // Not supported in ROCKSDB_LITE, in which case the function will
-  // return Status::NotSupported.
-  static Status OpenForReadOnly(const Options& options, const std::string& name,
-                                DB** dbptr,
-                                bool error_if_wal_file_exists = false);
-
-  // Open the database for read only with column families.
-  //
-  // When opening DB with read only, you can specify only a subset of column
-  // families in the database that should be opened. However, you always need
-  // to specify default column family. The default column family name is
-  // 'default' and it's stored in ROCKSDB_NAMESPACE::kDefaultColumnFamilyName
-  //
-  // Not supported in ROCKSDB_LITE, in which case the function will
-  // return Status::NotSupported.
-  static Status OpenForReadOnly(
-      const DBOptions& db_options, const std::string& name,
-      const std::vector<ColumnFamilyDescriptor>& column_families,
-      std::vector<ColumnFamilyHandle*>* handles, DB** dbptr,
-      bool error_if_wal_file_exists = false);
-
-  // OpenAsSecondary() creates a secondary instance that supports read-only
-  // operations and supports dynamic catch up with the primary (through a
-  // call to TryCatchUpWithPrimary()).
-  //
-  // All DB interfaces that modify data, like put/delete, will return error.
-  // Automatic Flush and Compactions are disabled and any manual calls
-  // to Flush/Compaction will return error.
-  //
-  // Multiple secondary instances can co-exist at the same time.
-  //
-
-  // Open DB as secondary instance
-  //
-  // The options argument specifies the options to open the secondary instance.
-  // Options.max_open_files should be set to -1.
-  // The name argument specifies the name of the primary db that you have used
-  // to open the primary instance.
-  // The secondary_path argument points to a directory where the secondary
-  // instance stores its info log.
-  // The dbptr is an out-arg corresponding to the opened secondary instance.
-  // The pointer points to a heap-allocated database, and the caller should
-  // delete it after use.
-  //
-  // Return OK on success, non-OK on failures.
-  static Status OpenAsSecondary(const Options& options, const std::string& name,
-                                const std::string& secondary_path, DB** dbptr);
-
-  // Open DB as secondary instance with specified column families
-  //
-  // When opening DB in secondary mode, you can specify only a subset of column
-  // families in the database that should be opened. However, you always need
-  // to specify default column family. The default column family name is
-  // 'default' and it's stored in ROCKSDB_NAMESPACE::kDefaultColumnFamilyName
-  //
-  // Column families created by the primary after the secondary instance starts
-  // are currently ignored by the secondary instance.  Column families opened
-  // by secondary and dropped by the primary will be dropped by secondary as
-  // well (on next invocation of TryCatchUpWithPrimary()). However the user
-  // of the secondary instance can still access the data of such dropped column
-  // family as long as they do not destroy the corresponding column family
-  // handle.
-  //
-  // The options argument specifies the options to open the secondary instance.
-  // Options.max_open_files should be set to -1.
-  // The name argument specifies the name of the primary db that you have used
-  // to open the primary instance.
-  // The secondary_path argument points to a directory where the secondary
-  // instance stores its info log.
-  // The column_families argument specifies a list of column families to open.
-  // If default column family is not specified or if any specified column
-  // families does not exist, the function returns non-OK status.
-  // The handles is an out-arg corresponding to the opened database column
-  // family handles.
-  // The dbptr is an out-arg corresponding to the opened secondary instance.
-  // The pointer points to a heap-allocated database, and the caller should
-  // delete it after use. Before deleting the dbptr, the user should also
-  // delete the pointers stored in handles vector.
-  //
-  // Return OK on success, non-OK on failures.
-  static Status OpenAsSecondary(
-      const DBOptions& db_options, const std::string& name,
-      const std::string& secondary_path,
-      const std::vector<ColumnFamilyDescriptor>& column_families,
-      std::vector<ColumnFamilyHandle*>* handles, DB** dbptr);
-
-
   // Open DB and run the compaction.
   // It's a read-only operation, the result won't be installed to the DB, it
   // will be output to the `output_directory`. The API should only be used with
@@ -454,6 +438,10 @@ class DB {
   // If "end_key" comes before "start_key" according to the user's comparator,
   // a `Status::InvalidArgument` is returned.
   //
+  // WARNING: Do not use `Iterator::Refresh()` API on DBs where `DeleteRange()`
+  // has been used or will be used. This feature combination is neither
+  // supported nor programmatically prevented.
+  //
   // This feature is now usable in production, with the following caveats:
   // 1) Accumulating many range tombstones in the memtable will degrade read
   // performance; this can be avoided by manually flushing occasionally.
@@ -553,20 +541,16 @@ class DB {
     return Get(options, DefaultColumnFamily(), key, value, timestamp);
   }
 
-  // Populates the `merge_operands` array with all the merge operands in the DB
-  // for `key`. The `merge_operands` array will be populated in the order of
-  // insertion. The number of entries populated in `merge_operands` will be
-  // assigned to `*number_of_operands`.
-  //
-  // If the number of merge operands in DB for `key` is greater than
-  // `merge_operands_options.expected_max_number_of_operands`,
-  // `merge_operands` is not populated and the return value is
-  // `Status::Incomplete`. In that case, `*number_of_operands` will be assigned
-  // the number of merge operands found in the DB for `key`.
-  //
-  // `merge_operands`- Points to an array of at-least
+  // Returns all the merge operands corresponding to the key. If the
+  // number of merge operands in DB is greater than
+  // merge_operands_options.expected_max_number_of_operands
+  // no merge operands are returned and status is Incomplete. Merge operands
+  // returned are in the order of insertion.
+  // merge_operands- Points to an array of at-least
   //             merge_operands_options.expected_max_number_of_operands and the
-  //             caller is responsible for allocating it.
+  //             caller is responsible for allocating it. If the status
+  //             returned is Incomplete then number_of_operands will contain
+  //             the total number of merge operands found in DB for key.
   virtual Status GetMergeOperands(
       const ReadOptions& options, ColumnFamilyHandle* column_family,
       const Slice& key, PinnableSlice* merge_operands,
@@ -940,8 +924,6 @@ class DB {
 
     //  "rocksdb.is-file-deletions-enabled" - returns 0 if deletion of obsolete
     //      files is enabled; otherwise, returns a non-zero number.
-    //  This name may be misleading because true(non-zero) means disable,
-    //  but we keep the name for backward compatibility.
     static const std::string kIsFileDeletionsEnabled;
 
     //  "rocksdb.num-snapshots" - returns number of unreleased snapshots of the
@@ -1219,44 +1201,20 @@ class DB {
     return CompactRange(options, DefaultColumnFamily(), begin, end);
   }
 
-  // Dynamically change column family options or table factory options in a
-  // running DB, for the specified column family. Only options internally
-  // marked as "mutable" can be changed. Options not listed in `opts_map` will
-  // keep their current values. See GetColumnFamilyOptionsFromMap() in
-  // convenience.h for the details of `opts_map`. Not supported in LITE mode.
-  //
-  // USABILITY NOTE: SetOptions is intended only for expert users, and does
-  // not apply the same sanitization to options as the standard DB::Open code
-  // path does. Use with caution.
-  //
-  // RELIABILITY & PERFORMANCE NOTE: SetOptions is not fully stress-tested for
-  // reliability, and this is a slow call because a new OPTIONS file is
-  // serialized and persisted for each call. Use only infrequently.
-  //
-  // EXAMPLES:
-  //  s = db->SetOptions(cfh, {{"ttl", "36000"}});
-  //  s = db->SetOptions(cfh, {{"block_based_table_factory",
-  //                            "{prepopulate_block_cache=kDisable;}"}});
+  // TODO: documentation needed
+  // NOTE: SetOptions is intended only for expert users, and does not apply the
+  // same sanitization to options as the standard DB::Open code path does. Use
+  // with caution.
   virtual Status SetOptions(
       ColumnFamilyHandle* /*column_family*/,
-      const std::unordered_map<std::string, std::string>& /*opts_map*/) {
+      const std::unordered_map<std::string, std::string>& /*new_options*/) {
     return Status::NotSupported("Not implemented");
   }
-  // Shortcut for SetOptions on the default column family handle.
   virtual Status SetOptions(
       const std::unordered_map<std::string, std::string>& new_options) {
     return SetOptions(DefaultColumnFamily(), new_options);
   }
 
-  // Like SetOptions but for DBOptions, including the same caveats for
-  // usability, reliability, and performance. See GetDBOptionsFromMap() (and
-  // GetColumnFamilyOptionsFromMap()) in convenience.h for details on
-  // `opts_map`. Note supported in LITE mode.
-  //
-  // EXAMPLES:
-  //  s = db->SetDBOptions({{"max_subcompactions", "2"}});
-  //  s = db->SetDBOptions({{"stats_dump_period_sec", "0"},
-  //                        {"stats_persist_period_sec", "0"}});
   virtual Status SetDBOptions(
       const std::unordered_map<std::string, std::string>& new_options) = 0;
 
@@ -1341,8 +1299,6 @@ class DB {
   // Get Env object from the DB
   virtual Env* GetEnv() const = 0;
 
-  // A shortcut for GetEnv()->->GetFileSystem().get(), possibly cached for
-  // efficiency.
   virtual FileSystem* GetFileSystem() const;
 
   // Get DB Options that we use.  During the process of opening the
@@ -1717,6 +1673,7 @@ class DB {
   // secondary instance does not delete the corresponding column family
   // handles, the data of the column family is still accessible to the
   // secondary.
+  // TODO: we will support WAL tailing soon.
   virtual Status TryCatchUpWithPrimary() {
     return Status::NotSupported("Supported only by secondary instance");
   }

include/rocksdb/file_system.h

@@ -89,12 +89,6 @@ struct IOOptions {
   // Priority - high or low
   IOPriority prio;
 
-  // Priority used to charge rate limiter configured in file system level (if
-  // any)
-  // Limitation: right now RocksDB internal does not consider this
-  // rate_limiter_priority
-  Env::IOPriority rate_limiter_priority;
-
   // Type of data being read/written
   IOType type;
 
@@ -115,7 +109,6 @@ struct IOOptions {
   explicit IOOptions(bool force_dir_fsync_)
       : timeout(std::chrono::microseconds::zero()),
         prio(IOPriority::kIOLow),
-        rate_limiter_priority(Env::IO_TOTAL),
         type(IOType::kUnknown),
         force_dir_fsync(force_dir_fsync_) {}
 };

include/rocksdb/iterator.h

@@ -92,6 +92,10 @@ class Iterator : public Cleanable {
   // If supported, renew the iterator to represent the latest state. The
   // iterator will be invalidated after the call. Not supported if
   // ReadOptions.snapshot is given when creating the iterator.
+  //
+  // WARNING: Do not use `Iterator::Refresh()` API on DBs where `DeleteRange()`
+  // has been used or will be used. This feature combination is neither
+  // supported nor programmatically prevented.
   virtual Status Refresh() {
     return Status::NotSupported("Refresh() is not supported");
   }

include/rocksdb/metadata.h

@@ -72,10 +72,10 @@ struct LiveFileStorageInfo : public FileStorageInfo {
 // The metadata that describes an SST file. (Does not need to extend
 // LiveFileStorageInfo because SST files are always immutable.)
 struct SstFileMetaData : public FileStorageInfo {
-  SstFileMetaData() { file_type = kTableFile; }
+  SstFileMetaData() {}
 
   SstFileMetaData(const std::string& _file_name, uint64_t _file_number,
-                  const std::string& _directory, uint64_t _size,
+                  const std::string& _directory, size_t _size,
                   SequenceNumber _smallest_seqno, SequenceNumber _largest_seqno,
                   const std::string& _smallestkey,
                   const std::string& _largestkey, uint64_t _num_reads_sampled,

include/rocksdb/options.h

@@ -507,9 +507,6 @@ struct DBOptions {
   // - Reads associated with a `ReadOptions` can be charged at
   //   `ReadOptions::rate_limiter_priority` (see that option's API doc for usage
   //   and limitations).
-  // - Writes associated with a `WriteOptions` can be charged at
-  //   `WriteOptions::rate_limiter_priority` (see that option's API doc for
-  //   usage and limitations).
   //
   // Rate limiting is disabled if nullptr. If rate limiter is enabled,
   // bytes_per_sync is set to 1MB by default.
@@ -551,10 +548,6 @@ struct DBOptions {
   // on target_file_size_base and target_file_size_multiplier for level-based
   // compaction. For universal-style compaction, you can usually set it to -1.
   //
-  // A high value or -1 for this option can cause high memory usage.
-  // See BlockBasedTableOptions::reserve_table_reader_memory to constrain
-  // memory usage in case of block based table format.
-  //
   // Default: -1
   //
   // Dynamically changeable through SetDBOptions() API.

include/rocksdb/statistics.h

@@ -431,8 +431,6 @@ enum Tickers : uint32_t {
   NON_LAST_LEVEL_READ_BYTES,
   NON_LAST_LEVEL_READ_COUNT,
 
-  BLOCK_CHECKSUM_COMPUTE_COUNT,
-
   TICKER_ENUM_MAX
 };
 
@@ -536,8 +534,6 @@ enum Histograms : uint32_t {
   // Error handler statistics
   ERROR_HANDLER_AUTORESUME_RETRY_COUNT,
 
-  ASYNC_READ_BYTES,
-
   HISTOGRAM_ENUM_MAX,
 };
 

include/rocksdb/table.h

@@ -300,35 +300,12 @@ struct BlockBasedTableOptions {
   //
   // If additional temporary memory of Ribbon Filter uses up too much memory
   // relative to the avaible space left in the block cache
-  // at some point (i.e, causing a cache full under
-  // LRUCacheOptions::strict_capacity_limit = true), construction will fall back
-  // to Bloom Filter.
+  // at some point (i.e, causing a cache full when strict_capacity_limit =
+  // true), construction will fall back to Bloom Filter.
   //
   // Default: false
   bool reserve_table_builder_memory = false;
 
-  // If true, a dynamically updating charge to block cache, loosely based
-  // on the actual memory usage of table reader, will occur to account
-  // the memory, if block cache available.
-  //
-  // Charged memory usage includes:
-  // 1. Table properties
-  // 2. Index block/Filter block/Uncompression dictionary if stored in table
-  // reader (i.e, BlockBasedTableOptions::cache_index_and_filter_blocks ==
-  // false)
-  // 3. Some internal data structures
-  // 4. More to come...
-  //
-  // Note:
-  // If creation of a table reader uses up too much memory
-  // relative to the avaible space left in the block cache
-  // at some point (i.e, causing a cache full under
-  // LRUCacheOptions::strict_capacity_limit = true), such creation will fail
-  // with Status::MemoryLimit().
-  //
-  // Default: false
-  bool reserve_table_reader_memory = false;
-
   // Note: currently this option requires kTwoLevelIndexSearch to be set as
   // well.
   // TODO(myabandeh): remove the note above once the limitation is lifted

include/rocksdb/table_properties.h

@@ -301,10 +301,6 @@ struct TableProperties {
   // between tables. Keys match field names in this class instead
   // of using full property names.
   std::map<std::string, uint64_t> GetAggregatablePropertiesAsMap() const;
-
-  // Return the approximated memory usage of this TableProperties object,
-  // including memory used by the string properties and UserCollectedProperties
-  std::size_t ApproximateMemoryUsage() const;
 };
 
 // Extra properties

include/rocksdb/utilities/transaction.h

@@ -557,18 +557,6 @@ class Transaction {
 
   virtual Status RebuildFromWriteBatch(WriteBatch* src_batch) = 0;
 
-  // Note: data in the commit-time-write-batch bypasses concurrency control,
-  // thus should be used with great caution.
-  // For write-prepared/write-unprepared transactions,
-  // GetCommitTimeWriteBatch() can be used only if the transaction is started
-  // with
-  // `TransactionOptions::use_only_the_last_commit_time_batch_for_recovery` set
-  // to true. Otherwise, it is possible that two uncommitted versions of the
-  // same key exist in the database due to the current implementation (see the
-  // explanation in WritePreparedTxn::CommitInternal).
-  // During bottommost compaction, RocksDB may
-  // set the sequence numbers of both to zero once becoming committed, causing
-  // output SST file to have two identical internal keys.
   virtual WriteBatch* GetCommitTimeWriteBatch() = 0;
 
   virtual void SetLogNumber(uint64_t log) { log_number_ = log; }

include/rocksdb/utilities/transaction_db.h

@@ -160,7 +160,8 @@ struct TransactionDBOptions {
 
   // Increasing this value will increase the concurrency by dividing the lock
   // table (per column family) into more sub-tables, each with their own
-  // separate mutex.
+  // separate
+  // mutex.
   size_t num_stripes = 16;
 
   // If positive, specifies the default wait timeout in milliseconds when
@@ -170,7 +171,8 @@ struct TransactionDBOptions {
   // If 0, no waiting is done if a lock cannot instantly be acquired.
   // If negative, there is no timeout.  Not using a timeout is not recommended
   // as it can lead to deadlocks.  Currently, there is no deadlock-detection to
-  // recover from a deadlock.
+  // recover
+  // from a deadlock.
   int64_t transaction_lock_timeout = 1000;  // 1 second
 
   // If positive, specifies the wait timeout in milliseconds when writing a key
@@ -259,8 +261,6 @@ struct TransactionOptions {
   // meant to be used later during recovery. It enables an optimization to
   // postpone updating the memtable with CommitTimeWriteBatch to only
   // SwitchMemtable or recovery.
-  // This option does not affect write-committed. Only
-  // write-prepared/write-unprepared transactions will be affected.
   bool use_only_the_last_commit_time_batch_for_recovery = false;
 
   // TODO(agiardullo): TransactionDB does not yet support comparators that allow

include/rocksdb/utilities/transaction_db_mutex.h

@@ -18,6 +18,7 @@ namespace ROCKSDB_NAMESPACE {
 //
 // To open a TransactionDB with a custom TransactionDBMutexFactory, set
 // TransactionDBOptions.custom_mutex_factory.
+
 class TransactionDBMutex {
  public:
   virtual ~TransactionDBMutex() {}

include/rocksdb/write_buffer_manager.h

@@ -158,7 +158,7 @@ class WriteBufferManager final {
   std::atomic<size_t> memory_used_;
   // Memory that hasn't been scheduled to free.
   std::atomic<size_t> memory_active_;
-  std::shared_ptr<CacheReservationManager> cache_res_mgr_;
+  std::unique_ptr<CacheReservationManager> cache_res_mgr_;
   // Protects cache_res_mgr_
   std::mutex cache_res_mgr_mu_;
 

java/CMakeLists.txt

@@ -7,7 +7,7 @@ endif()
 set(CMAKE_JAVA_COMPILE_FLAGS -source 7)
 
 set(JNI_NATIVE_SOURCES
-        rocksjni/backup_engine_options.cc
+        rocksjni/backupablejni.cc
         rocksjni/backupenginejni.cc
         rocksjni/cassandra_compactionfilterjni.cc
         rocksjni/cassandra_value_operator.cc

java/Makefile

@@ -187,7 +187,6 @@ JAVA_TESTS = \
 	org.rocksdb.TtlDBTest\
 	org.rocksdb.StatisticsTest\
 	org.rocksdb.StatisticsCollectorTest\
-	org.rocksdb.VerifyChecksumsTest\
 	org.rocksdb.WalFilterTest\
 	org.rocksdb.WALRecoveryModeTest\
 	org.rocksdb.WriteBatchHandlerTest\

java/rocksjni/iterator.cc

@@ -6,15 +6,13 @@
 // This file implements the "bridge" between Java and C++ and enables
 // calling c++ ROCKSDB_NAMESPACE::Iterator methods from Java side.
 
-#include "rocksdb/iterator.h"
-
 #include <jni.h>
 #include <stdio.h>
 #include <stdlib.h>
-
 #include <algorithm>
 
 #include "include/org_rocksdb_RocksIterator.h"
+#include "rocksdb/iterator.h"
 #include "rocksjni/portal.h"
 
 /*
@@ -89,7 +87,7 @@ void Java_org_rocksdb_RocksIterator_prev0(JNIEnv* /*env*/, jobject /*jobj*/,
  * Signature: (J)V
  */
 void Java_org_rocksdb_RocksIterator_refresh0(JNIEnv* env, jobject /*jobj*/,
-                                             jlong handle) {
+                                            jlong handle) {
   auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::Iterator*>(handle);
   ROCKSDB_NAMESPACE::Status s = it->Refresh();
 
@@ -108,31 +106,19 @@ void Java_org_rocksdb_RocksIterator_refresh0(JNIEnv* env, jobject /*jobj*/,
 void Java_org_rocksdb_RocksIterator_seek0(JNIEnv* env, jobject /*jobj*/,
                                           jlong handle, jbyteArray jtarget,
                                           jint jtarget_len) {
-  auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::Iterator*>(handle);
-  auto seek = [&it](ROCKSDB_NAMESPACE::Slice& target_slice) {
-    it->Seek(target_slice);
-  };
-  ROCKSDB_NAMESPACE::JniUtil::k_op_region(seek, env, jtarget, 0, jtarget_len);
-}
+  jbyte* target = env->GetByteArrayElements(jtarget, nullptr);
+  if (target == nullptr) {
+    // exception thrown: OutOfMemoryError
+    return;
+  }
+
+  ROCKSDB_NAMESPACE::Slice target_slice(reinterpret_cast<char*>(target),
+                                        jtarget_len);
 
-/*
- * This method supports fetching into indirect byte buffers;
- * the Java wrapper extracts the byte[] and passes it here.
- * In this case, the buffer offset of the key may be non-zero.
- *
- * Class:     org_rocksdb_RocksIterator
- * Method:    seek0
- * Signature: (J[BII)V
- */
-void Java_org_rocksdb_RocksIterator_seekByteArray0(
-    JNIEnv* env, jobject /*jobj*/, jlong handle, jbyteArray jtarget,
-    jint jtarget_off, jint jtarget_len) {
   auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::Iterator*>(handle);
-  auto seek = [&it](ROCKSDB_NAMESPACE::Slice& target_slice) {
-    it->Seek(target_slice);
-  };
-  ROCKSDB_NAMESPACE::JniUtil::k_op_region(seek, env, jtarget, jtarget_off,
-                                          jtarget_len);
+  it->Seek(target_slice);
+
+  env->ReleaseByteArrayElements(jtarget, target, JNI_ABORT);
 }
 
 /*
@@ -177,31 +163,19 @@ void Java_org_rocksdb_RocksIterator_seekForPrev0(JNIEnv* env, jobject /*jobj*/,
                                                  jlong handle,
                                                  jbyteArray jtarget,
                                                  jint jtarget_len) {
-  auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::Iterator*>(handle);
-  auto seek = [&it](ROCKSDB_NAMESPACE::Slice& target_slice) {
-    it->SeekForPrev(target_slice);
-  };
-  ROCKSDB_NAMESPACE::JniUtil::k_op_region(seek, env, jtarget, 0, jtarget_len);
-}
+  jbyte* target = env->GetByteArrayElements(jtarget, nullptr);
+  if (target == nullptr) {
+    // exception thrown: OutOfMemoryError
+    return;
+  }
+
+  ROCKSDB_NAMESPACE::Slice target_slice(reinterpret_cast<char*>(target),
+                                        jtarget_len);
 
-/*
- * This method supports fetching into indirect byte buffers;
- * the Java wrapper extracts the byte[] and passes it here.
- * In this case, the buffer offset of the key may be non-zero.
- *
- * Class:     org_rocksdb_RocksIterator
- * Method:    seek0
- * Signature: (J[BII)V
- */
-void Java_org_rocksdb_RocksIterator_seekForPrevByteArray0(
-    JNIEnv* env, jobject /*jobj*/, jlong handle, jbyteArray jtarget,
-    jint jtarget_off, jint jtarget_len) {
   auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::Iterator*>(handle);
-  auto seek = [&it](ROCKSDB_NAMESPACE::Slice& target_slice) {
-    it->SeekForPrev(target_slice);
-  };
-  ROCKSDB_NAMESPACE::JniUtil::k_op_region(seek, env, jtarget, jtarget_off,
-                                          jtarget_len);
+  it->SeekForPrev(target_slice);
+
+  env->ReleaseByteArrayElements(jtarget, target, JNI_ABORT);
 }
 
 /*
@@ -257,29 +231,6 @@ jint Java_org_rocksdb_RocksIterator_keyDirect0(JNIEnv* env, jobject /*jobj*/,
                                                   jtarget_off, jtarget_len);
 }
 
-/*
- * This method supports fetching into indirect byte buffers;
- * the Java wrapper extracts the byte[] and passes it here.
- *
- * Class:     org_rocksdb_RocksIterator
- * Method:    keyByteArray0
- * Signature: (J[BII)I
- */
-jint Java_org_rocksdb_RocksIterator_keyByteArray0(JNIEnv* env, jobject /*jobj*/,
-                                                  jlong handle, jbyteArray jkey,
-                                                  jint jkey_off,
-                                                  jint jkey_len) {
-  auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::Iterator*>(handle);
-  ROCKSDB_NAMESPACE::Slice key_slice = it->key();
-  jsize copy_size = std::min(static_cast<uint32_t>(key_slice.size()),
-                             static_cast<uint32_t>(jkey_len));
-  env->SetByteArrayRegion(
-      jkey, jkey_off, copy_size,
-      const_cast<jbyte*>(reinterpret_cast<const jbyte*>(key_slice.data())));
-
-  return static_cast<jsize>(key_slice.size());
-}
-
 /*
  * Class:     org_rocksdb_RocksIterator
  * Method:    value0
@@ -316,25 +267,3 @@ jint Java_org_rocksdb_RocksIterator_valueDirect0(JNIEnv* env, jobject /*jobj*/,
   return ROCKSDB_NAMESPACE::JniUtil::copyToDirect(env, value_slice, jtarget,
                                                   jtarget_off, jtarget_len);
 }
-
-/*
- * This method supports fetching into indirect byte buffers;
- * the Java wrapper extracts the byte[] and passes it here.
- *
- * Class:     org_rocksdb_RocksIterator
- * Method:    valueByteArray0
- * Signature: (J[BII)I
- */
-jint Java_org_rocksdb_RocksIterator_valueByteArray0(
-    JNIEnv* env, jobject /*jobj*/, jlong handle, jbyteArray jvalue_target,
-    jint jvalue_off, jint jvalue_len) {
-  auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::Iterator*>(handle);
-  ROCKSDB_NAMESPACE::Slice value_slice = it->value();
-  jsize copy_size = std::min(static_cast<uint32_t>(value_slice.size()),
-                             static_cast<uint32_t>(jvalue_len));
-  env->SetByteArrayRegion(
-      jvalue_target, jvalue_off, copy_size,
-      const_cast<jbyte*>(reinterpret_cast<const jbyte*>(value_slice.data())));
-
-  return static_cast<jsize>(value_slice.size());
-}

java/rocksjni/portal.h

@@ -2127,7 +2127,7 @@ class JniUtil {
         std::function<ROCKSDB_NAMESPACE::Status(ROCKSDB_NAMESPACE::Slice)> op,
         JNIEnv* env, jobject /*jobj*/, jbyteArray jkey, jint jkey_len) {
       jbyte* key = env->GetByteArrayElements(jkey, nullptr);
-      if (env->ExceptionCheck()) {
+      if(env->ExceptionCheck()) {
         // exception thrown: OutOfMemoryError
         return nullptr;
       }
@@ -2137,7 +2137,7 @@ class JniUtil {
 
       auto status = op(key_slice);
 
-      if (key != nullptr) {
+      if(key != nullptr) {
         env->ReleaseByteArrayElements(jkey, key, JNI_ABORT);
       }
 
@@ -2145,37 +2145,6 @@ class JniUtil {
           new ROCKSDB_NAMESPACE::Status(status));
     }
 
-    /*
-     * Helper for operations on a key which is a region of an array
-     * Used to extract the common code from seek/seekForPrev.
-     * Possible that it can be generalised from that.
-     *
-     * We use GetByteArrayRegion to copy the key region of the whole array into
-     * a char[] We suspect this is not much slower than GetByteArrayElements,
-     * which probably copies anyway.
-     */
-    static void k_op_region(std::function<void(ROCKSDB_NAMESPACE::Slice&)> op,
-                            JNIEnv* env, jbyteArray jkey, jint jkey_off,
-                            jint jkey_len) {
-      const std::unique_ptr<char[]> key(new char[jkey_len]);
-      if (key == nullptr) {
-        jclass oom_class = env->FindClass("/lang/java/OutOfMemoryError");
-        env->ThrowNew(oom_class,
-                      "Memory allocation failed in RocksDB JNI function");
-        return;
-      }
-      env->GetByteArrayRegion(jkey, jkey_off, jkey_len,
-                              reinterpret_cast<jbyte*>(key.get()));
-      if (env->ExceptionCheck()) {
-        // exception thrown: OutOfMemoryError
-        return;
-      }
-
-      ROCKSDB_NAMESPACE::Slice key_slice(reinterpret_cast<char*>(key.get()),
-                                         jkey_len);
-      op(key_slice);
-    }
-
     /*
      * Helper for operations on a value
      * for example WriteBatchWithIndex->GetFromBatch
@@ -3468,7 +3437,7 @@ class BackupEngineJni
                                 BackupEngineJni> {
  public:
   /**
-   * Get the Java Class org.rocksdb.BackupEngine
+   * Get the Java Class org.rocksdb.BackupableEngine
    *
    * @param env A pointer to the Java environment
    *
@@ -5086,8 +5055,6 @@ class TickerTypeJni {
         return -0x2C;
       case ROCKSDB_NAMESPACE::Tickers::NON_LAST_LEVEL_READ_COUNT:
         return -0x2D;
-      case ROCKSDB_NAMESPACE::Tickers::BLOCK_CHECKSUM_COMPUTE_COUNT:
-        return -0x2E;
       case ROCKSDB_NAMESPACE::Tickers::TICKER_ENUM_MAX:
         // 0x5F was the max value in the initial copy of tickers to Java.
         // Since these values are exposed directly to Java clients, we keep
@@ -5457,8 +5424,6 @@ class TickerTypeJni {
         return ROCKSDB_NAMESPACE::Tickers::NON_LAST_LEVEL_READ_BYTES;
       case -0x2D:
         return ROCKSDB_NAMESPACE::Tickers::NON_LAST_LEVEL_READ_COUNT;
-      case -0x2E:
-        return ROCKSDB_NAMESPACE::Tickers::BLOCK_CHECKSUM_COMPUTE_COUNT;
       case 0x5F:
         // 0x5F was the max value in the initial copy of tickers to Java.
         // Since these values are exposed directly to Java clients, we keep
@@ -5586,9 +5551,7 @@ class HistogramTypeJni {
       case ROCKSDB_NAMESPACE::Histograms::NUM_SST_READ_PER_LEVEL:
         return 0x31;
       case ROCKSDB_NAMESPACE::Histograms::ERROR_HANDLER_AUTORESUME_RETRY_COUNT:
-        return 0x32;
-      case ROCKSDB_NAMESPACE::Histograms::ASYNC_READ_BYTES:
-        return 0x33;
+        return 0x31;
       case ROCKSDB_NAMESPACE::Histograms::HISTOGRAM_ENUM_MAX:
         // 0x1F for backwards compatibility on current minor version.
         return 0x1F;
@@ -5706,8 +5669,6 @@ class HistogramTypeJni {
       case 0x32:
         return ROCKSDB_NAMESPACE::Histograms::
             ERROR_HANDLER_AUTORESUME_RETRY_COUNT;
-      case 0x33:
-        return ROCKSDB_NAMESPACE::Histograms::ASYNC_READ_BYTES;
       case 0x1F:
         // 0x1F for backwards compatibility on current minor version.
         return ROCKSDB_NAMESPACE::Histograms::HISTOGRAM_ENUM_MAX;
@@ -6578,8 +6539,6 @@ class ChecksumTypeJni {
         return 0x2;
       case ROCKSDB_NAMESPACE::ChecksumType::kxxHash64:
         return 0x3;
-      case ROCKSDB_NAMESPACE::ChecksumType::kXXH3:
-        return 0x4;
       default:
         return 0x7F;  // undefined
     }

java/rocksjni/rocksjni.cc

@@ -829,11 +829,9 @@ bool rocksdb_single_delete_helper(
     const ROCKSDB_NAMESPACE::WriteOptions& write_options,
     ROCKSDB_NAMESPACE::ColumnFamilyHandle* cf_handle, jbyteArray jkey,
     jint jkey_len) {
-  jbyte* key = new jbyte[jkey_len];
-  env->GetByteArrayRegion(jkey, 0, jkey_len, key);
-  if (env->ExceptionCheck()) {
-    // exception thrown: ArrayIndexOutOfBoundsException
-    delete[] key;
+  jbyte* key = env->GetByteArrayElements(jkey, nullptr);
+  if (key == nullptr) {
+    // exception thrown: OutOfMemoryError
     return false;
   }
   ROCKSDB_NAMESPACE::Slice key_slice(reinterpret_cast<char*>(key), jkey_len);
@@ -846,7 +844,10 @@ bool rocksdb_single_delete_helper(
     s = db->SingleDelete(write_options, key_slice);
   }
 
-  delete[] key;
+  // trigger java unref on key and value.
+  // by passing JNI_ABORT, it will simply release the reference without
+  // copying the result back to the java byte array.
+  env->ReleaseByteArrayElements(jkey, key, JNI_ABORT);
 
   if (s.ok()) {
     return true;

java/rocksjni/sst_file_reader_iterator.cc

@@ -206,29 +206,6 @@ jint Java_org_rocksdb_SstFileReaderIterator_keyDirect0(
                                                   jtarget_off, jtarget_len);
 }
 
-/*
- * This method supports fetching into indirect byte buffers;
- * the Java wrapper extracts the byte[] and passes it here.
- *
- * Class:     org_rocksdb_SstFileReaderIterator
- * Method:    keyByteArray0
- * Signature: (J[BII)I
- */
-jint Java_org_rocksdb_SstFileReaderIterator_keyByteArray0(
-    JNIEnv* env, jobject /*jobj*/, jlong handle, jbyteArray jkey, jint jkey_off,
-    jint jkey_len) {
-  auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::Iterator*>(handle);
-  ROCKSDB_NAMESPACE::Slice key_slice = it->key();
-  auto slice_size = key_slice.size();
-  jsize copy_size = std::min(static_cast<uint32_t>(slice_size),
-                             static_cast<uint32_t>(jkey_len));
-  env->SetByteArrayRegion(
-      jkey, jkey_off, copy_size,
-      const_cast<jbyte*>(reinterpret_cast<const jbyte*>(key_slice.data())));
-
-  return static_cast<jsize>(slice_size);
-}
-
 /*
  * Class:     org_rocksdb_SstFileReaderIterator
  * Method:    valueDirect0
@@ -243,29 +220,6 @@ jint Java_org_rocksdb_SstFileReaderIterator_valueDirect0(
                                                   jtarget_off, jtarget_len);
 }
 
-/*
- * This method supports fetching into indirect byte buffers;
- * the Java wrapper extracts the byte[] and passes it here.
- *
- * Class:     org_rocksdb_SstFileReaderIterator
- * Method:    valueByteArray0
- * Signature: (J[BII)I
- */
-jint Java_org_rocksdb_SstFileReaderIterator_valueByteArray0(
-    JNIEnv* env, jobject /*jobj*/, jlong handle, jbyteArray jvalue_target,
-    jint jvalue_off, jint jvalue_len) {
-  auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::Iterator*>(handle);
-  ROCKSDB_NAMESPACE::Slice value_slice = it->value();
-  auto slice_size = value_slice.size();
-  jsize copy_size = std::min(static_cast<uint32_t>(slice_size),
-                             static_cast<uint32_t>(jvalue_len));
-  env->SetByteArrayRegion(
-      jvalue_target, jvalue_off, copy_size,
-      const_cast<jbyte*>(reinterpret_cast<const jbyte*>(value_slice.data())));
-
-  return static_cast<jsize>(slice_size);
-}
-
 /*
  * Class:     org_rocksdb_SstFileReaderIterator
  * Method:    seekDirect0
@@ -298,60 +252,6 @@ void Java_org_rocksdb_SstFileReaderIterator_seekForPrevDirect0(
                                           jtarget_len);
 }
 
-/*
- * This method supports fetching into indirect byte buffers;
- * the Java wrapper extracts the byte[] and passes it here.
- *
- * Class:     org_rocksdb_SstFileReaderIterator
- * Method:    seekByteArray0
- * Signature: (J[BII)V
- */
-void Java_org_rocksdb_SstFileReaderIterator_seekByteArray0(
-    JNIEnv* env, jobject /*jobj*/, jlong handle, jbyteArray jtarget,
-    jint jtarget_off, jint jtarget_len) {
-  const std::unique_ptr<char[]> target(new char[jtarget_len]);
-  if (target == nullptr) {
-    jclass oom_class = env->FindClass("/lang/java/OutOfMemoryError");
-    env->ThrowNew(oom_class,
-                  "Memory allocation failed in RocksDB JNI function");
-    return;
-  }
-  env->GetByteArrayRegion(jtarget, jtarget_off, jtarget_len,
-                          reinterpret_cast<jbyte*>(target.get()));
-
-  ROCKSDB_NAMESPACE::Slice target_slice(target.get(), jtarget_len);
-
-  auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::Iterator*>(handle);
-  it->Seek(target_slice);
-}
-
-/*
- * This method supports fetching into indirect byte buffers;
- * the Java wrapper extracts the byte[] and passes it here.
- *
- * Class:     org_rocksdb_SstFileReaderIterator
- * Method:    seekForPrevByteArray0
- * Signature: (J[BII)V
- */
-void Java_org_rocksdb_SstFileReaderIterator_seekForPrevByteArray0(
-    JNIEnv* env, jobject /*jobj*/, jlong handle, jbyteArray jtarget,
-    jint jtarget_off, jint jtarget_len) {
-  const std::unique_ptr<char[]> target(new char[jtarget_len]);
-  if (target == nullptr) {
-    jclass oom_class = env->FindClass("/lang/java/OutOfMemoryError");
-    env->ThrowNew(oom_class,
-                  "Memory allocation failed in RocksDB JNI function");
-    return;
-  }
-  env->GetByteArrayRegion(jtarget, jtarget_off, jtarget_len,
-                          reinterpret_cast<jbyte*>(target.get()));
-
-  ROCKSDB_NAMESPACE::Slice target_slice(target.get(), jtarget_len);
-
-  auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::Iterator*>(handle);
-  it->SeekForPrev(target_slice);
-}
-
 /*
  * Class:     org_rocksdb_SstFileReaderIterator
  * Method:    refresh0

java/rocksjni/write_batch_with_index.cc

@@ -735,11 +735,9 @@ void Java_org_rocksdb_WBWIRocksIterator_seek0(JNIEnv* env, jobject /*jobj*/,
                                               jlong handle, jbyteArray jtarget,
                                               jint jtarget_len) {
   auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::WBWIIterator*>(handle);
-  jbyte* target = new jbyte[jtarget_len];
-  env->GetByteArrayRegion(jtarget, 0, jtarget_len, target);
-  if (env->ExceptionCheck()) {
-    // exception thrown: ArrayIndexOutOfBoundsException
-    delete[] target;
+  jbyte* target = env->GetByteArrayElements(jtarget, nullptr);
+  if (target == nullptr) {
+    // exception thrown: OutOfMemoryError
     return;
   }
 
@@ -748,7 +746,7 @@ void Java_org_rocksdb_WBWIRocksIterator_seek0(JNIEnv* env, jobject /*jobj*/,
 
   it->Seek(target_slice);
 
-  delete[] target;
+  env->ReleaseByteArrayElements(jtarget, target, JNI_ABORT);
 }
 
 /*
@@ -767,33 +765,6 @@ void Java_org_rocksdb_WBWIRocksIterator_seekDirect0(
                                           jtarget_len);
 }
 
-/*
- * This method supports fetching into indirect byte buffers;
- * the Java wrapper extracts the byte[] and passes it here.
- *
- * Class:     org_rocksdb_WBWIRocksIterator
- * Method:    seekByteArray0
- * Signature: (J[BII)V
- */
-void Java_org_rocksdb_WBWIRocksIterator_seekByteArray0(
-    JNIEnv* env, jobject /*jobj*/, jlong handle, jbyteArray jtarget,
-    jint jtarget_off, jint jtarget_len) {
-  const std::unique_ptr<char[]> target(new char[jtarget_len]);
-  if (target == nullptr) {
-    jclass oom_class = env->FindClass("/lang/java/OutOfMemoryError");
-    env->ThrowNew(oom_class,
-                  "Memory allocation failed in RocksDB JNI function");
-    return;
-  }
-  env->GetByteArrayRegion(jtarget, jtarget_off, jtarget_len,
-                          reinterpret_cast<jbyte*>(target.get()));
-
-  ROCKSDB_NAMESPACE::Slice target_slice(target.get(), jtarget_len);
-
-  auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::WBWIIterator*>(handle);
-  it->Seek(target_slice);
-}
-
 /*
  * Class:     org_rocksdb_WBWIRocksIterator
  * Method:    seekForPrev0
@@ -805,11 +776,9 @@ void Java_org_rocksdb_WBWIRocksIterator_seekForPrev0(JNIEnv* env,
                                                      jbyteArray jtarget,
                                                      jint jtarget_len) {
   auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::WBWIIterator*>(handle);
-  jbyte* target = new jbyte[jtarget_len];
-  env->GetByteArrayRegion(jtarget, 0, jtarget_len, target);
-  if (env->ExceptionCheck()) {
-    // exception thrown: ArrayIndexOutOfBoundsException
-    delete[] target;
+  jbyte* target = env->GetByteArrayElements(jtarget, nullptr);
+  if (target == nullptr) {
+    // exception thrown: OutOfMemoryError
     return;
   }
 
@@ -818,50 +787,7 @@ void Java_org_rocksdb_WBWIRocksIterator_seekForPrev0(JNIEnv* env,
 
   it->SeekForPrev(target_slice);
 
-  delete[] target;
-}
-
-/*
- * Class:     org_rocksdb_WBWIRocksIterator
- * Method:    seekForPrevDirect0
- * Signature: (JLjava/nio/ByteBuffer;II)V
- */
-void Java_org_rocksdb_WBWIRocksIterator_seekForPrevDirect0(
-    JNIEnv* env, jobject /*jobj*/, jlong handle, jobject jtarget,
-    jint jtarget_off, jint jtarget_len) {
-  auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::WBWIIterator*>(handle);
-  auto seek_for_prev = [&it](ROCKSDB_NAMESPACE::Slice& target_slice) {
-    it->SeekForPrev(target_slice);
-  };
-  ROCKSDB_NAMESPACE::JniUtil::k_op_direct(seek_for_prev, env, jtarget,
-                                          jtarget_off, jtarget_len);
-}
-
-/*
- * This method supports fetching into indirect byte buffers;
- * the Java wrapper extracts the byte[] and passes it here.
- *
- * Class:     org_rocksdb_WBWIRocksIterator
- * Method:    seekForPrevByteArray0
- * Signature: (J[BII)V
- */
-void Java_org_rocksdb_WBWIRocksIterator_seekForPrevByteArray0(
-    JNIEnv* env, jobject /*jobj*/, jlong handle, jbyteArray jtarget,
-    jint jtarget_off, jint jtarget_len) {
-  const std::unique_ptr<char[]> target(new char[jtarget_len]);
-  if (target == nullptr) {
-    jclass oom_class = env->FindClass("/lang/java/OutOfMemoryError");
-    env->ThrowNew(oom_class,
-                  "Memory allocation failed in RocksDB JNI function");
-    return;
-  }
-  env->GetByteArrayRegion(jtarget, jtarget_off, jtarget_len,
-                          reinterpret_cast<jbyte*>(target.get()));
-
-  ROCKSDB_NAMESPACE::Slice target_slice(target.get(), jtarget_len);
-
-  auto* it = reinterpret_cast<ROCKSDB_NAMESPACE::WBWIIterator*>(handle);
-  it->SeekForPrev(target_slice);
+  env->ReleaseByteArrayElements(jtarget, target, JNI_ABORT);
 }
 
 /*

java/src/main/java/org/rocksdb/AbstractRocksIterator.java

@@ -55,40 +55,30 @@ public abstract class AbstractRocksIterator<P extends RocksObject>
   }
 
   @Override
-  public void seek(final byte[] target) {
+  public void seek(byte[] target) {
     assert (isOwningHandle());
     seek0(nativeHandle_, target, target.length);
   }
 
-  @Override
-  public void seekForPrev(final byte[] target) {
-    assert (isOwningHandle());
-    seekForPrev0(nativeHandle_, target, target.length);
-  }
+ @Override
+ public void seekForPrev(byte[] target) {
+   assert (isOwningHandle());
+   seekForPrev0(nativeHandle_, target, target.length);
+ }
 
-  @Override
-  public void seek(final ByteBuffer target) {
-    assert (isOwningHandle());
-    if (target.isDirect()) {
-      seekDirect0(nativeHandle_, target, target.position(), target.remaining());
-    } else {
-      seekByteArray0(nativeHandle_, target.array(), target.arrayOffset() + target.position(),
-          target.remaining());
-    }
-    target.position(target.limit());
-  }
+ @Override
+ public void seek(ByteBuffer target) {
+   assert (isOwningHandle() && target.isDirect());
+   seekDirect0(nativeHandle_, target, target.position(), target.remaining());
+   target.position(target.limit());
+ }
 
-  @Override
-  public void seekForPrev(final ByteBuffer target) {
-    assert (isOwningHandle());
-    if (target.isDirect()) {
-      seekForPrevDirect0(nativeHandle_, target, target.position(), target.remaining());
-    } else {
-      seekForPrevByteArray0(nativeHandle_, target.array(), target.arrayOffset() + target.position(),
-          target.remaining());
-    }
-    target.position(target.limit());
-  }
+ @Override
+ public void seekForPrev(ByteBuffer target) {
+   assert (isOwningHandle() && target.isDirect());
+   seekForPrevDirect0(nativeHandle_, target, target.position(), target.remaining());
+   target.position(target.limit());
+ }
 
   @Override
   public void next() {
@@ -139,8 +129,5 @@ public abstract class AbstractRocksIterator<P extends RocksObject>
   abstract void seekForPrev0(long handle, byte[] target, int targetLen);
   abstract void seekDirect0(long handle, ByteBuffer target, int targetOffset, int targetLen);
   abstract void seekForPrevDirect0(long handle, ByteBuffer target, int targetOffset, int targetLen);
-  abstract void seekByteArray0(long handle, byte[] target, int targetOffset, int targetLen);
-  abstract void seekForPrevByteArray0(long handle, byte[] target, int targetOffset, int targetLen);
-
   abstract void status0(long handle) throws RocksDBException;
 }

java/src/main/java/org/rocksdb/BackupEngine.java

@@ -226,8 +226,8 @@ public class BackupEngine extends RocksObject implements AutoCloseable {
         restoreOptions.nativeHandle_);
   }
 
-  private native static long open(final long env, final long backupEngineOptions)
-      throws RocksDBException;
+  private native static long open(final long env,
+      final long backupableDbOptions) throws RocksDBException;
 
   private native void createNewBackup(final long handle, final long dbHandle,
       final boolean flushBeforeBackup) throws RocksDBException;