Fixing crash test and other refactoring

.github/workflows/pr-jobs.yml

@@ -484,7 +484,7 @@ jobs:
     runs-on:
       labels: 4-core-ubuntu
     container:
-      image: evolvedbinary/rocksjava:centos7_x64-be
+      image: evolvedbinary/rocksjava:centos6_x64-be
       options: --shm-size=16gb
     steps:
     # The docker image is intentionally based on an OS that has an older GLIBC version.
@@ -512,7 +512,7 @@ jobs:
     runs-on:
       labels: 4-core-ubuntu
     container:
-      image: evolvedbinary/rocksjava:centos7_x64-be
+      image: evolvedbinary/rocksjava:centos6_x64-be
       options: --shm-size=16gb
     steps:
     # The docker image is intentionally based on an OS that has an older GLIBC version.
@@ -607,7 +607,7 @@ jobs:
     runs-on:
       labels: 4-core-ubuntu
     container:
-      image: evolvedbinary/rocksjava:alpine3_x64-be
+      image: evolvedbinary/rocksjava:rockylinux8_x64-be
       options: --shm-size=16gb
     steps:
     - uses: actions/checkout@v4.1.0

HISTORY.md

@@ -1,36 +1,6 @@
 # Rocksdb Change Log
 > NOTE: Entries for next release do not go here. Follow instructions in `unreleased_history/README.txt`
 
-## 10.4.2 (07/09/2025)
-### Bug Fixes
-* Fix a race condition between concurrent DB::Open sharing the same SstFileManager instance.
-
-## 10.4.1 (07/01/2025)
-### Behavior Changes
-* RocksDB now triggers eligible compactions every 12 hours when periodic compaction is configured. This solves a limitation of the compaction trigger mechanism, which would only trigger compaction after specific events like flush, compaction, or SetOptions.
-### Bug Fixes
-* Fix a bug in BackupEngine that can crash backup due to a null FSWritableFile passed to WritableFileWriter.
-
-## 10.4.0 (06/20/2025)
-### New Features
-* Add a new CF option `memtable_avg_op_scan_flush_trigger` that supports triggering memtable flush when an iterator scans through an expensive range of keys, with the average number of skipped keys from the active memtable exceeding the threshold.
-* Vector based memtable now supports concurrent writers (DBOptions::allow_concurrent_memtable_write) #13675.
-* Add new experimental `TransactionOptions::large_txn_commit_optimize_byte_threshold` to enable optimizations for large transaction commit by transaction batch data size.
-* Add a new option `CompactionOptionsUniversal::reduce_file_locking` and if it's true, auto universal compaction picking will adjust to minimize locking of input files when bottom priority compactions are waiting to run. This can increase the likelihood of existing L0s being selected for compaction, thereby improving write stall and reducing read regression.
-* Add new `format_version=7` to aid experimental support of custom compression algorithms with CompressionManager and block-based table. This format version includes changing the format of `TableProperties::compression_name`.
-
-### Public API Changes
-* Change NewExternalTableFactory to return a unique_ptr instead of shared_ptr.
-* Add an optional min file size requirement for deletion triggered compaction. It can be specified when creating `CompactOnDeletionCollectorFactory`.
-
-### Behavior Changes
-* `TransactionOptions::large_txn_commit_optimize_threshold` now has default value 0 for disabled. `TransactionDBOptions::txn_commit_bypass_memtable_threshold` now has no effect on transactions.
-
-### Bug Fixes
-* Fix a bug where CreateColumnFamilyWithImport() could miss the SST file for the memtable flush it triggered. The exported CF then may not contain the updates in the memtable when CreateColumnFamilyWithImport() is called.
-* Fix iterator operations returning NotImplemented status if disallow_memtable_writes and paranoid_memory_checks CF options are both set.
-* Fixed handling of file checksums in IngestExternalFile() to allow providing checksums using recognized but not necessarily the DB's preferred checksum function, to ease migration between checksum functions.
-
 ## 10.3.0 (05/17/2025)
 ### New Features
 * Add new experimental `CompactionOptionsFIFO::allow_trivial_copy_when_change_temperature` along with `CompactionOptionsFIFO::trivial_copy_buffer_size` to allow optimizing FIFO compactions with tiering when kChangeTemperature to move files from source tier FileSystem to another tier FileSystem via trivial and direct copying raw sst file instead of reading thru the content of the SST file then rebuilding the table files.

Makefile

@@ -2147,14 +2147,14 @@ ZLIB_DOWNLOAD_BASE ?= http://zlib.net
 BZIP2_VER ?= 1.0.8
 BZIP2_SHA256 ?= ab5a03176ee106d3f0fa90e381da478ddae405918153cca248e682cd0c4a2269
 BZIP2_DOWNLOAD_BASE ?= http://sourceware.org/pub/bzip2
-SNAPPY_VER ?= 1.2.2
-SNAPPY_SHA256 ?= 90f74bc1fbf78a6c56b3c4a082a05103b3a56bb17bca1a27e052ea11723292dc
+SNAPPY_VER ?= 1.2.1
+SNAPPY_SHA256 ?= 736aeb64d86566d2236ddffa2865ee5d7a82d26c9016b36218fcc27ea4f09f86
 SNAPPY_DOWNLOAD_BASE ?= https://github.com/google/snappy/archive
-LZ4_VER ?= 1.10.0
-LZ4_SHA256 ?= 537512904744b35e232912055ccf8ec66d768639ff3abe5788d90d792ec5f48b
+LZ4_VER ?= 1.9.4
+LZ4_SHA256 ?= 0b0e3aa07c8c063ddf40b082bdf7e37a1562bda40a0ff5272957f3e987e0e54b
 LZ4_DOWNLOAD_BASE ?= https://github.com/lz4/lz4/archive
-ZSTD_VER ?= 1.5.7
-ZSTD_SHA256 ?= 37d7284556b20954e56e1ca85b80226768902e2edabd3b649e9e72c0c9012ee3
+ZSTD_VER ?= 1.5.5
+ZSTD_SHA256 ?= 98e9c3d949d1b924e28e01eccb7deed865eefebf25c2f21c702e5cd5b63b85e1
 ZSTD_DOWNLOAD_BASE ?= https://github.com/facebook/zstd/archive
 CURL_SSL_OPTS ?= --tlsv1
 
@@ -2375,27 +2375,27 @@ rocksdbjavastaticreleasedocker: rocksdbjavastaticosx rocksdbjavastaticdockerx86
 
 rocksdbjavastaticdockerx86:
 	mkdir -p java/target
-	docker run --rm --name rocksdb_linux_x86-be --platform linux/386 --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:centos7_x86-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
+	docker run --rm --name rocksdb_linux_x86-be --platform linux/386 --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:centos6_x86-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
 
 rocksdbjavastaticdockerx86_64:
 	mkdir -p java/target
-	docker run --rm --name rocksdb_linux_x64-be --platform linux/amd64 --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:centos7_x64-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
+	docker run --rm --name rocksdb_linux_x64-be --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:centos6_x64-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
 
 rocksdbjavastaticdockerppc64le:
 	mkdir -p java/target
-	docker run --rm --name rocksdb_linux_ppc64le-be --platform linux/ppc64le --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:centos7_ppc64le-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
+	docker run --rm --name rocksdb_linux_ppc64le-be --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:centos7_ppc64le-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
 
 rocksdbjavastaticdockerarm64v8:
 	mkdir -p java/target
-	docker run --rm --name rocksdb_linux_arm64v8-be --platform linux/aarch64 --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:centos7_arm64v8-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
+	docker run --rm --name rocksdb_linux_arm64v8-be --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:centos7_arm64v8-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
 
 rocksdbjavastaticdockers390x:
 	mkdir -p java/target
-	docker run --rm --name rocksdb_linux_s390x-be --platform linux/s390x --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:ubuntu18_s390x-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
+	docker run --rm --name rocksdb_linux_s390x-be --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:ubuntu18_s390x-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
 
 rocksdbjavastaticdockerriscv64:
 	mkdir -p java/target
-	docker run --rm --name rocksdb_linux_riscv64-be --platform linux/riscv64 --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:ubuntu20_riscv64-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
+	docker run --rm --name rocksdb_linux_riscv64-be --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:ubuntu20_riscv64-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
 
 rocksdbjavastaticdockerx86musl:
 	mkdir -p java/target
@@ -2403,19 +2403,19 @@ rocksdbjavastaticdockerx86musl:
 
 rocksdbjavastaticdockerx86_64musl:
 	mkdir -p java/target
-	docker run --rm --name rocksdb_linux_x64-musl-be --platform linux/amd64 --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:alpine3_x64-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
+	docker run --rm --name rocksdb_linux_x64-musl-be --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:alpine3_x64-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
 
 rocksdbjavastaticdockerppc64lemusl:
 	mkdir -p java/target
-	docker run --rm --name rocksdb_linux_ppc64le-musl-be --platform linux/ppc64le --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:alpine3_ppc64le-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
+	docker run --rm --name rocksdb_linux_ppc64le-musl-be --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:alpine3_ppc64le-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
 
 rocksdbjavastaticdockerarm64v8musl:
 	mkdir -p java/target
-	docker run --rm --name rocksdb_linux_arm64v8-musl-be --platform linux/aarch64 --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:alpine3_arm64v8-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
+	docker run --rm --name rocksdb_linux_arm64v8-musl-be --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:alpine3_arm64v8-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
 
 rocksdbjavastaticdockers390xmusl:
 	mkdir -p java/target
-	docker run --rm --name rocksdb_linux_s390x-musl-be --platform linux/s390x --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:alpine3_s390x-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
+	docker run --rm --name rocksdb_linux_s390x-musl-be --attach stdin --attach stdout --attach stderr --volume $(HOME)/.m2:/root/.m2:ro --volume `pwd`:/rocksdb-host:ro --volume /rocksdb-local-build --volume `pwd`/java/target:/rocksdb-java-target --env DEBUG_LEVEL=$(DEBUG_LEVEL) --env J=$(J) evolvedbinary/rocksjava:alpine3_s390x-be /rocksdb-host/java/crossbuild/docker-build-linux.sh
 
 rocksdbjavastaticpublish: rocksdbjavastaticrelease rocksdbjavastaticpublishcentral
 
@@ -2470,8 +2470,8 @@ jtest_run:
 jtest: rocksdbjava
 	cd java;$(MAKE) sample test
 
-jpmd: rocksdbjavageneratepom
-	cd java;$(MAKE) java java_test pmd
+jpmd: rocksdbjava rocksdbjavageneratepom
+	cd java;$(MAKE) pmd
 
 jdb_bench:
 	cd java;$(MAKE) db_bench;

cache/compressed_secondary_cache.cc

@@ -80,16 +80,15 @@ std::unique_ptr<SecondaryCacheResultHandle> CompressedSecondaryCache::Lookup(
     ptr = reinterpret_cast<CacheAllocationPtr*>(handle_value);
     handle_value_charge = cache_->GetCharge(lru_handle);
     data_ptr = ptr->get();
-    const char* limit = ptr->get() + handle_value_charge;
-    data_ptr =
-        GetVarint32Ptr(data_ptr, limit, static_cast<uint32_t*>(&type_32));
+    data_ptr = GetVarint32Ptr(data_ptr, data_ptr + 1,
+                              static_cast<uint32_t*>(&type_32));
     type = static_cast<CompressionType>(type_32);
-    data_ptr =
-        GetVarint32Ptr(data_ptr, limit, static_cast<uint32_t*>(&source_32));
+    data_ptr = GetVarint32Ptr(data_ptr, data_ptr + 1,
+                              static_cast<uint32_t*>(&source_32));
     source = static_cast<CacheTier>(source_32);
     uint64_t data_size = 0;
-    data_ptr =
-        GetVarint64Ptr(data_ptr, limit, static_cast<uint64_t*>(&data_size));
+    data_ptr = GetVarint64Ptr(data_ptr, ptr->get() + handle_value_charge,
+                              static_cast<uint64_t*>(&data_size));
     assert(handle_value_charge > data_size);
     handle_value_charge = data_size;
   }

db/builder.cc

