From abf9ebc4bfb44cb7f87158fc38ce32e2c326f850 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Wed, 12 Jun 2024 18:47:11 -0700 Subject: [PATCH] Remove redundant no_io parameters to filter functions (#12762) Summary: Consolidate on already-present ReadOptions::read_tier Pull Request resolved: https://github.com/facebook/rocksdb/pull/12762 Test Plan: existing tests Reviewed By: hx235 Differential Revision: D58450516 Pulled By: pdillinger fbshipit-source-id: 1eec58c60beca73c6d5f2e9ae4442644920f8c30 --- table/block_based/block_based_table_reader.cc | 28 +++---- table/block_based/block_based_table_reader.h | 2 - .../block_based_table_reader_sync_and_async.h | 5 +- table/block_based/filter_block.h | 23 ++---- .../block_based/filter_block_reader_common.cc | 16 ++-- .../block_based/filter_block_reader_common.h | 4 +- table/block_based/full_filter_block.cc | 34 ++++---- table/block_based/full_filter_block.h | 19 ++--- table/block_based/full_filter_block_test.cc | 32 ++++---- table/block_based/partitioned_filter_block.cc | 79 +++++++++---------- table/block_based/partitioned_filter_block.h | 25 +++--- .../partitioned_filter_block_test.cc | 40 +++++----- test_util/testutil.cc | 1 + test_util/testutil.h | 6 ++ util/filter_bench.cc | 2 +- 15 files changed, 142 insertions(+), 174 deletions(-) diff --git a/table/block_based/block_based_table_reader.cc b/table/block_based/block_based_table_reader.cc index 78d62b8210..9892c2056f 100644 --- a/table/block_based/block_based_table_reader.cc +++ b/table/block_based/block_based_table_reader.cc @@ -2023,14 +2023,11 @@ bool BlockBasedTable::PrefixRangeMayMatch( FilterBlockReader* const filter = rep_->filter.get(); *filter_checked = false; if (filter != nullptr) { - const bool no_io = read_options.read_tier == kBlockCacheTier; - const Slice* const const_ikey_ptr = &internal_key; may_match = filter->RangeMayExist( read_options.iterate_upper_bound, user_key_without_ts, prefix_extractor, rep_->internal_comparator.user_comparator(), const_ikey_ptr, - filter_checked, need_upper_bound_check, no_io, lookup_context, - read_options); + filter_checked, need_upper_bound_check, lookup_context, read_options); } return may_match; @@ -2110,7 +2107,7 @@ FragmentedRangeTombstoneIterator* BlockBasedTable::NewRangeTombstoneIterator( } bool BlockBasedTable::FullFilterKeyMayMatch( - FilterBlockReader* filter, const Slice& internal_key, const bool no_io, + FilterBlockReader* filter, const Slice& internal_key, const SliceTransform* prefix_extractor, GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) const { @@ -2123,7 +2120,7 @@ bool BlockBasedTable::FullFilterKeyMayMatch( size_t ts_sz = rep_->internal_comparator.user_comparator()->timestamp_size(); Slice user_key_without_ts = StripTimestampFromUserKey(user_key, ts_sz); if (rep_->whole_key_filtering) { - may_match = filter->KeyMayMatch(user_key_without_ts, no_io, const_ikey_ptr, + may_match = filter->KeyMayMatch(user_key_without_ts, const_ikey_ptr, get_context, lookup_context, read_options); if (may_match) { RecordTick(rep_->ioptions.stats, BLOOM_FILTER_FULL_POSITIVE); @@ -2137,7 +2134,7 @@ bool BlockBasedTable::FullFilterKeyMayMatch( // FIXME ^^^: there should be no reason for Get() to depend on current // prefix_extractor at all. It should always use table_prefix_extractor. may_match = filter->PrefixMayMatch( - prefix_extractor->Transform(user_key_without_ts), no_io, const_ikey_ptr, + prefix_extractor->Transform(user_key_without_ts), const_ikey_ptr, get_context, lookup_context, read_options); RecordTick(rep_->ioptions.stats, BLOOM_FILTER_PREFIX_CHECKED); if (may_match) { @@ -2153,7 +2150,7 @@ bool BlockBasedTable::FullFilterKeyMayMatch( } void BlockBasedTable::FullFilterKeysMayMatch( - FilterBlockReader* filter, MultiGetRange* range, const bool no_io, + FilterBlockReader* filter, MultiGetRange* range, const SliceTransform* prefix_extractor, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) const { @@ -2163,7 +2160,7 @@ void BlockBasedTable::FullFilterKeysMayMatch( uint64_t before_keys = range->KeysLeft(); assert(before_keys > 0); // Caller should ensure if (rep_->whole_key_filtering) { - filter->KeysMayMatch(range, no_io, lookup_context, read_options); + filter->KeysMayMatch(range, lookup_context, read_options); uint64_t after_keys = range->KeysLeft(); if (after_keys) { RecordTick(rep_->ioptions.stats, BLOOM_FILTER_FULL_POSITIVE, after_keys); @@ -2179,7 +2176,7 @@ void BlockBasedTable::FullFilterKeysMayMatch( } else if (!PrefixExtractorChanged(prefix_extractor)) { // FIXME ^^^: there should be no reason for MultiGet() to depend on current // prefix_extractor at all. It should always use table_prefix_extractor. - filter->PrefixesMayMatch(range, prefix_extractor, false, lookup_context, + filter->PrefixesMayMatch(range, prefix_extractor, lookup_context, read_options); RecordTick(rep_->ioptions.stats, BLOOM_FILTER_PREFIX_CHECKED, before_keys); uint64_t after_keys = range->KeysLeft(); @@ -2285,7 +2282,6 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key, assert(key.size() >= 8); // key must be internal key assert(get_context != nullptr); Status s; - const bool no_io = read_options.read_tier == kBlockCacheTier; FilterBlockReader* const filter = !skip_filters ? rep_->filter.get() : nullptr; @@ -2304,7 +2300,7 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key, } TEST_SYNC_POINT("BlockBasedTable::Get:BeforeFilterMatch"); const bool may_match = - FullFilterKeyMayMatch(filter, key, no_io, prefix_extractor, get_context, + FullFilterKeyMayMatch(filter, key, prefix_extractor, get_context, &lookup_context, read_options); TEST_SYNC_POINT("BlockBasedTable::Get:AfterFilterMatch"); if (may_match) { @@ -2354,7 +2350,8 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key, /*for_compaction=*/false, /*async_read=*/false, tmp_status, /*use_block_cache_for_lookup=*/true); - if (no_io && biter.status().IsIncomplete()) { + if (read_options.read_tier == kBlockCacheTier && + biter.status().IsIncomplete()) { // couldn't get block from block_cache // Update Saver.state to Found because we are only looking for // whether we can guarantee the key is not there when "no_io" is set @@ -2466,7 +2463,6 @@ Status BlockBasedTable::MultiGetFilter(const ReadOptions& read_options, // First check the full filter // If full filter not useful, Then go into each block - const bool no_io = read_options.read_tier == kBlockCacheTier; uint64_t tracing_mget_id = BlockCacheTraceHelper::kReservedGetId; if (mget_range->begin()->get_context) { tracing_mget_id = mget_range->begin()->get_context->get_tracing_get_id(); @@ -2474,8 +2470,8 @@ Status BlockBasedTable::MultiGetFilter(const ReadOptions& read_options, BlockCacheLookupContext lookup_context{ TableReaderCaller::kUserMultiGet, tracing_mget_id, /*_get_from_user_specified_snapshot=*/read_options.snapshot != nullptr}; - FullFilterKeysMayMatch(filter, mget_range, no_io, prefix_extractor, - &lookup_context, read_options); + FullFilterKeysMayMatch(filter, mget_range, prefix_extractor, &lookup_context, + read_options); return Status::OK(); } diff --git a/table/block_based/block_based_table_reader.h b/table/block_based/block_based_table_reader.h index d940386d12..1aadc62e01 100644 --- a/table/block_based/block_based_table_reader.h +++ b/table/block_based/block_based_table_reader.h @@ -469,14 +469,12 @@ class BlockBasedTable : public TableReader { std::unique_ptr* index_reader); bool FullFilterKeyMayMatch(FilterBlockReader* filter, const Slice& user_key, - const bool no_io, const SliceTransform* prefix_extractor, GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) const; void FullFilterKeysMayMatch(FilterBlockReader* filter, MultiGetRange* range, - const bool no_io, const SliceTransform* prefix_extractor, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) const; diff --git a/table/block_based/block_based_table_reader_sync_and_async.h b/table/block_based/block_based_table_reader_sync_and_async.h index 1facef4a42..6ab330dea5 100644 --- a/table/block_based/block_based_table_reader_sync_and_async.h +++ b/table/block_based/block_based_table_reader_sync_and_async.h @@ -362,7 +362,6 @@ DEFINE_SYNC_AND_ASYNC(void, BlockBasedTable::MultiGet) // First check the full filter // If full filter not useful, Then go into each block - const bool no_io = read_options.read_tier == kBlockCacheTier; uint64_t tracing_mget_id = BlockCacheTraceHelper::kReservedGetId; if (sst_file_range.begin()->get_context) { tracing_mget_id = sst_file_range.begin()->get_context->get_tracing_get_id(); @@ -372,7 +371,7 @@ DEFINE_SYNC_AND_ASYNC(void, BlockBasedTable::MultiGet) BlockCacheLookupContext metadata_lookup_context{ TableReaderCaller::kUserMultiGet, tracing_mget_id, /*_get_from_user_specified_snapshot=*/read_options.snapshot != nullptr}; - FullFilterKeysMayMatch(filter, &sst_file_range, no_io, prefix_extractor, + FullFilterKeysMayMatch(filter, &sst_file_range, prefix_extractor, &metadata_lookup_context, read_options); if (!sst_file_range.empty()) { @@ -668,7 +667,7 @@ DEFINE_SYNC_AND_ASYNC(void, BlockBasedTable::MultiGet) biter->status().IsIncomplete()) { // couldn't get block from block_cache // Update Saver.state to Found because we are only looking for - // whether we can guarantee the key is not there when "no_io" is set + // whether we can guarantee the key is not there with kBlockCacheTier get_context->MarkKeyMayExist(); break; } diff --git a/table/block_based/filter_block.h b/table/block_based/filter_block.h index 299b43ebc0..1730974489 100644 --- a/table/block_based/filter_block.h +++ b/table/block_based/filter_block.h @@ -100,39 +100,34 @@ class FilterBlockReader { FilterBlockReader& operator=(const FilterBlockReader&) = delete; /** - * If no_io is set, then it returns true if it cannot answer the query without - * reading data from disk. This is used in PartitionedFilterBlockReader to - * avoid reading partitions that are not in block cache already - * * Normally filters are built on only the user keys and the InternalKey is not * needed for a query. The index in PartitionedFilterBlockReader however is * built upon InternalKey and must be provided via const_ikey_ptr when running * queries. */ - virtual bool KeyMayMatch(const Slice& key, const bool no_io, - const Slice* const const_ikey_ptr, + virtual bool KeyMayMatch(const Slice& key, const Slice* const const_ikey_ptr, GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) = 0; - virtual void KeysMayMatch(MultiGetRange* range, const bool no_io, + virtual void KeysMayMatch(MultiGetRange* range, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) { for (auto iter = range->begin(); iter != range->end(); ++iter) { const Slice ukey_without_ts = iter->ukey_without_ts; const Slice ikey = iter->ikey; GetContext* const get_context = iter->get_context; - if (!KeyMayMatch(ukey_without_ts, no_io, &ikey, get_context, - lookup_context, read_options)) { + if (!KeyMayMatch(ukey_without_ts, &ikey, get_context, lookup_context, + read_options)) { range->SkipKey(iter); } } } /** - * no_io and const_ikey_ptr here means the same as in KeyMayMatch + * Similar to KeyMayMatch */ - virtual bool PrefixMayMatch(const Slice& prefix, const bool no_io, + virtual bool PrefixMayMatch(const Slice& prefix, const Slice* const const_ikey_ptr, GetContext* get_context, BlockCacheLookupContext* lookup_context, @@ -140,7 +135,6 @@ class FilterBlockReader { virtual void PrefixesMayMatch(MultiGetRange* range, const SliceTransform* prefix_extractor, - const bool no_io, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) { for (auto iter = range->begin(); iter != range->end(); ++iter) { @@ -148,8 +142,8 @@ class FilterBlockReader { const Slice ikey = iter->ikey; GetContext* const get_context = iter->get_context; if (prefix_extractor->InDomain(ukey_without_ts) && - !PrefixMayMatch(prefix_extractor->Transform(ukey_without_ts), no_io, - &ikey, get_context, lookup_context, read_options)) { + !PrefixMayMatch(prefix_extractor->Transform(ukey_without_ts), &ikey, + get_context, lookup_context, read_options)) { range->SkipKey(iter); } } @@ -178,7 +172,6 @@ class FilterBlockReader { const Comparator* /*comparator*/, const Slice* const const_ikey_ptr, bool* filter_checked, bool need_upper_bound_check, - bool no_io, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) = 0; }; diff --git a/table/block_based/filter_block_reader_common.cc b/table/block_based/filter_block_reader_common.cc index aeacc7bf54..343e9406b5 100644 --- a/table/block_based/filter_block_reader_common.cc +++ b/table/block_based/filter_block_reader_common.cc @@ -67,8 +67,7 @@ bool FilterBlockReaderCommon::cache_filter_blocks() const { template Status FilterBlockReaderCommon::GetOrReadFilterBlock( - bool no_io, GetContext* get_context, - BlockCacheLookupContext* lookup_context, + GetContext* get_context, BlockCacheLookupContext* lookup_context, CachableEntry* filter_block, const ReadOptions& read_options) const { assert(filter_block); @@ -78,12 +77,7 @@ Status FilterBlockReaderCommon::GetOrReadFilterBlock( return Status::OK(); } - ReadOptions ro = read_options; - if (no_io) { - ro.read_tier = kBlockCacheTier; - } - - return ReadFilterBlock(table_, nullptr /* prefetch_buffer */, ro, + return ReadFilterBlock(table_, nullptr /* prefetch_buffer */, read_options, cache_filter_blocks(), get_context, lookup_context, filter_block); } @@ -102,8 +96,8 @@ bool FilterBlockReaderCommon::RangeMayExist( const Slice* iterate_upper_bound, const Slice& user_key_without_ts, const SliceTransform* prefix_extractor, const Comparator* comparator, const Slice* const const_ikey_ptr, bool* filter_checked, - bool need_upper_bound_check, bool no_io, - BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) { + bool need_upper_bound_check, BlockCacheLookupContext* lookup_context, + const ReadOptions& read_options) { if (!prefix_extractor || !prefix_extractor->InDomain(user_key_without_ts)) { *filter_checked = false; return true; @@ -115,7 +109,7 @@ bool FilterBlockReaderCommon::RangeMayExist( return true; } else { *filter_checked = true; - return PrefixMayMatch(prefix, no_io, const_ikey_ptr, + return PrefixMayMatch(prefix, const_ikey_ptr, /* get_context */ nullptr, lookup_context, read_options); } diff --git a/table/block_based/filter_block_reader_common.h b/table/block_based/filter_block_reader_common.h index fc75e66f88..5502ed7065 100644 --- a/table/block_based/filter_block_reader_common.h +++ b/table/block_based/filter_block_reader_common.h @@ -38,7 +38,7 @@ class FilterBlockReaderCommon : public FilterBlockReader { const SliceTransform* prefix_extractor, const Comparator* comparator, const Slice* const const_ikey_ptr, bool* filter_checked, - bool need_upper_bound_check, bool no_io, + bool need_upper_bound_check, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) override; @@ -58,7 +58,7 @@ class FilterBlockReaderCommon : public FilterBlockReader { bool whole_key_filtering() const; bool cache_filter_blocks() const; - Status GetOrReadFilterBlock(bool no_io, GetContext* get_context, + Status GetOrReadFilterBlock(GetContext* get_context, BlockCacheLookupContext* lookup_context, CachableEntry* filter_block, const ReadOptions& read_options) const; diff --git a/table/block_based/full_filter_block.cc b/table/block_based/full_filter_block.cc index 0d7d9a5996..8f31d9e83a 100644 --- a/table/block_based/full_filter_block.cc +++ b/table/block_based/full_filter_block.cc @@ -123,7 +123,7 @@ FullFilterBlockReader::FullFilterBlockReader( CachableEntry&& filter_block) : FilterBlockReaderCommon(t, std::move(filter_block)) {} -bool FullFilterBlockReader::KeyMayMatch(const Slice& key, const bool no_io, +bool FullFilterBlockReader::KeyMayMatch(const Slice& key, const Slice* const /*const_ikey_ptr*/, GetContext* get_context, BlockCacheLookupContext* lookup_context, @@ -131,7 +131,7 @@ bool FullFilterBlockReader::KeyMayMatch(const Slice& key, const bool no_io, if (!whole_key_filtering()) { return true; } - return MayMatch(key, no_io, get_context, lookup_context, read_options); + return MayMatch(key, get_context, lookup_context, read_options); } std::unique_ptr FullFilterBlockReader::Create( @@ -162,19 +162,19 @@ std::unique_ptr FullFilterBlockReader::Create( } bool FullFilterBlockReader::PrefixMayMatch( - const Slice& prefix, const bool no_io, - const Slice* const /*const_ikey_ptr*/, GetContext* get_context, - BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) { - return MayMatch(prefix, no_io, get_context, lookup_context, read_options); + const Slice& prefix, const Slice* const /*const_ikey_ptr*/, + GetContext* get_context, BlockCacheLookupContext* lookup_context, + const ReadOptions& read_options) { + return MayMatch(prefix, get_context, lookup_context, read_options); } -bool FullFilterBlockReader::MayMatch(const Slice& entry, bool no_io, +bool FullFilterBlockReader::MayMatch(const Slice& entry, GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) const { CachableEntry filter_block; - const Status s = GetOrReadFilterBlock(no_io, get_context, lookup_context, + const Status s = GetOrReadFilterBlock(get_context, lookup_context, &filter_block, read_options); if (!s.ok()) { IGNORE_STATUS_IF_ERROR(s); @@ -199,32 +199,30 @@ bool FullFilterBlockReader::MayMatch(const Slice& entry, bool no_io, } void FullFilterBlockReader::KeysMayMatch( - MultiGetRange* range, const bool no_io, - BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) { + MultiGetRange* range, BlockCacheLookupContext* lookup_context, + const ReadOptions& read_options) { if (!whole_key_filtering()) { // Simply return. Don't skip any key - consider all keys as likely to be // present return; } - MayMatch(range, no_io, nullptr, lookup_context, read_options); + MayMatch(range, nullptr, lookup_context, read_options); } void FullFilterBlockReader::PrefixesMayMatch( MultiGetRange* range, const SliceTransform* prefix_extractor, - const bool no_io, BlockCacheLookupContext* lookup_context, - const ReadOptions& read_options) { - MayMatch(range, no_io, prefix_extractor, lookup_context, read_options); + BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) { + MayMatch(range, prefix_extractor, lookup_context, read_options); } -void FullFilterBlockReader::MayMatch(MultiGetRange* range, bool no_io, +void FullFilterBlockReader::MayMatch(MultiGetRange* range, const SliceTransform* prefix_extractor, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) const { CachableEntry filter_block; - const Status s = - GetOrReadFilterBlock(no_io, range->begin()->get_context, lookup_context, - &filter_block, read_options); + const Status s = GetOrReadFilterBlock( + range->begin()->get_context, lookup_context, &filter_block, read_options); if (!s.ok()) { IGNORE_STATUS_IF_ERROR(s); return; diff --git a/table/block_based/full_filter_block.h b/table/block_based/full_filter_block.h index 5bffd53520..99deb05383 100644 --- a/table/block_based/full_filter_block.h +++ b/table/block_based/full_filter_block.h @@ -102,41 +102,38 @@ class FullFilterBlockReader FilePrefetchBuffer* prefetch_buffer, bool use_cache, bool prefetch, bool pin, BlockCacheLookupContext* lookup_context); - bool KeyMayMatch(const Slice& key, const bool no_io, - const Slice* const const_ikey_ptr, GetContext* get_context, + bool KeyMayMatch(const Slice& key, const Slice* const const_ikey_ptr, + GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) override; - bool PrefixMayMatch(const Slice& prefix, const bool no_io, - const Slice* const const_ikey_ptr, + bool PrefixMayMatch(const Slice& prefix, const Slice* const const_ikey_ptr, GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) override; - void KeysMayMatch(MultiGetRange* range, const bool no_io, + void KeysMayMatch(MultiGetRange* range, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) override; // Used in partitioned filter code void KeysMayMatch2(MultiGetRange* range, const SliceTransform* /*prefix_extractor*/, - const bool no_io, BlockCacheLookupContext* lookup_context, + BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) { - KeysMayMatch(range, no_io, lookup_context, read_options); + KeysMayMatch(range, lookup_context, read_options); } void PrefixesMayMatch(MultiGetRange* range, const SliceTransform* prefix_extractor, - const bool no_io, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) override; size_t ApproximateMemoryUsage() const override; private: - bool MayMatch(const Slice& entry, bool no_io, GetContext* get_context, + bool MayMatch(const Slice& entry, GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) const; - void MayMatch(MultiGetRange* range, bool no_io, - const SliceTransform* prefix_extractor, + void MayMatch(MultiGetRange* range, const SliceTransform* prefix_extractor, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) const; }; diff --git a/table/block_based/full_filter_block_test.cc b/table/block_based/full_filter_block_test.cc index 154c8c0907..083d287763 100644 --- a/table/block_based/full_filter_block_test.cc +++ b/table/block_based/full_filter_block_test.cc @@ -115,7 +115,7 @@ TEST_F(PluginFullFilterBlockTest, PluginEmptyBuilder) { FullFilterBlockReader reader(table_.get(), std::move(block)); // Remain same symantic with blockbased filter ASSERT_TRUE(reader.KeyMayMatch("foo", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); } @@ -136,31 +136,31 @@ TEST_F(PluginFullFilterBlockTest, PluginSingleChunk) { FullFilterBlockReader reader(table_.get(), std::move(block)); ASSERT_TRUE(reader.KeyMayMatch("foo", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(reader.KeyMayMatch("bar", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(reader.KeyMayMatch("box", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(reader.KeyMayMatch("hello", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(reader.KeyMayMatch("foo", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(!reader.KeyMayMatch("missing", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(!reader.KeyMayMatch("other", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); } @@ -185,7 +185,7 @@ TEST_F(FullFilterBlockTest, EmptyBuilder) { FullFilterBlockReader reader(table_.get(), std::move(block)); // Remain same symantic with blockbased filter ASSERT_TRUE(reader.KeyMayMatch("foo", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); } @@ -285,31 +285,31 @@ TEST_F(FullFilterBlockTest, SingleChunk) { FullFilterBlockReader reader(table_.get(), std::move(block)); ASSERT_TRUE(reader.KeyMayMatch("foo", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(reader.KeyMayMatch("bar", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(reader.KeyMayMatch("box", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(reader.KeyMayMatch("hello", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(reader.KeyMayMatch("foo", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(!reader.KeyMayMatch("missing", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); ASSERT_TRUE(!reader.KeyMayMatch("other", - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); } diff --git a/table/block_based/partitioned_filter_block.cc b/table/block_based/partitioned_filter_block.cc index 8e20d84d8d..6df8df92b3 100644 --- a/table/block_based/partitioned_filter_block.cc +++ b/table/block_based/partitioned_filter_block.cc @@ -225,7 +225,7 @@ std::unique_ptr PartitionedFilterBlockReader::Create( } bool PartitionedFilterBlockReader::KeyMayMatch( - const Slice& key, const bool no_io, const Slice* const const_ikey_ptr, + const Slice& key, const Slice* const const_ikey_ptr, GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) { assert(const_ikey_ptr != nullptr); @@ -233,36 +233,35 @@ bool PartitionedFilterBlockReader::KeyMayMatch( return true; } - return MayMatch(key, no_io, const_ikey_ptr, get_context, lookup_context, + return MayMatch(key, const_ikey_ptr, get_context, lookup_context, read_options, &FullFilterBlockReader::KeyMayMatch); } void PartitionedFilterBlockReader::KeysMayMatch( - MultiGetRange* range, const bool no_io, - BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) { + MultiGetRange* range, BlockCacheLookupContext* lookup_context, + const ReadOptions& read_options) { if (!whole_key_filtering()) { return; // Any/all may match } - MayMatch(range, nullptr, no_io, lookup_context, read_options, + MayMatch(range, nullptr, lookup_context, read_options, &FullFilterBlockReader::KeysMayMatch2); } bool PartitionedFilterBlockReader::PrefixMayMatch( - const Slice& prefix, const bool no_io, const Slice* const const_ikey_ptr, + const Slice& prefix, const Slice* const const_ikey_ptr, GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) { assert(const_ikey_ptr != nullptr); - return MayMatch(prefix, no_io, const_ikey_ptr, get_context, lookup_context, + return MayMatch(prefix, const_ikey_ptr, get_context, lookup_context, read_options, &FullFilterBlockReader::PrefixMayMatch); } void PartitionedFilterBlockReader::PrefixesMayMatch( MultiGetRange* range, const SliceTransform* prefix_extractor, - const bool no_io, BlockCacheLookupContext* lookup_context, - const ReadOptions& read_options) { + BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) { assert(prefix_extractor); - MayMatch(range, prefix_extractor, no_io, lookup_context, read_options, + MayMatch(range, prefix_extractor, lookup_context, read_options, &FullFilterBlockReader::PrefixesMayMatch); } @@ -295,8 +294,8 @@ BlockHandle PartitionedFilterBlockReader::GetFilterPartitionHandle( Status PartitionedFilterBlockReader::GetFilterPartitionBlock( FilePrefetchBuffer* prefetch_buffer, const BlockHandle& fltr_blk_handle, - bool no_io, GetContext* get_context, - BlockCacheLookupContext* lookup_context, const ReadOptions& _read_options, + GetContext* get_context, BlockCacheLookupContext* lookup_context, + const ReadOptions& read_options, CachableEntry* filter_block) const { assert(table()); assert(filter_block); @@ -312,11 +311,6 @@ Status PartitionedFilterBlockReader::GetFilterPartitionBlock( } } - ReadOptions read_options = _read_options; - if (no_io) { - read_options.read_tier = kBlockCacheTier; - } - const Status s = table()->RetrieveBlock( prefetch_buffer, read_options, fltr_blk_handle, UncompressionDict::GetEmptyDict(), filter_block, get_context, @@ -328,12 +322,12 @@ Status PartitionedFilterBlockReader::GetFilterPartitionBlock( } bool PartitionedFilterBlockReader::MayMatch( - const Slice& slice, bool no_io, const Slice* const_ikey_ptr, - GetContext* get_context, BlockCacheLookupContext* lookup_context, - const ReadOptions& read_options, FilterFunction filter_function) const { + const Slice& slice, const Slice* const_ikey_ptr, GetContext* get_context, + BlockCacheLookupContext* lookup_context, const ReadOptions& read_options, + FilterFunction filter_function) const { CachableEntry filter_block; - Status s = GetOrReadFilterBlock(no_io, get_context, lookup_context, - &filter_block, read_options); + Status s = GetOrReadFilterBlock(get_context, lookup_context, &filter_block, + read_options); if (UNLIKELY(!s.ok())) { IGNORE_STATUS_IF_ERROR(s); return true; @@ -350,7 +344,7 @@ bool PartitionedFilterBlockReader::MayMatch( CachableEntry filter_partition_block; s = GetFilterPartitionBlock(nullptr /* prefetch_buffer */, filter_handle, - no_io, get_context, lookup_context, read_options, + get_context, lookup_context, read_options, &filter_partition_block); if (UNLIKELY(!s.ok())) { IGNORE_STATUS_IF_ERROR(s); @@ -359,17 +353,17 @@ bool PartitionedFilterBlockReader::MayMatch( FullFilterBlockReader filter_partition(table(), std::move(filter_partition_block)); - return (filter_partition.*filter_function)( - slice, no_io, const_ikey_ptr, get_context, lookup_context, read_options); + return (filter_partition.*filter_function)(slice, const_ikey_ptr, get_context, + lookup_context, read_options); } void PartitionedFilterBlockReader::MayMatch( - MultiGetRange* range, const SliceTransform* prefix_extractor, bool no_io, + MultiGetRange* range, const SliceTransform* prefix_extractor, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options, FilterManyFunction filter_function) const { CachableEntry filter_block; - Status s = GetOrReadFilterBlock(no_io, range->begin()->get_context, - lookup_context, &filter_block, read_options); + Status s = GetOrReadFilterBlock(range->begin()->get_context, lookup_context, + &filter_block, read_options); if (UNLIKELY(!s.ok())) { IGNORE_STATUS_IF_ERROR(s); return; // Any/all may match @@ -392,7 +386,7 @@ void PartitionedFilterBlockReader::MayMatch( if (!prev_filter_handle.IsNull() && this_filter_handle != prev_filter_handle) { MultiGetRange subrange(*range, start_iter_same_handle, iter); - MayMatchPartition(&subrange, prefix_extractor, prev_filter_handle, no_io, + MayMatchPartition(&subrange, prefix_extractor, prev_filter_handle, lookup_context, read_options, filter_function); range->AddSkipsFrom(subrange); start_iter_same_handle = iter; @@ -408,7 +402,7 @@ void PartitionedFilterBlockReader::MayMatch( } if (!prev_filter_handle.IsNull()) { MultiGetRange subrange(*range, start_iter_same_handle, range->end()); - MayMatchPartition(&subrange, prefix_extractor, prev_filter_handle, no_io, + MayMatchPartition(&subrange, prefix_extractor, prev_filter_handle, lookup_context, read_options, filter_function); range->AddSkipsFrom(subrange); } @@ -416,14 +410,12 @@ void PartitionedFilterBlockReader::MayMatch( void PartitionedFilterBlockReader::MayMatchPartition( MultiGetRange* range, const SliceTransform* prefix_extractor, - BlockHandle filter_handle, bool no_io, - BlockCacheLookupContext* lookup_context, const ReadOptions& read_options, - FilterManyFunction filter_function) const { + BlockHandle filter_handle, BlockCacheLookupContext* lookup_context, + const ReadOptions& read_options, FilterManyFunction filter_function) const { CachableEntry filter_partition_block; Status s = GetFilterPartitionBlock( - nullptr /* prefetch_buffer */, filter_handle, no_io, - range->begin()->get_context, lookup_context, read_options, - &filter_partition_block); + nullptr /* prefetch_buffer */, filter_handle, range->begin()->get_context, + lookup_context, read_options, &filter_partition_block); if (UNLIKELY(!s.ok())) { IGNORE_STATUS_IF_ERROR(s); return; // Any/all may match @@ -431,8 +423,8 @@ void PartitionedFilterBlockReader::MayMatchPartition( FullFilterBlockReader filter_partition(table(), std::move(filter_partition_block)); - (filter_partition.*filter_function)(range, prefix_extractor, no_io, - lookup_context, read_options); + (filter_partition.*filter_function)(range, prefix_extractor, lookup_context, + read_options); } size_t PartitionedFilterBlockReader::ApproximateMemoryUsage() const { @@ -458,8 +450,8 @@ Status PartitionedFilterBlockReader::CacheDependencies( CachableEntry filter_block; - Status s = GetOrReadFilterBlock(false /* no_io */, nullptr /* get_context */, - &lookup_context, &filter_block, ro); + Status s = GetOrReadFilterBlock(nullptr /* get_context */, &lookup_context, + &filter_block, ro); if (!s.ok()) { ROCKS_LOG_ERROR(rep->ioptions.logger, "Error retrieving top-level filter block while trying to " @@ -545,9 +537,10 @@ void PartitionedFilterBlockReader::EraseFromCacheBeforeDestruction( if (uncache_aggressiveness > 0) { CachableEntry top_level_block; - GetOrReadFilterBlock(/*no_io=*/true, /*get_context=*/nullptr, - /*lookup_context=*/nullptr, &top_level_block, - ReadOptions{}) + ReadOptions ro; + ro.read_tier = ReadTier::kBlockCacheTier; + GetOrReadFilterBlock(/*get_context=*/nullptr, + /*lookup_context=*/nullptr, &top_level_block, ro) .PermitUncheckedError(); if (!filter_map_.empty()) { diff --git a/table/block_based/partitioned_filter_block.h b/table/block_based/partitioned_filter_block.h index 3ff1661c65..24241cc779 100644 --- a/table/block_based/partitioned_filter_block.h +++ b/table/block_based/partitioned_filter_block.h @@ -111,22 +111,20 @@ class PartitionedFilterBlockReader FilePrefetchBuffer* prefetch_buffer, bool use_cache, bool prefetch, bool pin, BlockCacheLookupContext* lookup_context); - bool KeyMayMatch(const Slice& key, const bool no_io, - const Slice* const const_ikey_ptr, GetContext* get_context, + bool KeyMayMatch(const Slice& key, const Slice* const const_ikey_ptr, + GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) override; - void KeysMayMatch(MultiGetRange* range, const bool no_io, + void KeysMayMatch(MultiGetRange* range, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) override; - bool PrefixMayMatch(const Slice& prefix, const bool no_io, - const Slice* const const_ikey_ptr, + bool PrefixMayMatch(const Slice& prefix, const Slice* const const_ikey_ptr, GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) override; void PrefixesMayMatch(MultiGetRange* range, const SliceTransform* prefix_extractor, - const bool no_io, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options) override; @@ -138,30 +136,29 @@ class PartitionedFilterBlockReader const Slice& entry) const; Status GetFilterPartitionBlock( FilePrefetchBuffer* prefetch_buffer, const BlockHandle& handle, - bool no_io, GetContext* get_context, - BlockCacheLookupContext* lookup_context, const ReadOptions& read_options, + GetContext* get_context, BlockCacheLookupContext* lookup_context, + const ReadOptions& read_options, CachableEntry* filter_block) const; using FilterFunction = bool (FullFilterBlockReader::*)( - const Slice& slice, const bool no_io, const Slice* const const_ikey_ptr, + const Slice& slice, const Slice* const const_ikey_ptr, GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options); - bool MayMatch(const Slice& slice, bool no_io, const Slice* const_ikey_ptr, + bool MayMatch(const Slice& slice, const Slice* const_ikey_ptr, GetContext* get_context, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options, FilterFunction filter_function) const; using FilterManyFunction = void (FullFilterBlockReader::*)( MultiGetRange* range, const SliceTransform* prefix_extractor, - const bool no_io, BlockCacheLookupContext* lookup_context, - const ReadOptions& read_options); + BlockCacheLookupContext* lookup_context, const ReadOptions& read_options); void MayMatch(MultiGetRange* range, const SliceTransform* prefix_extractor, - bool no_io, BlockCacheLookupContext* lookup_context, + BlockCacheLookupContext* lookup_context, const ReadOptions& read_options, FilterManyFunction filter_function) const; void MayMatchPartition(MultiGetRange* range, const SliceTransform* prefix_extractor, - BlockHandle filter_handle, bool no_io, + BlockHandle filter_handle, BlockCacheLookupContext* lookup_context, const ReadOptions& read_options, FilterManyFunction filter_function) const; diff --git a/table/block_based/partitioned_filter_block_test.cc b/table/block_based/partitioned_filter_block_test.cc index 50bb77975b..1da7ed5354 100644 --- a/table/block_based/partitioned_filter_block_test.cc +++ b/table/block_based/partitioned_filter_block_test.cc @@ -198,24 +198,23 @@ class PartitionedFilterBlockTest std::unique_ptr reader( NewReader(builder, pib)); // Querying added keys - const bool no_io = true; std::vector keys = PrepareKeys(keys_without_ts, kKeyNum); for (const auto& key : keys) { auto ikey = InternalKey(key, 0, ValueType::kTypeValue); const Slice ikey_slice = Slice(*ikey.rep()); ASSERT_TRUE(reader->KeyMayMatch( - StripTimestampFromUserKey(key, ts_sz_), !no_io, &ikey_slice, + StripTimestampFromUserKey(key, ts_sz_), &ikey_slice, /*get_context=*/nullptr, - /*lookup_context=*/nullptr, ReadOptions())); + /*lookup_context=*/nullptr, test::kReadOptionsNoIo)); } { // querying a key twice auto ikey = InternalKey(keys[0], 0, ValueType::kTypeValue); const Slice ikey_slice = Slice(*ikey.rep()); ASSERT_TRUE(reader->KeyMayMatch( - StripTimestampFromUserKey(keys[0], ts_sz_), !no_io, &ikey_slice, + StripTimestampFromUserKey(keys[0], ts_sz_), &ikey_slice, /*get_context=*/nullptr, - /*lookup_context=*/nullptr, ReadOptions())); + /*lookup_context=*/nullptr, test::kReadOptionsNoIo)); } // querying missing keys std::vector missing_keys = @@ -225,15 +224,15 @@ class PartitionedFilterBlockTest const Slice ikey_slice = Slice(*ikey.rep()); if (empty) { ASSERT_TRUE(reader->KeyMayMatch( - StripTimestampFromUserKey(key, ts_sz_), !no_io, &ikey_slice, + StripTimestampFromUserKey(key, ts_sz_), &ikey_slice, /*get_context=*/nullptr, - /*lookup_context=*/nullptr, ReadOptions())); + /*lookup_context=*/nullptr, test::kReadOptionsNoIo)); } else { // assuming a good hash function ASSERT_FALSE(reader->KeyMayMatch( - StripTimestampFromUserKey(key, ts_sz_), !no_io, &ikey_slice, + StripTimestampFromUserKey(key, ts_sz_), &ikey_slice, /*get_context=*/nullptr, - /*lookup_context=*/nullptr, ReadOptions())); + /*lookup_context=*/nullptr, test::kReadOptionsNoIo)); } } } @@ -389,11 +388,10 @@ TEST_P(PartitionedFilterBlockTest, SamePrefixInMultipleBlocks) { for (const auto& key : pkeys) { auto ikey = InternalKey(key, 0, ValueType::kTypeValue); const Slice ikey_slice = Slice(*ikey.rep()); - ASSERT_TRUE(reader->PrefixMayMatch(prefix_extractor->Transform(key), - /*no_io=*/false, &ikey_slice, - /*get_context=*/nullptr, - /*lookup_context=*/nullptr, - ReadOptions())); + ASSERT_TRUE( + reader->PrefixMayMatch(prefix_extractor->Transform(key), &ikey_slice, + /*get_context=*/nullptr, + /*lookup_context=*/nullptr, ReadOptions())); } // Non-existent keys but with the same prefix const std::string pnonkeys_without_ts[4] = {"p-key9", "p-key11", "p-key21", @@ -403,11 +401,10 @@ TEST_P(PartitionedFilterBlockTest, SamePrefixInMultipleBlocks) { for (const auto& key : pnonkeys) { auto ikey = InternalKey(key, 0, ValueType::kTypeValue); const Slice ikey_slice = Slice(*ikey.rep()); - ASSERT_TRUE(reader->PrefixMayMatch(prefix_extractor->Transform(key), - /*no_io=*/false, &ikey_slice, - /*get_context=*/nullptr, - /*lookup_context=*/nullptr, - ReadOptions())); + ASSERT_TRUE( + reader->PrefixMayMatch(prefix_extractor->Transform(key), &ikey_slice, + /*get_context=*/nullptr, + /*lookup_context=*/nullptr, ReadOptions())); } } @@ -444,8 +441,7 @@ TEST_P(PartitionedFilterBlockTest, PrefixInWrongPartitionBug) { auto prefix = prefix_extractor->Transform(key); auto ikey = InternalKey(key, 0, ValueType::kTypeValue); const Slice ikey_slice = Slice(*ikey.rep()); - ASSERT_TRUE(reader->PrefixMayMatch(prefix, - /*no_io=*/false, &ikey_slice, + ASSERT_TRUE(reader->PrefixMayMatch(prefix, &ikey_slice, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ReadOptions())); @@ -477,4 +473,4 @@ int main(int argc, char** argv) { ROCKSDB_NAMESPACE::port::InstallStackTraceHandler(); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); -} \ No newline at end of file +} diff --git a/test_util/testutil.cc b/test_util/testutil.cc index ab926cef21..40531586af 100644 --- a/test_util/testutil.cc +++ b/test_util/testutil.cc @@ -46,6 +46,7 @@ const std::set kFooterFormatVersionsToTest{ kDefaultFormatVersion, kLatestFormatVersion, }; +const ReadOptionsNoIo kReadOptionsNoIo; std::string RandomKey(Random* rnd, int len, RandomKeyType type) { // Make sure to generate a wide variety of characters so we diff --git a/test_util/testutil.h b/test_util/testutil.h index 02accdc521..9e0499cb3d 100644 --- a/test_util/testutil.h +++ b/test_util/testutil.h @@ -898,5 +898,11 @@ int RegisterTestObjects(ObjectLibrary& library, const std::string& /*arg*/); // Register the testutil classes with the default ObjectRegistry/Library void RegisterTestLibrary(const std::string& arg = ""); + +struct ReadOptionsNoIo : public ReadOptions { + ReadOptionsNoIo() { read_tier = ReadTier::kBlockCacheTier; } +}; +extern const ReadOptionsNoIo kReadOptionsNoIo; + } // namespace test } // namespace ROCKSDB_NAMESPACE diff --git a/util/filter_bench.cc b/util/filter_bench.cc index 4e6dabf576..c94d58194c 100644 --- a/util/filter_bench.cc +++ b/util/filter_bench.cc @@ -726,7 +726,7 @@ double FilterBench::RandomQueryTest(uint32_t inside_threshold, bool dry_run, } else { may_match = info.full_block_reader_->KeyMayMatch( batch_slices[i], - /*no_io=*/false, /*const_ikey_ptr=*/nullptr, + /*const_ikey_ptr=*/nullptr, /*get_context=*/nullptr, /*lookup_context=*/nullptr, ROCKSDB_NAMESPACE::ReadOptions()); }