Remove compressed block cache (#11117)

Summary: Compressed block cache is replaced by compressed secondary cache. Remove the feature. Pull Request resolved: facebook/rocksdb#11117 Test Plan: See CI passes Reviewed By: pdillinger Differential Revision: D42700164 fbshipit-source-id: 6cbb24e460da29311150865f60ecb98637f9f67d
awitten1 · Jan 25, 2023 · 2800aa0 · 2800aa0
1 parent 4a91853
commit 2800aa0
Show file tree

Hide file tree

Showing 30 changed files with 72 additions and 1,099 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -3,6 +3,9 @@
 ### Bug Fixes
 * Fixed a data race on `ColumnFamilyData::flush_reason` caused by concurrent flushes.
 
+### Feature Removal
+* The feature block_cache_compressed is removed. Statistics related to it are removed too.
+
 ## 7.10.0 (01/23/2023)
 ### Behavior changes
 * Make best-efforts recovery verify SST unique ID before Version construction (#10962)

diff --git a/cache/lru_cache_test.cc b/cache/lru_cache_test.cc
@@ -1784,43 +1784,6 @@ TEST_F(DBSecondaryCacheTest, SecondaryCacheFailureTest) {
   Destroy(options);
 }
 
-TEST_F(DBSecondaryCacheTest, TestSecondaryWithCompressedCache) {
-  if (!Snappy_Supported()) {
-    ROCKSDB_GTEST_SKIP("Compressed cache test requires snappy support");
-    return;
-  }
-  LRUCacheOptions opts(2000 /* capacity */, 0 /* num_shard_bits */,
-                       false /* strict_capacity_limit */,
-                       0.5 /* high_pri_pool_ratio */,
-                       nullptr /* memory_allocator */, kDefaultToAdaptiveMutex,
-                       kDontChargeCacheMetadata);
-  std::shared_ptr<TestSecondaryCache> secondary_cache(
-      new TestSecondaryCache(2048 * 1024));
-  opts.secondary_cache = secondary_cache;
-  std::shared_ptr<Cache> cache = NewLRUCache(opts);
-  BlockBasedTableOptions table_options;
-  table_options.block_cache_compressed = cache;
-  table_options.no_block_cache = true;
-  table_options.block_size = 1234;
-  Options options = GetDefaultOptions();
-  options.compression = kSnappyCompression;
-  options.create_if_missing = true;
-  options.table_factory.reset(NewBlockBasedTableFactory(table_options));
-  DestroyAndReopen(options);
-  Random rnd(301);
-  const int N = 6;
-  for (int i = 0; i < N; i++) {
-    // Partly compressible
-    std::string p_v = rnd.RandomString(507) + std::string(500, ' ');
-    ASSERT_OK(Put(Key(i), p_v));
-  }
-  ASSERT_OK(Flush());
-  for (int i = 0; i < 2 * N; i++) {
-    std::string v = Get(Key(i % N));
-    ASSERT_EQ(1007, v.size());
-  }
-}
-
 TEST_F(LRUCacheSecondaryCacheTest, BasicWaitAllTest) {
   LRUCacheOptions opts(1024 /* capacity */, 2 /* num_shard_bits */,
                        false /* strict_capacity_limit */,

diff --git a/db/c.cc b/db/c.cc
@@ -2620,14 +2620,6 @@ void rocksdb_block_based_options_set_block_cache(
   }
 }
 
-void rocksdb_block_based_options_set_block_cache_compressed(
-    rocksdb_block_based_table_options_t* options,
-    rocksdb_cache_t* block_cache_compressed) {
-  if (block_cache_compressed) {
-    options->rep.block_cache_compressed = block_cache_compressed->rep;
-  }
-}
-
 void rocksdb_block_based_options_set_whole_key_filtering(
     rocksdb_block_based_table_options_t* options, unsigned char v) {
   options->rep.whole_key_filtering = v;

diff --git a/db/db_basic_test.cc b/db/db_basic_test.cc
@@ -3364,17 +3364,13 @@ INSTANTIATE_TEST_CASE_P(DBBasicTestTrackWal, DBBasicTestTrackWal,
 
 class DBBasicTestMultiGet : public DBTestBase {
  public:
-  DBBasicTestMultiGet(std::string test_dir, int num_cfs, bool compressed_cache,
+  DBBasicTestMultiGet(std::string test_dir, int num_cfs,
                       bool uncompressed_cache, bool _compression_enabled,
                       bool _fill_cache, uint32_t compression_parallel_threads)
       : DBTestBase(test_dir, /*env_do_fsync=*/false) {
     compression_enabled_ = _compression_enabled;
     fill_cache_ = _fill_cache;
 
-    if (compressed_cache) {
-      std::shared_ptr<Cache> cache = NewLRUCache(1048576);
-      compressed_cache_ = std::make_shared<MyBlockCache>(cache);
-    }
     if (uncompressed_cache) {
       std::shared_ptr<Cache> cache = NewLRUCache(1048576);
       uncompressed_cache_ = std::make_shared<MyBlockCache>(cache);
@@ -3418,7 +3414,6 @@ class DBBasicTestMultiGet : public DBTestBase {
     } else {
       table_options.pin_l0_filter_and_index_blocks_in_cache = true;
     }
-    table_options.block_cache_compressed = compressed_cache_;
     table_options.flush_block_policy_factory.reset(
         new MyFlushBlockPolicyFactory());
     options.table_factory.reset(NewBlockBasedTableFactory(table_options));
@@ -3601,16 +3596,14 @@ class DBBasicTestMultiGet : public DBTestBase {
   std::vector<std::string> cf_names_;
 };
 
-class DBBasicTestWithParallelIO
-    : public DBBasicTestMultiGet,
-      public testing::WithParamInterface<
-          std::tuple<bool, bool, bool, bool, uint32_t>> {
+class DBBasicTestWithParallelIO : public DBBasicTestMultiGet,
+                                  public testing::WithParamInterface<
+                                      std::tuple<bool, bool, bool, uint32_t>> {
  public:
   DBBasicTestWithParallelIO()
       : DBBasicTestMultiGet("/db_basic_test_with_parallel_io", 1,
                             std::get<0>(GetParam()), std::get<1>(GetParam()),
-                            std::get<2>(GetParam()), std::get<3>(GetParam()),
-                            std::get<4>(GetParam())) {}
+                            std::get<2>(GetParam()), std::get<3>(GetParam())) {}
 };
 
 TEST_P(DBBasicTestWithParallelIO, MultiGet) {
@@ -3941,13 +3934,12 @@ TEST_P(DBBasicTestWithParallelIO, MultiGetWithMissingFile) {
 
 INSTANTIATE_TEST_CASE_P(ParallelIO, DBBasicTestWithParallelIO,
                         // Params are as follows -
-                        // Param 0 - Compressed cache enabled
-                        // Param 1 - Uncompressed cache enabled
-                        // Param 2 - Data compression enabled
-                        // Param 3 - ReadOptions::fill_cache
-                        // Param 4 - CompressionOptions::parallel_threads
+                        // Param 0 - Uncompressed cache enabled
+                        // Param 1 - Data compression enabled
+                        // Param 2 - ReadOptions::fill_cache
+                        // Param 3 - CompressionOptions::parallel_threads
                         ::testing::Combine(::testing::Bool(), ::testing::Bool(),
-                                           ::testing::Bool(), ::testing::Bool(),
+                                           ::testing::Bool(),
                                            ::testing::Values(1, 4)));
 
 // Forward declaration
@@ -4158,9 +4150,8 @@ class DBBasicTestMultiGetDeadline : public DBBasicTestMultiGet,
   DBBasicTestMultiGetDeadline()
       : DBBasicTestMultiGet(
             "db_basic_test_multiget_deadline" /*Test dir*/,
-            10 /*# of column families*/, false /*compressed cache enabled*/,
-            true /*uncompressed cache enabled*/, true /*compression enabled*/,
-            true /*ReadOptions.fill_cache*/,
+            10 /*# of column families*/, true /*uncompressed cache enabled*/,
+            true /*compression enabled*/, true /*ReadOptions.fill_cache*/,
             1 /*# of parallel compression threads*/) {}
 
   inline void CheckStatus(std::vector<Status>& statuses, size_t num_ok) {