diff --git a/cache/secondary_cache_adapter.cc b/cache/secondary_cache_adapter.cc index 9c1b960102..57a77bc7fc 100644 --- a/cache/secondary_cache_adapter.cc +++ b/cache/secondary_cache_adapter.cc @@ -271,7 +271,8 @@ Status CacheWithSecondaryAdapter::Insert(const Slice& key, ObjectPtr value, // Warm up the secondary cache with the compressed block. The secondary // cache may choose to ignore it based on the admission policy. if (value != nullptr && !compressed_value.empty() && - adm_policy_ == TieredAdmissionPolicy::kAdmPolicyThreeQueue) { + adm_policy_ == TieredAdmissionPolicy::kAdmPolicyThreeQueue && + helper->IsSecondaryCacheCompatible()) { Status status = secondary_cache_->InsertSaved(key, compressed_value, type); assert(status.ok() || status.IsNotSupported()); } diff --git a/cache/tiered_secondary_cache_test.cc b/cache/tiered_secondary_cache_test.cc index e4b4202265..f38a358ad6 100644 --- a/cache/tiered_secondary_cache_test.cc +++ b/cache/tiered_secondary_cache_test.cc @@ -765,6 +765,54 @@ TEST_F(DBTieredSecondaryCacheTest, IterateTest) { Destroy(options); } +TEST_F(DBTieredSecondaryCacheTest, VolatileTierTest) { + if (!LZ4_Supported()) { + ROCKSDB_GTEST_SKIP("This test requires LZ4 support."); + return; + } + + BlockBasedTableOptions table_options; + // We want a block cache of size 5KB, and a compressed secondary cache of + // size 5KB. However, we specify a block cache size of 256KB here in order + // to take into account the cache reservation in the block cache on + // behalf of the compressed cache. The unit of cache reservation is 256KB. + // The effective block cache capacity will be calculated as 256 + 5 = 261KB, + // and 256KB will be reserved for the compressed cache, leaving 5KB for + // the primary block cache. We only have to worry about this here because + // the cache size is so small. + table_options.block_cache = NewCache(256 * 1024, 5 * 1024, 256 * 1024); + table_options.block_size = 4 * 1024; + table_options.cache_index_and_filter_blocks = false; + Options options = GetDefaultOptions(); + options.create_if_missing = true; + options.compression = kLZ4Compression; + options.table_factory.reset(NewBlockBasedTableFactory(table_options)); + + // Disable paranoid_file_checks so that flush will not read back the newly + // written file + options.paranoid_file_checks = false; + options.lowest_used_cache_tier = CacheTier::kVolatileTier; + DestroyAndReopen(options); + Random rnd(301); + const int N = 256; + for (int i = 0; i < N; i++) { + std::string p_v; + test::CompressibleString(&rnd, 0.5, 1007, &p_v); + ASSERT_OK(Put(Key(i), p_v)); + } + + ASSERT_OK(Flush()); + + // Since lowest_used_cache_tier is the volatile tier, nothing should be + // inserted in the secondary cache. + std::string v = Get(Key(0)); + ASSERT_EQ(1007, v.size()); + ASSERT_EQ(nvm_sec_cache()->num_insert_saved(), 0u); + ASSERT_EQ(nvm_sec_cache()->num_misses(), 0u); + + Destroy(options); +} + class DBTieredAdmPolicyTest : public DBTieredSecondaryCacheTest, public testing::WithParamInterface {}; diff --git a/unreleased_history/bug_fixes/skip_insertion_tiered_sec_cache.md b/unreleased_history/bug_fixes/skip_insertion_tiered_sec_cache.md new file mode 100644 index 0000000000..7dcbe099fd --- /dev/null +++ b/unreleased_history/bug_fixes/skip_insertion_tiered_sec_cache.md @@ -0,0 +1 @@ +Skip insertion of compressed blocks in the secondary cache if the lowest_used_cache_tier DB option is kVolatileTier.