From a4405fd9812e741a20ecd631e517059cb394b756 Mon Sep 17 00:00:00 2001 From: Zhichao Cao Date: Mon, 24 May 2021 08:35:20 -0700 Subject: [PATCH] fix lru caching test and fix reference binding to null pointer (#8326) Summary: Fix for https://github.com/facebook/rocksdb/issues/8315. Inhe lru caching test, 5100 is not enough to hold meta block and first block in some random case, increase to 6100. Fix the reference binding to null pointer, use template. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8326 Test Plan: make check Reviewed By: pdillinger Differential Revision: D28625666 Pulled By: zhichao-cao fbshipit-source-id: 97b85306ae3d09bfb74addc7c65e57fe55a976a5 --- cache/lru_cache_test.cc | 4 ++-- table/block_based/block_based_table_reader.cc | 9 ++++----- table/block_based/block_like_traits.h | 3 +-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/cache/lru_cache_test.cc b/cache/lru_cache_test.cc index 31e730f07..17b72f28d 100644 --- a/cache/lru_cache_test.cc +++ b/cache/lru_cache_test.cc @@ -659,14 +659,14 @@ TEST_F(DBSecondaryCacheTest, TestSecondaryCacheCorrectness1) { Destroy(options); } -// In this test, the block cache size is set to 5100, after insert 6 KV-pairs +// In this test, the block cache size is set to 6100, after insert 6 KV-pairs // and flush, there are 5 blocks in this SST file, 2 data blocks and 3 meta // blocks. block_1 size is 4096 and block_2 size is 2056. The total size // of the meta blocks are about 900 to 1000. Therefore, we can successfully // insert and cache block_1 in the block cache (this is the different place // from TestSecondaryCacheCorrectness1) TEST_F(DBSecondaryCacheTest, TestSecondaryCacheCorrectness2) { - LRUCacheOptions opts(5100, 0, false, 0.5, nullptr, kDefaultToAdaptiveMutex, + LRUCacheOptions opts(6100, 0, false, 0.5, nullptr, kDefaultToAdaptiveMutex, kDontChargeCacheMetadata); std::shared_ptr secondary_cache( new TestSecondaryCache(2048 * 1024)); diff --git a/table/block_based/block_based_table_reader.cc b/table/block_based/block_based_table_reader.cc index c92b9d3de..ddcd8154b 100644 --- a/table/block_based/block_based_table_reader.cc +++ b/table/block_based/block_based_table_reader.cc @@ -1124,9 +1124,8 @@ Status BlockBasedTable::GetDataBlockFromCache( Statistics* statistics = rep_->ioptions.statistics.get(); bool using_zstd = rep_->blocks_definitely_zstd_compressed; const FilterPolicy* filter_policy = rep_->filter_policy; - Cache::CreateCallback create_cb = - GetCreateCallback(read_amp_bytes_per_bit, statistics, using_zstd, - filter_policy, *block->GetValue()); + Cache::CreateCallback create_cb = GetCreateCallback( + read_amp_bytes_per_bit, statistics, using_zstd, filter_policy); // Lookup uncompressed cache first if (block_cache != nullptr) { @@ -1151,8 +1150,8 @@ Status BlockBasedTable::GetDataBlockFromCache( assert(!compressed_block_cache_key.empty()); BlockContents contents; - Cache::CreateCallback create_cb_special = GetCreateCallback( - read_amp_bytes_per_bit, statistics, using_zstd, filter_policy, contents); + Cache::CreateCallback create_cb_special = GetCreateCallback( + read_amp_bytes_per_bit, statistics, using_zstd, filter_policy); block_cache_compressed_handle = block_cache_compressed->Lookup( compressed_block_cache_key, BlocklikeTraits::GetCacheItemHelper(block_type), diff --git a/table/block_based/block_like_traits.h b/table/block_based/block_like_traits.h index 55562bdad..ccfa8bc56 100644 --- a/table/block_based/block_like_traits.h +++ b/table/block_based/block_like_traits.h @@ -23,8 +23,7 @@ Cache::CacheItemHelper* GetCacheItemHelperForRole(); template Cache::CreateCallback GetCreateCallback(size_t read_amp_bytes_per_bit, Statistics* statistics, bool using_zstd, - const FilterPolicy* filter_policy, - const TBlocklike& /*block*/) { + const FilterPolicy* filter_policy) { return [read_amp_bytes_per_bit, statistics, using_zstd, filter_policy]( void* buf, size_t size, void** out_obj, size_t* charge) -> Status { assert(buf != nullptr);