@@ -218,7 +218,8 @@ Status BuildTable(
       const Slice& key = c_iter.key();
       const Slice& value = c_iter.value();
       ParsedInternalKey ikey = c_iter.ikey();
-      Slice key_after_flush = key;
+      key_after_flush_buf.assign(key.data(), key.size());
+      Slice key_after_flush = key_after_flush_buf;
       Slice value_after_flush = value;
 
       if (ikey.type == kTypeValuePreferredSeqno) {
@@ -236,7 +237,6 @@ Status BuildTable(
               std::min(smallest_preferred_seqno, preferred_seqno);
         } else {
           // Cannot get a useful preferred seqno, convert it to a kTypeValue.
-          key_after_flush_buf.assign(key.data(), key.size());
           UpdateInternalKey(&key_after_flush_buf, ikey.sequence, kTypeValue);
           ikey = ParsedInternalKey(ikey.user_key, ikey.sequence, kTypeValue);
           key_after_flush = key_after_flush_buf;

db/column_family.cc

@@ -110,48 +110,23 @@ void GetInternalTblPropCollFactory(
   }
 }
 
-Status CheckCompressionSupportedWithManager(
-    CompressionType type, UnownedPtr<CompressionManager> mgr) {
-  if (mgr) {
-    if (!mgr->SupportsCompressionType(type)) {
-      return Status::NotSupported("Compression type " +
-                                  CompressionTypeToString(type) +
-                                  " is not recognized/supported by this "
-                                  "version of CompressionManager " +
-                                  mgr->GetId());
-    }
-  } else {
-    if (!CompressionTypeSupported(type)) {
-      if (type <= kLastBuiltinCompression) {
-        return Status::InvalidArgument("Compression type " +
-                                       CompressionTypeToString(type) +
-                                       " is not linked with the binary.");
-      } else {
-        return Status::NotSupported(
-            "Compression type " + CompressionTypeToString(type) +
-            " is not recognized/supported by built-in CompressionManager.");
-      }
-    }
-  }
-  return Status::OK();
-}
-
 Status CheckCompressionSupported(const ColumnFamilyOptions& cf_options) {
   if (!cf_options.compression_per_level.empty()) {
     for (size_t level = 0; level < cf_options.compression_per_level.size();
          ++level) {
-      Status s = CheckCompressionSupportedWithManager(
-          cf_options.compression_per_level[level],
-          cf_options.compression_manager.get());
-      if (!s.ok()) {
-        return s;
+      if (!CompressionTypeSupported(cf_options.compression_per_level[level])) {
+        return Status::InvalidArgument(
+            "Compression type " +
+            CompressionTypeToString(cf_options.compression_per_level[level]) +
+            " is not linked with the binary.");
       }
     }
   } else {
-    Status s = CheckCompressionSupportedWithManager(
-        cf_options.compression, cf_options.compression_manager.get());
-    if (!s.ok()) {
-      return s;
+    if (!CompressionTypeSupported(cf_options.compression)) {
+      return Status::InvalidArgument(
+          "Compression type " +
+          CompressionTypeToString(cf_options.compression) +
+          " is not linked with the binary.");
     }
   }
   if (cf_options.compression_opts.zstd_max_train_bytes > 0) {
@@ -1633,8 +1608,6 @@ Status ColumnFamilyData::SetOptions(
   Status s = GetColumnFamilyOptionsFromMap(config_opts, cf_opts, options_map,
                                            &cf_opts);
   if (s.ok()) {
-    // FIXME: we should call SanitizeOptions() too or consolidate it with
-    // ValidateOptions().
     s = ValidateOptions(db_opts, cf_opts);
   }
   if (s.ok()) {

db/compaction/compaction_job_test.cc

@@ -458,10 +458,9 @@ class CompactionJobTestBase : public testing::Test {
       ReadOptions read_opts;
       Status s = cf_options_.table_factory->NewTableReader(
           read_opts,
-          TableReaderOptions(cfd->ioptions(), /*prefix_extractor=*/nullptr,
-                             /*compression_manager=*/nullptr, FileOptions(),
+          TableReaderOptions(cfd->ioptions(), nullptr, FileOptions(),
                              cfd_->internal_comparator(),
-                             /*block_protection_bytes_per_key=*/0),
+                             0 /* block_protection_bytes_per_key */),
           std::move(freader), file_size, &table_reader, false);
       ASSERT_OK(s);
       assert(table_reader);

db/convenience.cc

@@ -93,10 +93,9 @@ Status VerifySstFileChecksumInternal(const Options& options,
           nullptr /* file_read_hist */, ioptions.rate_limiter.get()));
   const bool kImmortal = true;
   auto reader_options = TableReaderOptions(
-      ioptions, options.prefix_extractor, options.compression_manager.get(),
-      env_options, internal_comparator, options.block_protection_bytes_per_key,
-      false /* skip_filters */, !kImmortal, false /* force_direct_prefetch */,
-      -1 /* level */);
+      ioptions, options.prefix_extractor, env_options, internal_comparator,
+      options.block_protection_bytes_per_key, false /* skip_filters */,
+      !kImmortal, false /* force_direct_prefetch */, -1 /* level */);
   reader_options.largest_seqno = largest_seqno;
   s = options.table_factory->NewTableReader(
       read_options, reader_options, std::move(file_reader), file_size,

db/db_block_cache_test.cc

@@ -829,78 +829,68 @@ TEST_F(DBBlockCacheTest, CacheCompressionDict) {
   const int kNumEntriesPerFile = 128;
   const int kNumBytesPerEntry = 1024;
 
-  std::vector<CompressionType> dict_compressions =
-      GetSupportedDictCompressions();
+  // Try all the available libraries that support dictionary compression
+  std::vector<CompressionType> compression_types;
+  if (Zlib_Supported()) {
+    compression_types.push_back(kZlibCompression);
+  }
+  if (LZ4_Supported()) {
+    compression_types.push_back(kLZ4Compression);
+    compression_types.push_back(kLZ4HCCompression);
+  }
+  if (ZSTD_Supported()) {
+    compression_types.push_back(kZSTD);
+  }
   Random rnd(301);
-  // Format version before and after compression handling changes
-  for (int format_version : {6, 7}) {
-    // Test all supported compression types because (at least historically)
-    // dictionary compression could be enabled and a dictionary block saved
-    // but ignored by some compression types. Ensure we at least don't crash
-    // or return corruption for those.
-    for (auto compression_type : GetSupportedCompressions()) {
-      // Extra handling checks only for types actually supporting dictionary
-      // compression.
-      bool dict_supported =
-          std::count(dict_compressions.begin(), dict_compressions.end(),
-                     compression_type) > 0;
+  for (auto compression_type : compression_types) {
+    Options options = CurrentOptions();
+    options.bottommost_compression = compression_type;
+    options.bottommost_compression_opts.max_dict_bytes = 4096;
+    options.bottommost_compression_opts.enabled = true;
+    options.create_if_missing = true;
+    options.num_levels = 2;
+    options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
+    options.target_file_size_base = kNumEntriesPerFile * kNumBytesPerEntry;
+    BlockBasedTableOptions table_options;
+    table_options.cache_index_and_filter_blocks = true;
+    table_options.block_cache.reset(new MockCache());
+    options.table_factory.reset(NewBlockBasedTableFactory(table_options));
+    DestroyAndReopen(options);
 
-      Options options = CurrentOptions();
-      options.bottommost_compression = compression_type;
-      options.bottommost_compression_opts.max_dict_bytes = 4096;
-      options.bottommost_compression_opts.enabled = true;
-      options.create_if_missing = true;
-      options.num_levels = 2;
-      options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
-      options.target_file_size_base = kNumEntriesPerFile * kNumBytesPerEntry;
-      BlockBasedTableOptions table_options;
-      table_options.cache_index_and_filter_blocks = true;
-      table_options.block_cache.reset(new MockCache());
-      table_options.format_version = format_version;
-      options.table_factory.reset(NewBlockBasedTableFactory(table_options));
-      DestroyAndReopen(options);
+    RecordCacheCountersForCompressionDict(options);
 
-      RecordCacheCountersForCompressionDict(options);
-
-      for (int i = 0; i < kNumFiles; ++i) {
-        ASSERT_EQ(i, NumTableFilesAtLevel(0, 0));
-        for (int j = 0; j < kNumEntriesPerFile; ++j) {
-          std::string value = rnd.RandomString(kNumBytesPerEntry);
-          ASSERT_OK(Put(Key(j * kNumFiles + i), value.c_str()));
-        }
-        ASSERT_OK(Flush());
-      }
-      ASSERT_OK(dbfull()->TEST_WaitForCompact());
-      ASSERT_EQ(0, NumTableFilesAtLevel(0));
-      ASSERT_EQ(kNumFiles, NumTableFilesAtLevel(1));
-
-      if (dict_supported) {
-        // Compression dictionary blocks are preloaded.
-        CheckCacheCountersForCompressionDict(
-            options, kNumFiles /* expected_compression_dict_misses */,
-            0 /* expected_compression_dict_hits */,
-            kNumFiles /* expected_compression_dict_inserts */);
-      }
-
-      // Seek to a key in a file. It should cause the SST's dictionary
-      // meta-block to be read.
-      RecordCacheCounters(options);
-      RecordCacheCountersForCompressionDict(options);
-      ReadOptions read_options;
-      ASSERT_NE("NOT_FOUND", Get(Key(kNumFiles * kNumEntriesPerFile - 1)));
-
-      if (dict_supported) {
-        // Two block hits: index and dictionary since they are prefetched
-        // One block missed/added: data block
-        CheckCacheCounters(options, 1 /* expected_misses */,
-                           2 /* expected_hits */, 1 /* expected_inserts */,
-                           0 /* expected_failures */);
-        CheckCacheCountersForCompressionDict(
-            options, 0 /* expected_compression_dict_misses */,
-            1 /* expected_compression_dict_hits */,
-            0 /* expected_compression_dict_inserts */);
+    for (int i = 0; i < kNumFiles; ++i) {
+      ASSERT_EQ(i, NumTableFilesAtLevel(0, 0));
+      for (int j = 0; j < kNumEntriesPerFile; ++j) {
+        std::string value = rnd.RandomString(kNumBytesPerEntry);
+        ASSERT_OK(Put(Key(j * kNumFiles + i), value.c_str()));
       }
+      ASSERT_OK(Flush());
     }
+    ASSERT_OK(dbfull()->TEST_WaitForCompact());
+    ASSERT_EQ(0, NumTableFilesAtLevel(0));
+    ASSERT_EQ(kNumFiles, NumTableFilesAtLevel(1));
+
+    // Compression dictionary blocks are preloaded.
+    CheckCacheCountersForCompressionDict(
+        options, kNumFiles /* expected_compression_dict_misses */,
+        0 /* expected_compression_dict_hits */,
+        kNumFiles /* expected_compression_dict_inserts */);
+
+    // Seek to a key in a file. It should cause the SST's dictionary meta-block
+    // to be read.
+    RecordCacheCounters(options);
+    RecordCacheCountersForCompressionDict(options);
+    ReadOptions read_options;
+    ASSERT_NE("NOT_FOUND", Get(Key(kNumFiles * kNumEntriesPerFile - 1)));
+    // Two block hits: index and dictionary since they are prefetched
+    // One block missed/added: data block
+    CheckCacheCounters(options, 1 /* expected_misses */, 2 /* expected_hits */,
+                       1 /* expected_inserts */, 0 /* expected_failures */);
+    CheckCacheCountersForCompressionDict(
+        options, 0 /* expected_compression_dict_misses */,
+        1 /* expected_compression_dict_hits */,
+        0 /* expected_compression_dict_inserts */);
   }
 }
 

db/db_compaction_test.cc

@@ -11093,59 +11093,6 @@ TEST_F(DBCompactionTest, RecordNewestKeyTimeForTtlCompaction) {
   ASSERT_OK(dbfull()->TEST_WaitForCompact());
   ASSERT_EQ(NumTableFilesAtLevel(0), 0);
 }
-
-class PeriodicCompactionListener : public EventListener {
- public:
-  explicit PeriodicCompactionListener() {}
-  void OnCompactionBegin(DB* /*db*/, const CompactionJobInfo& ci) override {
-    if (ci.compaction_reason == CompactionReason::kPeriodicCompaction) {
-      ++num_periodic_compactions;
-    }
-  }
-
-  std::atomic<int> num_periodic_compactions = 0;
-};
-
-TEST_F(DBCompactionTest, PeriodicTask) {
-  // Tests that when no trigger event is fired (flush/compaction/setoptions),
-  // periodic compaction is still triggered by a scheduled periodic function.
-  auto mock_clock = std::make_shared<MockSystemClock>(env_->GetSystemClock());
-  mock_clock->SetCurrentTime(100);
-  mock_clock->InstallTimedWaitFixCallback();
-  auto mock_env = std::make_unique<CompositeEnvWrapper>(env_, mock_clock);
-  SyncPoint::GetInstance()->SetCallBack(
-      "DBImpl::StartPeriodicTaskScheduler:Init", [&](void* arg) {
-        auto periodic_task_scheduler_ptr =
-            static_cast<PeriodicTaskScheduler*>(arg);
-        periodic_task_scheduler_ptr->TEST_OverrideTimer(mock_clock.get());
-      });
-
-  Options options;
-  options.env = mock_env.get();
-  options.compaction_style = kCompactionStyleUniversal;
-  options.statistics = CreateDBStatistics();
-  int kPeriodicCompactionSeconds = 7 * 24 * 60 * 60;  // 1 week
-  options.periodic_compaction_seconds = kPeriodicCompactionSeconds;
-  options.num_levels = 50;
-  auto listener = std::make_shared<PeriodicCompactionListener>();
-  options.listeners.push_back(listener);
-  ASSERT_OK(TryReopen(options));
-
-  Random* rnd = Random::GetTLSInstance();
-  for (int k = 0; k < 10; ++k) {
-    ASSERT_OK(Put(Key(k), rnd->RandomString(100)));
-  }
-  ASSERT_OK(Flush());
-  ASSERT_OK(db_->CompactRange({}, nullptr, nullptr));
-  ASSERT_EQ(1, NumTableFilesAtLevel(49));
-
-  dbfull()->TEST_WaitForPeriodicTaskRun(
-      [&] { mock_clock->MockSleepForSeconds(kPeriodicCompactionSeconds + 1); });
-  ASSERT_OK(db_->WaitForCompact({}));
-
-  ASSERT_EQ(listener->num_periodic_compactions, 1);
-  Close();
-}
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

db/db_impl/db_impl.cc

@@ -253,9 +253,6 @@ DBImpl::DBImpl(const DBOptions& options, const std::string& dbname,
   periodic_task_functions_.emplace(
       PeriodicTaskType::kRecordSeqnoTime,
       [this]() { this->RecordSeqnoToTimeMapping(); });
-  periodic_task_functions_.emplace(
-      PeriodicTaskType::kTriggerCompaction,
-      [this]() { this->TriggerPeriodicCompaction(); });
 
   versions_.reset(new VersionSet(
       dbname_, &immutable_db_options_, file_options_, table_cache_.get(),
@@ -790,8 +787,7 @@ Status DBImpl::StartPeriodicTaskScheduler() {
     Status s = periodic_task_scheduler_.Register(
         PeriodicTaskType::kDumpStats,
         periodic_task_functions_.at(PeriodicTaskType::kDumpStats),
-        mutable_db_options_.stats_dump_period_sec,
-        /*run_immediately=*/true);
+        mutable_db_options_.stats_dump_period_sec);
     if (!s.ok()) {
       return s;
     }
@@ -800,8 +796,7 @@ Status DBImpl::StartPeriodicTaskScheduler() {
     Status s = periodic_task_scheduler_.Register(
         PeriodicTaskType::kPersistStats,
         periodic_task_functions_.at(PeriodicTaskType::kPersistStats),
-        mutable_db_options_.stats_persist_period_sec,
-        /*run_immediately=*/true);
+        mutable_db_options_.stats_persist_period_sec);
     if (!s.ok()) {
       return s;
     }
@@ -809,15 +804,7 @@ Status DBImpl::StartPeriodicTaskScheduler() {
 
   Status s = periodic_task_scheduler_.Register(
       PeriodicTaskType::kFlushInfoLog,
-      periodic_task_functions_.at(PeriodicTaskType::kFlushInfoLog),
-      /*run_immediately=*/true);
-
-  if (s.ok()) {
-    s = periodic_task_scheduler_.Register(
-        PeriodicTaskType::kTriggerCompaction,
-        periodic_task_functions_.at(PeriodicTaskType::kTriggerCompaction),
-        /*run_immediately=*/false);
-  }
+      periodic_task_functions_.at(PeriodicTaskType::kFlushInfoLog));
 
   return s;
 }
@@ -868,7 +855,7 @@ Status DBImpl::RegisterRecordSeqnoTimeWorker() {
     s = periodic_task_scheduler_.Register(
         PeriodicTaskType::kRecordSeqnoTime,
         periodic_task_functions_.at(PeriodicTaskType::kRecordSeqnoTime),
-        seqno_time_cadence, /*run_immediately=*/true);
+        seqno_time_cadence);
   }
 
   return s;
@@ -1378,7 +1365,7 @@ Status DBImpl::SetDBOptions(
         s = periodic_task_scheduler_.Register(
             PeriodicTaskType::kDumpStats,
             periodic_task_functions_.at(PeriodicTaskType::kDumpStats),
-            new_options.stats_dump_period_sec, /*run_immediately=*/true);
+            new_options.stats_dump_period_sec);
       }
       if (new_options.max_total_wal_size !=
           mutable_db_options_.max_total_wal_size) {
@@ -1393,7 +1380,7 @@ Status DBImpl::SetDBOptions(
           s = periodic_task_scheduler_.Register(
               PeriodicTaskType::kPersistStats,
               periodic_task_functions_.at(PeriodicTaskType::kPersistStats),
-              new_options.stats_persist_period_sec, /*run_immediately=*/true);
+              new_options.stats_persist_period_sec);
         }
       }
       mutex_.Lock();
@@ -6897,35 +6884,6 @@ void DBImpl::RecordSeqnoToTimeMapping() {
   sv_context.Clean();
 }
 
-void DBImpl::TriggerPeriodicCompaction() {
-  TEST_SYNC_POINT("DBImpl::TriggerPeriodicCompaction:StartRunning");
-  {
-    InstrumentedMutexLock l(&mutex_);
-    ROCKS_LOG_INFO(immutable_db_options_.info_log,
-                   "Running the periodic task to trigger compactions.");
-
-    for (ColumnFamilyData* cfd : *versions_->GetColumnFamilySet()) {
-      if (cfd->IsDropped()) {
-        continue;
-      }
-      if (cfd->GetLatestCFOptions().periodic_compaction_seconds &&
-          !cfd->queued_for_compaction()) {
-        cfd->current()->storage_info()->ComputeCompactionScore(
-            cfd->ioptions(), cfd->GetLatestMutableCFOptions());
-        EnqueuePendingCompaction(cfd);
-        if (cfd->queued_for_compaction()) {
-          ROCKS_LOG_INFO(immutable_db_options_.info_log,
-                         "Periodic task to trigger compaction queued Column "
-                         "family [%s] for compaction.",
-                         cfd->GetName().c_str());
-        }
-      }
-    }
-    MaybeScheduleFlushOrCompaction();
-    bg_cv_.SignalAll();
-  }
-}
-
 void DBImpl::TrackOrUntrackFiles(
     const std::vector<std::string>& existing_data_files, bool track) {
   auto sfm = static_cast_with_check<SstFileManagerImpl>(

db/db_impl/db_impl.h

@@ -1286,12 +1286,6 @@ class DBImpl : public DB {
   // For the background timer job
   void RecordSeqnoToTimeMapping();
 
-  // Compactions rely on an event triggers like flush/compaction/SetOptions.
-  // We need to trigger periodic compactions even when there is no such trigger.
-  // This function checks and schedules available compactions and will run
-  // periodically.
-  void TriggerPeriodicCompaction();
-
   // REQUIRES: DB mutex held
   std::pair<SequenceNumber, uint64_t> GetSeqnoToTimeSample() const;
 

db/db_impl/db_impl_write.cc

@@ -856,7 +856,8 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options,
             write_group, current_sequence, column_family_memtables_.get(),
             &flush_scheduler_, &trim_history_scheduler_,
             write_options.ignore_missing_column_families,
-            0 /*recovery_log_number*/, this, seq_per_batch_, batch_per_txn_);
+            0 /*recovery_log_number*/, this, parallel, seq_per_batch_,
+            batch_per_txn_);
       } else {
         write_group.last_sequence = last_sequence;
         write_thread_.LaunchParallelMemTableWriters(&write_group);
@@ -1114,7 +1115,7 @@ Status DBImpl::PipelinedWriteImpl(const WriteOptions& write_options,
           memtable_write_group, w.sequence, column_family_memtables_.get(),
           &flush_scheduler_, &trim_history_scheduler_,
           write_options.ignore_missing_column_families, 0 /*log_number*/, this,
-          seq_per_batch_, batch_per_txn_);
+          false /*concurrent_memtable_writes*/, seq_per_batch_, batch_per_txn_);
       if (memtable_write_group.status
               .ok()) {  // Don't publish a partial batch write
         versions_->SetLastSequence(memtable_write_group.last_sequence);

db/db_iter.cc

@@ -511,6 +511,7 @@ bool DBIter::FindNextUserEntryInternal(bool skipping_saved_key,
 
             valid_ = true;
             return true;
+            break;
           case kTypeMerge:
             if (!PrepareValueInternal()) {
               return false;
@@ -522,6 +523,7 @@ bool DBIter::FindNextUserEntryInternal(bool skipping_saved_key,
             current_entry_is_merged_ = true;
             valid_ = true;
             return MergeValuesNewToOld();  // Go to a different state machine
+            break;
           default:
             valid_ = false;
             status_ = Status::Corruption(
@@ -1122,6 +1124,7 @@ bool DBIter::FindValueForCurrentKey() {
         }
         return true;
       }
+      break;
     case kTypeValue:
     case kTypeValuePreferredSeqno:
       SetValueAndColumnsFromPlain(pinned_value_);

db/db_memtable_test.cc

@@ -424,96 +424,6 @@ TEST_F(DBMemTableTest, IntegrityChecks) {
     ASSERT_FALSE(iter->Valid());
   }
 }
-
-TEST_F(DBMemTableTest, VectorConcurrentInsert) {
-  Options options;
-  options.create_if_missing = true;
-  options.create_missing_column_families = true;
-  options.allow_concurrent_memtable_write = true;
-  options.memtable_factory.reset(new VectorRepFactory());
-  DestroyAndReopen(options);
-  CreateAndReopenWithCF({"cf1"}, options);
-
-  // Multi-threaded writes
-  {
-    WriteOptions write_options;
-    std::vector<port::Thread> threads;
-    for (int i = 0; i < 10; ++i) {
-      threads.emplace_back([&, i]() {
-        int start = i * 100;
-        int end = start + 100;
-        WriteBatch batch;
-        for (int j = start; j < end; ++j) {
-          ASSERT_OK(
-              batch.Put(handles_[0], Key(j), "value" + std::to_string(j)));
-        }
-        ASSERT_OK(db_->Write(write_options, &batch));
-      });
-    }
-    for (auto& t : threads) {
-      t.join();
-    }
-
-    std::unique_ptr<Iterator> iter(
-        db_->NewIterator(ReadOptions(), handles_[0]));
-    iter->SeekToFirst();
-    for (int i = 0; i < 1000; ++i) {
-      ASSERT_TRUE(iter->Valid());
-      ASSERT_EQ(iter->key().ToString(), Key(i));
-      ASSERT_EQ(iter->value().ToString(), "value" + std::to_string(i));
-      iter->Next();
-    }
-    ASSERT_FALSE(iter->Valid());
-    ASSERT_OK(iter->status());
-  }
-
-  // Multi-threaded writes, multi CF
-  {
-    WriteOptions write_options;
-    std::vector<port::Thread> threads;
-    for (int i = 0; i < 10; ++i) {
-      threads.emplace_back([&, i]() {
-        int start = i * 100;
-        int end = start + 100;
-        WriteBatch batch;
-        for (int j = start; j < end; ++j) {
-          ASSERT_OK(batch.Put(handles_[0], Key(j), "CF0" + std::to_string(j)));
-          ASSERT_OK(batch.Put(handles_[1], Key(j), "CF1" + std::to_string(j)));
-        }
-        ASSERT_OK(db_->Write(write_options, &batch));
-      });
-    }
-
-    for (auto& t : threads) {
-      t.join();
-    }
-
-    std::unique_ptr<Iterator> iter0(
-        db_->NewIterator(ReadOptions(), handles_[0]));
-    std::unique_ptr<Iterator> iter1(
-        db_->NewIterator(ReadOptions(), handles_[1]));
-    iter0->SeekToFirst();
-    iter1->SeekToFirst();
-    for (int i = 0; i < 1000; ++i) {
-      ASSERT_TRUE(iter0->Valid());
-      ASSERT_EQ(iter0->key().ToString(), Key(i));
-      ASSERT_EQ(iter0->value().ToString(), "CF0" + std::to_string(i));
-      iter0->Next();
-
-      ASSERT_TRUE(iter1->Valid());
-      ASSERT_EQ(iter1->key().ToString(), Key(i));
-      ASSERT_EQ(iter1->value().ToString(), "CF1" + std::to_string(i));
-      iter1->Next();
-    }
-    ASSERT_FALSE(iter0->Valid());
-    ASSERT_OK(iter0->status());
-    ASSERT_FALSE(iter1->Valid());
-    ASSERT_OK(iter1->status());
-  }
-
-  ASSERT_OK(Flush(0));
-  ASSERT_OK(Flush(1));
-}
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

db/db_test.cc

@@ -6209,8 +6209,8 @@ TEST_F(DBTest, L0L1L2AndUpHitCounter) {
 }
 
 TEST_F(DBTest, EncodeDecompressedBlockSizeTest) {
-  // Allow testing format_version=1
-  bool& allow_unsupported_fv = TEST_AllowUnsupportedFormatVersion();
+  bool& allow_unsupported_fv =
+      BlockBasedTableFactory::AllowUnsupportedFormatVersion();
   SaveAndRestore guard(&allow_unsupported_fv);
   ASSERT_FALSE(allow_unsupported_fv);
 

db/db_test2.cc

@@ -24,7 +24,6 @@
 #include "rocksdb/persistent_cache.h"
 #include "rocksdb/trace_record.h"
 #include "rocksdb/trace_record_result.h"
-#include "rocksdb/utilities/object_registry.h"
 #include "rocksdb/utilities/replayer.h"
 #include "rocksdb/wal_filter.h"
 #include "test_util/testutil.h"
@@ -1886,48 +1885,52 @@ TEST_F(DBTest2, CompressionOptions) {
 
 TEST_F(DBTest2, RoundRobinManager) {
   if (ZSTD_Supported()) {
-    auto mgr =
-        std::make_shared<RoundRobinManager>(GetBuiltinV2CompressionManager());
+    auto mgr = std::make_shared<RoundRobinManager>(
+        GetDefaultBuiltinCompressionManager());
 
-    std::vector<std::string> values;
-    for (bool use_wrapper : {true}) {
-      SCOPED_TRACE((use_wrapper ? "With " : "No ") + std::string("wrapper"));
+    for (CompressionType type : {kZSTD}) {
+      std::vector<std::string> values;
+      for (bool use_wrapper : {true}) {
+        SCOPED_TRACE("Compression type: " + std::to_string(type) +
+                     (use_wrapper ? " with " : " no ") + "wrapper");
 
-      Options options = CurrentOptions();
-      options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
-      options.statistics->set_stats_level(StatsLevel::kExceptTimeForMutex);
-      BlockBasedTableOptions bbto;
-      bbto.enable_index_compression = false;
-      options.table_factory.reset(NewBlockBasedTableFactory(bbto));
-      options.compression_manager = use_wrapper ? mgr : nullptr;
-      DestroyAndReopen(options);
+        Options options = CurrentOptions();
+        options.compression = type;
+        options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
+        options.statistics->set_stats_level(StatsLevel::kExceptTimeForMutex);
+        BlockBasedTableOptions bbto;
+        bbto.enable_index_compression = false;
+        options.table_factory.reset(NewBlockBasedTableFactory(bbto));
+        options.compression_manager = use_wrapper ? mgr : nullptr;
+        DestroyAndReopen(options);
 
-      Random rnd(301);
-      constexpr int kCount = 13;
+        Random rnd(301);
+        constexpr int kCount = 13;
 
-      // Highly compressible blocks, except 1 non-compressible. Half of the
-      // compressible are morked for bypass and 1 marked for rejection. Values
-      // are large enough to ensure just 1 k-v per block.
-      for (int i = 0; i < kCount; ++i) {
-        std::string value;
-        if (i == 6) {
-          // One non-compressible block
-          value = rnd.RandomBinaryString(20000);
-        } else {
-          test::CompressibleString(&rnd, 0.1, 20000, &value);
+        // Highly compressible blocks, except 1 non-compressible. Half of the
+        // compressible are morked for bypass and 1 marked for rejection. Values
+        // are large enough to ensure just 1 k-v per block.
+        for (int i = 0; i < kCount; ++i) {
+          std::string value;
+          if (i == 6) {
+            // One non-compressible block
+            value = rnd.RandomBinaryString(20000);
+          } else {
+            test::CompressibleString(&rnd, 0.1, 20000, &value);
+          }
+          values.push_back(value);
+          ASSERT_OK(Put(Key(i), value));
+          ASSERT_EQ(Get(Key(i)), value);
         }
-        values.push_back(value);
-        ASSERT_OK(Put(Key(i), value));
-        ASSERT_EQ(Get(Key(i)), value);
-      }
-      ASSERT_OK(Flush());
+        ASSERT_OK(Flush());
 
-      // Ensure well-formed for reads
-      for (int i = 0; i < kCount; ++i) {
-        ASSERT_NE(Get(Key(i)), "NOT_FOUND");
-        ASSERT_EQ(Get(Key(i)), values[i]);
+        // Ensure well-formed for reads
+        for (int i = 0; i < kCount; ++i) {
+          ASSERT_NE(Get(Key(i)), "NOT_FOUND");
+          ASSERT_EQ(Get(Key(i)), values[i]);
+        }
+        ASSERT_EQ(Get(Key(kCount)), "NOT_FOUND");
       }
-      ASSERT_EQ(Get(Key(kCount)), "NOT_FOUND");
     }
   }
 }
@@ -1935,50 +1938,55 @@ TEST_F(DBTest2, RoundRobinManager) {
 TEST_F(DBTest2, RandomMixedCompressionManager) {
   if (ZSTD_Supported()) {
     auto mgr = std::make_shared<RandomMixedCompressionManager>(
-        GetBuiltinV2CompressionManager());
-    std::vector<std::string> values;
-    for (bool use_wrapper : {true}) {
-      SCOPED_TRACE((use_wrapper ? "With " : "No ") + std::string("wrapper"));
+        GetDefaultBuiltinCompressionManager());
+    // Currently mixedmanager only supports with preffered compression manager
+    // zstd
+    for (CompressionType type : {kZSTD}) {
+      std::vector<std::string> values;
+      for (bool use_wrapper : {true}) {
+        SCOPED_TRACE("Compression type: " + std::to_string(type) +
+                     (use_wrapper ? " with " : " no ") + "wrapper");
 
-      Options options = CurrentOptions();
-      options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
-      options.statistics->set_stats_level(StatsLevel::kExceptTimeForMutex);
-      BlockBasedTableOptions bbto;
-      bbto.enable_index_compression = false;
-      options.table_factory.reset(NewBlockBasedTableFactory(bbto));
-      options.compression_manager = use_wrapper ? mgr : nullptr;
-      DestroyAndReopen(options);
+        Options options = CurrentOptions();
+        options.compression = type;
+        options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
+        options.statistics->set_stats_level(StatsLevel::kExceptTimeForMutex);
+        BlockBasedTableOptions bbto;
+        bbto.enable_index_compression = false;
+        options.table_factory.reset(NewBlockBasedTableFactory(bbto));
+        options.compression_manager = use_wrapper ? mgr : nullptr;
+        DestroyAndReopen(options);
 
-      Random rnd(301);
-      constexpr int kCount = 13;
+        Random rnd(301);
+        constexpr int kCount = 13;
 
-      // Highly compressible blocks, except 1 non-compressible. Half of the
-      // compressible are morked for bypass and 1 marked for rejection. Values
-      // are large enough to ensure just 1 k-v per block.
-      for (int i = 0; i < kCount; ++i) {
-        std::string value;
-        if (i == 6) {
-          // One non-compressible block
-          value = rnd.RandomBinaryString(20000);
-        } else {
-          test::CompressibleString(&rnd, 0.1, 20000, &value);
+        // Highly compressible blocks, except 1 non-compressible. Half of the
+        // compressible are morked for bypass and 1 marked for rejection. Values
+        // are large enough to ensure just 1 k-v per block.
+        for (int i = 0; i < kCount; ++i) {
+          std::string value;
+          if (i == 6) {
+            // One non-compressible block
+            value = rnd.RandomBinaryString(20000);
+          } else {
+            test::CompressibleString(&rnd, 0.1, 20000, &value);
+          }
+          values.push_back(value);
+          ASSERT_OK(Put(Key(i), value));
+          ASSERT_EQ(Get(Key(i)), value);
         }
-        values.push_back(value);
-        ASSERT_OK(Put(Key(i), value));
-        ASSERT_EQ(Get(Key(i)), value);
-      }
-      ASSERT_OK(Flush());
+        ASSERT_OK(Flush());
 
-      // Ensure well-formed for reads
-      for (int i = 0; i < kCount; ++i) {
-        ASSERT_NE(Get(Key(i)), "NOT_FOUND");
-        ASSERT_EQ(Get(Key(i)), values[i]);
+        // Ensure well-formed for reads
+        for (int i = 0; i < kCount; ++i) {
+          ASSERT_NE(Get(Key(i)), "NOT_FOUND");
+          ASSERT_EQ(Get(Key(i)), values[i]);
+        }
+        ASSERT_EQ(Get(Key(kCount)), "NOT_FOUND");
       }
-      ASSERT_EQ(Get(Key(kCount)), "NOT_FOUND");
     }
   }
 }
-
 TEST_F(DBTest2, CompressionManagerWrapper) {
   // Test that we can use a custom CompressionManager to wrap the built-in
   // CompressionManager, thus adopting a custom *strategy* based on existing
@@ -1992,7 +2000,6 @@ TEST_F(DBTest2, CompressionManagerWrapper) {
 
   struct MyCompressor : public CompressorWrapper {
     using CompressorWrapper::CompressorWrapper;
-    const char* Name() const override { return "MyCompressor"; }
 
     Status CompressBlock(Slice uncompressed_data,
                          std::string* compressed_output,
@@ -2019,7 +2026,7 @@ TEST_F(DBTest2, CompressionManagerWrapper) {
   };
   struct MyManager : public CompressionManagerWrapper {
     using CompressionManagerWrapper::CompressionManagerWrapper;
-    const char* Name() const override { return "MyManager"; }
+    const char* Name() const override { return wrapped_->Name(); }
     std::unique_ptr<Compressor> GetCompressorForSST(
         const FilterBuildingContext& context, const CompressionOptions& opts,
         CompressionType preferred) override {
@@ -2027,7 +2034,7 @@ TEST_F(DBTest2, CompressionManagerWrapper) {
           wrapped_->GetCompressorForSST(context, opts, preferred));
     }
   };
-  auto mgr = std::make_shared<MyManager>(GetBuiltinV2CompressionManager());
+  auto mgr = std::make_shared<MyManager>(GetDefaultBuiltinCompressionManager());
 
   for (CompressionType type : GetSupportedCompressions()) {
     for (bool use_wrapper : {false, true}) {
@@ -2095,331 +2102,6 @@ TEST_F(DBTest2, CompressionManagerWrapper) {
   }
 }
 
-TEST_F(DBTest2, CompressionManagerCustomCompression) {
-  // Test that we can use a custom CompressionManager to implement custom
-  // compression algorithms, and that there are appropriate schema guard rails
-  // to ensure data is not processed by the wrong algorithm.
-  using Compressor8A = test::CompressorCustomAlg<kCustomCompression8A>;
-  using Compressor8B = test::CompressorCustomAlg<kCustomCompression8B>;
-  using Compressor8C = test::CompressorCustomAlg<kCustomCompression8C>;
-
-  if (!Compressor8A::Supported() || !LZ4_Supported()) {
-    fprintf(stderr,
-            "Prerequisite compression library not supported. Skipping\n");
-    return;
-  }
-
-  class MyManager : public CompressionManager {
-   public:
-    explicit MyManager(const char* compat_name) : compat_name_(compat_name) {}
-    const char* Name() const override { return name_.c_str(); }
-    const char* CompatibilityName() const override { return compat_name_; }
-
-    bool SupportsCompressionType(CompressionType type) const override {
-      return type == kCustomCompression8A || type == kCustomCompression8B ||
-             type == kCustomCompression8C ||
-             GetBuiltinV2CompressionManager()->SupportsCompressionType(type);
-    }
-
-    int used_compressor8A_count_ = 0;
-    int used_compressor8B_count_ = 0;
-    int used_compressor8C_count_ = 0;
-
-    std::unique_ptr<Compressor> GetCompressor(const CompressionOptions& opts,
-                                              CompressionType type) override {
-      switch (static_cast<unsigned char>(type)) {
-        case kCustomCompression8A:
-          used_compressor8A_count_++;
-          return std::make_unique<Compressor8A>();
-        case kCustomCompression8B:
-          used_compressor8B_count_++;
-          return std::make_unique<Compressor8B>();
-        case kCustomCompression8C:
-          used_compressor8C_count_++;
-          return std::make_unique<Compressor8C>();
-        // Also support built-in compression algorithms
-        default:
-          return GetBuiltinV2CompressionManager()->GetCompressor(opts, type);
-      }
-    }
-
-    std::shared_ptr<Decompressor> GetDecompressor() override {
-      return std::make_shared<test::DecompressorCustomAlg>();
-    }
-
-    RelaxedAtomic<CompressionType> last_specific_decompressor_type_{
-        kNoCompression};
-
-    std::shared_ptr<Decompressor> GetDecompressorForTypes(
-        const CompressionType* types_begin,
-        const CompressionType* types_end) override {
-      assert(types_end > types_begin);
-      last_specific_decompressor_type_.StoreRelaxed(*types_begin);
-      auto decomp = std::make_shared<test::DecompressorCustomAlg>();
-      decomp->SetAllowedTypes(types_begin, types_end);
-      return decomp;
-    }
-
-    void AddFriend(const std::shared_ptr<CompressionManager>& mgr) {
-      friends_[mgr->CompatibilityName()] = mgr;
-    }
-    std::shared_ptr<CompressionManager> FindCompatibleCompressionManager(
-        Slice compatibility_name) override {
-      std::shared_ptr<CompressionManager> rv =
-          CompressionManager::FindCompatibleCompressionManager(
-              compatibility_name);
-      if (!rv) {
-        auto it = friends_.find(compatibility_name.ToString());
-        if (it != friends_.end()) {
-          return it->second.lock();
-        }
-      }
-      return rv;
-    }
-
-   private:
-    const char* compat_name_;
-    std::string name_;
-    // weak_ptr to avoid cycles
-    std::map<std::string, std::weak_ptr<CompressionManager>> friends_;
-  };
-
-  for (bool use_dict : {false, true}) {
-    SCOPED_TRACE(use_dict ? "With dict" : "No dict");
-
-    // Although these compression managers are actually compatible, we must
-    // respect their distinct compatibility names and treat them as incompatible
-    // (or else risk processing data incorrectly)
-    // NOTE: these are not registered in ObjectRegistry to test what happens
-    // when the original CompressionManager might not be available, but
-    // mgr_bar will be registered during the test, with different names to
-    // prevent interference between iterations.
-    auto mgr_foo = std::make_shared<MyManager>("Foo");
-    auto mgr_bar = std::make_shared<MyManager>(use_dict ? "Bar1" : "Bar2");
-
-    // And this one claims to be fully compatible with the built-in compression
-    // manager when it's not fully compatible (for custom CompressionTypes)
-    auto mgr_claim_compatible = std::make_shared<MyManager>("BuiltinV2");
-
-    constexpr uint16_t kValueSize = 10000;
-
-    Options options = CurrentOptions();
-    options.level0_file_num_compaction_trigger = 20;
-    BlockBasedTableOptions bbto;
-    bbto.enable_index_compression = false;
-    bbto.format_version = 6;  // Before custom compression alg support
-    options.table_factory.reset(NewBlockBasedTableFactory(bbto));
-    // Claims not to use custom compression (and doesn't unless setting a custom
-    // CompressionType)
-    options.compression_manager = mgr_claim_compatible;
-    // Use a built-in compression type with dictionary support
-    options.compression = kLZ4Compression;
-    options.compression_opts.max_dict_bytes = kValueSize / 2;
-    DestroyAndReopen(options);
-
-    Random rnd(404);
-    std::string value;
-    ASSERT_OK(
-        Put("a", test::CompressibleString(&rnd, 0.1, kValueSize, &value)));
-    ASSERT_OK(Flush());
-
-    // That data should be readable without access to the original compression
-    // manager, because it used the built-in CompatibilityName and a built-in
-    // CompressionType
-    options.compression_manager = nullptr;
-    Reopen(options);
-    ASSERT_EQ(Get("a"), value);
-
-    // Verify it was compressed
-    Range r = {"a", "a0"};
-    TablePropertiesCollection tables_properties;
-    ASSERT_OK(db_->GetPropertiesOfTablesInRange(db_->DefaultColumnFamily(), &r,
-                                                1, &tables_properties));
-    ASSERT_EQ(tables_properties.size(), 1U);
-    EXPECT_LT(tables_properties.begin()->second->data_size, kValueSize / 2);
-    EXPECT_EQ(tables_properties.begin()->second->compression_name, "LZ4");
-
-    // Disallow setting a custom CompressionType with a CompressionManager
-    // claiming to be built-in compatible.
-    options.compression_manager = mgr_claim_compatible;
-    options.compression = kCustomCompression8A;
-    ASSERT_EQ(TryReopen(options).code(), Status::Code::kInvalidArgument);
-
-    options.compression_manager = nullptr;
-    options.compression = kCustomCompressionFE;
-    ASSERT_EQ(TryReopen(options).code(), Status::Code::kInvalidArgument);
-    options.compression =
-        static_cast<CompressionType>(kLastBuiltinCompression + 1);
-    ASSERT_EQ(TryReopen(options).code(), Status::Code::kInvalidArgument);
-
-    // Custom compression schema (different CompatibilityName) not supported
-    // before format_version=7
-    options.compression_manager = mgr_foo;
-    options.compression = kLZ4Compression;
-    ASSERT_EQ(TryReopen(options).code(), Status::Code::kInvalidArgument);
-
-    // Set format version supporting custom compression
-    bbto.format_version = 7;
-    options.table_factory.reset(NewBlockBasedTableFactory(bbto));
-
-    // Custom compression type not supported with built-in schema name, even
-    // with format_version=7
-    options.compression_manager = mgr_claim_compatible;
-    options.compression = kCustomCompression8B;
-    ASSERT_EQ(TryReopen(options).code(), Status::Code::kInvalidArgument);
-
-    // Custom compression schema, but specifying a custom compression type it
-    // doesn't support.
-    options.compression_manager = mgr_foo;
-    options.compression = kCustomCompressionF0;
-    ASSERT_EQ(TryReopen(options).code(), Status::Code::kNotSupported);
-
-    // Using a built-in compression type with fv=7 but named custom schema
-    options.compression = kLZ4Compression;
-    Reopen(options);
-    ASSERT_OK(
-        Put("b", test::CompressibleString(&rnd, 0.1, kValueSize, &value)));
-    ASSERT_OK(Flush());
-    ASSERT_EQ(NumTableFilesAtLevel(0), 2);
-    ASSERT_EQ(Get("b"), value);
-
-    // Verify it was compressed with LZ4
-    r = {"b", "b0"};
-    tables_properties.clear();
-    ASSERT_OK(db_->GetPropertiesOfTablesInRange(db_->DefaultColumnFamily(), &r,
-                                                1, &tables_properties));
-    ASSERT_EQ(tables_properties.size(), 1U);
-    EXPECT_LT(tables_properties.begin()->second->data_size, kValueSize / 2);
-    // Uses new format for "compression_name" property
-    EXPECT_EQ(tables_properties.begin()->second->compression_name, "Foo;04;");
-    EXPECT_EQ(mgr_foo->last_specific_decompressor_type_.LoadRelaxed(),
-              kLZ4Compression);
-
-    // Custom compression type
-    options.compression = kCustomCompression8A;
-    Reopen(options);
-    ASSERT_OK(
-        Put("c", test::CompressibleString(&rnd, 0.1, kValueSize, &value)));
-    EXPECT_EQ(mgr_foo->used_compressor8A_count_, 0);
-    ASSERT_OK(Flush());
-    ASSERT_EQ(NumTableFilesAtLevel(0), 3);
-    ASSERT_EQ(Get("c"), value);
-    EXPECT_EQ(mgr_foo->used_compressor8A_count_, 1);
-
-    // Verify it was compressed with custom format
-    r = {"c", "c0"};
-    tables_properties.clear();
-    ASSERT_OK(db_->GetPropertiesOfTablesInRange(db_->DefaultColumnFamily(), &r,
-                                                1, &tables_properties));
-    ASSERT_EQ(tables_properties.size(), 1U);
-    EXPECT_LT(tables_properties.begin()->second->data_size, kValueSize / 2);
-    EXPECT_EQ(tables_properties.begin()->second->compression_name, "Foo;8A;");
-    EXPECT_EQ(mgr_foo->last_specific_decompressor_type_.LoadRelaxed(),
-              kCustomCompression8A);
-
-    // Also dynamically changeable, because the compression manager will respect
-    // the current setting as reported under the legacy logic
-    ASSERT_OK(dbfull()->SetOptions({{"compression", "kLZ4Compression"}}));
-    ASSERT_OK(
-        Put("d", test::CompressibleString(&rnd, 0.1, kValueSize, &value)));
-    ASSERT_OK(Flush());
-    ASSERT_EQ(NumTableFilesAtLevel(0), 4);
-    ASSERT_EQ(Get("d"), value);
-
-    // Verify it was compressed with LZ4
-    r = {"d", "d0"};
-    tables_properties.clear();
-    ASSERT_OK(db_->GetPropertiesOfTablesInRange(db_->DefaultColumnFamily(), &r,
-                                                1, &tables_properties));
-    ASSERT_EQ(tables_properties.size(), 1U);
-    EXPECT_LT(tables_properties.begin()->second->data_size, kValueSize / 2);
-    EXPECT_EQ(tables_properties.begin()->second->compression_name, "Foo;04;");
-    EXPECT_EQ(mgr_foo->last_specific_decompressor_type_.LoadRelaxed(),
-              kLZ4Compression);
-
-    // Dynamically changeable to custom compressions also
-    ASSERT_OK(dbfull()->SetOptions({{"compression", "kCustomCompression8B"}}));
-    ASSERT_OK(
-        Put("e", test::CompressibleString(&rnd, 0.1, kValueSize, &value)));
-    ASSERT_OK(Flush());
-    ASSERT_EQ(NumTableFilesAtLevel(0), 5);
-    ASSERT_EQ(Get("e"), value);
-
-    // Verify it was compressed with custom format
-    r = {"e", "e0"};
-    tables_properties.clear();
-    ASSERT_OK(db_->GetPropertiesOfTablesInRange(db_->DefaultColumnFamily(), &r,
-                                                1, &tables_properties));
-    ASSERT_EQ(tables_properties.size(), 1U);
-    EXPECT_LT(tables_properties.begin()->second->data_size, kValueSize / 2);
-    EXPECT_EQ(tables_properties.begin()->second->compression_name, "Foo;8B;");
-    EXPECT_EQ(mgr_foo->last_specific_decompressor_type_.LoadRelaxed(),
-              kCustomCompression8B);
-
-    // Fails to re-open with incompatible compression manager (can't find
-    // compression manager Foo because it's not registered nor known by Bar)
-    options.compression_manager = mgr_bar;
-    options.compression = kLZ4Compression;
-    ASSERT_EQ(TryReopen(options).code(), Status::Code::kNotSupported);
-
-    // But should re-open if we make Bar aware of the Foo compression manager
-    mgr_bar->AddFriend(mgr_foo);
-    Reopen(options);
-
-    // Can still read everything
-    ASSERT_EQ(Get("a").size(), kValueSize);
-    ASSERT_EQ(Get("b").size(), kValueSize);
-    ASSERT_EQ(Get("c").size(), kValueSize);
-    ASSERT_EQ(Get("d").size(), kValueSize);
-    ASSERT_EQ(Get("e").size(), kValueSize);
-
-    // Add a file using mgr_bar
-    ASSERT_OK(
-        Put("f", test::CompressibleString(&rnd, 0.1, kValueSize, &value)));
-    ASSERT_OK(Flush());
-    ASSERT_EQ(NumTableFilesAtLevel(0), 6);
-    ASSERT_EQ(Get("f"), value);
-
-    // Verify it was compressed appropriately
-    r = {"f", "f0"};
-    tables_properties.clear();
-    ASSERT_OK(db_->GetPropertiesOfTablesInRange(db_->DefaultColumnFamily(), &r,
-                                                1, &tables_properties));
-    ASSERT_EQ(tables_properties.size(), 1U);
-    EXPECT_LT(tables_properties.begin()->second->data_size, kValueSize / 2);
-    EXPECT_EQ(mgr_bar->last_specific_decompressor_type_.LoadRelaxed(),
-              kLZ4Compression);
-
-    // Fails to re-open with incompatible compression manager (can't find
-    // compression manager Bar because it's not registered nor known by Foo)
-    options.compression_manager = mgr_foo;
-    ASSERT_EQ(TryReopen(options).code(), Status::Code::kNotSupported);
-
-    // Register and re-open
-    auto& library = *ObjectLibrary::Default();
-    library.AddFactory<CompressionManager>(
-        mgr_bar->CompatibilityName(),
-        [mgr_bar](const std::string& /*uri*/,
-                  std::unique_ptr<CompressionManager>* guard,
-                  std::string* /*errmsg*/) {
-          *guard = std::make_unique<MyManager>(mgr_bar->CompatibilityName());
-          return guard->get();
-        });
-    Reopen(options);
-
-    // Can still read everything
-    ASSERT_EQ(Get("a").size(), kValueSize);
-    ASSERT_EQ(Get("b").size(), kValueSize);
-    ASSERT_EQ(Get("c").size(), kValueSize);
-    ASSERT_EQ(Get("d").size(), kValueSize);
-    ASSERT_EQ(Get("e").size(), kValueSize);
-    ASSERT_EQ(Get("f").size(), kValueSize);
-
-    // TODO: test old version of a compression manager unable to read a
-    // compression type
-  }
-}
-
 class CompactionStallTestListener : public EventListener {
  public:
   CompactionStallTestListener()

db/external_sst_file_basic_test.cc

@@ -16,7 +16,6 @@
 #include "test_util/testharness.h"
 #include "test_util/testutil.h"
 #include "util/defer.h"
-#include "util/file_checksum_helper.h"
 #include "util/random.h"
 #include "utilities/fault_injection_env.h"
 
@@ -347,8 +346,7 @@ class ChecksumVerifyHelper {
 
   Status GetSingleFileChecksumAndFuncName(
       const std::string& file_path, std::string* file_checksum,
-      std::string* file_checksum_func_name,
-      const std::string& requested_func_name = {}) {
+      std::string* file_checksum_func_name) {
     Status s;
     EnvOptions soptions;
     std::unique_ptr<SequentialFile> file_reader;
@@ -366,8 +364,6 @@ class ChecksumVerifyHelper {
       return Status::OK();
     } else {
       FileChecksumGenContext gen_context;
-      gen_context.file_name = file_path;
-      gen_context.requested_checksum_func_name = requested_func_name;
       std::unique_ptr<FileChecksumGenerator> file_checksum_gen =
           file_checksum_gen_factory->CreateFileChecksumGenerator(gen_context);
       *file_checksum_func_name = file_checksum_gen->Name();
@@ -443,50 +439,10 @@ TEST_F(ExternalSSTFileBasicTest, BasicWithFileChecksumCrc32c) {
   DestroyAndRecreateExternalSSTFilesDir();
 }
 
-namespace {
-class VariousFileChecksumGenerator : public FileChecksumGenCrc32c {
- public:
-  explicit VariousFileChecksumGenerator(const std::string& name)
-      : FileChecksumGenCrc32c({}), name_(name) {}
-
-  const char* Name() const override { return name_.c_str(); }
-
-  std::string GetChecksum() const override {
-    return FileChecksumGenCrc32c::GetChecksum() + "_" + name_;
-  }
-
- private:
-  const std::string name_;
-};
-
-class VariousFileChecksumGenFactory : public FileChecksumGenFactory {
- public:
-  std::unique_ptr<FileChecksumGenerator> CreateFileChecksumGenerator(
-      const FileChecksumGenContext& context) override {
-    static RelaxedAtomic<int> counter{0};
-    if (Slice(context.requested_checksum_func_name).starts_with("Various")) {
-      return std::make_unique<VariousFileChecksumGenerator>(
-          context.requested_checksum_func_name);
-    } else if (context.requested_checksum_func_name.empty()) {
-      // Lacking a specific request, use a different function name for each
-      // result.
-      return std::make_unique<VariousFileChecksumGenerator>(
-          "Various" + std::to_string(counter.FetchAddRelaxed(1)));
-    } else {
-      return nullptr;
-    }
-  }
-
-  static const char* kClassName() { return "VariousFileChecksumGenFactory"; }
-  const char* Name() const override { return kClassName(); }
-};
-}  // namespace
-
 TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
   Options old_options = CurrentOptions();
   Options options = CurrentOptions();
-  options.file_checksum_gen_factory =
-      std::make_shared<VariousFileChecksumGenFactory>();
+  options.file_checksum_gen_factory = GetFileChecksumGenCrc32cFactory();
   const ImmutableCFOptions ioptions(options);
   ChecksumVerifyHelper checksum_helper(options);
 
@@ -507,8 +463,7 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
   ASSERT_EQ(file1_info.largest_key, Key(1099));
   std::string file_checksum1, file_checksum_func_name1;
   ASSERT_OK(checksum_helper.GetSingleFileChecksumAndFuncName(
-      file1, &file_checksum1, &file_checksum_func_name1,
-      file1_info.file_checksum_func_name));
+      file1, &file_checksum1, &file_checksum_func_name1));
   ASSERT_EQ(file1_info.file_checksum, file_checksum1);
   ASSERT_EQ(file1_info.file_checksum_func_name, file_checksum_func_name1);
 
@@ -527,8 +482,7 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
   ASSERT_EQ(file2_info.largest_key, Key(1299));
   std::string file_checksum2, file_checksum_func_name2;
   ASSERT_OK(checksum_helper.GetSingleFileChecksumAndFuncName(
-      file2, &file_checksum2, &file_checksum_func_name2,
-      file2_info.file_checksum_func_name));
+      file2, &file_checksum2, &file_checksum_func_name2));
   ASSERT_EQ(file2_info.file_checksum, file_checksum2);
   ASSERT_EQ(file2_info.file_checksum_func_name, file_checksum_func_name2);
 
@@ -547,8 +501,7 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
   ASSERT_EQ(file3_info.largest_key, Key(1499));
   std::string file_checksum3, file_checksum_func_name3;
   ASSERT_OK(checksum_helper.GetSingleFileChecksumAndFuncName(
-      file3, &file_checksum3, &file_checksum_func_name3,
-      file3_info.file_checksum_func_name));
+      file3, &file_checksum3, &file_checksum_func_name3));
   ASSERT_EQ(file3_info.file_checksum, file_checksum3);
   ASSERT_EQ(file3_info.file_checksum_func_name, file_checksum_func_name3);
 
@@ -567,8 +520,7 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
   ASSERT_EQ(file4_info.largest_key, Key(1799));
   std::string file_checksum4, file_checksum_func_name4;
   ASSERT_OK(checksum_helper.GetSingleFileChecksumAndFuncName(
-      file4, &file_checksum4, &file_checksum_func_name4,
-      file4_info.file_checksum_func_name));
+      file4, &file_checksum4, &file_checksum_func_name4));
   ASSERT_EQ(file4_info.file_checksum, file_checksum4);
   ASSERT_EQ(file4_info.file_checksum_func_name, file_checksum_func_name4);
 
@@ -587,8 +539,7 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
   ASSERT_EQ(file5_info.largest_key, Key(1999));
   std::string file_checksum5, file_checksum_func_name5;
   ASSERT_OK(checksum_helper.GetSingleFileChecksumAndFuncName(
-      file5, &file_checksum5, &file_checksum_func_name5,
-      file5_info.file_checksum_func_name));
+      file5, &file_checksum5, &file_checksum_func_name5));
   ASSERT_EQ(file5_info.file_checksum, file_checksum5);
   ASSERT_EQ(file5_info.file_checksum_func_name, file_checksum_func_name5);
 
@@ -607,8 +558,7 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
   ASSERT_EQ(file6_info.largest_key, Key(2199));
   std::string file_checksum6, file_checksum_func_name6;
   ASSERT_OK(checksum_helper.GetSingleFileChecksumAndFuncName(
-      file6, &file_checksum6, &file_checksum_func_name6,
-      file6_info.file_checksum_func_name));
+      file6, &file_checksum6, &file_checksum_func_name6));
   ASSERT_EQ(file6_info.file_checksum, file_checksum6);
   ASSERT_EQ(file6_info.file_checksum_func_name, file_checksum_func_name6);
 
@@ -678,23 +628,18 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
   }
   ASSERT_OK(env_->FileExists(file2));
 
-  // Enable verify_file_checksum option. No checksum information is provided,
-  // so it is generated when ingesting. The configured checksum factory will
-  // use a different function than before.
-  s = AddFileWithFileChecksum({file3}, {}, {}, true, false, false, false);
+  // Enable verify_file_checksum option
+  // No checksum information is provided, generate it when ingesting
+  std::vector<std::string> checksum, checksum_func;
+  s = AddFileWithFileChecksum({file3}, checksum, checksum_func, true, false,
+                              false, false);
   ASSERT_OK(s) << s.ToString();
   std::vector<LiveFileMetaData> live_files2;
   dbfull()->GetLiveFilesMetaData(&live_files2);
   for (const auto& f : live_files2) {
     if (set1.find(f.name) == set1.end()) {
-      // Recomputed checksum, different function
-      EXPECT_NE(f.file_checksum_func_name, file_checksum_func_name3);
-      std::string cur_checksum3, cur_checksum_func_name3;
-      ASSERT_OK(checksum_helper.GetSingleFileChecksumAndFuncName(
-          dbname_ + f.name, &cur_checksum3, &cur_checksum_func_name3,
-          f.file_checksum_func_name));
-      EXPECT_EQ(f.file_checksum, cur_checksum3);
-      EXPECT_EQ(f.file_checksum_func_name, cur_checksum_func_name3);
+      ASSERT_EQ(f.file_checksum, file_checksum3);
+      ASSERT_EQ(f.file_checksum_func_name, file_checksum_func_name3);
       set1.insert(f.name);
     }
   }
@@ -708,9 +653,8 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
   ASSERT_NOK(s) << s.ToString();
 
   // Does not enable verify_file_checksum options
-  // Checksum function name is recognized, so store the checksum being ingested.
-  std::string file_checksum_func_name4alt = "VariousABCD";
-  s = AddFileWithFileChecksum({file4}, {"asd"}, {file_checksum_func_name4alt},
+  // Checksum function name matches, store the checksum being ingested.
+  s = AddFileWithFileChecksum({file4}, {"asd"}, {file_checksum_func_name4},
                               false, false, false, false);
   ASSERT_OK(s) << s.ToString();
   std::vector<LiveFileMetaData> live_files3;
@@ -719,7 +663,7 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
     if (set1.find(f.name) == set1.end()) {
       ASSERT_FALSE(f.file_checksum == file_checksum4);
       ASSERT_EQ(f.file_checksum, "asd");
-      ASSERT_EQ(f.file_checksum_func_name, file_checksum_func_name4alt);
+      ASSERT_EQ(f.file_checksum_func_name, file_checksum_func_name4);
       set1.insert(f.name);
     }
   }
@@ -728,8 +672,7 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
 
   // enable verify_file_checksum options, DB enable checksum, and enable
   // write_global_seq. So the checksum stored is different from the one
-  // ingested due to the sequence number changes. The checksum function name
-  // may also change since the checksum is recomputed.
+  // ingested due to the sequence number changes.
   s = AddFileWithFileChecksum({file5}, {file_checksum5},
                               {file_checksum_func_name5}, true, false, false,
                               true);
@@ -738,14 +681,11 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
   dbfull()->GetLiveFilesMetaData(&live_files4);
   for (const auto& f : live_files4) {
     if (set1.find(f.name) == set1.end()) {
-      // Recomputed checksum, different function
-      EXPECT_NE(f.file_checksum_func_name, file_checksum_func_name5);
       std::string cur_checksum5, cur_checksum_func_name5;
       ASSERT_OK(checksum_helper.GetSingleFileChecksumAndFuncName(
-          dbname_ + f.name, &cur_checksum5, &cur_checksum_func_name5,
-          f.file_checksum_func_name));
-      EXPECT_EQ(f.file_checksum, cur_checksum5);
-      EXPECT_EQ(f.file_checksum_func_name, cur_checksum_func_name5);
+          dbname_ + f.name, &cur_checksum5, &cur_checksum_func_name5));
+      ASSERT_EQ(f.file_checksum, cur_checksum5);
+      ASSERT_EQ(f.file_checksum_func_name, file_checksum_func_name5);
       set1.insert(f.name);
     }
   }
@@ -753,22 +693,18 @@ TEST_F(ExternalSSTFileBasicTest, IngestFileWithFileChecksum) {
   ASSERT_OK(env_->FileExists(file5));
 
   // Does not enable verify_file_checksum options and also the ingested file
-  // checksum information is empty. DB will generate and store file checksum
-  // in Manifest, which could be different from the previous invocation.
-  s = AddFileWithFileChecksum({file6}, {}, {}, false, false, false, false);
+  // checksum information is empty. DB will generate and store the checksum
+  // in Manifest.
+  std::vector<std::string> files_c6, files_name6;
+  s = AddFileWithFileChecksum({file6}, files_c6, files_name6, false, false,
+                              false, false);
   ASSERT_OK(s) << s.ToString();
   std::vector<LiveFileMetaData> live_files6;
   dbfull()->GetLiveFilesMetaData(&live_files6);
   for (const auto& f : live_files6) {
     if (set1.find(f.name) == set1.end()) {
-      // Recomputed checksum, different function
-      EXPECT_NE(f.file_checksum_func_name, file_checksum_func_name6);
-      std::string cur_checksum6, cur_checksum_func_name6;
-      ASSERT_OK(checksum_helper.GetSingleFileChecksumAndFuncName(
-          dbname_ + f.name, &cur_checksum6, &cur_checksum_func_name6,
-          f.file_checksum_func_name));
-      EXPECT_EQ(f.file_checksum, cur_checksum6);
-      EXPECT_EQ(f.file_checksum_func_name, cur_checksum_func_name6);
+      ASSERT_EQ(f.file_checksum, file_checksum6);
+      ASSERT_EQ(f.file_checksum_func_name, file_checksum_func_name6);
       set1.insert(f.name);
     }
   }

db/external_sst_file_ingestion_job.cc

@@ -260,6 +260,10 @@ Status ExternalSstFileIngestionJob::Prepare(
     } else {
       need_generate_file_checksum_ = true;
     }
+    FileChecksumGenContext gen_context;
+    std::unique_ptr<FileChecksumGenerator> file_checksum_gen =
+        db_options_.file_checksum_gen_factory->CreateFileChecksumGenerator(
+            gen_context);
     std::vector<std::string> generated_checksums;
     std::vector<std::string> generated_checksum_func_names;
     // Step 1: generate the checksum for ingested sst file.
@@ -267,9 +271,7 @@ Status ExternalSstFileIngestionJob::Prepare(
       for (size_t i = 0; i < files_to_ingest_.size(); i++) {
         std::string generated_checksum;
         std::string generated_checksum_func_name;
-        std::string requested_checksum_func_name =
-            i < files_checksum_func_names.size() ? files_checksum_func_names[i]
-                                                 : "";
+        std::string requested_checksum_func_name;
         // TODO: rate limit file reads for checksum calculation during file
         // ingestion.
         // TODO: plumb Env::IOActivity
@@ -312,50 +314,40 @@ Status ExternalSstFileIngestionJob::Prepare(
             if (files_checksum_func_names[i] !=
                 generated_checksum_func_names[i]) {
               status = Status::InvalidArgument(
-                  "DB file checksum gen factory " +
-                  std::string(db_options_.file_checksum_gen_factory->Name()) +
-                  " generated checksum function name " +
-                  generated_checksum_func_names[i] + " for file " +
-                  external_files_paths[i] +
-                  " which does not match requested/provided " +
-                  files_checksum_func_names[i]);
+                  "Checksum function name does not match with the checksum "
+                  "function name of this DB");
+              ROCKS_LOG_WARN(
+                  db_options_.info_log,
+                  "Sst file checksum verification of file: %s failed: %s",
+                  external_files_paths[i].c_str(), status.ToString().c_str());
               break;
             }
             if (files_checksums[i] != generated_checksums[i]) {
               status = Status::Corruption(
-                  "Checksum verification mismatch for ingestion file " +
-                  external_files_paths[i] + " using function " +
-                  generated_checksum_func_names[i] + ". Expected: " +
-                  Slice(files_checksums[i]).ToString(/*hex=*/true) +
-                  " Computed: " +
-                  Slice(generated_checksums[i]).ToString(/*hex=*/true));
+                  "Ingested checksum does not match with the generated "
+                  "checksum");
+              ROCKS_LOG_WARN(
+                  db_options_.info_log,
+                  "Sst file checksum verification of file: %s failed: %s",
+                  files_to_ingest_[i].internal_file_path.c_str(),
+                  status.ToString().c_str());
               break;
             }
           }
         } else {
-          // If verify_file_checksum is not enabled, we only verify the factory
-          // recognizes the checksum function name. If it does not match, fail
-          // the ingestion. If matches, we trust the ingested checksum
-          // information and store in the Manifest.
+          // If verify_file_checksum is not enabled, we only verify the
+          // checksum function name. If it does not match, fail the ingestion.
+          // If matches, we trust the ingested checksum information and store
+          // in the Manifest.
           for (size_t i = 0; i < files_to_ingest_.size(); i++) {
-            FileChecksumGenContext gen_context;
-            gen_context.file_name = files_to_ingest_[i].internal_file_path;
-            gen_context.requested_checksum_func_name =
-                files_checksum_func_names[i];
-            auto file_checksum_gen =
-                db_options_.file_checksum_gen_factory
-                    ->CreateFileChecksumGenerator(gen_context);
-
-            if (file_checksum_gen == nullptr ||
-                files_checksum_func_names[i] != file_checksum_gen->Name()) {
+            if (files_checksum_func_names[i] != file_checksum_gen->Name()) {
               status = Status::InvalidArgument(
-                  "Checksum function name " + files_checksum_func_names[i] +
-                  " for file " + external_files_paths[i] +
-                  " not recognized by DB checksum gen factory" +
-                  db_options_.file_checksum_gen_factory->Name() +
-                  (file_checksum_gen ? (" Returned function " +
-                                        std::string(file_checksum_gen->Name()))
-                                     : ""));
+                  "Checksum function name does not match with the checksum "
+                  "function name of this DB");
+              ROCKS_LOG_WARN(
+                  db_options_.info_log,
+                  "Sst file checksum verification of file: %s failed: %s",
+                  external_files_paths[i].c_str(), status.ToString().c_str());
               break;
             }
             files_to_ingest_[i].file_checksum = files_checksums[i];
@@ -370,11 +362,12 @@ Status ExternalSstFileIngestionJob::Prepare(
         status = Status::InvalidArgument(
             "The checksum information of ingested sst files are nonempty and "
             "the size of checksums or the size of the checksum function "
-            "names does not match with the number of ingested sst files");
-      }
-      if (!status.ok()) {
-        ROCKS_LOG_WARN(db_options_.info_log, "Ingestion failed: %s",
-                       status.ToString().c_str());
+            "names "
+            "does not match with the number of ingested sst files");
+        ROCKS_LOG_WARN(
+            db_options_.info_log,
+            "The ingested sst files checksum information is incomplete: %s",
+            status.ToString().c_str());
       }
     }
   }
@@ -841,8 +834,7 @@ Status ExternalSstFileIngestionJob::ResetTableReader(
       ro,
       TableReaderOptions(
           cfd_->ioptions(), sv->mutable_cf_options.prefix_extractor,
-          sv->mutable_cf_options.compression_manager.get(), env_options_,
-          cfd_->internal_comparator(),
+          env_options_, cfd_->internal_comparator(),
           sv->mutable_cf_options.block_protection_bytes_per_key,
           /*skip_filters*/ false, /*immortal*/ false,
           /*force_direct_prefetch*/ false, /*level*/ -1,

db/import_column_family_job.cc

@@ -324,8 +324,7 @@ Status ImportColumnFamilyJob::GetIngestedFileInfo(
   status = sv->mutable_cf_options.table_factory->NewTableReader(
       TableReaderOptions(
           cfd_->ioptions(), sv->mutable_cf_options.prefix_extractor,
-          sv->mutable_cf_options.compression_manager.get(), env_options_,
-          cfd_->internal_comparator(),
+          env_options_, cfd_->internal_comparator(),
           sv->mutable_cf_options.block_protection_bytes_per_key,
           /*skip_filters*/ false, /*immortal*/ false,
           /*force_direct_prefetch*/ false, /*level*/ -1,

db/memtable.cc

@@ -196,11 +196,10 @@ bool MemTable::ShouldFlushNow() {
   // allocate one more block.
   const double kAllowOverAllocationRatio = 0.6;
 
-  // range deletion use skip list which allocates all memeory through `arena_`
-  assert(range_del_table_->ApproximateMemoryUsage() == 0);
   // If arena still have room for new block allocation, we can safely say it
   // shouldn't flush.
   auto allocated_memory = table_->ApproximateMemoryUsage() +
+                          range_del_table_->ApproximateMemoryUsage() +
                           arena_.MemoryAllocatedBytes();
 
   approximate_memory_usage_.store(allocated_memory, std::memory_order_relaxed);

db/memtable.h

@@ -688,7 +688,6 @@ class MemTable final : public ReadOnlyMemTable {
   // Update counters and flush status after inserting a whole write batch
   // Used in concurrent memtable inserts.
   void BatchPostProcess(const MemTablePostProcessInfo& update_counters) {
-    table_->BatchPostProcess();
     num_entries_.fetch_add(update_counters.num_entries,
                            std::memory_order_relaxed);
     data_size_.fetch_add(update_counters.data_size, std::memory_order_relaxed);
@@ -699,10 +698,6 @@ class MemTable final : public ReadOnlyMemTable {
     if (update_counters.num_range_deletes > 0) {
       num_range_deletes_.fetch_add(update_counters.num_range_deletes,
                                    std::memory_order_relaxed);
-      // noop for skip-list memtable
-      // Besides correctness test in stress test, memtable flush record count
-      // check will catch this if it were not noop.
-      // range_del_table_->BatchPostProcess();
     }
     UpdateFlushState();
   }

db/periodic_task_scheduler.cc

@@ -26,7 +26,6 @@ static const std::map<PeriodicTaskType, uint64_t> kDefaultPeriodSeconds = {
     {PeriodicTaskType::kPersistStats, kInvalidPeriodSec},
     {PeriodicTaskType::kFlushInfoLog, 10},
     {PeriodicTaskType::kRecordSeqnoTime, kInvalidPeriodSec},
-    {PeriodicTaskType::kTriggerCompaction, 12 * 60 * 60}  // 12 hours
 };
 
 static const std::map<PeriodicTaskType, std::string> kPeriodicTaskTypeNames = {
@@ -34,20 +33,16 @@ static const std::map<PeriodicTaskType, std::string> kPeriodicTaskTypeNames = {
     {PeriodicTaskType::kPersistStats, "pst_st"},
     {PeriodicTaskType::kFlushInfoLog, "flush_info_log"},
     {PeriodicTaskType::kRecordSeqnoTime, "record_seq_time"},
-    {PeriodicTaskType::kTriggerCompaction, "trigger_compaction"},
 };
 
 Status PeriodicTaskScheduler::Register(PeriodicTaskType task_type,
-                                       const PeriodicTaskFunc& fn,
-                                       bool run_immediately) {
-  return Register(task_type, fn, kDefaultPeriodSeconds.at(task_type),
-                  run_immediately);
+                                       const PeriodicTaskFunc& fn) {
+  return Register(task_type, fn, kDefaultPeriodSeconds.at(task_type));
 }
 
 Status PeriodicTaskScheduler::Register(PeriodicTaskType task_type,
                                        const PeriodicTaskFunc& fn,
-                                       uint64_t repeat_period_seconds,
-                                       bool run_immediately) {
+                                       uint64_t repeat_period_seconds) {
   MutexLock l(&timer_mutex);
   static std::atomic<uint64_t> initial_delay(0);
 
@@ -70,13 +65,10 @@ Status PeriodicTaskScheduler::Register(PeriodicTaskType task_type,
   std::string unique_id =
       kPeriodicTaskTypeNames.at(task_type) + std::to_string(id_++);
 
-  uint64_t initial_delay_micros =
-      (initial_delay.fetch_add(1) % repeat_period_seconds) * kMicrosInSecond;
-  if (!run_immediately) {
-    initial_delay_micros += repeat_period_seconds * kMicrosInSecond;
-  }
-  bool succeeded = timer_->Add(fn, unique_id, initial_delay_micros,
-                               repeat_period_seconds * kMicrosInSecond);
+  bool succeeded = timer_->Add(
+      fn, unique_id,
+      (initial_delay.fetch_add(1) % repeat_period_seconds) * kMicrosInSecond,
+      repeat_period_seconds * kMicrosInSecond);
   if (!succeeded) {
     return Status::Aborted("Failed to register periodic task");
   }

db/periodic_task_scheduler.h

@@ -21,7 +21,6 @@ enum class PeriodicTaskType : uint8_t {
   kPersistStats,
   kFlushInfoLog,
   kRecordSeqnoTime,
-  kTriggerCompaction,
   kMax,
 };
 
@@ -43,16 +42,13 @@ class PeriodicTaskScheduler {
   PeriodicTaskScheduler& operator=(PeriodicTaskScheduler&&) = delete;
 
   // Register a task with its default repeat period. Thread safe call.
-  // @param run_immediately If true, the task will run soon after it's
-  // scheduled, instead of waiting for the repeat period.
-  Status Register(PeriodicTaskType task_type, const PeriodicTaskFunc& fn,
-                  bool run_immediately);
+  Status Register(PeriodicTaskType task_type, const PeriodicTaskFunc& fn);
 
   // Register a task with specified repeat period. 0 is an invalid argument
   // (kInvalidPeriodSec). To stop the task, please use Unregister().
   // Thread safe call.
   Status Register(PeriodicTaskType task_type, const PeriodicTaskFunc& fn,
-                  uint64_t repeat_period_seconds, bool run_immediately);
+                  uint64_t repeat_period_seconds);
 
   // Unregister the task. Thread safe call.
   Status Unregister(PeriodicTaskType task_type);

db/periodic_task_scheduler_test.cc

@@ -56,12 +56,6 @@ TEST_F(PeriodicTaskSchedulerTest, Basic) {
   SyncPoint::GetInstance()->SetCallBack(
       "DBImpl::FlushInfoLog:StartRunning",
       [&](void*) { flush_info_log_counter++; });
-
-  int trigger_compaction_counter = 0;
-  SyncPoint::GetInstance()->SetCallBack(
-      "DBImpl::TriggerPeriodicCompaction:StartRunning",
-      [&](void*) { trigger_compaction_counter++; });
-
   SyncPoint::GetInstance()->EnableProcessing();
 
   Reopen(options);
@@ -76,7 +70,7 @@ TEST_F(PeriodicTaskSchedulerTest, Basic) {
 
   const PeriodicTaskScheduler& scheduler =
       dbfull()->TEST_GetPeriodicTaskScheduler();
-  ASSERT_EQ((int)PeriodicTaskType::kMax - 1, scheduler.TEST_GetValidTaskNum());
+  ASSERT_EQ(3, scheduler.TEST_GetValidTaskNum());
 
   ASSERT_EQ(1, dump_st_counter);
   ASSERT_EQ(1, pst_st_counter);
@@ -109,14 +103,14 @@ TEST_F(PeriodicTaskSchedulerTest, Basic) {
   ASSERT_EQ(3, pst_st_counter);
   ASSERT_EQ(4, flush_info_log_counter);
 
-  ASSERT_EQ(2u, scheduler.TEST_GetValidTaskNum());
+  ASSERT_EQ(1u, scheduler.TEST_GetValidTaskNum());
 
   // Re-enable one task
   ASSERT_OK(dbfull()->SetDBOptions({{"stats_dump_period_sec", "5"}}));
   ASSERT_EQ(5u, dbfull()->GetDBOptions().stats_dump_period_sec);
   ASSERT_EQ(0u, dbfull()->GetDBOptions().stats_persist_period_sec);
 
-  ASSERT_EQ(3, scheduler.TEST_GetValidTaskNum());
+  ASSERT_EQ(2, scheduler.TEST_GetValidTaskNum());
 
   dbfull()->TEST_WaitForPeriodicTaskRun(
       [&] { mock_clock_->MockSleepForSeconds(static_cast<int>(kPeriodSec)); });
@@ -124,16 +118,6 @@ TEST_F(PeriodicTaskSchedulerTest, Basic) {
   ASSERT_EQ(3, pst_st_counter);
   ASSERT_EQ(5, flush_info_log_counter);
 
-  ASSERT_EQ(0, trigger_compaction_counter);
-  dbfull()->TEST_WaitForPeriodicTaskRun([&] {
-    mock_clock_->MockSleepForSeconds(static_cast<int>(12 * 60 * 60));
-  });
-  ASSERT_EQ(1, trigger_compaction_counter);
-  dbfull()->TEST_WaitForPeriodicTaskRun([&] {
-    mock_clock_->MockSleepForSeconds(static_cast<int>(12 * 60 * 60));
-  });
-  ASSERT_EQ(2, trigger_compaction_counter);
-
   Close();
 }
 
@@ -166,9 +150,7 @@ TEST_F(PeriodicTaskSchedulerTest, MultiInstances) {
   auto dbi = static_cast_with_check<DBImpl>(dbs[kInstanceNum - 1]);
 
   const PeriodicTaskScheduler& scheduler = dbi->TEST_GetPeriodicTaskScheduler();
-  // kRecordSeqnoTime is not registered since the feature is not enabled
-  ASSERT_EQ(kInstanceNum * ((int)PeriodicTaskType::kMax - 1),
-            scheduler.TEST_GetValidTaskNum());
+  ASSERT_EQ(kInstanceNum * 3, scheduler.TEST_GetValidTaskNum());
 
   int expected_run = kInstanceNum;
   dbi->TEST_WaitForPeriodicTaskRun(

db/table_cache.cc

@@ -146,8 +146,7 @@ Status TableCache::GetTableReader(
     s = mutable_cf_options.table_factory->NewTableReader(
         ro,
         TableReaderOptions(
-            ioptions_, mutable_cf_options.prefix_extractor,
-            mutable_cf_options.compression_manager.get(), file_options,
+            ioptions_, mutable_cf_options.prefix_extractor, file_options,
             internal_comparator,
             mutable_cf_options.block_protection_bytes_per_key, skip_filters,
             immortal_tables_, false /* force_direct_prefetch */, level,

db/write_batch.cc

@@ -551,6 +551,9 @@ Status WriteBatchInternal::Iterate(const WriteBatch* wb,
 
     if (LIKELY(!s.IsTryAgain())) {
       last_was_try_again = false;
+      tag = 0;
+      column_family = 0;  // default
+
       s = ReadRecordFromWriteBatch(&input, &tag, &column_family, &key, &value,
                                    &blob, &xid, &write_unix_time);
       if (!s.ok()) {
@@ -1894,6 +1897,7 @@ Status WriteBatch::VerifyChecksum() const {
     // ReadRecordFromWriteBatch
     key.clear();
     value.clear();
+    column_family = 0;
     s = ReadRecordFromWriteBatch(&input, &tag, &column_family, &key, &value,
                                  &blob, &xid, /*write_unix_time=*/nullptr);
     if (!s.ok()) {
@@ -3210,11 +3214,11 @@ Status WriteBatchInternal::InsertInto(
     ColumnFamilyMemTables* memtables, FlushScheduler* flush_scheduler,
     TrimHistoryScheduler* trim_history_scheduler,
     bool ignore_missing_column_families, uint64_t recovery_log_number, DB* db,
-    bool seq_per_batch, bool batch_per_txn) {
+    bool concurrent_memtable_writes, bool seq_per_batch, bool batch_per_txn) {
   MemTableInserter inserter(
       sequence, memtables, flush_scheduler, trim_history_scheduler,
       ignore_missing_column_families, recovery_log_number, db,
-      /*concurrent_memtable_writes=*/false, nullptr /* prot_info */,
+      concurrent_memtable_writes, nullptr /* prot_info */,
       nullptr /*has_valid_writes*/, seq_per_batch, batch_per_txn);
   for (auto w : write_group) {
     if (w->CallbackFailed()) {

db/write_batch_internal.h

@@ -185,19 +185,18 @@ class WriteBatchInternal {
   // If flush_scheduler is non-null, it will be invoked if the memtable
   // should be flushed.
   //
-  // This overload is for non-concurrent insertion only.
+  // Under concurrent use, the caller is responsible for making sure that
+  // the memtables object itself is thread-local.
   static Status InsertInto(
       WriteThread::WriteGroup& write_group, SequenceNumber sequence,
       ColumnFamilyMemTables* memtables, FlushScheduler* flush_scheduler,
       TrimHistoryScheduler* trim_history_scheduler,
       bool ignore_missing_column_families = false, uint64_t log_number = 0,
-      DB* db = nullptr, bool seq_per_batch = false, bool batch_per_txn = true);
+      DB* db = nullptr, bool concurrent_memtable_writes = false,
+      bool seq_per_batch = false, bool batch_per_txn = true);
 
   // Convenience form of InsertInto when you have only one batch
   // next_seq returns the seq after last sequence number used in MemTable insert
-  //
-  // Under concurrent use, the caller is responsible for making sure that
-  // the memtables object itself is thread-local.
   static Status InsertInto(
       const WriteBatch* batch, ColumnFamilyMemTables* memtables,
       FlushScheduler* flush_scheduler,

db_stress_tool/db_stress_compression_manager.h

@@ -1,65 +0,0 @@
-//  Copyright (c) Meta Platforms, Inc. and affiliates.
-//  This source code is licensed under both the GPLv2 (found in the
-//  COPYING file in the root directory) and Apache 2.0 License
-//  (found in the LICENSE.Apache file in the root directory).
-
-#pragma once
-
-#include "test_util/testutil.h"
-
-namespace ROCKSDB_NAMESPACE {
-
-class DbStressCustomCompressionManager : public CompressionManager {
- public:
-  const char* Name() const override {
-    return "DbStressCustomCompressionManager";
-  }
-  const char* CompatibilityName() const override { return "DbStressCustom1"; }
-
-  bool SupportsCompressionType(CompressionType type) const override {
-    return default_->SupportsCompressionType(type) ||
-           type == kCustomCompressionAA || type == kCustomCompressionAB ||
-           type == kCustomCompressionAC;
-  }
-
-  std::unique_ptr<Compressor> GetCompressor(const CompressionOptions& opts,
-                                            CompressionType type) override {
-    // db_stress never specifies a custom type, so we randomly use them anyway
-    // when this compression manager is used.
-    std::array<CompressionType, 4> choices = {
-        type, kCustomCompressionAA, kCustomCompressionAB, kCustomCompressionAC};
-    type = choices[Random::GetTLSInstance()->Uniform(4)];
-    switch (static_cast<unsigned char>(type)) {
-      case kCustomCompressionAA:
-        return std::make_unique<
-            test::CompressorCustomAlg<kCustomCompressionAA>>();
-      case kCustomCompressionAB:
-        return std::make_unique<
-            test::CompressorCustomAlg<kCustomCompressionAB>>();
-      case kCustomCompressionAC:
-        return std::make_unique<
-            test::CompressorCustomAlg<kCustomCompressionAC>>();
-      // Also support built-in compression algorithms
-      default:
-        return GetBuiltinV2CompressionManager()->GetCompressor(opts, type);
-    }
-  }
-
-  std::shared_ptr<Decompressor> GetDecompressor() override {
-    return std::make_shared<test::DecompressorCustomAlg>();
-  }
-
-  std::shared_ptr<Decompressor> GetDecompressorForTypes(
-      const CompressionType* types_begin,
-      const CompressionType* types_end) override {
-    auto decomp = std::make_shared<test::DecompressorCustomAlg>();
-    decomp->SetAllowedTypes(types_begin, types_end);
-    return decomp;
-  }
-
- protected:
-  std::shared_ptr<CompressionManager> default_ =
-      GetBuiltinV2CompressionManager();
-};
-
-}  // namespace ROCKSDB_NAMESPACE

db_stress_tool/db_stress_test_base.cc

@@ -11,7 +11,6 @@
 #include <ios>
 #include <thread>
 
-#include "db_stress_tool/db_stress_compression_manager.h"
 #include "db_stress_tool/db_stress_listener.h"
 #include "rocksdb/io_status.h"
 #include "rocksdb/options.h"
@@ -3413,37 +3412,32 @@ void StressTest::Open(SharedState* shared, bool reopen) {
     InitializeOptionsFromFlags(cache_, filter_policy_, options_);
   }
   InitializeOptionsGeneral(cache_, filter_policy_, sqfc_factory_, options_);
-  {
-    // We must register any compression managers with a custom
-    // CompatibilityName() so that if it was used in a past invocation but not
-    // the current invocation, we can still read the SST files requiring it.
-    static std::once_flag loaded;
-    std::call_once(loaded, [&]() {
-      TEST_AllowUnsupportedFormatVersion() = true;
-      auto& library = *ObjectLibrary::Default();
-      library.AddFactory<CompressionManager>(
-          DbStressCustomCompressionManager().CompatibilityName(),
-          [](const std::string& /*uri*/,
-             std::unique_ptr<CompressionManager>* guard,
-             std::string* /*errmsg*/) {
-            *guard = std::make_unique<DbStressCustomCompressionManager>();
-            return guard->get();
-          });
-    });
-  }
-  if (!strcasecmp(FLAGS_compression_manager.c_str(), "custom")) {
-    options_.compression_manager =
-        std::make_shared<DbStressCustomCompressionManager>();
-  } else if (!strcasecmp(FLAGS_compression_manager.c_str(), "mixed")) {
-    options_.compression_manager =
-        std::make_shared<RoundRobinManager>(GetBuiltinV2CompressionManager());
-  } else if (!strcasecmp(FLAGS_compression_manager.c_str(), "randommixed")) {
-    options_.compression_manager =
-        std::make_shared<RandomMixedCompressionManager>(
-            GetBuiltinV2CompressionManager());
+  if (!strcasecmp(FLAGS_compression_manager.c_str(), "mixed") ||
+      !strcasecmp(FLAGS_compression_manager.c_str(), "randommixed")) {
+    // Currently limited to ZSTD compression. Table property compression_name
+    // needs to set to zstd for now even when there can be more than one
+    // algorithm in the table under your compressor.
+    if (!ZSTD_Supported()) {
+      fprintf(stderr,
+              "ZSTD compression not supported thus mixed compression cannot be "
+              "used\n");
+      exit(1);
+    }
+    if (!strcasecmp(FLAGS_compression_manager.c_str(), "mixed")) {
+      auto mgr = std::make_shared<RoundRobinManager>(
+          GetDefaultBuiltinCompressionManager());
+      options_.compression_manager = mgr;
+    } else if (!strcasecmp(FLAGS_compression_manager.c_str(), "randommixed")) {
+      auto mgr = std::make_shared<RandomMixedCompressionManager>(
+          GetDefaultBuiltinCompressionManager());
+      options_.compression_manager = mgr;
+    }
+    options_.compression = kZSTD;
+    options_.bottommost_compression = kZSTD;
+
   } else if (!strcasecmp(FLAGS_compression_manager.c_str(), "autoskip")) {
     options_.compression_manager =
-        CreateAutoSkipCompressionManager(GetBuiltinV2CompressionManager());
+        CreateAutoSkipCompressionManager(GetDefaultBuiltinCompressionManager());
   } else if (!strcasecmp(FLAGS_compression_manager.c_str(), "none")) {
     // Nothing to do using default compression manager
   } else {

env/env_posix.cc

@@ -169,9 +169,8 @@ class PosixClock : public SystemClock {
     struct timespec ts;
     clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts);
     return (static_cast<uint64_t>(ts.tv_sec) * 1000000000 + ts.tv_nsec) / 1000;
-#else
-    return 0;
 #endif
+    return 0;
   }
 
   uint64_t CPUNanos() override {
@@ -180,9 +179,8 @@ class PosixClock : public SystemClock {
     struct timespec ts;
     clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts);
     return static_cast<uint64_t>(ts.tv_sec) * 1000000000 + ts.tv_nsec;
-#else
-    return 0;
 #endif
+    return 0;
   }
 
   void SleepForMicroseconds(int micros) override { usleep(micros); }

env/fs_posix.cc

@@ -868,6 +868,7 @@ class PosixFileSystem : public FileSystem {
       IOOptions opts;
       return CreateDirIfMissing(*result, opts, nullptr);
     }
+    return IOStatus::OK();
   }
 
   IOStatus GetFreeSpace(const std::string& fname, const IOOptions& /*opts*/,

file/sst_file_manager_impl.h

@@ -162,6 +162,7 @@ class SstFileManagerImpl : public SstFileManager {
   void Close();
 
   void SetStatisticsPtr(const std::shared_ptr<Statistics>& stats) override {
+    stats_ = stats;
     delete_scheduler_.SetStatisticsPtr(stats);
   }
 
@@ -215,6 +216,7 @@ class SstFileManagerImpl : public SstFileManager {
   std::list<ErrorHandler*> error_handler_list_;
   // Pointer to ErrorHandler instance that is currently processing recovery
   ErrorHandler* cur_instance_;
+  std::shared_ptr<Statistics> stats_;
 };
 
 }  // namespace ROCKSDB_NAMESPACE

fuzz/sst_file_writer_fuzzer.cc

@@ -91,8 +91,7 @@ TableReader* NewTableReader(const std::string& sst_file_path,
   }
   if (s.ok()) {
     ImmutableOptions iopts(options, cf_ioptions);
-    TableReaderOptions t_opt(iopts, /*prefix_extractor=*/nullptr,
-                             /*compression_manager=*/nullptr, env_options,
+    TableReaderOptions t_opt(iopts, /*prefix_extractor=*/nullptr, env_options,
                              cf_ioptions.internal_comparator,
                              0 /* block_protection_bytes_per_key */);
     t_opt.largest_seqno = kMaxSequenceNumber;

include/rocksdb/advanced_compression.h

@@ -55,15 +55,6 @@ class Compressor {
   Compressor() = default;
   virtual ~Compressor() = default;
 
-  // Class name for logging / debugging purposes
-  virtual const char* Name() const = 0;
-
-  // Potentially more elaborate identifier for logging / debugging purposes
-  virtual std::string GetId() const {
-    std::string id = Name();
-    return id;
-  }
-
   // Returns the max total bytes of for all sampled blocks for creating the data
   // dictionary, or zero indicating dictionary compression should not be
   // used/configured. This will typically be called after
@@ -237,19 +228,21 @@ class Decompressor {
   // EXTENSIBLE or reinterpret_cast-able by custom Compressor implementations
   struct WorkingArea {};
 
+ protected:
   // To allow for flexible re-use / reclaimation, we have explicit Obtain and
   // Release functions, which are typically wrapped in a special RAII smart
   // pointer. For example, a WorkingArea could be saved/recycled in thread-local
   // or core-local storage, or heap managed, etc., though an explicit
   // WorkingArea is only advised for repeated decompression (by a single
-  // thread). ReleaseWorkingArea() in not intended to be called directly, but
-  // used by ManagedWorkingArea.
+  // thread).
+
   virtual void ReleaseWorkingArea(WorkingArea* wa) {
     // Default implementation: no working area
     (void)wa;
     assert(wa == nullptr);
   }
 
+ public:
   using ManagedWorkingArea =
       ManagedPtr<WorkingArea, Decompressor, &Decompressor::ReleaseWorkingArea>;
 
@@ -273,11 +266,11 @@ class Decompressor {
   // dictionary is processed into a form reusable by repeated compressions in
   // many threads, that happens within this call.
   //
-  // Must return OK if and only if storing a result in `out`. Otherwise, could
-  // return values like NotSupported - dictionary compression is not (yet)
-  // supported for this kind of Decompressor. Corruption - dictionary is
-  // malformed (though many implementations will accept any data as a
-  // dictionary)
+  // Must return OK if storing a result in `out`. Otherwise, could return values
+  // like NotSupported - dictionary compression is not (yet) supported for this
+  // kind of Decompressor.
+  // Corruption - dictionary is malformed (though many implementations will
+  // accept any data as a dictionary)
   virtual Status MaybeCloneForDict(const Slice& /*serialized_dict*/,
                                    std::unique_ptr<Decompressor>* /*out*/) {
     return Status::NotSupported(
@@ -353,31 +346,22 @@ class CompressionManager
   // should have the same CompatibilityName(), so that a compatible
   // CompressionManager/Decompressor might be used if the original is
   // unavailable. (Name() can be useful in addition to CompatibilityName() for
-  // understanding what compression strategy was used.) This name should be
-  // limited to legal variable names in C++ (alphanumeric and underscores).
+  // understanding what compression strategy was used.)
   virtual const char* CompatibilityName() const = 0;
 
   // Default implementation checks the current compatibility name and returns
   // this CompressionManager (via `out`) if appropriate, and otherwise defers
-  // to CreateFromString(). Failure should simply be a matter of "not found" in
-  // which case nullptr is returned.
-  virtual std::shared_ptr<CompressionManager> FindCompatibleCompressionManager(
-      Slice compatibility_name);
+  // to CreateFromString().
+  virtual Status FindCompatibleCompressionManager(
+      Slice compatibility_name, std::shared_ptr<CompressionManager>* out);
 
-  // Create or find a CompressionManager from a string, including built-in
+  // Create a CompressionManager from a string, including built-in
   // CompressionManager types.
   // TODO: ObjectLibrary stuff
   static Status CreateFromString(const ConfigOptions& config_options,
                                  const std::string& id,
                                  std::shared_ptr<CompressionManager>* result);
 
-  // Returns false iff a configuration that would pass the given compression
-  // type to GetCompressor/GetCompressorForSST should be rejected (not
-  // supported)
-  virtual bool SupportsCompressionType(CompressionType type) const = 0;
-
-  // TODO: function to check compatibility with or sanitize CompressionOptions
-
   // ************************* Compressor creation *********************** //
   // Returning nullptr means compression is entirely disabled for the file,
   // which is valid at the discretion of the CompressionManager. Returning
@@ -426,14 +410,6 @@ class CompressionManager
     // Safe default implementation
     return GetDecompressor();
   }
-
-  // Get a decompressor that is allowed to have support only for the
-  // CompressionTypes used by the given Compressor.
-  virtual std::shared_ptr<Decompressor> GetDecompressorForCompressor(
-      const Compressor& compressor) {
-    // Reasonable default implementation
-    return GetDecompressorOptimizeFor(compressor.GetPreferredCompressionType());
-  }
 };
 
 // ************************* Utility wrappers etc. *********************** //
@@ -477,51 +453,6 @@ class CompressorWrapper : public Compressor {
   std::unique_ptr<Compressor> wrapped_;
 };
 
-class DecompressorWrapper : public Decompressor {
- public:
-  explicit DecompressorWrapper(std::shared_ptr<Decompressor> decompressor)
-      : wrapped_(std::move(decompressor)) {}
-  // No copies
-  DecompressorWrapper(const DecompressorWrapper&) = delete;
-  DecompressorWrapper& operator=(const DecompressorWrapper&) = delete;
-
-  const char* Name() const override { return wrapped_->Name(); }
-
-  void ReleaseWorkingArea(WorkingArea* wa) override {
-    wrapped_->ReleaseWorkingArea(wa);
-  }
-
-  ManagedWorkingArea ObtainWorkingArea(CompressionType preferred) override {
-    return wrapped_->ObtainWorkingArea(preferred);
-  }
-
-  const Slice& GetSerializedDict() const override {
-    return wrapped_->GetSerializedDict();
-  }
-
-  Status MaybeCloneForDict(const Slice& serialized_dict,
-                           std::unique_ptr<Decompressor>* out) override {
-    // NOTE: derived class probably needs to override this to ensure a
-    // derived wrapper around the new Decompressor
-    return wrapped_->MaybeCloneForDict(serialized_dict, out);
-  }
-
-  size_t ApproximateOwnedMemoryUsage() const override {
-    return wrapped_->ApproximateOwnedMemoryUsage();
-  }
-
-  Status ExtractUncompressedSize(Args& args) override {
-    return wrapped_->ExtractUncompressedSize(args);
-  }
-
-  Status DecompressBlock(const Args& args, char* uncompressed_output) override {
-    return wrapped_->DecompressBlock(args, uncompressed_output);
-  }
-
- protected:
-  std::shared_ptr<Decompressor> wrapped_;
-};
-
 // TODO: CompressorBase, for custom compressions
 
 class CompressionManagerWrapper : public CompressionManager {
@@ -534,13 +465,10 @@ class CompressionManagerWrapper : public CompressionManager {
     return wrapped_->CompatibilityName();
   }
 
-  std::shared_ptr<CompressionManager> FindCompatibleCompressionManager(
-      Slice compatibility_name) override {
-    return wrapped_->FindCompatibleCompressionManager(compatibility_name);
-  }
-
-  bool SupportsCompressionType(CompressionType type) const override {
-    return wrapped_->SupportsCompressionType(type);
+  Status FindCompatibleCompressionManager(
+      Slice compatibility_name,
+      std::shared_ptr<CompressionManager>* out) override {
+    return wrapped_->FindCompatibleCompressionManager(compatibility_name, out);
   }
 
   std::unique_ptr<Compressor> GetCompressorForSST(
@@ -569,26 +497,15 @@ class CompressionManagerWrapper : public CompressionManager {
     return wrapped_->GetDecompressorForTypes(types_begin, types_end);
   }
 
-  std::shared_ptr<Decompressor> GetDecompressorForCompressor(
-      const Compressor& compressor) override {
-    return wrapped_->GetDecompressorForCompressor(compressor);
-  }
-
  protected:
   std::shared_ptr<CompressionManager> wrapped_;
 };
 
-// Compression manager that implements the second schema for RocksDB built-in
-// compression support. (The first schema is intentionally not provided here.)
-// *** CURRENT STATE ***
-// This is currently the latest schema for built-in compression, and the
-// compression manager used when compression_manager=nullptr.
-const std::shared_ptr<CompressionManager>& GetBuiltinV2CompressionManager();
-
-// NOTE: No GetLatestBuiltinCompressionManager() is provided because that could
-// lead to unexpected schema changes for user CompressionManagers building on
-// the built-in schema, in the unlikely/rare case of a new built-in schema.
-
+// Compression manager that implements built-in compression strategy. The
+// behavior of
+// compression_manager=nullptr with this
+const std::shared_ptr<CompressionManager>&
+GetDefaultBuiltinCompressionManager();
 // Gets CompressionManager designed for the automated compression strategy.
 // This may include deciding to compress or not.
 // In future should be able to select compression algorithm based on the CPU

include/rocksdb/compression_type.h

@@ -18,148 +18,16 @@ namespace ROCKSDB_NAMESPACE {
 enum CompressionType : unsigned char {
   // NOTE: do not change the values of existing entries, as these are
   // part of the persistent format on disk.
-  kNoCompression = 0x00,
-  kSnappyCompression = 0x01,
-  kZlibCompression = 0x02,
-  kBZip2Compression = 0x03,
-  kLZ4Compression = 0x04,
-  kLZ4HCCompression = 0x05,
-  kXpressCompression = 0x06,
-  kZSTD = 0x07,
-  kLastBuiltinCompression = kZSTD,
+  kNoCompression = 0x0,
+  kSnappyCompression = 0x1,
+  kZlibCompression = 0x2,
+  kBZip2Compression = 0x3,
+  kLZ4Compression = 0x4,
+  kLZ4HCCompression = 0x5,
+  kXpressCompression = 0x6,
+  kZSTD = 0x7,
 
-  // Reserved for future use: up to 0x7F
-
-  // For use by user custom CompressionManagers
-  kCustomCompression80 = 0x80,
-  kFirstCustomCompression = kCustomCompression80,
-  kCustomCompression81 = 0x81,
-  kCustomCompression82 = 0x82,
-  kCustomCompression83 = 0x83,
-  kCustomCompression84 = 0x84,
-  kCustomCompression85 = 0x85,
-  kCustomCompression86 = 0x86,
-  kCustomCompression87 = 0x87,
-  kCustomCompression88 = 0x88,
-  kCustomCompression89 = 0x89,
-  kCustomCompression8A = 0x8A,
-  kCustomCompression8B = 0x8B,
-  kCustomCompression8C = 0x8C,
-  kCustomCompression8D = 0x8D,
-  kCustomCompression8E = 0x8E,
-  kCustomCompression8F = 0x8F,
-  kCustomCompression90 = 0x90,
-  kCustomCompression91 = 0x91,
-  kCustomCompression92 = 0x92,
-  kCustomCompression93 = 0x93,
-  kCustomCompression94 = 0x94,
-  kCustomCompression95 = 0x95,
-  kCustomCompression96 = 0x96,
-  kCustomCompression97 = 0x97,
-  kCustomCompression98 = 0x98,
-  kCustomCompression99 = 0x99,
-  kCustomCompression9A = 0x9A,
-  kCustomCompression9B = 0x9B,
-  kCustomCompression9C = 0x9C,
-  kCustomCompression9D = 0x9D,
-  kCustomCompression9E = 0x9E,
-  kCustomCompression9F = 0x9F,
-  kCustomCompressionA0 = 0xA0,
-  kCustomCompressionA1 = 0xA1,
-  kCustomCompressionA2 = 0xA2,
-  kCustomCompressionA3 = 0xA3,
-  kCustomCompressionA4 = 0xA4,
-  kCustomCompressionA5 = 0xA5,
-  kCustomCompressionA6 = 0xA6,
-  kCustomCompressionA7 = 0xA7,
-  kCustomCompressionA8 = 0xA8,
-  kCustomCompressionA9 = 0xA9,
-  kCustomCompressionAA = 0xAA,
-  kCustomCompressionAB = 0xAB,
-  kCustomCompressionAC = 0xAC,
-  kCustomCompressionAD = 0xAD,
-  kCustomCompressionAE = 0xAE,
-  kCustomCompressionAF = 0xAF,
-  kCustomCompressionB0 = 0xB0,
-  kCustomCompressionB1 = 0xB1,
-  kCustomCompressionB2 = 0xB2,
-  kCustomCompressionB3 = 0xB3,
-  kCustomCompressionB4 = 0xB4,
-  kCustomCompressionB5 = 0xB5,
-  kCustomCompressionB6 = 0xB6,
-  kCustomCompressionB7 = 0xB7,
-  kCustomCompressionB8 = 0xB8,
-  kCustomCompressionB9 = 0xB9,
-  kCustomCompressionBA = 0xBA,
-  kCustomCompressionBB = 0xBB,
-  kCustomCompressionBC = 0xBC,
-  kCustomCompressionBD = 0xBD,
-  kCustomCompressionBE = 0xBE,
-  kCustomCompressionBF = 0xBF,
-  kCustomCompressionC0 = 0xC0,
-  kCustomCompressionC1 = 0xC1,
-  kCustomCompressionC2 = 0xC2,
-  kCustomCompressionC3 = 0xC3,
-  kCustomCompressionC4 = 0xC4,
-  kCustomCompressionC5 = 0xC5,
-  kCustomCompressionC6 = 0xC6,
-  kCustomCompressionC7 = 0xC7,
-  kCustomCompressionC8 = 0xC8,
-  kCustomCompressionC9 = 0xC9,
-  kCustomCompressionCA = 0xCA,
-  kCustomCompressionCB = 0xCB,
-  kCustomCompressionCC = 0xCC,
-  kCustomCompressionCD = 0xCD,
-  kCustomCompressionCE = 0xCE,
-  kCustomCompressionCF = 0xCF,
-  kCustomCompressionD0 = 0xD0,
-  kCustomCompressionD1 = 0xD1,
-  kCustomCompressionD2 = 0xD2,
-  kCustomCompressionD3 = 0xD3,
-  kCustomCompressionD4 = 0xD4,
-  kCustomCompressionD5 = 0xD5,
-  kCustomCompressionD6 = 0xD6,
-  kCustomCompressionD7 = 0xD7,
-  kCustomCompressionD8 = 0xD8,
-  kCustomCompressionD9 = 0xD9,
-  kCustomCompressionDA = 0xDA,
-  kCustomCompressionDB = 0xDB,
-  kCustomCompressionDC = 0xDC,
-  kCustomCompressionDD = 0xDD,
-  kCustomCompressionDE = 0xDE,
-  kCustomCompressionDF = 0xDF,
-  kCustomCompressionE0 = 0xE0,
-  kCustomCompressionE1 = 0xE1,
-  kCustomCompressionE2 = 0xE2,
-  kCustomCompressionE3 = 0xE3,
-  kCustomCompressionE4 = 0xE4,
-  kCustomCompressionE5 = 0xE5,
-  kCustomCompressionE6 = 0xE6,
-  kCustomCompressionE7 = 0xE7,
-  kCustomCompressionE8 = 0xE8,
-  kCustomCompressionE9 = 0xE9,
-  kCustomCompressionEA = 0xEA,
-  kCustomCompressionEB = 0xEB,
-  kCustomCompressionEC = 0xEC,
-  kCustomCompressionED = 0xED,
-  kCustomCompressionEE = 0xEE,
-  kCustomCompressionEF = 0xEF,
-  kCustomCompressionF0 = 0xF0,
-  kCustomCompressionF1 = 0xF1,
-  kCustomCompressionF2 = 0xF2,
-  kCustomCompressionF3 = 0xF3,
-  kCustomCompressionF4 = 0xF4,
-  kCustomCompressionF5 = 0xF5,
-  kCustomCompressionF6 = 0xF6,
-  kCustomCompressionF7 = 0xF7,
-  kCustomCompressionF8 = 0xF8,
-  kCustomCompressionF9 = 0xF9,
-  kCustomCompressionFA = 0xFA,
-  kCustomCompressionFB = 0xFB,
-  kCustomCompressionFC = 0xFC,
-  kCustomCompressionFD = 0xFD,
-  kCustomCompressionFE = 0xFE,
-  kLastCustomCompression = kCustomCompressionFE,
+  // TODO: add enum values for user custom compression types
 
   // kDisableCompressionOption is used to disable some compression options.
   kDisableCompressionOption = 0xff,

include/rocksdb/external_table.h

@@ -269,7 +269,7 @@ class ExternalTableFactory : public Customizable {
 
 // Allocate a TableFactory that wraps around an ExternalTableFactory. Use this
 // to allocate and set in ColumnFamilyOptions::table_factory.
-std::unique_ptr<TableFactory> NewExternalTableFactory(
+std::shared_ptr<TableFactory> NewExternalTableFactory(
     std::shared_ptr<ExternalTableFactory> inner_factory);
 
 }  // namespace ROCKSDB_NAMESPACE

include/rocksdb/file_checksum.h

@@ -80,8 +80,7 @@ class FileChecksumGenFactory : public Customizable {
       const ConfigOptions& options, const std::string& value,
       std::shared_ptr<FileChecksumGenFactory>* result);
 
-  // Create a new FileChecksumGenerator. Recommended to return nullptr if the
-  // requested function name is not recognized.
+  // Create a new FileChecksumGenerator.
   virtual std::unique_ptr<FileChecksumGenerator> CreateFileChecksumGenerator(
       const FileChecksumGenContext& context) = 0;
 

include/rocksdb/memtablerep.h

@@ -162,12 +162,6 @@ class MemTableRep {
     return true;
   }
 
-  // Only used after concurrent memtable inserts.
-  // This function will be called by each writer after all writes are done
-  // through InsertConcurrently().
-  // This is used by VectorRep to do batched writes for concurrent inserts.
-  virtual void BatchPostProcess() {}
-
   // Returns true iff an entry that compares equal to key is in the collection.
   virtual bool Contains(const char* key) const = 0;
 
@@ -403,11 +397,6 @@ class SkipListFactory : public MemTableRepFactory {
 // the vector is sorted. This is useful for workloads where iteration is very
 // rare and writes are generally not issued after reads begin.
 //
-// Concurrent inserts are supported by buffering writes in thread-local vectors
-// for each write batch. To optimize performance for concurrent inserts, it is
-// recommended to perform batched writes, and enable unordered_write (refer to
-// the option comment for its impact on read consistency).
-//
 // Parameters:
 //   count: Passed to the constructor of the underlying std::vector of each
 //     VectorRep. On initialization, the underlying array will be at least count
@@ -429,8 +418,6 @@ class VectorRepFactory : public MemTableRepFactory {
   MemTableRep* CreateMemTableRep(const MemTableRep::KeyComparator&, Allocator*,
                                  const SliceTransform*,
                                  Logger* logger) override;
-
-  bool IsInsertConcurrentlySupported() const override { return true; }
 };
 
 // This class contains a fixed array of buckets, each

include/rocksdb/table.h

@@ -561,10 +561,6 @@ struct BlockBasedTableOptions {
   // misplaced within or between files is as likely to fail checksum
   // verification as random corruption. Also checksum-protects SST footer.
   // Can be read by RocksDB versions >= 8.6.0.
-  // 7 -- Support for custom compression algorithms with a CompressionManager
-  // using a non-built-in CompatibilityName(). See `compression_manager` in
-  // ColumnFamilyOptions. Also changes the format of TableProperties field
-  // `compression_name`. Can be read by RocksDB versions >= 10.4.0.
   //
   // Using the default setting of format_version is strongly recommended, so
   // that available enhancements are adopted eventually and automatically. The

include/rocksdb/table_properties.h

@@ -109,10 +109,6 @@ class TablePropertiesCollector {
   // table.
   // @params key    the user key that is inserted into the table.
   // @params value  the value that is inserted into the table.
-  // @params file_size the current file size. For BlockBasedTable, this
-  //         includes all the data blocks written so far, upto but not including
-  //         the current block being built. With parallel compression, data
-  //         blocks are written async so it depends on the compression progress.
   virtual Status AddUserKey(const Slice& key, const Slice& value,
                             EntryType /*type*/, SequenceNumber /*seq*/,
                             uint64_t /*file_size*/) {
@@ -147,7 +143,7 @@ class TablePropertiesCollector {
   // The name of the properties collector can be used for debugging purpose.
   virtual const char* Name() const = 0;
 
-  // Return whether the output file should be further compacted
+  // EXPERIMENTAL Return whether the output file should be further compacted
   virtual bool NeedCompact() const { return false; }
 
   // For internal use only.
@@ -348,20 +344,7 @@ struct TableProperties {
   // {collector_name[1]},{collector_name[2]},{collector_name[3]} ..
   std::string property_collectors_names;
 
-  // Identifies the compression algorithm or schema used in the file.
-  // Specifically:
-  // * For format_version < 7, it is one of several names for built-in
-  // compression types. Because of how some previous versions of RocksDB
-  // behave, this must be set to "ZSTD" if any blocks are compressed
-  // with zstd and must NOT be set to "NoCompression" if any blocks are
-  // compressed.
-  // * For format_version >= 7, the format is
-  //   <compatibility_name>;<hex-coded compression types>;<future use>
-  // where <compatibility_name> is the CompatibilityName() of the
-  // CompressionManager used for the file, or empty if compression was
-  // disabled; <hex-coded compression types> represents a sorted set of
-  // CompressionType values used in the file other than kNoCompression, each
-  // as 2-digit hex, e.g. 04 for LZ$, 07 for ZSTD, etc.
+  // The compression algo used to compress the SST files.
   std::string compression_name;
 
   // Compression options used to compress the SST files.

include/rocksdb/utilities/table_properties_collectors.h

@@ -23,20 +23,15 @@ class CompactOnDeletionCollectorFactory
   // A factory of a table property collector that marks a SST
   // file as need-compaction when it observe at least "D" deletion
   // entries in any "N" consecutive entries, or the ratio of tombstone
-  // entries >= deletion_ratio for the entire file.
+  // entries >= deletion_ratio.
   //
   // @param sliding_window_size "N"
   // @param deletion_trigger "D"
   // @param deletion_ratio, if <= 0 or > 1, disable triggering compaction
   //     based on deletion ratio.
-  // @param min_file_size, a file needs to be at least this size to be marked
-  //     for compaction. See comments above
-  //     TablePropertiesCollector::AddUserKey() for limitations/inaccuracies on
-  //     the file size.
   CompactOnDeletionCollectorFactory(size_t sliding_window_size,
                                     size_t deletion_trigger,
-                                    double deletion_ratio,
-                                    uint64_t min_file_size = 0);
+                                    double deletion_ratio);
 
   ~CompactOnDeletionCollectorFactory() override {}
 
@@ -64,12 +59,6 @@ class CompactOnDeletionCollectorFactory
   }
 
   double GetDeletionRatio() const { return deletion_ratio_.load(); }
-
-  uint64_t GetMinFileSize() const { return min_file_size_.load(); }
-  void SetMinFileSize(uint64_t min_file_size) {
-    min_file_size_.store(min_file_size);
-  }
-
   static const char* kClassName() { return "CompactOnDeletionCollector"; }
   const char* Name() const override { return kClassName(); }
 
@@ -79,7 +68,6 @@ class CompactOnDeletionCollectorFactory
   std::atomic<size_t> sliding_window_size_;
   std::atomic<size_t> deletion_trigger_;
   std::atomic<double> deletion_ratio_;
-  std::atomic<uint64_t> min_file_size_;
 };
 
 // Creates a factory of a table property collector that marks a SST
@@ -97,8 +85,7 @@ class CompactOnDeletionCollectorFactory
 std::shared_ptr<CompactOnDeletionCollectorFactory>
 NewCompactOnDeletionCollectorFactory(size_t sliding_window_size,
                                      size_t deletion_trigger,
-                                     double deletion_ratio = 0,
-                                     uint64_t min_file_size = 0);
+                                     double deletion_ratio = 0);
 
 // A factory of a table property collector that marks a SST file as
 // need-compaction when for the tiering use case, it observes, among all the

include/rocksdb/version.h

@@ -13,7 +13,7 @@
 // minor or major version number planned for release.
 #define ROCKSDB_MAJOR 10
 #define ROCKSDB_MINOR 4
-#define ROCKSDB_PATCH 2
+#define ROCKSDB_PATCH 0
 
 // Do not use these. We made the mistake of declaring macros starting with
 // double underscore. Now we have to live with our choice. We'll deprecate these

java/src/main/java/org/rocksdb/NativeLibraryLoader.java

@@ -30,14 +30,6 @@ public class NativeLibraryLoader {
   private static final String tempFilePrefix = "librocksdbjni";
   private static final String tempFileSuffix = Environment.getJniLibraryExtension();
 
-  /**
-   * If you set the System Property ROCKS_JAVA_DEBUG_NLL can be to true
-   * messages about attempts to load the native library will be printed
-   * to std out.
-   */
-  private static boolean DEBUG_LOADING =
-      "true".equals(System.getProperty("ROCKS_JAVA_DEBUG_NLL", "false"));
-
   /**
    * Get a reference to the NativeLibraryLoader
    *
@@ -63,7 +55,7 @@ public class NativeLibraryLoader {
    *
    * @throws java.io.IOException if a filesystem operation fails.
    */
-  @SuppressWarnings({"PMD.EmptyCatchBlock", "PMD.SystemPrintln"})
+  @SuppressWarnings("PMD.EmptyCatchBlock")
   public synchronized void loadLibrary(final String tmpDir) throws IOException {
     try {
       // try dynamic library
@@ -71,9 +63,6 @@ public class NativeLibraryLoader {
       return;
     } catch (final UnsatisfiedLinkError ule) {
       // ignore - try from static library
-      if (DEBUG_LOADING) {
-        System.out.println("Unable to load shared dynamic library: " + sharedLibraryName);
-      }
     }
 
     try {
@@ -82,9 +71,6 @@ public class NativeLibraryLoader {
       return;
     } catch (final UnsatisfiedLinkError ule) {
       // ignore - then try static library fallback or from jar
-      if (DEBUG_LOADING) {
-        System.out.println("Unable to load shared static library: " + jniLibraryName);
-      }
     }
 
     if (fallbackJniLibraryName != null) {
@@ -94,10 +80,6 @@ public class NativeLibraryLoader {
         return;
       } catch (final UnsatisfiedLinkError ule) {
         // ignore - then try from jar
-        if (DEBUG_LOADING) {
-          System.out.println(
-              "Unable to load shared static fallback library: " + fallbackJniLibraryName);
-        }
       }
     }
 
@@ -155,23 +137,18 @@ public class NativeLibraryLoader {
     }
   }
 
-  @SuppressWarnings({"PMD.UseProperClassLoader", "PMD.UseTryWithResources", "PMD.SystemPrintln"})
+  @SuppressWarnings({"PMD.UseProperClassLoader", "PMD.UseTryWithResources"})
   File loadLibraryFromJarToTemp(final String tmpDir) throws IOException {
     try (InputStream is = getClass().getClassLoader().getResourceAsStream(jniLibraryFileName)) {
       if (is != null) {
         final File temp = createTemp(tmpDir, jniLibraryFileName);
         Files.copy(is, temp.toPath(), StandardCopyOption.REPLACE_EXISTING);
         return temp;
-      } else {
-        if (DEBUG_LOADING) {
-          System.out.println("Unable to find: " + jniLibraryFileName + " on the classpath");
-        }
       }
     }
 
     if (fallbackJniLibraryFileName == null) {
-      throw new RuntimeException(
-          jniLibraryFileName + " was not found inside JAR, and there is no fallback.");
+      throw new RuntimeException(fallbackJniLibraryFileName + " was not found inside JAR.");
     }
 
     try (InputStream is =
@@ -180,16 +157,10 @@ public class NativeLibraryLoader {
         final File temp = createTemp(tmpDir, fallbackJniLibraryFileName);
         Files.copy(is, temp.toPath(), StandardCopyOption.REPLACE_EXISTING);
         return temp;
-      } else {
-        if (DEBUG_LOADING) {
-          System.out.println(
-              "Unable to find fallback: " + fallbackJniLibraryFileName + " on the classpath");
-        }
       }
     }
 
-    throw new RuntimeException("Neither " + jniLibraryFileName + " or " + fallbackJniLibraryFileName
-        + " were found inside the JAR, and there is no fallback.");
+    throw new RuntimeException(jniLibraryFileName + " was not found inside JAR.");
   }
 
   /**

java/src/main/java/org/rocksdb/RocksDB.java

@@ -84,7 +84,13 @@ public class RocksDB extends RocksObject {
       return;
     }
 
-    waitForLibraryToBeLoaded();
+    while (libraryLoaded.get() == LibraryState.LOADING) {
+      try {
+        Thread.sleep(10);
+      } catch(final InterruptedException e) {
+        //ignore
+      }
+    }
   }
 
   /**
@@ -140,28 +146,12 @@ public class RocksDB extends RocksObject {
       return;
     }
 
-    waitForLibraryToBeLoaded();
-  }
-
-  private static void waitForLibraryToBeLoaded() {
-    final long wait = 10; // Time to wait before re-checking if another thread loaded the library
-    final long timeout =
-        10 * 1000; // Maximum time to wait for another thread to load the library (10 seconds)
-    long waited = 0;
-    try {
-      while (libraryLoaded.get() == LibraryState.LOADING) {
-        Thread.sleep(wait);
-        waited += wait;
-
-        if (waited >= timeout) {
-          throw new RuntimeException(
-              "Exceeded timeout whilst trying to load the RocksDB shared library");
-        }
+    while (libraryLoaded.get() == LibraryState.LOADING) {
+      try {
+        Thread.sleep(10);
+      } catch(final InterruptedException e) {
+        //ignore
       }
-    } catch (final InterruptedException e) {
-      // restore interrupted status
-      Thread.currentThread().interrupt();
-      throw new RuntimeException("Interrupted whilst trying to load the RocksDB shared library", e);
     }
   }
 

memtable/vectorrep.cc

@@ -30,8 +30,6 @@ class VectorRep : public MemTableRep {
   // collection.
   void Insert(KeyHandle handle) override;
 
-  void InsertConcurrently(KeyHandle handle) override;
-
   // Returns true iff an entry that compares equal to key is in the collection.
   bool Contains(const char* key) const override;
 
@@ -42,8 +40,6 @@ class VectorRep : public MemTableRep {
   void Get(const LookupKey& k, void* callback_args,
            bool (*callback_func)(void* arg, const char* entry)) override;
 
-  void BatchPostProcess() override;
-
   ~VectorRep() override = default;
 
   class Iterator : public MemTableRep::Iterator {
@@ -104,40 +100,19 @@ class VectorRep : public MemTableRep {
 
  private:
   friend class Iterator;
-  ALIGN_AS(CACHE_LINE_SIZE) RelaxedAtomic<size_t> bucket_size_;
   using Bucket = std::vector<const char*>;
   std::shared_ptr<Bucket> bucket_;
   mutable port::RWMutex rwlock_;
   bool immutable_;
   bool sorted_;
   const KeyComparator& compare_;
-  // Thread-local vector to buffer concurrent writes.
-  using TlBucket = std::vector<const char*>;
-  ThreadLocalPtr tl_writes_;
-
-  static void DeleteTlBucket(void* ptr) {
-    auto* v = static_cast<TlBucket*>(ptr);
-    delete v;
-  }
 };
 
 void VectorRep::Insert(KeyHandle handle) {
   auto* key = static_cast<char*>(handle);
-  {
-    WriteLock l(&rwlock_);
-    assert(!immutable_);
-    bucket_->push_back(key);
-  }
-  bucket_size_.FetchAddRelaxed(1);
-}
-
-void VectorRep::InsertConcurrently(KeyHandle handle) {
-  auto* v = static_cast<TlBucket*>(tl_writes_.Get());
-  if (!v) {
-    v = new TlBucket();
-    tl_writes_.Reset(v);
-  }
-  v->push_back(static_cast<char*>(handle));
+  WriteLock l(&rwlock_);
+  assert(!immutable_);
+  bucket_->push_back(key);
 }
 
 // Returns true iff an entry that compares equal to key is in the collection.
@@ -152,35 +127,19 @@ void VectorRep::MarkReadOnly() {
 }
 
 size_t VectorRep::ApproximateMemoryUsage() {
-  return bucket_size_.LoadRelaxed() *
-         sizeof(std::remove_reference<decltype(*bucket_)>::type::value_type);
-}
-
-void VectorRep::BatchPostProcess() {
-  auto* v = static_cast<TlBucket*>(tl_writes_.Get());
-  if (v) {
-    {
-      WriteLock l(&rwlock_);
-      assert(!immutable_);
-      for (auto& key : *v) {
-        bucket_->push_back(key);
-      }
-    }
-    bucket_size_.FetchAddRelaxed(v->size());
-    delete v;
-    tl_writes_.Reset(nullptr);
-  }
+  return sizeof(bucket_) + sizeof(*bucket_) +
+         bucket_->size() *
+             sizeof(
+                 std::remove_reference<decltype(*bucket_)>::type::value_type);
 }
 
 VectorRep::VectorRep(const KeyComparator& compare, Allocator* allocator,
                      size_t count)
     : MemTableRep(allocator),
-      bucket_size_(0),
       bucket_(new Bucket()),
       immutable_(false),
       sorted_(false),
-      compare_(compare),
-      tl_writes_(DeleteTlBucket) {
+      compare_(compare) {
   bucket_.get()->reserve(count);
 }
 

options/configurable.cc

@@ -272,8 +272,7 @@ Status ConfigurableHelper::ConfigureOptions(
   if (config_options.ignore_unknown_options) {
     s = Status::OK();
   } else if (s.ok() && unused == nullptr && !remaining.empty()) {
-    s = Status::NotFound("Extra option not recognized",
-                         remaining.begin()->first);
+    s = Status::NotFound("Could not find option: ", remaining.begin()->first);
   }
   return s;
 }
@@ -370,7 +369,7 @@ Status ConfigurableHelper::ConfigureSingleOption(
   const auto opt_info =
       FindOption(configurable, opt_name, &elem_name, &opt_ptr);
   if (opt_info == nullptr) {
-    return Status::NotFound("Could not find option", name);
+    return Status::NotFound("Could not find option: ", name);
   } else {
     return ConfigureOption(config_options, configurable, *opt_info, opt_name,
                            elem_name, value, opt_ptr);
@@ -466,7 +465,7 @@ Status ConfigurableHelper::ConfigureOption(
     return configurable.ParseOption(config_options, opt_info, name, value,
                                     opt_ptr);
   } else {
-    return Status::NotFound("Unknown how to configure option", name);
+    return Status::NotFound("Could not find option: ", name);
   }
 }
 

options/options_helper.cc

@@ -386,133 +386,6 @@ std::unordered_map<std::string, CompressionType>
         {"kLZ4HCCompression", kLZ4HCCompression},
         {"kXpressCompression", kXpressCompression},
         {"kZSTD", kZSTD},
-        {"kCustomCompression80", kCustomCompression80},
-        {"kCustomCompression81", kCustomCompression81},
-        {"kCustomCompression82", kCustomCompression82},
-        {"kCustomCompression83", kCustomCompression83},
-        {"kCustomCompression84", kCustomCompression84},
-        {"kCustomCompression85", kCustomCompression85},
-        {"kCustomCompression86", kCustomCompression86},
-        {"kCustomCompression87", kCustomCompression87},
-        {"kCustomCompression88", kCustomCompression88},
-        {"kCustomCompression89", kCustomCompression89},
-        {"kCustomCompression8A", kCustomCompression8A},
-        {"kCustomCompression8B", kCustomCompression8B},
-        {"kCustomCompression8C", kCustomCompression8C},
-        {"kCustomCompression8D", kCustomCompression8D},
-        {"kCustomCompression8E", kCustomCompression8E},
-        {"kCustomCompression8F", kCustomCompression8F},
-        {"kCustomCompression90", kCustomCompression90},
-        {"kCustomCompression91", kCustomCompression91},
-        {"kCustomCompression92", kCustomCompression92},
-        {"kCustomCompression93", kCustomCompression93},
-        {"kCustomCompression94", kCustomCompression94},
-        {"kCustomCompression95", kCustomCompression95},
-        {"kCustomCompression96", kCustomCompression96},
-        {"kCustomCompression97", kCustomCompression97},
-        {"kCustomCompression98", kCustomCompression98},
-        {"kCustomCompression99", kCustomCompression99},
-        {"kCustomCompression9A", kCustomCompression9A},
-        {"kCustomCompression9B", kCustomCompression9B},
-        {"kCustomCompression9C", kCustomCompression9C},
-        {"kCustomCompression9D", kCustomCompression9D},
-        {"kCustomCompression9E", kCustomCompression9E},
-        {"kCustomCompression9F", kCustomCompression9F},
-        {"kCustomCompressionA0", kCustomCompressionA0},
-        {"kCustomCompressionA1", kCustomCompressionA1},
-        {"kCustomCompressionA2", kCustomCompressionA2},
-        {"kCustomCompressionA3", kCustomCompressionA3},
-        {"kCustomCompressionA4", kCustomCompressionA4},
-        {"kCustomCompressionA5", kCustomCompressionA5},
-        {"kCustomCompressionA6", kCustomCompressionA6},
-        {"kCustomCompressionA7", kCustomCompressionA7},
-        {"kCustomCompressionA8", kCustomCompressionA8},
-        {"kCustomCompressionA9", kCustomCompressionA9},
-        {"kCustomCompressionAA", kCustomCompressionAA},
-        {"kCustomCompressionAB", kCustomCompressionAB},
-        {"kCustomCompressionAC", kCustomCompressionAC},
-        {"kCustomCompressionAD", kCustomCompressionAD},
-        {"kCustomCompressionAE", kCustomCompressionAE},
-        {"kCustomCompressionAF", kCustomCompressionAF},
-        {"kCustomCompressionB0", kCustomCompressionB0},
-        {"kCustomCompressionB1", kCustomCompressionB1},
-        {"kCustomCompressionB2", kCustomCompressionB2},
-        {"kCustomCompressionB3", kCustomCompressionB3},
-        {"kCustomCompressionB4", kCustomCompressionB4},
-        {"kCustomCompressionB5", kCustomCompressionB5},
-        {"kCustomCompressionB6", kCustomCompressionB6},
-        {"kCustomCompressionB7", kCustomCompressionB7},
-        {"kCustomCompressionB8", kCustomCompressionB8},
-        {"kCustomCompressionB9", kCustomCompressionB9},
-        {"kCustomCompressionBA", kCustomCompressionBA},
-        {"kCustomCompressionBB", kCustomCompressionBB},
-        {"kCustomCompressionBC", kCustomCompressionBC},
-        {"kCustomCompressionBD", kCustomCompressionBD},
-        {"kCustomCompressionBE", kCustomCompressionBE},
-        {"kCustomCompressionBF", kCustomCompressionBF},
-        {"kCustomCompressionC0", kCustomCompressionC0},
-        {"kCustomCompressionC1", kCustomCompressionC1},
-        {"kCustomCompressionC2", kCustomCompressionC2},
-        {"kCustomCompressionC3", kCustomCompressionC3},
-        {"kCustomCompressionC4", kCustomCompressionC4},
-        {"kCustomCompressionC5", kCustomCompressionC5},
-        {"kCustomCompressionC6", kCustomCompressionC6},
-        {"kCustomCompressionC7", kCustomCompressionC7},
-        {"kCustomCompressionC8", kCustomCompressionC8},
-        {"kCustomCompressionC9", kCustomCompressionC9},
-        {"kCustomCompressionCA", kCustomCompressionCA},
-        {"kCustomCompressionCB", kCustomCompressionCB},
-        {"kCustomCompressionCC", kCustomCompressionCC},
-        {"kCustomCompressionCD", kCustomCompressionCD},
-        {"kCustomCompressionCE", kCustomCompressionCE},
-        {"kCustomCompressionCF", kCustomCompressionCF},
-        {"kCustomCompressionD0", kCustomCompressionD0},
-        {"kCustomCompressionD1", kCustomCompressionD1},
-        {"kCustomCompressionD2", kCustomCompressionD2},
-        {"kCustomCompressionD3", kCustomCompressionD3},
-        {"kCustomCompressionD4", kCustomCompressionD4},
-        {"kCustomCompressionD5", kCustomCompressionD5},
-        {"kCustomCompressionD6", kCustomCompressionD6},
-        {"kCustomCompressionD7", kCustomCompressionD7},
-        {"kCustomCompressionD8", kCustomCompressionD8},
-        {"kCustomCompressionD9", kCustomCompressionD9},
-        {"kCustomCompressionDA", kCustomCompressionDA},
-        {"kCustomCompressionDB", kCustomCompressionDB},
-        {"kCustomCompressionDC", kCustomCompressionDC},
-        {"kCustomCompressionDD", kCustomCompressionDD},
-        {"kCustomCompressionDE", kCustomCompressionDE},
-        {"kCustomCompressionDF", kCustomCompressionDF},
-        {"kCustomCompressionE0", kCustomCompressionE0},
-        {"kCustomCompressionE1", kCustomCompressionE1},
-        {"kCustomCompressionE2", kCustomCompressionE2},
-        {"kCustomCompressionE3", kCustomCompressionE3},
-        {"kCustomCompressionE4", kCustomCompressionE4},
-        {"kCustomCompressionE5", kCustomCompressionE5},
-        {"kCustomCompressionE6", kCustomCompressionE6},
-        {"kCustomCompressionE7", kCustomCompressionE7},
-        {"kCustomCompressionE8", kCustomCompressionE8},
-        {"kCustomCompressionE9", kCustomCompressionE9},
-        {"kCustomCompressionEA", kCustomCompressionEA},
-        {"kCustomCompressionEB", kCustomCompressionEB},
-        {"kCustomCompressionEC", kCustomCompressionEC},
-        {"kCustomCompressionED", kCustomCompressionED},
-        {"kCustomCompressionEE", kCustomCompressionEE},
-        {"kCustomCompressionEF", kCustomCompressionEF},
-        {"kCustomCompressionF0", kCustomCompressionF0},
-        {"kCustomCompressionF1", kCustomCompressionF1},
-        {"kCustomCompressionF2", kCustomCompressionF2},
-        {"kCustomCompressionF3", kCustomCompressionF3},
-        {"kCustomCompressionF4", kCustomCompressionF4},
-        {"kCustomCompressionF5", kCustomCompressionF5},
-        {"kCustomCompressionF6", kCustomCompressionF6},
-        {"kCustomCompressionF7", kCustomCompressionF7},
-        {"kCustomCompressionF8", kCustomCompressionF8},
-        {"kCustomCompressionF9", kCustomCompressionF9},
-        {"kCustomCompressionFA", kCustomCompressionFA},
-        {"kCustomCompressionFB", kCustomCompressionFB},
-        {"kCustomCompressionFC", kCustomCompressionFC},
-        {"kCustomCompressionFD", kCustomCompressionFD},
-        {"kCustomCompressionFE", kCustomCompressionFE},
         {"kDisableCompressionOption", kDisableCompressionOption}};
 
 const std::vector<CompressionType>& GetSupportedCompressions() {
@@ -1037,7 +910,7 @@ Status OptionTypeInfo::Parse(const ConfigOptions& config_options,
                                        : value;
 
     if (opt_ptr == nullptr) {
-      return Status::NotFound("Nullptr option", opt_name);
+      return Status::NotFound("Could not find option", opt_name);
     } else if (parse_func_ != nullptr) {
       ConfigOptions copy = config_options;
       copy.invoke_prepare_options = false;

options/options_settable_test.cc

@@ -703,11 +703,8 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) {
       new_options->compaction_options_fifo.file_temperature_age_thresholds[0]
           .age,
       12345);
-  // TODO: try to enhance ObjectLibrary to support singletons
-  // ASSERT_EQ(new_options->compression_manager,
-  //           GetBuiltinV2CompressionManager());
-  ASSERT_STREQ(new_options->compression_manager->Name(),
-               GetBuiltinV2CompressionManager()->Name());
+  ASSERT_EQ(new_options->compression_manager,
+            GetBuiltinCompressionManager(/*compression_format_version*/ 2));
 
   ColumnFamilyOptions rnd_filled_options = *new_options;
 

options/options_test.cc

@@ -2031,7 +2031,7 @@ TEST_F(OptionsTest, GetStringFromCompressionType) {
   ASSERT_EQ(res, "kZlibCompression");
 
   ASSERT_NOK(
-      GetStringFromCompressionType(&res, static_cast<CompressionType>(0x7F)));
+      GetStringFromCompressionType(&res, static_cast<CompressionType>(-10)));
 }
 
 TEST_F(OptionsTest, OnlyMutableDBOptions) {

port/port_posix.cc

@@ -220,9 +220,8 @@ int GetMaxOpenFiles() {
     return std::numeric_limits<int>::max();
   }
   return static_cast<int>(no_files_limit.rlim_cur);
-#else
-  return -1;
 #endif
+  return -1;
 }
 
 void* cacheline_aligned_alloc(size_t size) {

table/block_based/block_based_table_builder.cc

@@ -524,7 +524,6 @@ struct BlockBasedTableBuilder::Rep {
   std::string last_ikey;  // Internal key or empty (unset)
   const Slice* first_key_in_next_block = nullptr;
   bool warm_cache = false;
-  bool uses_explicit_compression_manager = false;
 
   uint64_t sample_for_compression;
   std::atomic<uint64_t> compressible_input_data_bytes;
@@ -561,11 +560,6 @@ struct BlockBasedTableBuilder::Rep {
   // in verifying data blocks.
   UnownedPtr<Decompressor> data_block_verify_decompressor;
 
-  // Set of compression types used for blocks in this file (mixing compression
-  // algorithms in a single file is allowed, using a CompressionManager)
-  SmallEnumSet<CompressionType, kDisableCompressionOption>
-      compression_types_used;
-
   // Working area for basic_compressor when compression_parallel_threads==1
   WorkingAreaPair basic_working_area;
   // Working areas for data_block_compressor, for each of
@@ -776,26 +770,16 @@ struct BlockBasedTableBuilder::Rep {
       assert(filter_context.level_at_creation < filter_context.num_levels);
     }
 
+    // TODO: get CompressionManager from options and sort out properties
+    auto mgr = tbo.moptions.compression_manager;
+    if (mgr == nullptr) {
+      mgr = GetBuiltinCompressionManager(
+          GetCompressFormatForVersion(table_opt.format_version));
+    }
+    props.compression_name = CompressionTypeToString(tbo.compression_type);
     props.compression_options =
         CompressionOptionsToString(tbo.compression_opts);
 
-    auto* mgr = tbo.moptions.compression_manager.get();
-    if (mgr == nullptr) {
-      uses_explicit_compression_manager = false;
-      mgr = GetBuiltinCompressionManager(
-                GetCompressFormatForVersion(
-                    static_cast<uint32_t>(table_opt.format_version)))
-                .get();
-    } else {
-      uses_explicit_compression_manager = true;
-
-      // Stuff some extra debugging info as extra pseudo-options. Using
-      // underscore prefix to indicate they are special.
-      props.compression_options.append("_compression_manager=");
-      props.compression_options.append(mgr->GetId());
-      props.compression_options.append("; ");
-    }
-
     // Sanitize to only allowing compression when it saves space.
     max_compressed_bytes_per_kb =
         std::min(int{1023}, tbo.compression_opts.max_compressed_bytes_per_kb);
@@ -826,23 +810,22 @@ struct BlockBasedTableBuilder::Rep {
               data_block_compressor->ObtainWorkingArea();
         }
       }
-      basic_decompressor = mgr->GetDecompressorForCompressor(*basic_compressor);
+      basic_decompressor =
+          mgr->GetDecompressorOptimizeFor(tbo.compression_type);
       create_context.decompressor = basic_decompressor.get();
 
       if (table_options.verify_compression) {
         verify_decompressor = basic_decompressor.get();
         if (table_options.enable_index_compression) {
-          basic_working_area.verify = verify_decompressor->ObtainWorkingArea(
-              basic_compressor->GetPreferredCompressionType());
+          basic_working_area.verify =
+              verify_decompressor->ObtainWorkingArea(tbo.compression_type);
         }
         if (state == State::kUnbuffered) {
-          assert(data_block_compressor);
-          data_block_verify_decompressor = verify_decompressor.get();
           for (uint32_t i = 0; i < compression_parallel_threads; i++) {
             data_block_working_areas[i].verify =
-                data_block_verify_decompressor->ObtainWorkingArea(
-                    data_block_compressor->GetPreferredCompressionType());
+                verify_decompressor->ObtainWorkingArea(tbo.compression_type);
           }
+          data_block_verify_decompressor = verify_decompressor.get();
         }
       }
     }
@@ -947,7 +930,6 @@ struct BlockBasedTableBuilder::Rep {
     // Default is UINT64_MAX for unknown. Setting it to 0 here
     // to allow updating it by taking max in BlockBasedTableBuilder::Add().
     props.key_largest_seqno = 0;
-    PrePopulateCompressionProperties(mgr);
 
     if (FormatVersionUsesContextChecksum(table_options.format_version)) {
       // Must be non-zero and semi- or quasi-random
@@ -972,95 +954,6 @@ struct BlockBasedTableBuilder::Rep {
   Rep(const Rep&) = delete;
   Rep& operator=(const Rep&) = delete;
 
-  void PrePopulateCompressionProperties(UnownedPtr<CompressionManager> mgr) {
-    if (FormatVersionUsesCompressionManagerName(table_options.format_version)) {
-      assert(mgr);
-      // Use newer compression_name property
-      props.compression_name.reserve(32);
-      // If compression is disabled, use empty manager name
-      if (basic_compressor) {
-        props.compression_name.append(mgr->CompatibilityName());
-      }
-      props.compression_name.push_back(';');
-      // Rest of property to be filled out at the end of building the file
-    } else {
-      // Use legacy compression_name property, populated at the end of building
-      // the file. Not compatible with compression managers using custom
-      // algorithms / compression types.
-      assert(Slice(mgr->CompatibilityName())
-                 .compare(GetBuiltinCompressionManager(
-                              GetCompressFormatForVersion(
-                                  static_cast<uint32_t>(props.format_version)))
-                              ->CompatibilityName()) == 0);
-    }
-  }
-  void PostPopulateCompressionProperties() {
-    // Do not include "no compression" in the set. It's not really useful
-    // information whether there are any uncompressed blocks. Some kinds of
-    // blocks are never compressed anyway.
-    compression_types_used.Remove(kNoCompression);
-    size_t ctype_count = compression_types_used.count();
-
-    if (uses_explicit_compression_manager) {
-      // Stuff some extra debugging info as extra pseudo-options. Using
-      // underscore prefix to indicate they are special.
-      std::string& compression_options = props.compression_options;
-      compression_options.append("_compressor=");
-      compression_options.append(data_block_compressor
-                                     ? data_block_compressor->GetId()
-                                     : std::string{});
-      compression_options.append("; ");
-    } else {
-      // No explicit compression manager
-      assert(compression_types_used.count() <= 1);
-    }
-
-    std::string& compression_name = props.compression_name;
-    if (FormatVersionUsesCompressionManagerName(table_options.format_version)) {
-      // Fill in extended field of "compression name" property, which is the set
-      // of compression types used, sorted by unsigned byte and then hex
-      // encoded with two digits each (so that table properties are human
-      // readable).
-      assert(*compression_name.rbegin() == ';');
-      size_t pos = compression_name.size();
-      // Make space for the field contents
-      compression_name.append(ctype_count * 2, '\0');
-      char* ptr = compression_name.data() + pos;
-      // Populate the field contents
-      for (CompressionType t : compression_types_used) {
-        PutBaseChars<16>(&ptr, /*digits=*/2, static_cast<unsigned char>(t),
-                         /*uppercase=*/true);
-      }
-      assert(ptr == compression_name.data() + pos + ctype_count * 2);
-      // Allow additional fields in the future
-      compression_name.push_back(';');
-    } else {
-      // Use legacy compression naming. To adhere to requirements described in
-      // TableProperties::compression_name, we might have to replace the name
-      // based on the legacy configured compression type.
-      assert(compression_name.empty());
-      if (ctype_count == 0) {
-        // We could get a slight performance boost in the reader by marking the
-        // file as "no compression" if compression is configured but
-        // consistently rejected, but that would give misleading info for
-        // debugging purposes. So instead we record the configured compression
-        // type, matching the historical behavior.
-        if (data_block_compressor) {
-          compression_name = CompressionTypeToString(
-              data_block_compressor->GetPreferredCompressionType());
-        } else {
-          assert(basic_compressor == nullptr);
-          compression_name = CompressionTypeToString(kNoCompression);
-        }
-      } else if (compression_types_used.Contains(kZSTD)) {
-        compression_name = CompressionTypeToString(kZSTD);
-      } else {
-        compression_name =
-            CompressionTypeToString(*compression_types_used.begin());
-      }
-    }
-  }
-
  private:
   // Synchronize status & io_status accesses across threads from main thread,
   // compression thread and write thread in parallel compression.
@@ -1539,7 +1432,6 @@ void BlockBasedTableBuilder::WriteMaybeCompressedBlock(
     }
   }
 
-  r->compression_types_used.Add(comp_type);
   std::array<char, kBlockTrailerSize> trailer;
   trailer[0] = comp_type;
   uint32_t checksum = ComputeBuiltinChecksumWithLastByte(
@@ -1891,9 +1783,6 @@ void BlockBasedTableBuilder::WritePropertiesBlock(
     }
     property_collectors_names += "]";
     rep_->props.property_collectors_names = property_collectors_names;
-
-    rep_->PostPopulateCompressionProperties();
-
     if (rep_->table_options.index_type ==
         BlockBasedTableOptions::kTwoLevelIndexSearch) {
       assert(rep_->p_index_builder_ != nullptr);
@@ -2041,8 +1930,6 @@ void BlockBasedTableBuilder::EnterUnbuffered() {
   if (kNumBlocksBuffered == 0) {
     // The below code is neither safe nor necessary for handling zero data
     // blocks.
-    // For PostPopulateCompressionProperties()
-    r->data_block_compressor = r->basic_compressor.get();
     return;
   }
 
@@ -2080,8 +1967,6 @@ void BlockBasedTableBuilder::EnterUnbuffered() {
     }
   }
 
-  assert(samples.sample_data.size() > 0);
-
   // final sample data block flushed, now we can generate dictionary
   r->compressor_with_dict = r->basic_compressor->MaybeCloneSpecialized(
       CacheEntryRole::kDataBlock, std::move(samples));

table/block_based/block_based_table_factory.cc

@@ -469,7 +469,7 @@ void BlockBasedTableFactory::InitializeOptions() {
   }
 
   if (table_options_.format_version < kMinSupportedFormatVersion) {
-    if (TEST_AllowUnsupportedFormatVersion()) {
+    if (AllowUnsupportedFormatVersion()) {
       // Allow old format version for testing.
       // And relevant old sanitization.
       if (table_options_.format_version == 0 &&
@@ -569,11 +569,9 @@ Status BlockBasedTableFactory::NewTableReader(
       file_size, table_reader_options.block_protection_bytes_per_key,
       table_reader, table_reader_options.tail_size,
       shared_state_->table_reader_cache_res_mgr,
-      table_reader_options.prefix_extractor,
-      table_reader_options.compression_manager,
-      prefetch_index_and_filter_in_cache, table_reader_options.skip_filters,
-      table_reader_options.level, table_reader_options.immortal,
-      table_reader_options.largest_seqno,
+      table_reader_options.prefix_extractor, prefetch_index_and_filter_in_cache,
+      table_reader_options.skip_filters, table_reader_options.level,
+      table_reader_options.immortal, table_reader_options.largest_seqno,
       table_reader_options.force_direct_prefetch,
       &shared_state_->tail_prefetch_stats,
       table_reader_options.block_cache_tracer,
@@ -610,67 +608,28 @@ Status BlockBasedTableFactory::ValidateOptions(
         "Enable pin_l0_filter_and_index_blocks_in_cache, "
         ", but block cache is disabled");
   }
-  if (!IsSupportedFormatVersion(table_options_.format_version) &&
-      !TEST_AllowUnsupportedFormatVersion()) {
+  if (!IsSupportedFormatVersion(table_options_.format_version)) {
     return Status::InvalidArgument(
         "Unsupported BlockBasedTable format_version. Please check "
         "include/rocksdb/table.h for more info");
   }
-  bool using_builtin_compatible_compression = true;
-  if (cf_opts.compression_manager &&
-      strcmp(cf_opts.compression_manager->CompatibilityName(),
-             GetBuiltinCompressionManager(
-                 GetCompressFormatForVersion(table_options_.format_version))
-                 ->CompatibilityName()) != 0) {
-    if (FormatVersionUsesCompressionManagerName(
-            table_options_.format_version)) {
-      using_builtin_compatible_compression = false;
-    } else {
-      return Status::InvalidArgument(
-          "Using a CompressionManager incompatible with built-in (custom "
-          "CompatibilityName()) is not supported for format_version < 7");
-    }
+  if (table_options_.block_align && (cf_opts.compression != kNoCompression)) {
+    return Status::InvalidArgument(
+        "Enable block_align, but compression "
+        "enabled");
   }
-  auto validate_compression_type_fn = [&](CompressionType ctype,
-                                          const char* context) {
-    if (ctype == kNoCompression) {
-      return Status::OK();
-    }
-    if (ctype == kDisableCompressionOption) {
-      if (strcmp(context, "compression") == 0) {
+  if (table_options_.block_align &&
+      cf_opts.bottommost_compression != kDisableCompressionOption &&
+      cf_opts.bottommost_compression != kNoCompression) {
+    return Status::InvalidArgument(
+        "Enable block_align, but bottommost_compression enabled");
+  }
+  if (table_options_.block_align) {
+    for (auto level_compression : cf_opts.compression_per_level) {
+      if (level_compression != kDisableCompressionOption &&
+          level_compression != kNoCompression) {
         return Status::InvalidArgument(
-            "kDisableCompressionOption not permitted for option: "
-            "compression");
-      } else {
-        return Status::OK();
-      }
-    }
-    if (table_options_.block_align) {
-      return Status::InvalidArgument("Enable block_align, but " +
-                                     std::string(context) + " enabled");
-    }
-    if (ctype > kLastBuiltinCompression &&
-        using_builtin_compatible_compression) {
-      return Status::InvalidArgument(
-          "Using a CompressionType other than built-in ...");  // TODO
-    }
-    // Otherwise
-    return Status::OK();
-  };
-  {
-    Status s = validate_compression_type_fn(cf_opts.compression, "compression");
-    if (!s.ok()) {
-      return s;
-    }
-    s = validate_compression_type_fn(cf_opts.bottommost_compression,
-                                     "bottommost_compression");
-    if (!s.ok()) {
-      return s;
-    }
-    for (auto ctype : cf_opts.compression_per_level) {
-      s = validate_compression_type_fn(ctype, "compression_per_level");
-      if (!s.ok()) {
-        return s;
+            "Enable block_align, but compression_per_level enabled");
       }
     }
   }
@@ -965,6 +924,11 @@ Status BlockBasedTableFactory::ParseOption(const ConfigOptions& config_options,
   return status;
 }
 
+bool& BlockBasedTableFactory::AllowUnsupportedFormatVersion() {
+  static bool allow = false;
+  return allow;
+}
+
 Status GetBlockBasedTableOptionsFromString(
     const ConfigOptions& config_options,
     const BlockBasedTableOptions& table_options, const std::string& opts_str,

table/block_based/block_based_table_factory.h

@@ -89,6 +89,10 @@ class BlockBasedTableFactory : public TableFactory {
 
   static constexpr int kMinSupportedFormatVersion = 2;
 
+  // Set to true to allow unit testing of writing unsupported block-based table
+  // format versions (to test read side)
+  static bool& AllowUnsupportedFormatVersion();
+
  protected:
   const void* GetOptionsPtr(const std::string& name) const override;
   Status ParseOption(const ConfigOptions& config_options,

table/block_based/block_based_table_reader.cc

@@ -562,112 +562,6 @@ Status GetGlobalSequenceNumber(const TableProperties& table_properties,
 
   return Status::OK();
 }
-
-Status GetDecompressor(const std::string& compression_name,
-                       UnownedPtr<CompressionManager> compression_manager,
-                       uint32_t table_format_version,
-                       std::shared_ptr<Decompressor>* out_decompressor) {
-  if (compression_name.empty()) {
-    // Very old file (before RocksDB 4.9.0) that might contain compressed
-    // blocks. Get a general decompressor for the format version.
-    auto mgr_to_use = GetBuiltinCompressionManager(
-        GetCompressFormatForVersion(table_format_version));
-    *out_decompressor = mgr_to_use->GetDecompressor();
-    return Status::OK();
-  }
-  if (FormatVersionUsesCompressionManagerName(table_format_version)) {
-    constexpr char kFieldSep = ';';
-    size_t separator_pos = compression_name.find_first_of(kFieldSep);
-    if (separator_pos == std::string::npos) {
-      return Status::Corruption(
-          "Missing separator in compression_name property");
-    }
-    // Built with explicit CompressionManager and schema support for
-    // identifying its compatibility name, which is the first field here.
-    Slice compatibility_name(compression_name.data(), separator_pos);
-    std::shared_ptr<CompressionManager> mgr_to_use;
-    if (compression_manager) {
-      // First attempt to go through the compression manager configured for
-      // writing new files, for efficiency (usually correct) and not forcing
-      // use of ObjectLibrary registration (dependency injection).
-      mgr_to_use = compression_manager->FindCompatibleCompressionManager(
-          compatibility_name);
-    }
-    if (mgr_to_use == nullptr) {
-      ConfigOptions strict;
-      strict.ignore_unknown_options = false;
-      strict.ignore_unsupported_options = false;
-      Status s = CompressionManager::CreateFromString(
-          strict, compatibility_name.ToString(), &mgr_to_use);
-      // Even though we might be able to recover from "not found" if only
-      // built-in compression types are used (would be checked below), it
-      // would provide misleading or unreliable success to allow that to
-      // succeed.
-      if (!s.ok()) {
-        return s;
-      }
-      assert(mgr_to_use || compatibility_name == kNullptrString ||
-             compatibility_name.empty());
-    }
-
-    // Second field is set of compression types actually used in the file
-    size_t start_pos = separator_pos + 1;
-    separator_pos = compression_name.find_first_of(kFieldSep, start_pos);
-    if (UNLIKELY(separator_pos == std::string::npos)) {
-      return Status::Corruption("Missing second field from compression_name");
-    }
-    if (UNLIKELY((separator_pos - start_pos) & 1)) {
-      return Status::Corruption(
-          "Second field of compression_name has odd size");
-    }
-    size_t count = (separator_pos - start_pos) / 2;
-    auto ctypes = std::make_unique<CompressionType[]>(count);
-    const char* ptr = compression_name.data() + start_pos;
-    for (size_t i = 0; i < count; ++i) {
-      uint64_t val = 0;
-      bool success = ParseBaseChars<16>(&ptr, 2, &val);
-      if (UNLIKELY(!success || val == kNoCompression ||
-                   val >= kDisableCompressionOption)) {
-        return Status::Corruption(
-            "Error parsing second field of compression_name");
-      }
-      ctypes[i] = static_cast<CompressionType>(val);
-    }
-    if (mgr_to_use) {
-      *out_decompressor = mgr_to_use->GetDecompressorForTypes(
-          ctypes.get(), ctypes.get() + count);
-      assert(*out_decompressor || count == 0);
-    } else {
-      // Compression/decompression disabled
-      *out_decompressor = nullptr;
-      assert(count == 0);
-    }
-    // Can ignore possible additional future fields
-  } else {
-    // No explicit CompressionManager, e.g. legacy file support where
-    // decompressing with built-in CompressionManager works.
-    CompressionType saved_comp_type =
-        CompressionTypeFromString(compression_name);
-    if (saved_comp_type == kDisableCompressionOption) {
-      // Unrecognized. For RocksDB versions able to read format_version=7,
-      // this is considered an error so that we can continue to evolve the
-      // schema of the compression_name property and report good error
-      // messages.
-      return Status::Corruption("Unrecognized compression_name: " +
-                                compression_name);
-    } else if (saved_comp_type != kNoCompression) {
-      // Use built-in compression manager
-      auto mgr_to_use = GetBuiltinCompressionManager(
-          GetCompressFormatForVersion(table_format_version));
-      *out_decompressor =
-          mgr_to_use->GetDecompressorOptimizeFor(saved_comp_type);
-    } else {
-      // No compression -> decompressor not needed
-      *out_decompressor = nullptr;
-    }
-  }
-  return Status::OK();
-}
 }  // namespace
 
 void BlockBasedTable::SetupBaseCacheKey(const TableProperties* properties,
@@ -735,7 +629,6 @@ Status BlockBasedTable::Open(
     std::unique_ptr<TableReader>* table_reader, uint64_t tail_size,
     std::shared_ptr<CacheReservationManager> table_reader_cache_res_mgr,
     const std::shared_ptr<const SliceTransform>& prefix_extractor,
-    UnownedPtr<CompressionManager> compression_manager,
     const bool prefetch_index_and_filter_in_cache, const bool skip_filters,
     const int level, const bool immortal_table,
     const SequenceNumber largest_seqno, const bool force_direct_prefetch,
@@ -803,8 +696,7 @@ Status BlockBasedTable::Open(
     }
     return s;
   }
-  if (!IsSupportedFormatVersion(footer.format_version()) &&
-      !TEST_AllowUnsupportedFormatVersion()) {
+  if (!IsSupportedFormatVersion(footer.format_version())) {
     return Status::Corruption(
         "Unknown Footer version. Maybe this file was created with newer "
         "version of RocksDB?");
@@ -854,13 +746,17 @@ Status BlockBasedTable::Open(
     return s;
   }
 
-  // Read compression metadata and configure decompressor
-  s = GetDecompressor(
+  CompressionType saved_comp_type = CompressionTypeFromString(
       rep->table_properties ? rep->table_properties->compression_name
-                            : std::string{},
-      compression_manager, footer.format_version(), &rep->decompressor);
-  if (!s.ok()) {
-    return s;
+                            : std::string{});
+  if (saved_comp_type != kNoCompression) {
+    // Includes "unrecognized" or "unspecified" case, including some old files
+    // before the compression_name table property was introduced in
+    // version 4.9.0
+    // TODO: custom CompressionManager
+    auto mgr = GetBuiltinCompressionManager(
+        GetCompressFormatForVersion(footer.format_version()));
+    rep->decompressor = mgr->GetDecompressorOptimizeFor(saved_comp_type);
   }
 
   // Populate BlockCreateContext

table/block_based/block_based_table_reader.h

@@ -106,7 +106,6 @@ class BlockBasedTable : public TableReader {
       std::shared_ptr<CacheReservationManager> table_reader_cache_res_mgr =
           nullptr,
       const std::shared_ptr<const SliceTransform>& prefix_extractor = nullptr,
-      UnownedPtr<CompressionManager> compression_manager = nullptr,
       bool prefetch_index_and_filter_in_cache = true, bool skip_filters = false,
       int level = -1, const bool immortal_table = false,
       const SequenceNumber largest_seqno = 0,

table/block_based/block_based_table_reader_test.cc

@@ -163,11 +163,10 @@ class BlockBasedTableReaderBaseTest : public testing::Test {
                                 bool user_defined_timestamps_persisted = true) {
     const MutableCFOptions moptions(options_);
     TableReaderOptions table_reader_options = TableReaderOptions(
-        ioptions, moptions.prefix_extractor, moptions.compression_manager.get(),
-        foptions, comparator, 0 /* block_protection_bytes_per_key */,
-        false /* _skip_filters */, false /* _immortal */,
-        false /* _force_direct_prefetch */, -1 /* _level */,
-        nullptr /* _block_cache_tracer */,
+        ioptions, moptions.prefix_extractor, foptions, comparator,
+        0 /* block_protection_bytes_per_key */, false /* _skip_filters */,
+        false /* _immortal */, false /* _force_direct_prefetch */,
+        -1 /* _level */, nullptr /* _block_cache_tracer */,
         0 /* _max_file_size_for_l0_meta_pin */, "" /* _cur_db_session_id */,
         0 /* _cur_file_num */, {} /* _unique_id */, 0 /* _largest_seqno */,
         0 /* _tail_size */, user_defined_timestamps_persisted);

table/block_based/data_block_hash_index_test.cc

@@ -582,8 +582,7 @@ void TestBoundary(InternalKey& ik1, std::string& v1, InternalKey& ik2,
   const bool kSkipFilters = true;
   const bool kImmortal = true;
   ASSERT_OK(moptions.table_factory->NewTableReader(
-      TableReaderOptions(ioptions, moptions.prefix_extractor,
-                         nullptr /* compression_manager */, soptions,
+      TableReaderOptions(ioptions, moptions.prefix_extractor, soptions,
                          internal_comparator,
                          0 /* block_protection_bytes_per_key */, !kSkipFilters,
                          !kImmortal, level_),

table/external_table.cc

@@ -473,10 +473,10 @@ class ExternalTableFactoryAdapter : public TableFactory {
 
 }  // namespace
 
-std::unique_ptr<TableFactory> NewExternalTableFactory(
+std::shared_ptr<TableFactory> NewExternalTableFactory(
     std::shared_ptr<ExternalTableFactory> inner_factory) {
-  std::unique_ptr<TableFactory> res;
-  res = std::make_unique<ExternalTableFactoryAdapter>(std::move(inner_factory));
+  std::shared_ptr<TableFactory> res;
+  res.reset(new ExternalTableFactoryAdapter(std::move(inner_factory)));
   return res;
 }
 

table/format.cc

@@ -229,8 +229,7 @@ Status FooterBuilder::Build(uint64_t magic_number, uint32_t format_version,
                             const BlockHandle& index_handle,
                             uint32_t base_context_checksum) {
   assert(magic_number != Footer::kNullTableMagicNumber);
-  assert(IsSupportedFormatVersion(format_version) ||
-         TEST_AllowUnsupportedFormatVersion());
+  assert(IsSupportedFormatVersion(format_version));
 
   char* part2;
   char* part3;
@@ -363,8 +362,7 @@ Status Footer::DecodeFrom(Slice input, uint64_t input_offset,
   } else {
     part3_ptr = magic_ptr - 4;
     format_version_ = DecodeFixed32(part3_ptr);
-    if (UNLIKELY(!IsSupportedFormatVersion(format_version_) &&
-                 !TEST_AllowUnsupportedFormatVersion())) {
+    if (UNLIKELY(!IsSupportedFormatVersion(format_version_))) {
       return Status::Corruption("Corrupt or unsupported format_version: " +
                                 std::to_string(format_version_));
     }
@@ -477,11 +475,6 @@ std::string Footer::ToString() const {
   return result;
 }
 
-bool& TEST_AllowUnsupportedFormatVersion() {
-  static bool allow = false;
-  return allow;
-}
-
 static Status ReadFooterFromFileInternal(const IOOptions& opts,
                                          RandomAccessFileReader* file,
                                          FileSystem& fs,

table/format.h

@@ -157,15 +157,10 @@ inline uint32_t GetCompressFormatForVersion(uint32_t format_version) {
   // As of format_version 2, we encode compressed block with
   // compress_format_version == 2. Before that, the version is 1.
   // DO NOT CHANGE THIS FUNCTION, it affects disk format
-  // As of format_version 7 and opening up to custom compression, the
-  // compression format version is essentially independent of the block-based
-  // table format version, and encoded in the compression_name table property.
-  // Thus, this function can go away once we remove support for reading
-  // format_version=1.
   return format_version >= 2 ? 2 : 1;
 }
 
-constexpr uint32_t kLatestFormatVersion = 7;
+constexpr uint32_t kLatestFormatVersion = 6;
 
 inline bool IsSupportedFormatVersion(uint32_t version) {
   return version <= kLatestFormatVersion;
@@ -180,10 +175,6 @@ inline bool FormatVersionUsesIndexHandleInFooter(uint32_t version) {
   return version < 6;
 }
 
-inline bool FormatVersionUsesCompressionManagerName(uint32_t version) {
-  return version >= 7;
-}
-
 // Footer encapsulates the fixed information stored at the tail end of every
 // SST file. In general, it should only include things that cannot go
 // elsewhere under the metaindex block. For example, checksum_type is
@@ -317,10 +308,6 @@ class FooterBuilder {
   std::array<char, Footer::kMaxEncodedLength> data_;
 };
 
-// Set to true to allow unit testing of writing unsupported block-based table
-// format versions (to test read side)
-bool& TEST_AllowUnsupportedFormatVersion();
-
 // Read the footer from file
 // If enforce_table_magic_number != 0, ReadFooterFromFile() will return
 // corruption if table_magic number is not equal to enforce_table_magic_number

table/sst_file_dumper.cc

@@ -172,8 +172,7 @@ Status SstFileDumper::NewTableReader(
     const InternalKeyComparator& /*internal_comparator*/, uint64_t file_size,
     std::unique_ptr<TableReader>* /*table_reader*/) {
   auto t_opt = TableReaderOptions(
-      ioptions_, moptions_.prefix_extractor,
-      moptions_.compression_manager.get(), soptions_, internal_comparator_,
+      ioptions_, moptions_.prefix_extractor, soptions_, internal_comparator_,
       0 /* block_protection_bytes_per_key */, false /* skip_filters */,
       false /* immortal */, true /* force_direct_prefetch */, -1 /* level */,
       nullptr /* block_cache_tracer */, 0 /* max_file_size_for_l0_meta_pin */,

table/sst_file_reader.cc

@@ -62,8 +62,7 @@ Status SstFileReader::Open(const std::string& file_path) {
   }
   if (s.ok()) {
     TableReaderOptions t_opt(
-        r->ioptions, r->moptions.prefix_extractor,
-        r->moptions.compression_manager.get(), r->soptions,
+        r->ioptions, r->moptions.prefix_extractor, r->soptions,
         r->ioptions.internal_comparator,
         r->moptions.block_protection_bytes_per_key,
         /*skip_filters*/ false, /*immortal*/ false,

table/table_builder.h

@@ -24,7 +24,6 @@
 #include "rocksdb/table_properties.h"
 #include "table/unique_id_impl.h"
 #include "trace_replay/block_cache_tracer.h"
-#include "util/cast_util.h"
 
 namespace ROCKSDB_NAMESPACE {
 
@@ -36,7 +35,6 @@ struct TableReaderOptions {
   TableReaderOptions(
       const ImmutableOptions& _ioptions,
       const std::shared_ptr<const SliceTransform>& _prefix_extractor,
-      UnownedPtr<CompressionManager> _compression_manager,
       const EnvOptions& _env_options,
       const InternalKeyComparator& _internal_comparator,
       uint8_t _block_protection_bytes_per_key, bool _skip_filters = false,
@@ -48,7 +46,6 @@ struct TableReaderOptions {
       uint64_t _tail_size = 0, bool _user_defined_timestamps_persisted = true)
       : ioptions(_ioptions),
         prefix_extractor(_prefix_extractor),
-        compression_manager(_compression_manager),
         env_options(_env_options),
         internal_comparator(_internal_comparator),
         skip_filters(_skip_filters),
@@ -67,9 +64,6 @@ struct TableReaderOptions {
 
   const ImmutableOptions& ioptions;
   const std::shared_ptr<const SliceTransform>& prefix_extractor;
-  // NOTE: the compression manager is not saved, just potentially a decompressor
-  // from it, so we don't need a shared_ptr copy
-  UnownedPtr<CompressionManager> compression_manager;
   const EnvOptions& env_options;
   const InternalKeyComparator& internal_comparator;
   // This is only used for BlockBasedTable (reader)

table/table_reader_bench.cc

@@ -145,9 +145,8 @@ void TableReaderBenchmark(Options& opts, EnvOptions& env_options,
     std::unique_ptr<RandomAccessFileReader> file_reader(
         new RandomAccessFileReader(std::move(raf), file_name));
     s = opts.table_factory->NewTableReader(
-        TableReaderOptions(ioptions, moptions.prefix_extractor,
-                           moptions.compression_manager.get(), env_options, ikc,
-                           0 /* block_protection_bytes_per_key */),
+        TableReaderOptions(ioptions, moptions.prefix_extractor, env_options,
+                           ikc, 0 /* block_protection_bytes_per_key */),
         std::move(file_reader), file_size, &table_reader);
     if (!s.ok()) {
       fprintf(stderr, "Open Table Error: %s\n", s.ToString().c_str());

table/table_test.cc

@@ -444,8 +444,7 @@ class TableConstructor : public Constructor {
 
     file_reader_.reset(new RandomAccessFileReader(std::move(source), "test"));
     return moptions.table_factory->NewTableReader(
-        TableReaderOptions(ioptions, moptions.prefix_extractor,
-                           moptions.compression_manager.get(), soptions,
+        TableReaderOptions(ioptions, moptions.prefix_extractor, soptions,
                            *last_internal_comparator_,
                            0 /* block_protection_bytes_per_key */,
                            /*skip_filters*/ false,
@@ -1795,23 +1794,18 @@ TEST_P(BlockBasedTableTest, IndexUncompressed) {
 #endif  // SNAPPY
 
 TEST_P(BlockBasedTableTest, BlockBasedTableProperties2) {
-  TableConstructor c(&reverse_key_comparator,
-                     true /* convert_to_internal_key_ */);
+  TableConstructor c(&reverse_key_comparator);
   std::vector<std::string> keys;
   stl_wrappers::KVMap kvmap;
 
-  for (CompressionType ct : {kNoCompression, kSnappyCompression}) {
-    if (!Snappy_Supported() && ct == kSnappyCompression) {
-      continue;
-    }
+  {
     Options options;
-    options.compression = ct;
+    options.compression = CompressionType::kNoCompression;
     BlockBasedTableOptions table_options = GetBlockBasedTableOptions();
     options.table_factory.reset(NewBlockBasedTableFactory(table_options));
 
     const ImmutableOptions ioptions(options);
     const MutableCFOptions moptions(options);
-    c.Add("blah", std::string(200, 'x'));  // something to compress
     c.Finish(options, ioptions, moptions, table_options,
              GetPlainInternalComparator(options.comparator), &keys, &kvmap);
 
@@ -1828,13 +1822,7 @@ TEST_P(BlockBasedTableTest, BlockBasedTableProperties2) {
     // No filter policy is used
     ASSERT_EQ("", props.filter_policy_name);
     // Compression type == that set:
-    if (FormatVersionUsesCompressionManagerName(table_options.format_version)) {
-      ASSERT_EQ(ct == kNoCompression ? ";;" : "BuiltinV2;01;",
-                props.compression_name);
-    } else {
-      ASSERT_EQ(ct == kNoCompression ? "NoCompression" : "Snappy",
-                props.compression_name);
-    }
+    ASSERT_EQ("NoCompression", props.compression_name);
     c.ResetTableReader();
   }
 
@@ -5344,8 +5332,7 @@ TEST_P(BlockBasedTableTest, DISABLED_TableWithGlobalSeqno) {
         new RandomAccessFileReader(std::move(source), ""));
 
     options.table_factory->NewTableReader(
-        TableReaderOptions(ioptions, moptions.prefix_extractor,
-                           moptions.compression_manager.get(), EnvOptions(),
+        TableReaderOptions(ioptions, moptions.prefix_extractor, EnvOptions(),
                            ikc, 0 /* block_protection_bytes_per_key */),
         std::move(file_reader), ss_rw.contents().size(), &table_reader);
 
@@ -5520,8 +5507,7 @@ TEST_P(BlockBasedTableTest, BlockAlignTest) {
   const MutableCFOptions moptions2(options2);
 
   ASSERT_OK(moptions.table_factory->NewTableReader(
-      TableReaderOptions(ioptions2, moptions2.prefix_extractor,
-                         moptions2.compression_manager.get(), EnvOptions(),
+      TableReaderOptions(ioptions2, moptions2.prefix_extractor, EnvOptions(),
                          GetPlainInternalComparator(options2.comparator),
                          0 /* block_protection_bytes_per_key */),
       std::move(file_reader), sink->contents().size(), &table_reader));
@@ -7400,7 +7386,8 @@ TEST_F(ExternalTableTest, IngestionTest) {
 
 int main(int argc, char** argv) {
   // Opt-in this whole test file
-  ROCKSDB_NAMESPACE::TEST_AllowUnsupportedFormatVersion() = true;
+  ROCKSDB_NAMESPACE::BlockBasedTableFactory::AllowUnsupportedFormatVersion() =
+      true;
 
   ROCKSDB_NAMESPACE::port::InstallStackTraceHandler();
   ::testing::InitGoogleTest(&argc, argv);

test_util/testutil.h

@@ -23,7 +23,6 @@
 #include "rocksdb/slice.h"
 #include "rocksdb/table.h"
 #include "table/internal_iterator.h"
-#include "util/defer.h"
 #include "util/mutexlock.h"
 
 #ifdef ROCKSDB_UNITTESTS_WITH_CUSTOM_OBJECTS_FROM_STATIC_LIBS
@@ -72,7 +71,7 @@ bool ShouldPersistUDT(const UserDefinedTimestampTestMode& test_mode);
 // Store in *dst a string of length "len" that will compress to
 // "N*compressed_fraction" bytes and return a Slice that references
 // the generated data.
-Slice CompressibleString(Random* rnd, double compressed_to_fraction, int len,
+Slice CompressibleString(Random* rnd, double compressed_fraction, int len,
                          std::string* dst);
 
 #ifndef NDEBUG
@@ -732,136 +731,6 @@ class StringFS : public FileSystemWrapper {
   std::unordered_map<std::string, std::string> files_;
 };
 
-// A compressor that essentially implements a custom compression algorithm
-// by leveraging an existing compression algorithm and putting a custom header
-// on it to detect any attempts to decompress it with the wrong compression
-// type or dictionary.
-template <CompressionType kCompression>
-struct CompressorCustomAlg : public CompressorWrapper {
-  static bool Supported() { return LZ4_Supported(); }
-
-  explicit CompressorCustomAlg(
-      std::unique_ptr<Compressor> wrapped =
-          GetBuiltinV2CompressionManager()->GetCompressor({}, kLZ4Compression))
-      : CompressorWrapper(std::move(wrapped)),
-        dictionary_hash_(GetSliceHash(wrapped_->GetSerializedDict())) {
-    static_assert(kCompression > kLastBuiltinCompression);
-  }
-
-  const char* Name() const override { return "CompressorCustomAlg"; }
-
-  CompressionType GetPreferredCompressionType() const override {
-    return kCompression;
-  }
-
-  Status CompressBlock(Slice uncompressed_data, std::string* compressed_output,
-                       CompressionType* out_compression_type,
-                       ManagedWorkingArea* working_area) override {
-    Status s = wrapped_->CompressBlock(uncompressed_data, compressed_output,
-                                       out_compression_type, working_area);
-    if (s.ok() && *out_compression_type != kNoCompression) {
-      assert(*out_compression_type == kLZ4Compression);
-      std::string header(/*size=*/5, 0);
-      header[0] = lossless_cast<char>(kCompression);
-      EncodeFixed32(&header[1], dictionary_hash_);
-      compressed_output->insert(0, header);
-      *out_compression_type = kCompression;
-    }
-    return s;
-  }
-
-  std::unique_ptr<Compressor> MaybeCloneSpecialized(
-      CacheEntryRole block_type, DictSampleArgs&& dict_samples) override {
-    auto clone =
-        wrapped_->MaybeCloneSpecialized(block_type, std::move(dict_samples));
-    return std::make_unique<CompressorCustomAlg>(std::move(clone));
-  }
-
- protected:
-  uint32_t dictionary_hash_;
-};
-
-// A decompressor suitable for all the instantiable CompressorCustomAlg
-// implementations. Can be configured to check that it is only used to
-// decompress certain types using SetAllowedTypes().
-struct DecompressorCustomAlg : public DecompressorWrapper {
-  using TypeSet = SmallEnumSet<CompressionType, kDisableCompressionOption>;
-
-  DecompressorCustomAlg(std::shared_ptr<Decompressor> wrapped =
-                            GetBuiltinV2CompressionManager()->GetDecompressor())
-      : DecompressorWrapper(std::move(wrapped)),
-        dictionary_hash_(GetSliceHash(wrapped_->GetSerializedDict())),
-        allowed_types_(TypeSet::All()) {}
-
-  const char* Name() const override { return "DecompressorCustomAlg"; }
-
-  Status MaybeCloneForDict(const Slice& serialized_dict,
-                           std::unique_ptr<Decompressor>* out) override {
-    Status s = wrapped_->MaybeCloneForDict(serialized_dict, out);
-    if (s.ok()) {
-      assert(*out != nullptr);
-      auto clone = std::make_unique<DecompressorCustomAlg>(std::move(*out));
-      clone->SetAllowedTypes(allowed_types_);
-      *out = std::move(clone);
-      assert(out->get()->GetSerializedDict() == serialized_dict);
-    } else {
-      assert(*out == nullptr);
-    }
-    return s;
-  }
-
-  Status ExtractUncompressedSize(Args& args) override {
-    if (args.compression_type >= kFirstCustomCompression &&
-        args.compression_type <= kLastCustomCompression) {
-      assert(args.compressed_data.size() > 0);
-      assert(args.compressed_data[0] ==
-             lossless_cast<char>(args.compression_type));
-      assert(DecodeFixed32(args.compressed_data.data() + 1) ==
-             dictionary_hash_);
-      // Strip off our header because ExtractUncompressedSize() is also going
-      // to strip off the uncompressed size data.
-      args.compressed_data.remove_prefix(5);
-      // It's ok to modify other parts of args if we restore to original
-      SaveAndRestore<CompressionType> save_compression_type(
-          &args.compression_type, kLZ4Compression);
-      return wrapped_->ExtractUncompressedSize(args);
-    } else {
-      // Also support built-in compressions
-      return wrapped_->ExtractUncompressedSize(args);
-    }
-  }
-
-  Status DecompressBlock(const Args& args, char* uncompressed_output) override {
-    if (args.compression_type >= kFirstCustomCompression &&
-        args.compression_type <= kLastCustomCompression) {
-      // Also allowed to copy args and modify
-      Args modified_args = args;
-      modified_args.compression_type = kLZ4Compression;
-      return wrapped_->DecompressBlock(modified_args, uncompressed_output);
-    } else {
-      // Also support built-in compressions
-      return wrapped_->DecompressBlock(args, uncompressed_output);
-    }
-  }
-
-  void SetAllowedTypes(const CompressionType* types_begin,
-                       const CompressionType* types_end) {
-    TypeSet allowed_types;
-    for (auto type = types_begin; type != types_end; ++type) {
-      allowed_types.Add(*type);
-    }
-    allowed_types_ = std::move(allowed_types);
-  }
-
-  void SetAllowedTypes(TypeSet allowed_types) {
-    allowed_types_ = std::move(allowed_types);
-  }
-
- protected:
-  uint32_t dictionary_hash_;
-  SmallEnumSet<CompressionType, kDisableCompressionOption> allowed_types_;
-};
-
 // Randomly initialize the given DBOptions
 void RandomInitDBOptions(DBOptions* db_opt, Random* rnd);
 

tools/db_bench_tool.cc

@@ -1730,10 +1730,6 @@ DEFINE_uint64(stats_history_buffer_size,
 DEFINE_bool(avoid_flush_during_recovery,
             ROCKSDB_NAMESPACE::Options().avoid_flush_during_recovery,
             "If true, avoids flushing the recovered WAL data where possible.");
-
-DEFINE_bool(avoid_flush_during_shutdown,
-            ROCKSDB_NAMESPACE::Options().avoid_flush_during_shutdown,
-            "If true, avoids flushing the recovered WAL data where possible.");
 DEFINE_int64(multiread_stride, 0,
              "Stride length for the keys in a MultiGet batch");
 DEFINE_bool(multiread_batched, false, "Use the new MultiGet API");
@@ -4268,7 +4264,6 @@ class Benchmark {
     options.stats_history_buffer_size =
         static_cast<size_t>(FLAGS_stats_history_buffer_size);
     options.avoid_flush_during_recovery = FLAGS_avoid_flush_during_recovery;
-    options.avoid_flush_during_shutdown = FLAGS_avoid_flush_during_shutdown;
 
     options.compression_opts.level = FLAGS_compression_level;
     options.compression_opts.max_dict_bytes = FLAGS_compression_max_dict_bytes;
@@ -4500,7 +4495,6 @@ class Benchmark {
       block_based_options.block_restart_interval = FLAGS_block_restart_interval;
       block_based_options.index_block_restart_interval =
           FLAGS_index_block_restart_interval;
-      TEST_AllowUnsupportedFormatVersion() = true;
       block_based_options.format_version =
           static_cast<uint32_t>(FLAGS_format_version);
       block_based_options.read_amp_bytes_per_bit = FLAGS_read_amp_bytes_per_bit;
@@ -4653,7 +4647,7 @@ class Benchmark {
         options.bottommost_compression = kZSTD;
 
         mgr = std::make_shared<RoundRobinManager>(
-            GetBuiltinV2CompressionManager());
+            GetDefaultBuiltinCompressionManager());
       } else if (!strcasecmp(FLAGS_compression_manager.c_str(), "autoskip")) {
         options.compression = FLAGS_compression_type_e;
         if (FLAGS_compression_type_e == kNoCompression) {
@@ -4662,8 +4656,8 @@ class Benchmark {
                   "autoskip");
           ErrorExit();
         }
-        mgr =
-            CreateAutoSkipCompressionManager(GetBuiltinV2CompressionManager());
+        mgr = CreateAutoSkipCompressionManager(
+            GetDefaultBuiltinCompressionManager());
       } else {
         // not defined -> exit with error
         fprintf(stderr, "Requested compression manager not supported");

tools/db_crashtest.py

@@ -178,7 +178,7 @@ default_params = {
     "verify_checksum": 1,
     "write_buffer_size": lambda: random.choice([1024 * 1024, 4 * 1024 * 1024]),
     "writepercent": 35,
-    "format_version": lambda: random.choice([2, 3, 4, 5, 6, 7, 7]),
+    "format_version": lambda: random.choice([2, 3, 4, 5, 6, 6]),
     "index_block_restart_interval": lambda: random.choice(range(1, 16)),
     "use_multiget": lambda: random.randint(0, 1),
     "use_get_entity": lambda: random.choice([0] * 7 + [1]),
@@ -350,11 +350,7 @@ default_params = {
     "ingest_wbwi_one_in": lambda: random.choice([0, 0, 100, 500]),
     "universal_reduce_file_locking": lambda: random.randint(0, 1),
     "compression_manager": lambda: random.choice(
-        ["mixed"] * 1
-        + ["none"] * 2
-        + ["autoskip"] * 2
-        + ["randommixed"] * 2
-        + ["custom"] * 3
+        ["mixed"] * 1 + ["none"] * 2 + ["autoskip"] * 2 + ["randommixed"] * 2
     ),
 }
 
@@ -524,7 +520,6 @@ cf_consistency_params = {
     "ingest_external_file_one_in": 0,
     # `CfConsistencyStressTest::TestIterateAgainstExpected()` is not implemented.
     "verify_iterator_with_expected_state_one_in": 0,
-    "memtablerep": random.choice(["skip_list"] * 9 + ["vector"]),
 }
 
 # For pessimistic transaction db
@@ -727,10 +722,6 @@ def finalize_and_sanitize(src_params):
         else:
             dest_params["mock_direct_io"] = True
 
-    if dest_params.get("memtablerep") == "vector":
-        dest_params["inplace_update_support"] = 0
-        dest_params["paranoid_memory_checks"] = 0
-
     if dest_params["test_batches_snapshots"] == 1:
         dest_params["enable_compaction_filter"] = 0
         dest_params["inplace_update_support"] = 0
@@ -958,6 +949,8 @@ def finalize_and_sanitize(src_params):
         # disable atomic flush.
         if dest_params["test_best_efforts_recovery"] == 0:
             dest_params["disable_wal"] = 0
+    if dest_params.get("allow_concurrent_memtable_write", 1) == 1:
+        dest_params["memtablerep"] = "skip_list"
     if (
         dest_params.get("enable_compaction_filter", 0) == 1
         or dest_params.get("inplace_update_support", 0) == 1
@@ -1008,19 +1001,19 @@ def finalize_and_sanitize(src_params):
             # have to disable metadata write fault injection to other file
             dest_params["exclude_wal_from_write_fault_injection"] = 1
             dest_params["metadata_write_fault_one_in"] = 0
-    # Disabling block align if mixed manager is being used
-    if dest_params.get("compression_manager") == "custom":
-        if dest_params.get("block_align") == 1:
-            dest_params["block_align"] = 0
-        if dest_params["format_version"] < 7:
-            dest_params["format_version"] = 7
-    elif (
+    # Disabling block align if mixed manager is neing used
+    if (
         dest_params.get("compression_manager") == "mixed"
         or dest_params.get("compression_manager") == "randommixed"
     ):
-        dest_params["block_align"] = 0
+        if dest_params.get("block_align") == 1:
+            dest_params["block_align"] = 0
+        dest_params["compression_type"] = "zstd"
+        dest_params["bottommost_compression_type"] = "zstd"
     elif dest_params.get("compression_manager") == "autoskip":
-        # ensuring the compression is being used
+        # disabling compression parallel threads if auto skip manager is being used as the predictor is not thread safe
+        dest_params["compression_parallel_threads"] = 1
+        # esuring the compression is being used
         if dest_params.get("compression_type") == "none":
             dest_params["compression_type"] = random.choice(
                 ["snappy", "zlib", "lz4", "lz4hc", "xpress", "zstd"]

tools/ldb_cmd.cc

@@ -870,8 +870,8 @@ bool LDBCommand::ParseCompressionTypeOption(
       }
       options_.compression = kZSTD;
       options_.bottommost_compression = kZSTD;
-      auto mgr =
-          std::make_shared<RoundRobinManager>(GetBuiltinV2CompressionManager());
+      auto mgr = std::make_shared<RoundRobinManager>(
+          GetDefaultBuiltinCompressionManager());
       options_.compression_manager = mgr;
 
       // Need to list zstd in the compression_name table property if it's

tools/sst_dump_tool.cc

@@ -197,8 +197,6 @@ int SSTDumpTool::Run(int argc, char const* const* argv, Options options) {
 
   int64_t tmp_val;
 
-  TEST_AllowUnsupportedFormatVersion() = true;
-
   for (int i = 1; i < argc; i++) {
     if (strncmp(argv[i], "--env_uri=", 10) == 0) {
       env_uri = argv[i] + 10;

unreleased_history/behavior_changes/large-txn-default-val.md

@@ -0,0 +1 @@
+* `TransactionOptions::large_txn_commit_optimize_threshold` now has default value 0 for disabled. `TransactionDBOptions::txn_commit_bypass_memtable_threshold` now has no effect on transactions.

unreleased_history/bug_fixes/create-with-import.md

@@ -0,0 +1 @@
+* Fix a bug where CreateColumnFamilyWithImport() could miss the SST file for the memtable flush it triggered. The exported CF then may not contain the updates in the memtable when CreateColumnFamilyWithImport() is called.

unreleased_history/bug_fixes/disallow_memtable_writes_paranoid_checks.md

@@ -0,0 +1 @@
+Fix iterator operations returning NotImplemented status if disallow_memtable_writes and paranoid_memory_checks CF options are both set.

unreleased_history/new_features/avg-flush-trigger.md

@@ -0,0 +1 @@
+* Add a new CF option `memtable_avg_op_scan_flush_trigger` that supports triggering memtable flush when an iterator scans through an expensive range of keys, with the average number of skipped keys from the active memtable exceeding the threshold.

unreleased_history/new_features/large-txn-byte-threshold.md

@@ -0,0 +1 @@
+* Add new experimental `TransactionOptions::large_txn_commit_optimize_byte_threshold` to enable optimizations for large transaction commit by transaction batch data size.

unreleased_history/new_features/reduce_file_locking.md

@@ -0,0 +1 @@
+Add a new option `CompactionOptionsUniversal::reduce_file_locking` and if it's true, auto universal compaction picking will adjust to minimize locking of input files when bottom priority compactions are waiting to run. This can increase the likelihood of existing L0s being selected for compaction, thereby improving write stall and reducing read regression.

util/auto_skip_compressor.cc

@@ -45,41 +45,27 @@ bool CompressionRejectionProbabilityPredictor::Record(
 AutoSkipCompressorWrapper::AutoSkipCompressorWrapper(
     std::unique_ptr<Compressor> compressor, const CompressionOptions& opts)
     : CompressorWrapper::CompressorWrapper(std::move(compressor)),
-      kOpts(opts),
+      opts_(opts),
       predictor_(
           std::make_shared<CompressionRejectionProbabilityPredictor>(10)) {}
 
-const char* AutoSkipCompressorWrapper::Name() const {
-  return "AutoSkipCompressorWrapper";
-}
-
 Status AutoSkipCompressorWrapper::CompressBlock(
     Slice uncompressed_data, std::string* compressed_output,
     CompressionType* out_compression_type, ManagedWorkingArea* wa) {
-  // Check if the managed working area is provided or owned by this object.
-  // If not, bypass auto-skip logic since the working area lacks a predictor to
-  // record or make necessary decisions to compress or bypass compression of the
-  // block
-  if (wa == nullptr || wa->owner() != this) {
-    return wrapped_->CompressBlock(uncompressed_data, compressed_output,
-                                   out_compression_type, wa);
-  }
   bool exploration =
       Random::GetTLSInstance()->PercentTrue(kExplorationPercentage);
   TEST_SYNC_POINT_CALLBACK(
       "AutoSkipCompressorWrapper::CompressBlock::exploitOrExplore",
       &exploration);
-  auto autoskip_wa = static_cast<AutoSkipWorkingArea*>(wa->get());
   if (exploration) {
     return CompressBlockAndRecord(uncompressed_data, compressed_output,
-                                  out_compression_type, autoskip_wa);
+                                  out_compression_type, wa);
   } else {
-    auto predictor_ptr = autoskip_wa->predictor;
-    auto prediction = predictor_ptr->Predict();
+    auto prediction = predictor_->Predict();
     if (prediction <= kProbabilityCutOff) {
       // decide to compress
       return CompressBlockAndRecord(uncompressed_data, compressed_output,
-                                    out_compression_type, autoskip_wa);
+                                    out_compression_type, wa);
     } else {
       // decide to bypass compression
       *out_compression_type = kNoCompression;
@@ -89,22 +75,13 @@ Status AutoSkipCompressorWrapper::CompressBlock(
   return Status::OK();
 }
 
-Compressor::ManagedWorkingArea AutoSkipCompressorWrapper::ObtainWorkingArea() {
-  auto wrap_wa = wrapped_->ObtainWorkingArea();
-  return ManagedWorkingArea(new AutoSkipWorkingArea(std::move(wrap_wa)), this);
-}
-void AutoSkipCompressorWrapper::ReleaseWorkingArea(WorkingArea* wa) {
-  delete static_cast<AutoSkipWorkingArea*>(wa);
-}
-
 Status AutoSkipCompressorWrapper::CompressBlockAndRecord(
     Slice uncompressed_data, std::string* compressed_output,
-    CompressionType* out_compression_type, AutoSkipWorkingArea* wa) {
+    CompressionType* out_compression_type, ManagedWorkingArea* wa) {
   Status status = wrapped_->CompressBlock(uncompressed_data, compressed_output,
-                                          out_compression_type, &(wa->wrapped));
+                                          out_compression_type, wa);
   // determine if it was rejected or compressed
-  auto predictor_ptr = wa->predictor;
-  predictor_ptr->Record(uncompressed_data, compressed_output, kOpts);
+  predictor_->Record(uncompressed_data, compressed_output, opts_);
   return status;
 }
 
@@ -126,6 +103,6 @@ std::unique_ptr<Compressor> AutoSkipCompressorManager::GetCompressorForSST(
 std::shared_ptr<CompressionManagerWrapper> CreateAutoSkipCompressionManager(
     std::shared_ptr<CompressionManager> wrapped) {
   return std::make_shared<AutoSkipCompressorManager>(
-      wrapped == nullptr ? GetBuiltinV2CompressionManager() : wrapped);
+      wrapped == nullptr ? GetDefaultBuiltinCompressionManager() : wrapped);
 }
 }  // namespace ROCKSDB_NAMESPACE

util/auto_skip_compressor.h

@@ -10,9 +10,11 @@
 #include <memory>
 
 #include "rocksdb/advanced_compression.h"
+#include "util/compression.h"
 
 namespace ROCKSDB_NAMESPACE {
 // Predict rejection probability using a moving window approach
+// This class is not thread safe
 class CompressionRejectionProbabilityPredictor {
  public:
   CompressionRejectionProbabilityPredictor(int window_size)
@@ -32,50 +34,23 @@ class CompressionRejectionProbabilityPredictor {
   size_t window_size_;
 };
 
-class AutoSkipWorkingArea : public Compressor::WorkingArea {
- public:
-  explicit AutoSkipWorkingArea(Compressor::ManagedWorkingArea&& wa)
-      : wrapped(std::move(wa)),
-        predictor(
-            std::make_shared<CompressionRejectionProbabilityPredictor>(10)) {}
-  ~AutoSkipWorkingArea() {}
-  AutoSkipWorkingArea(const AutoSkipWorkingArea&) = delete;
-  AutoSkipWorkingArea& operator=(const AutoSkipWorkingArea&) = delete;
-  AutoSkipWorkingArea(AutoSkipWorkingArea&& other) noexcept
-      : wrapped(std::move(other.wrapped)),
-        predictor(std::move(other.predictor)) {}
-
-  AutoSkipWorkingArea& operator=(AutoSkipWorkingArea&& other) noexcept {
-    if (this != &other) {
-      wrapped = std::move(other.wrapped);
-      predictor = std::move(other.predictor);
-    }
-    return *this;
-  }
-  Compressor::ManagedWorkingArea wrapped;
-  std::shared_ptr<CompressionRejectionProbabilityPredictor> predictor;
-};
-
 class AutoSkipCompressorWrapper : public CompressorWrapper {
  public:
-  const char* Name() const override;
   explicit AutoSkipCompressorWrapper(std::unique_ptr<Compressor> compressor,
                                      const CompressionOptions& opts);
 
   Status CompressBlock(Slice uncompressed_data, std::string* compressed_output,
                        CompressionType* out_compression_type,
                        ManagedWorkingArea* wa) override;
-  ManagedWorkingArea ObtainWorkingArea() override;
-  void ReleaseWorkingArea(WorkingArea* wa) override;
 
  private:
   Status CompressBlockAndRecord(Slice uncompressed_data,
                                 std::string* compressed_output,
                                 CompressionType* out_compression_type,
-                                AutoSkipWorkingArea* wa);
+                                ManagedWorkingArea* wa);
   static constexpr int kExplorationPercentage = 10;
   static constexpr int kProbabilityCutOff = 50;
-  const CompressionOptions kOpts;
+  const CompressionOptions opts_;
   std::shared_ptr<CompressionRejectionProbabilityPredictor> predictor_;
 };
 

util/compression.cc

@@ -7,7 +7,6 @@
 
 #include "options/options_helper.h"
 #include "rocksdb/convenience.h"
-#include "rocksdb/utilities/object_registry.h"
 
 namespace ROCKSDB_NAMESPACE {
 
@@ -133,8 +132,8 @@ Status Decompressor::ExtractUncompressedSize(Args& args) {
   // payload. (RocksDB compress_format_version=2 except Snappy)
   //
   // This is historically a varint32, but it is preliminarily generalized
-  // to varint64, in case that is supported on the write side for some
-  // algorithms.
+  // to varint64. (TODO: support that on the write side, at least for some
+  // codecs, in BBT format_version=7)
   if (LIKELY(GetVarint64(&args.compressed_data, &args.uncompressed_size))) {
     if (LIKELY(args.uncompressed_size <= SIZE_MAX)) {
       return Status::OK();
@@ -156,8 +155,6 @@ namespace {
 
 class BuiltinCompressorV1 : public Compressor {
  public:
-  const char* Name() const override { return "BuiltinCompressorV1"; }
-
   explicit BuiltinCompressorV1(const CompressionOptions& opts,
                                CompressionType type)
       : opts_(opts), type_(type) {
@@ -195,8 +192,6 @@ class BuiltinCompressorV1 : public Compressor {
 
 class BuiltinCompressorV2 : public Compressor {
  public:
-  const char* Name() const override { return "BuiltinCompressorV2"; }
-
   explicit BuiltinCompressorV2(const CompressionOptions& opts,
                                CompressionType type,
                                CompressionDict&& dict = {})
@@ -253,7 +248,8 @@ class BuiltinCompressorV2 : public Compressor {
 
   // TODO: use ZSTD_CCtx directly
   ManagedWorkingArea ObtainWorkingArea() override {
-    return ManagedWorkingArea(new CompressionContext(type_, opts_), this);
+    return ManagedWorkingArea(
+        static_cast<WorkingArea*>(new CompressionContext(type_, opts_)), this);
   }
   void ReleaseWorkingArea(WorkingArea* wa) override {
     delete static_cast<CompressionContext*>(wa);
@@ -352,8 +348,6 @@ class BuiltinCompressionManagerV1 : public CompressionManager {
 
   std::unique_ptr<Compressor> GetCompressor(const CompressionOptions& opts,
                                             CompressionType type) override {
-    // At the time of deprecating the writing of new format_version=1 files,
-    // ZSTD was the last supported built-in compression type.
     if (type > kZSTD) {
       // Unrecognized; fall back on default compression
       type = ColumnFamilyOptions{}.compression;
@@ -369,10 +363,6 @@ class BuiltinCompressionManagerV1 : public CompressionManager {
     return std::shared_ptr<Decompressor>(shared_from_this(), &decompressor_);
   }
 
-  bool SupportsCompressionType(CompressionType type) const override {
-    return CompressionTypeSupported(type);
-  }
-
  protected:
   BuiltinDecompressorV1 decompressor_;
 };
@@ -675,34 +665,6 @@ class BuiltinDecompressorV2 : public Decompressor {
   }
 };
 
-class BuiltinDecompressorV2SnappyOnly : public BuiltinDecompressorV2 {
- public:
-  const char* Name() const override {
-    return "BuiltinDecompressorV2SnappyOnly";
-  }
-
-  Status ExtractUncompressedSize(Args& args) override {
-    assert(args.compression_type == kSnappyCompression);
-#ifdef SNAPPY
-    size_t uncompressed_length = 0;
-    if (!snappy::GetUncompressedLength(args.compressed_data.data(),
-                                       args.compressed_data.size(),
-                                       &uncompressed_length)) {
-      return Status::Corruption("Error reading snappy compressed length");
-    }
-    args.uncompressed_size = uncompressed_length;
-    return Status::OK();
-#else
-    return Status::NotSupported("Snappy not supported in this build");
-#endif
-  }
-
-  Status DecompressBlock(const Args& args, char* uncompressed_output) override {
-    assert(args.compression_type == kSnappyCompression);
-    return Snappy_DecompressBlock(args, uncompressed_output);
-  }
-};
-
 class BuiltinDecompressorV2WithDict : public BuiltinDecompressorV2 {
  public:
   explicit BuiltinDecompressorV2WithDict(const Slice& dict) : dict_(dict) {}
@@ -746,17 +708,6 @@ class BuiltinDecompressorV2WithDict : public BuiltinDecompressorV2 {
 
 Status BuiltinDecompressorV2::MaybeCloneForDict(
     const Slice& dict, std::unique_ptr<Decompressor>* out) {
-  // Because of unfortunate decisions in handling built-in compression types,
-  // all the compression types before ZSTD that do not actually support
-  // dictionary compression pretend to support it. Specifically, we have to be
-  // able to read files with a compression dictionary block using those
-  // compression types even though the compression dictionary is ignored by
-  // the compression algorithm. And the Decompressor has to return the
-  // configured dictionary from GetSerializedDict() even if it is ignored. This
-  // unfortunately means that a new schema version (BuiltinV3?) would be needed
-  // toactually support dictionary compression in the future for these
-  // algorithms (if the libraries add support).
-  // TODO: can we make this a better/cleaner experience?
   *out = std::make_unique<BuiltinDecompressorV2WithDict>(dict);
   return Status::OK();
 }
@@ -873,7 +824,7 @@ class BuiltinCompressionManagerV2 : public CompressionManager {
       // No acceptable compression ratio => no compression
       return nullptr;
     }
-    if (type > kLastBuiltinCompression) {
+    if (type > kZSTD) {
       // Unrecognized; fall back on default compression
       type = ColumnFamilyOptions{}.compression;
     }
@@ -900,40 +851,16 @@ class BuiltinCompressionManagerV2 : public CompressionManager {
   std::shared_ptr<Decompressor> GetDecompressorForTypes(
       const CompressionType* types_begin,
       const CompressionType* types_end) override {
-    if (types_begin == types_end) {
-      return nullptr;
-    } else if (types_begin + 1 == types_end &&
-               *types_begin == kSnappyCompression) {
-      return GetSnappyDecompressor();
-    } else if (std::find(types_begin, types_end, kZSTD)) {
+    if (std::find(types_begin, types_end, kZSTD)) {
       return GetZstdDecompressor();
     } else {
       return GetGeneralDecompressor();
     }
   }
-  std::shared_ptr<Decompressor> GetDecompressorForCompressor(
-      const Compressor& compressor) override {
-#ifdef ROCKSDB_USE_RTTI
-    // To be extra safe, only optimize here if we are certain we are not
-    // looking at a wrapped compressor, so that we are sure it only uses that
-    // one compression type.
-    if (dynamic_cast<const BuiltinCompressorV2*>(&compressor)) {
-      CompressionType type = compressor.GetPreferredCompressionType();
-      return GetDecompressorForTypes(&type, &type + 1);
-    }
-#endif
-    // Fallback
-    return CompressionManager::GetDecompressorForCompressor(compressor);
-  }
-
-  bool SupportsCompressionType(CompressionType type) const override {
-    return CompressionTypeSupported(type);
-  }
 
  protected:
   BuiltinDecompressorV2 decompressor_;
   BuiltinDecompressorV2OptimizeZstd zstd_decompressor_;
-  BuiltinDecompressorV2SnappyOnly snappy_decompressor_;
 
   inline std::shared_ptr<Decompressor> GetGeneralDecompressor() {
     return std::shared_ptr<Decompressor>(shared_from_this(), &decompressor_);
@@ -943,11 +870,6 @@ class BuiltinCompressionManagerV2 : public CompressionManager {
     return std::shared_ptr<Decompressor>(shared_from_this(),
                                          &zstd_decompressor_);
   }
-
-  inline std::shared_ptr<Decompressor> GetSnappyDecompressor() {
-    return std::shared_ptr<Decompressor>(shared_from_this(),
-                                         &snappy_decompressor_);
-  }
 };
 
 const std::shared_ptr<BuiltinCompressionManagerV1>
@@ -960,68 +882,35 @@ const std::shared_ptr<BuiltinCompressionManagerV2>
 }  // namespace
 
 Status CompressionManager::CreateFromString(
-    const ConfigOptions& config_options, const std::string& value,
+    const ConfigOptions& /*config_options*/, const std::string& id,
     std::shared_ptr<CompressionManager>* result) {
-  if (value == kNullptrString || value.empty()) {
+  if (id == kNullptrString || id.empty()) {
     result->reset();
     return Status::OK();
-  }
-
-  static std::once_flag loaded;
-  std::call_once(loaded, [&]() {
-    auto& library = *ObjectLibrary::Default();
-    // TODO: try to enhance ObjectLibrary to support singletons
-    library.AddFactory<CompressionManager>(
-        kBuiltinCompressionManagerV1->CompatibilityName(),
-        [](const std::string& /*uri*/,
-           std::unique_ptr<CompressionManager>* guard,
-           std::string* /*errmsg*/) {
-          *guard = std::make_unique<BuiltinCompressionManagerV1>();
-          return guard->get();
-        });
-    library.AddFactory<CompressionManager>(
-        kBuiltinCompressionManagerV2->CompatibilityName(),
-        [](const std::string& /*uri*/,
-           std::unique_ptr<CompressionManager>* guard,
-           std::string* /*errmsg*/) {
-          *guard = std::make_unique<BuiltinCompressionManagerV2>();
-          return guard->get();
-        });
-  });
-
-  std::string id;
-  std::unordered_map<std::string, std::string> opt_map;
-  Status status = Customizable::GetOptionsMap(config_options, result->get(),
-                                              value, &id, &opt_map);
-  if (!status.ok()) {  // GetOptionsMap failed
-    return status;
-  } else if (id.empty()) {  // We have no Id but have options.  Not good
-    return Status::NotSupported("Cannot reset object ", id);
-  } else {
-    status = config_options.registry->NewSharedObject(id, result);
-  }
-  if (config_options.ignore_unsupported_options && status.IsNotSupported()) {
+  } else if (id.compare(kBuiltinCompressionManagerV1->CompatibilityName()) ==
+                 0 ||
+             id.compare(kBuiltinCompressionManagerV1->Name()) == 0) {
+    *result = kBuiltinCompressionManagerV1;
     return Status::OK();
-  } else if (status.ok()) {
-    status = Customizable::ConfigureNewObject(config_options, result->get(),
-                                              opt_map);
+  } else if (id.compare(kBuiltinCompressionManagerV2->CompatibilityName()) ==
+                 0 ||
+             id.compare(kBuiltinCompressionManagerV2->Name()) == 0) {
+    *result = kBuiltinCompressionManagerV2;
+    return Status::OK();
+  } else {
+    return Status::NotFound("Compatible compression manager for \"" + id +
+                            "\"");
   }
-  return status;
 }
 
-std::shared_ptr<CompressionManager>
-CompressionManager::FindCompatibleCompressionManager(Slice compatibility_name) {
+Status CompressionManager::FindCompatibleCompressionManager(
+    Slice compatibility_name, std::shared_ptr<CompressionManager>* out) {
   if (compatibility_name.compare(CompatibilityName()) == 0) {
-    return shared_from_this();
+    *out = shared_from_this();
+    return Status::OK();
   } else {
-    std::shared_ptr<CompressionManager> out;
-    Status s =
-        CreateFromString(ConfigOptions(), compatibility_name.ToString(), &out);
-    if (s.ok()) {
-      return out;
-    } else {
-      return nullptr;
-    }
+    return CreateFromString(ConfigOptions(), compatibility_name.ToString(),
+                            out);
   }
 }
 
@@ -1043,7 +932,8 @@ const std::shared_ptr<CompressionManager>& GetBuiltinCompressionManager(
   }
 }
 
-const std::shared_ptr<CompressionManager>& GetBuiltinV2CompressionManager() {
+const std::shared_ptr<CompressionManager>&
+GetDefaultBuiltinCompressionManager() {
   return GetBuiltinCompressionManager(2);
 }
 

util/compression.h

@@ -28,7 +28,6 @@
 #include "table/block_based/block_type.h"
 #include "test_util/sync_point.h"
 #include "util/atomic.h"
-#include "util/cast_util.h"
 #include "util/coding.h"
 #include "util/compression_context_cache.h"
 #include "util/string_util.h"
@@ -248,7 +247,6 @@ struct DecompressorDict {
       decompressor_ = std::make_unique<FailureDecompressor>(std::move(s));
     } else {
       assert(s.ok());
-      assert(decompressor_->GetSerializedDict() == dict);
     }
 
     memory_usage_ = sizeof(struct DecompressorDict);
@@ -699,7 +697,8 @@ inline bool CompressionTypeSupported(CompressionType compression_type) {
       return XPRESS_Supported();
     case kZSTD:
       return ZSTD_Supported();
-    default:  // Including custom compression types
+    default:
+      assert(false);
       return false;
   }
 }
@@ -727,7 +726,8 @@ inline bool DictCompressionTypeSupported(CompressionType compression_type) {
       // NB: dictionary supported since 0.5.0. See ZSTD_VERSION_NUMBER check
       // above.
       return ZSTD_Supported();
-    default:  // Including custom compression types
+    default:
+      assert(false);
       return false;
   }
 }
@@ -753,13 +753,9 @@ inline std::string CompressionTypeToString(CompressionType compression_type) {
       return "ZSTD";
     case kDisableCompressionOption:
       return "DisableOption";
-    default: {
-      bool is_custom = compression_type >= kFirstCustomCompression &&
-                       compression_type <= kLastCustomCompression;
-      unsigned char c = lossless_cast<unsigned char>(compression_type);
-      return (is_custom ? "Custom" : "Reserved") +
-             ToBaseCharsString<16>(2, c, /*uppercase=*/true);
-    }
+    default:
+      assert(false);
+      return "";
   }
 }
 

util/compression_test.cc

@@ -32,7 +32,7 @@ class AutoSkipTestFlushBlockPolicy : public FlushBlockPolicy {
         statistics_(statistics) {}
 
   bool Update(const Slice& /*key*/, const Slice& /*value*/) override {
-    auto nth_window = num_keys_ / window_;
+    auto multiple_of_10 = num_keys_ / window_;
     if (data_block_builder_.empty()) {
       // First key in this block
       return false;
@@ -49,10 +49,9 @@ class AutoSkipTestFlushBlockPolicy : public FlushBlockPolicy {
       };
       SyncPoint::GetInstance()->DisableProcessing();
       SyncPoint::GetInstance()->ClearAllCallBacks();
-      // We force exploration to set the predicted rejection ratio for odd
-      // window and then test that the prediction is exploited in the even
-      // window
-      if (nth_window % 2 == 0) {
+      // We force exploration to set the predicted rejection ratio and then test
+      // that the prediction is exploited
+      if (multiple_of_10 % 2 == 0) {
         SyncPoint::GetInstance()->SetCallBack(
             "AutoSkipCompressorWrapper::CompressBlock::exploitOrExplore",
             set_exploration);
@@ -73,43 +72,36 @@ class AutoSkipTestFlushBlockPolicy : public FlushBlockPolicy {
         bypassed_percentage = static_cast<int>(bypassed_count * 100 / total);
         compressed_percentage =
             static_cast<int>(compressed_count * 100 / total);
-        // use nth window to detect test cases and set the expected
-        switch (nth_window) {
-          case 1:
-            // In first window we only explore and thus here we verify that the
-            // correct prediction has been made by the end of the window
-            // Since 6 of 10 blocks are compression unfriendly, the predicted
-            // rejection ratio should be 60%
-            EXPECT_EQ(rejection_percentage, 60);
-            EXPECT_EQ(bypassed_percentage, 0);
-            EXPECT_EQ(compressed_percentage, 40);
-            break;
-          case 2:
-            // With the rejection ratio set to 0.6 all the blocks should be
-            // bypassed in next window
-            EXPECT_EQ(rejection_percentage, 0);
-            EXPECT_EQ(bypassed_percentage, 100);
-            EXPECT_EQ(compressed_percentage, 0);
-            break;
-          case 3:
-            // In third window we only explore and verify that the correct
-            // prediction has been made by the end of the window
-            // since 4 of 10 blocks are compression ufriendly, the predicted
-            // rejection ratio should be 40%
-            EXPECT_EQ(rejection_percentage, 40);
-            EXPECT_EQ(bypassed_percentage, 0);
-            EXPECT_EQ(compressed_percentage, 60);
-            break;
-          case 4:
-            // With the rejection ratio set to 0.4 all the blocks should be
-            // attempted to be compressed
-            // 6 of 10 blocks are compression unfriendly and thus should be
-            // rejected 4 of 10 blocks are compression friendly and thus should
-            // be compressed
-            EXPECT_EQ(rejection_percentage, 60);
-            EXPECT_EQ(bypassed_percentage, 0);
-            EXPECT_EQ(compressed_percentage, 40);
-        }
+      }
+      // use mulitple of 10 to get correct assertion
+      switch (multiple_of_10) {
+        case 1:
+          // This is exploration stage in which we set the rejection ratio to
+          // 0.6
+          EXPECT_EQ(rejection_percentage, 60);
+          EXPECT_EQ(bypassed_percentage, 0);
+          EXPECT_EQ(compressed_percentage, 40);
+          break;
+        case 2:
+          // With the rejection ratio set to 0.6 all the blocks should be
+          // bypassed in next window
+          EXPECT_EQ(rejection_percentage, 0);
+          EXPECT_EQ(bypassed_percentage, 100);
+          EXPECT_EQ(compressed_percentage, 0);
+          break;
+        case 3:
+          // This is exploration stage in which we set the rejection ratio to
+          // 0.4
+          EXPECT_EQ(rejection_percentage, 40);
+          EXPECT_EQ(bypassed_percentage, 0);
+          EXPECT_EQ(compressed_percentage, 60);
+          break;
+        case 4:
+          // With the rejection ratio set to 0.4 all the blocks should be
+          // attempted to be compressed
+          EXPECT_EQ(rejection_percentage, 60);
+          EXPECT_EQ(bypassed_percentage, 0);
+          EXPECT_EQ(compressed_percentage, 40);
       }
     }
     num_keys_++;
@@ -158,7 +150,7 @@ class DBAutoSkip : public DBTestBase {
         rnd_(231),
         key_index_(0) {
     options.compression_manager =
-        CreateAutoSkipCompressionManager(GetBuiltinV2CompressionManager());
+        CreateAutoSkipCompressionManager(GetDefaultBuiltinCompressionManager());
     auto statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
     options.statistics = statistics;
     options.statistics->set_stats_level(StatsLevel::kExceptTimeForMutex);
@@ -190,12 +182,7 @@ class DBAutoSkip : public DBTestBase {
   }
 };
 
-// FIXME: the test is failing the assertion in auto_skip_compressor.cc
-// when run on nightly build in build-linux-arm-test-full mode [1].
-//
-// [1]
-// auto_skip_compressor.cc:101: Assertion `preferred != kNoCompression' failed.
-TEST_F(DBAutoSkip, DISABLED_AutoSkipCompressionManager) {
+TEST_F(DBAutoSkip, AutoSkipCompressionManager) {
   if (GetSupportedCompressions().size() > 1) {
     const int kValueSize = 20000;
     // This will set the rejection ratio to 60%

util/simple_mixed_compressor.cc

@@ -16,17 +16,20 @@ namespace ROCKSDB_NAMESPACE {
 
 // MultiCompressorWrapper implementation
 MultiCompressorWrapper::MultiCompressorWrapper(const CompressionOptions& opts,
+                                               CompressionType type,
                                                CompressionDict&& dict) {
-  // TODO: make the compression manager a field
-  auto builtInManager = GetBuiltinV2CompressionManager();
+  assert(type != kNoCompression);
+  assert(type == kZSTD);
+  auto builtInManager = GetDefaultBuiltinCompressionManager();
   const auto& compressions = GetSupportedCompressions();
-  for (auto type : compressions) {
-    if (type == kNoCompression) {
+  for (auto type_ : compressions) {
+    if (type_ == kNoCompression) {
       continue;
     }
-    compressors_.push_back(builtInManager->GetCompressor(opts, type));
+    compressors_.push_back(builtInManager->GetCompressor(opts, type_));
   }
   (void)dict;
+  (void)type;
 }
 
 size_t MultiCompressorWrapper::GetMaxSampleSizeIfWantDict(
@@ -39,7 +42,7 @@ Slice MultiCompressorWrapper::GetSerializedDict() const {
 }
 
 CompressionType MultiCompressorWrapper::GetPreferredCompressionType() const {
-  return compressors_.back()->GetPreferredCompressionType();
+  return kZSTD;
 }
 
 Compressor::ManagedWorkingArea MultiCompressorWrapper::ObtainWorkingArea() {
@@ -48,17 +51,10 @@ Compressor::ManagedWorkingArea MultiCompressorWrapper::ObtainWorkingArea() {
 
 std::unique_ptr<Compressor> MultiCompressorWrapper::MaybeCloneSpecialized(
     CacheEntryRole block_type, DictSampleArgs&& dict_samples) {
-  // TODO: full dictionary compression support. Currently this just falls
-  // back on a non-multi compressor when asked to use a dictionary.
   return compressors_.back()->MaybeCloneSpecialized(block_type,
                                                     std::move(dict_samples));
 }
 
-// RandomMixedCompressor implementation
-const char* RandomMixedCompressor::Name() const {
-  return "RandomMixedCompressor";
-}
-
 Status RandomMixedCompressor::CompressBlock(
     Slice uncompressed_data, std::string* compressed_output,
     CompressionType* out_compression_type, ManagedWorkingArea* wa) {
@@ -70,20 +66,19 @@ Status RandomMixedCompressor::CompressBlock(
 }
 
 const char* RandomMixedCompressionManager::Name() const {
-  return "RandomMixedCompressionManager";
+  return wrapped_->Name();
+  // return "RandomMixedCompressionManager";
 }
 
 std::unique_ptr<Compressor> RandomMixedCompressionManager::GetCompressorForSST(
-    const FilterBuildingContext& /*context*/, const CompressionOptions& opts,
-    CompressionType /*preferred*/) {
-  return std::make_unique<RandomMixedCompressor>(opts);
+    const FilterBuildingContext& context, const CompressionOptions& opts,
+    CompressionType preferred) {
+  assert(preferred == kZSTD);
+  (void)context;
+  return std::make_unique<RandomMixedCompressor>(opts, preferred);
 }
 
 // RoundRobinCompressor implementation
-const char* RoundRobinCompressor::Name() const {
-  return "RoundRobinCompressor";
-}
-
 Status RoundRobinCompressor::CompressBlock(
     Slice uncompressed_data, std::string* compressed_output,
     CompressionType* out_compression_type, ManagedWorkingArea* wa) {
@@ -97,12 +92,17 @@ Status RoundRobinCompressor::CompressBlock(
 RelaxedAtomic<uint64_t> RoundRobinCompressor::block_counter{0};
 
 // RoundRobinManager implementation
-const char* RoundRobinManager::Name() const { return "RoundRobinManager"; }
+const char* RoundRobinManager::Name() const {
+  // return "RoundRobinManager";
+  return wrapped_->Name();
+}
 
 std::unique_ptr<Compressor> RoundRobinManager::GetCompressorForSST(
-    const FilterBuildingContext& /*context*/, const CompressionOptions& opts,
-    CompressionType /*preferred*/) {
-  return std::make_unique<RoundRobinCompressor>(opts);
+    const FilterBuildingContext& context, const CompressionOptions& opts,
+    CompressionType preferred) {
+  assert(preferred == kZSTD);
+  (void)context;
+  return std::make_unique<RoundRobinCompressor>(opts, preferred);
 }
 
 }  // namespace ROCKSDB_NAMESPACE

util/simple_mixed_compressor.h

@@ -8,6 +8,7 @@
 
 #pragma once
 #include <memory>
+#include <mutex>
 #include <vector>
 
 #include "compression.h"
@@ -18,6 +19,7 @@ namespace ROCKSDB_NAMESPACE {
 class MultiCompressorWrapper : public Compressor {
  public:
   explicit MultiCompressorWrapper(const CompressionOptions& opts,
+                                  CompressionType type,
                                   CompressionDict&& dict = {});
 
   size_t GetMaxSampleSizeIfWantDict(CacheEntryRole block_type) const override;
@@ -33,7 +35,6 @@ class MultiCompressorWrapper : public Compressor {
 
 struct RandomMixedCompressor : public MultiCompressorWrapper {
   using MultiCompressorWrapper::MultiCompressorWrapper;
-  const char* Name() const override;
   Status CompressBlock(Slice uncompressed_data, std::string* compressed_output,
                        CompressionType* out_compression_type,
                        ManagedWorkingArea* wa) override;
@@ -49,7 +50,6 @@ class RandomMixedCompressionManager : public CompressionManagerWrapper {
 
 struct RoundRobinCompressor : public MultiCompressorWrapper {
   using MultiCompressorWrapper::MultiCompressorWrapper;
-  const char* Name() const override;
   Status CompressBlock(Slice uncompressed_data, std::string* compressed_output,
                        CompressionType* out_compression_type,
                        ManagedWorkingArea* wa) override;

util/slice_test.cc

@@ -14,7 +14,6 @@
 #include "test_util/testharness.h"
 #include "test_util/testutil.h"
 #include "util/cast_util.h"
-#include "util/string_util.h"
 
 namespace ROCKSDB_NAMESPACE {
 
@@ -411,17 +410,6 @@ TEST(UnownedPtrTest, Tests) {
   }
 }
 
-TEST(ToBaseCharsStringTest, Tests) {
-  using ROCKSDB_NAMESPACE::ToBaseCharsString;
-  // Base 16
-  ASSERT_EQ(ToBaseCharsString<16>(5, 0, true), "00000");
-  ASSERT_EQ(ToBaseCharsString<16>(5, 42, true), "0002A");
-  ASSERT_EQ(ToBaseCharsString<16>(5, 42, false), "0002a");
-  ASSERT_EQ(ToBaseCharsString<16>(2, 255, false), "ff");
-  // Base 32
-  ASSERT_EQ(ToBaseCharsString<32>(2, 255, false), "7v");
-}
-
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

util/string_util.h

@@ -40,16 +40,6 @@ inline void PutBaseChars(char** buf, size_t n, uint64_t v, bool uppercase) {
   *buf += n;
 }
 
-// Construct a string of n digits from v in base kBase
-template <size_t kBase>
-inline std::string ToBaseCharsString(size_t n, uint64_t v, bool uppercase) {
-  std::string result;
-  result.resize(n);
-  char* buf = &result[0];
-  PutBaseChars<kBase>(&buf, n, v, uppercase);
-  return result;
-}
-
 // Parse n digits from *buf in base kBase to *v and advance *buf to the
 // position after what was read. On success, true is returned. On failure,
 // false is returned, *buf is placed at the first bad character, and *v

utilities/backup/backup_engine.cc

@@ -2372,11 +2372,7 @@ IOStatus BackupEngineImpl::CopyOrCreateFile(
 
   io_s = dst_env->GetFileSystem()->NewWritableFile(dst, dst_file_options,
                                                    &dst_file, nullptr);
-  if (!io_s.ok()) {
-    return io_s;
-  }
-
-  if (!src.empty()) {
+  if (io_s.ok() && !src.empty()) {
     auto src_file_options = FileOptions(src_env_options);
     src_file_options.temperature = *src_temperature;
     io_s = src_env->GetFileSystem()->NewSequentialFile(src, src_file_options,

utilities/table_properties_collectors/compact_on_deletion_collector.cc

@@ -17,19 +17,16 @@
 namespace ROCKSDB_NAMESPACE {
 
 CompactOnDeletionCollector::CompactOnDeletionCollector(
-    size_t sliding_window_size, size_t deletion_trigger, double deletion_ratio,
-    uint64_t min_file_size)
+    size_t sliding_window_size, size_t deletion_trigger, double deletion_ratio)
     : bucket_size_((sliding_window_size + kNumBuckets - 1) / kNumBuckets),
       current_bucket_(0),
       num_keys_in_current_bucket_(0),
       num_deletions_in_observation_window_(0),
       deletion_trigger_(deletion_trigger),
       deletion_ratio_(deletion_ratio),
-      min_file_size_(min_file_size),
-      cur_file_size_(0),
-      max_deletion_in_window_(0),
       deletion_ratio_enabled_(deletion_ratio > 0 && deletion_ratio <= 1),
-      need_compaction_(false) {
+      need_compaction_(false),
+      finished_(false) {
   memset(num_deletions_in_buckets_, 0, sizeof(size_t) * kNumBuckets);
 }
 
@@ -42,7 +39,7 @@ Status CompactOnDeletionCollector::AddUserKey(const Slice& /*key*/,
                                               const Slice& /*value*/,
                                               EntryType type,
                                               SequenceNumber /*seq*/,
-                                              uint64_t file_size) {
+                                              uint64_t /*file_size*/) {
   assert(!finished_);
   if (!bucket_size_ && !deletion_ratio_enabled_) {
     // This collector is effectively disabled
@@ -54,14 +51,11 @@ Status CompactOnDeletionCollector::AddUserKey(const Slice& /*key*/,
     return Status::OK();
   }
 
-  const bool is_delete = (type == kEntryDelete || type == kEntrySingleDelete ||
-                          type == kEntryDeleteWithTimestamp);
   if (deletion_ratio_enabled_) {
     total_entries_++;
-    if (is_delete) {
+    if (type == kEntryDelete) {
       deletion_entries_++;
     }
-    cur_file_size_ = file_size;
   }
 
   if (bucket_size_) {
@@ -82,20 +76,13 @@ Status CompactOnDeletionCollector::AddUserKey(const Slice& /*key*/,
     }
 
     num_keys_in_current_bucket_++;
-    if (is_delete) {
+    if (type == kEntryDelete) {
       num_deletions_in_observation_window_++;
       num_deletions_in_buckets_[current_bucket_]++;
-      if (num_deletions_in_observation_window_ >= max_deletion_in_window_) {
-        max_deletion_in_window_ = num_deletions_in_observation_window_;
+      if (num_deletions_in_observation_window_ >= deletion_trigger_) {
+        need_compaction_ = true;
       }
     }
-
-    // The file may qualify for compaction based on file size constraints,
-    // even if max_deletion_in_window_ is not updated.
-    if (max_deletion_in_window_ >= deletion_trigger_ &&
-        file_size >= min_file_size_) {
-      need_compaction_ = true;
-    }
   }
 
   return Status::OK();
@@ -103,8 +90,7 @@ Status CompactOnDeletionCollector::AddUserKey(const Slice& /*key*/,
 
 Status CompactOnDeletionCollector::Finish(
     UserCollectedProperties* /*properties*/) {
-  if (!need_compaction_ && deletion_ratio_enabled_ && total_entries_ > 0 &&
-      cur_file_size_ >= min_file_size_) {
+  if (!need_compaction_ && deletion_ratio_enabled_ && total_entries_ > 0) {
     double ratio = static_cast<double>(deletion_entries_) / total_entries_;
     need_compaction_ = ratio >= deletion_ratio_;
   }
@@ -167,43 +153,23 @@ static std::unordered_map<std::string, OptionTypeInfo>
             return Status::OK();
           },
           nullptr}},
-        {"min_file_size",
-         {0, OptionType::kUnknown, OptionVerificationType::kNormal,
-          OptionTypeFlags::kCompareNever | OptionTypeFlags::kMutable,
-          [](const ConfigOptions&, const std::string&, const std::string& value,
-             void* addr) {
-            auto* factory =
-                static_cast<CompactOnDeletionCollectorFactory*>(addr);
-            factory->SetMinFileSize(ParseUint64(value));
-            return Status::OK();
-          },
-          [](const ConfigOptions&, const std::string&, const void* addr,
-             std::string* value) {
-            const auto* factory =
-                static_cast<const CompactOnDeletionCollectorFactory*>(addr);
-            *value = std::to_string(factory->GetMinFileSize());
-            return Status::OK();
-          },
-          nullptr}},
 
 };
 
 CompactOnDeletionCollectorFactory::CompactOnDeletionCollectorFactory(
-    size_t sliding_window_size, size_t deletion_trigger, double deletion_ratio,
-    uint64_t min_file_size)
+    size_t sliding_window_size, size_t deletion_trigger, double deletion_ratio)
     : sliding_window_size_(sliding_window_size),
       deletion_trigger_(deletion_trigger),
-      deletion_ratio_(deletion_ratio),
-      min_file_size_(min_file_size) {
+      deletion_ratio_(deletion_ratio) {
   RegisterOptions("", this, &on_deletion_collector_type_info);
 }
 
 TablePropertiesCollector*
 CompactOnDeletionCollectorFactory::CreateTablePropertiesCollector(
     TablePropertiesCollectorFactory::Context /*context*/) {
-  return new CompactOnDeletionCollector(
-      sliding_window_size_.load(), deletion_trigger_.load(),
-      deletion_ratio_.load(), min_file_size_.load());
+  return new CompactOnDeletionCollector(sliding_window_size_.load(),
+                                        deletion_trigger_.load(),
+                                        deletion_ratio_.load());
 }
 
 std::string CompactOnDeletionCollectorFactory::ToString() const {
@@ -217,12 +183,10 @@ std::string CompactOnDeletionCollectorFactory::ToString() const {
 std::shared_ptr<CompactOnDeletionCollectorFactory>
 NewCompactOnDeletionCollectorFactory(size_t sliding_window_size,
                                      size_t deletion_trigger,
-                                     double deletion_ratio,
-                                     uint64_t min_file_size) {
+                                     double deletion_ratio) {
   return std::shared_ptr<CompactOnDeletionCollectorFactory>(
       new CompactOnDeletionCollectorFactory(sliding_window_size,
-                                            deletion_trigger, deletion_ratio,
-                                            min_file_size));
+                                            deletion_trigger, deletion_ratio));
 }
 
 namespace {

utilities/table_properties_collectors/compact_on_deletion_collector.h

@@ -11,8 +11,7 @@ namespace ROCKSDB_NAMESPACE {
 class CompactOnDeletionCollector : public TablePropertiesCollector {
  public:
   CompactOnDeletionCollector(size_t sliding_window_size,
-                             size_t deletion_trigger, double deletion_ratio,
-                             uint64_t min_file_size);
+                             size_t deletion_trigger, double deletion_raatio);
 
   // AddUserKey() will be called when a new key/value pair is inserted into the
   // table.
@@ -37,7 +36,7 @@ class CompactOnDeletionCollector : public TablePropertiesCollector {
   // The name of the properties collector can be used for debugging purpose.
   const char* Name() const override { return "CompactOnDeletionCollector"; }
 
-  // Return whether the output file should be further compacted
+  // EXPERIMENTAL Return whether the output file should be further compacted
   bool NeedCompact() const override { return need_compaction_; }
 
   static const int kNumBuckets = 128;
@@ -49,21 +48,18 @@ class CompactOnDeletionCollector : public TablePropertiesCollector {
   // "bucket_size_" keys.
   size_t num_deletions_in_buckets_[kNumBuckets];
   // the number of keys in a bucket
-  const size_t bucket_size_;
+  size_t bucket_size_;
 
   size_t current_bucket_;
   size_t num_keys_in_current_bucket_;
   size_t num_deletions_in_observation_window_;
-  const size_t deletion_trigger_;
+  size_t deletion_trigger_;
   const double deletion_ratio_;
+  const bool deletion_ratio_enabled_;
   size_t total_entries_ = 0;
   size_t deletion_entries_ = 0;
-  const size_t min_file_size_;
-  size_t cur_file_size_;
-  size_t max_deletion_in_window_;
-  const bool deletion_ratio_enabled_;
   // true if the current SST file needs to be compacted.
   bool need_compaction_;
-  bool finished_ = false;
+  bool finished_;
 };
 }  // namespace ROCKSDB_NAMESPACE

utilities/table_properties_collectors/compact_on_deletion_collector_test.cc

@@ -232,61 +232,6 @@ TEST(CompactOnDeletionCollector, SlidingWindow) {
   }
 }
 
-TEST(CompactOnDeletionCollector, MinFileSize) {
-  TablePropertiesCollectorFactory::Context context;
-  context.column_family_id =
-      TablePropertiesCollectorFactory::Context::kUnknownColumnFamily;
-  context.last_level_inclusive_max_seqno_threshold = kMaxSequenceNumber;
-
-  const size_t kWindowSize = 1000;
-  const size_t kDeletionTrigger = 800;
-  const double kDeletionRatio = 0.9;
-  const uint64_t kMinFileSize = 1 << 20;
-
-  for (uint64_t file_size : {(uint64_t)0, kMinFileSize - 1, kMinFileSize}) {
-    {
-      auto factory = NewCompactOnDeletionCollectorFactory(
-          kWindowSize, kDeletionTrigger, 0, kMinFileSize);
-      std::unique_ptr<TablePropertiesCollector> collector(
-          factory->CreateTablePropertiesCollector(context));
-
-      // Add enough deletions to meet the sliding window triggers
-      for (size_t i = 0; i < kWindowSize; i++) {
-        if (i < kDeletionTrigger) {
-          ASSERT_OK(collector->AddUserKey("key", "value", kEntryDelete, 0,
-                                          file_size));
-        } else {
-          ASSERT_OK(
-              collector->AddUserKey("key", "value", kEntryPut, 0, file_size));
-        }
-      }
-      ASSERT_OK(collector->Finish(nullptr));
-      ASSERT_EQ(collector->NeedCompact(), file_size >= kMinFileSize);
-    }
-
-    {
-      auto factory = NewCompactOnDeletionCollectorFactory(
-          kWindowSize, kDeletionTrigger, kDeletionRatio, kMinFileSize);
-
-      std::unique_ptr<TablePropertiesCollector> collector(
-          factory->CreateTablePropertiesCollector(context));
-
-      const size_t kTotalEntries = 100;
-      // Add all deletions to maximize tombstone ratio
-      for (size_t i = 0; i < kTotalEntries - 1; i++) {
-        ASSERT_OK(
-            collector->AddUserKey("key", "value", kEntrySingleDelete, 0, 0));
-      }
-      // Give update file size
-      ASSERT_OK(collector->AddUserKey("key", "value", kEntrySingleDelete, 0,
-                                      file_size));
-
-      ASSERT_OK(collector->Finish(nullptr));
-      ASSERT_EQ(collector->NeedCompact(), file_size >= kMinFileSize);
-    }
-  }
-}
-
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {