andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix iterator reading filter block despite read_tier == kBlockCacheTier (#6562)

Browse Source 
Summary:
We're seeing iterators with `ReadOptions::read_tier == kBlockCacheTier` sometimes doing file reads. Stack trace:

```
rocksdb::RandomAccessFileReader::Read(unsigned long, unsigned long, rocksdb::Slice*, char*, bool) const
rocksdb::BlockFetcher::ReadBlockContents()
rocksdb::Status rocksdb::BlockBasedTable::MaybeReadBlockAndLoadToCache<rocksdb::ParsedFullFilterBlock>(rocksdb::FilePrefetchBuffer*, rocksdb::ReadOptions const&, rocksdb::BlockHandle const&, rocksdb::UncompressionDict const&, rocksdb::CachableEntry<rocksdb::ParsedFullFilterBlock>*, rocksdb::BlockType, rocksdb::GetContext*, rocksdb::BlockCacheLookupContext*, rocksdb::BlockContents*) const
rocksdb::Status rocksdb::BlockBasedTable::RetrieveBlock<rocksdb::ParsedFullFilterBlock>(rocksdb::FilePrefetchBuffer*, rocksdb::ReadOptions const&, rocksdb::BlockHandle const&, rocksdb::UncompressionDict const&, rocksdb::CachableEntry<rocksdb::ParsedFullFilterBlock>*, rocksdb::BlockType, rocksdb::GetContext*, rocksdb::BlockCacheLookupContext*, bool, bool) const
rocksdb::FilterBlockReaderCommon<rocksdb::ParsedFullFilterBlock>::ReadFilterBlock(rocksdb::BlockBasedTable const*, rocksdb::FilePrefetchBuffer*, rocksdb::ReadOptions const&, bool, rocksdb::GetContext*, rocksdb::BlockCacheLookupContext*, rocksdb::CachableEntry<rocksdb::ParsedFullFilterBlock>*)
rocksdb::FilterBlockReaderCommon<rocksdb::ParsedFullFilterBlock>::GetOrReadFilterBlock(bool, rocksdb::GetContext*, rocksdb::BlockCacheLookupContext*, rocksdb::CachableEntry<rocksdb::ParsedFullFilterBlock>*) const
rocksdb::FullFilterBlockReader::MayMatch(rocksdb::Slice const&, bool, rocksdb::GetContext*, rocksdb::BlockCacheLookupContext*) const
rocksdb::FullFilterBlockReader::RangeMayExist(rocksdb::Slice const*, rocksdb::Slice const&, rocksdb::SliceTransform const*, rocksdb::Comparator const*, rocksdb::Slice const*, bool*, bool, rocksdb::BlockCacheLookupContext*)
rocksdb::BlockBasedTable::PrefixMayMatch(rocksdb::Slice const&, rocksdb::ReadOptions const&, rocksdb::SliceTransform const*, bool, rocksdb::BlockCacheLookupContext*) const
rocksdb::BlockBasedTableIterator<rocksdb::DataBlockIter, rocksdb::Slice>::SeekImpl(rocksdb::Slice const*)
rocksdb::ForwardIterator::SeekInternal(rocksdb::Slice const&, bool)
rocksdb::DBIter::Seek(rocksdb::Slice const&)
```

`BlockBasedTableIterator::CheckPrefixMayMatch` was missing a check for `kBlockCacheTier`. This PR adds it.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6562

Test Plan: deployed it to a logdevice test cluster and looked at logdevice's IO tracing.

Reviewed By: siying

Differential Revision: D20529368

Pulled By: al13n321

fbshipit-source-id: 65bf33964b1951464415c900336635fb20919611
This commit is contained in:
Mike Kolupaev 2020-03-26 15:18:03 -07:00 committed by Facebook GitHub Bot
parent e70629e5f7
commit 963af52f15
5 changed files with 13 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
db/version_set.h
View File

@ -698,10 +698,6 @@ class Version {
return storage_info_.user_comparator_; return storage_info_.user_comparator_;
} }
bool PrefixMayMatch(const ReadOptions& read_options,
InternalIterator* level_iter,
const Slice& internal_prefix) const;
// Returns true if the filter blocks in the specified level will not be // Returns true if the filter blocks in the specified level will not be
// checked during read operations. In certain cases (trivial move or preload), // checked during read operations. In certain cases (trivial move or preload),
// the filter block may already be cached, but we still do not access it such // the filter block may already be cached, but we still do not access it such

10
table/block_based/block_based_table_reader.cc
View File

@ -1905,7 +1905,9 @@ BlockBasedTable::PartitionedIndexIteratorState::NewSecondaryIterator(
// 2) Compare(prefix(key), key) <= 0. // 2) Compare(prefix(key), key) <= 0.
// 3) If Compare(key1, key2) <= 0, then Compare(prefix(key1), prefix(key2)) <= 0 // 3) If Compare(key1, key2) <= 0, then Compare(prefix(key1), prefix(key2)) <= 0
// //
// Otherwise, this method guarantees no I/O will be incurred. // If read_options.read_tier == kBlockCacheTier, this method will do no I/O and
// will return true if the filter block is not in memory and not found in block
// cache.
// //
// REQUIRES: this method shouldn't be called while the DB lock is held. // REQUIRES: this method shouldn't be called while the DB lock is held.
bool BlockBasedTable::PrefixMayMatch( bool BlockBasedTable::PrefixMayMatch(
@ -1939,12 +1941,14 @@ bool BlockBasedTable::PrefixMayMatch(
FilterBlockReader* const filter = rep_->filter.get(); FilterBlockReader* const filter = rep_->filter.get();
bool filter_checked = true; bool filter_checked = true;
if (filter != nullptr) { if (filter != nullptr) {
const bool no_io = read_options.read_tier == kBlockCacheTier;
if (!filter->IsBlockBased()) { if (!filter->IsBlockBased()) {
const Slice* const const_ikey_ptr = &internal_key; const Slice* const const_ikey_ptr = &internal_key;
may_match = filter->RangeMayExist( may_match = filter->RangeMayExist(
read_options.iterate_upper_bound, user_key, prefix_extractor, read_options.iterate_upper_bound, user_key, prefix_extractor,
rep_->internal_comparator.user_comparator(), const_ikey_ptr, rep_->internal_comparator.user_comparator(), const_ikey_ptr,
&filter_checked, need_upper_bound_check, lookup_context); &filter_checked, need_upper_bound_check, no_io, lookup_context);
} else { } else {
// if prefix_extractor changed for block based filter, skip filter // if prefix_extractor changed for block based filter, skip filter
if (need_upper_bound_check) { if (need_upper_bound_check) {
@ -1997,7 +2001,7 @@ bool BlockBasedTable::PrefixMayMatch(
// is the only on could potentially contain the prefix. // is the only on could potentially contain the prefix.
BlockHandle handle = iiter->value().handle; BlockHandle handle = iiter->value().handle;
may_match = filter->PrefixMayMatch( may_match = filter->PrefixMayMatch(
prefix, prefix_extractor, handle.offset(), /*no_io=*/false, prefix, prefix_extractor, handle.offset(), no_io,
/*const_key_ptr=*/nullptr, /*get_context=*/nullptr, lookup_context); /*const_key_ptr=*/nullptr, /*get_context=*/nullptr, lookup_context);
} }
} }

3
table/block_based/filter_block.h
View File

@ -161,13 +161,14 @@ class FilterBlockReader {
const Comparator* /*comparator*/, const Comparator* /*comparator*/,
const Slice* const const_ikey_ptr, const Slice* const const_ikey_ptr,
bool* filter_checked, bool need_upper_bound_check, bool* filter_checked, bool need_upper_bound_check,
bool no_io,
BlockCacheLookupContext* lookup_context) { BlockCacheLookupContext* lookup_context) {
if (need_upper_bound_check) { if (need_upper_bound_check) {
return true; return true;
} }
*filter_checked = true; *filter_checked = true;
Slice prefix = prefix_extractor->Transform(user_key); Slice prefix = prefix_extractor->Transform(user_key);
return PrefixMayMatch(prefix, prefix_extractor, kNotValid, false, return PrefixMayMatch(prefix, prefix_extractor, kNotValid, no_io,
const_ikey_ptr, /* get_context */ nullptr, const_ikey_ptr, /* get_context */ nullptr,
lookup_context); lookup_context);
} }

5
table/block_based/full_filter_block.cc
View File

@ -285,7 +285,8 @@ bool FullFilterBlockReader::RangeMayExist(
const Slice* iterate_upper_bound, const Slice& user_key, const Slice* iterate_upper_bound, const Slice& user_key,
const SliceTransform* prefix_extractor, const Comparator* comparator, const SliceTransform* prefix_extractor, const Comparator* comparator,
const Slice* const const_ikey_ptr, bool* filter_checked, const Slice* const const_ikey_ptr, bool* filter_checked,
bool need_upper_bound_check, BlockCacheLookupContext* lookup_context) { bool need_upper_bound_check, bool no_io,
BlockCacheLookupContext* lookup_context) {
if (!prefix_extractor || !prefix_extractor->InDomain(user_key)) { if (!prefix_extractor || !prefix_extractor->InDomain(user_key)) {
*filter_checked = false; *filter_checked = false;
return true; return true;
@ -297,7 +298,7 @@ bool FullFilterBlockReader::RangeMayExist(
return true; return true;
} else { } else {
*filter_checked = true; *filter_checked = true;
return PrefixMayMatch(prefix, prefix_extractor, kNotValid, false, return PrefixMayMatch(prefix, prefix_extractor, kNotValid, no_io,
const_ikey_ptr, /* get_context */ nullptr, const_ikey_ptr, /* get_context */ nullptr,
lookup_context); lookup_context);
} }

2
table/block_based/full_filter_block.h
View File

@ -119,7 +119,7 @@ class FullFilterBlockReader
const SliceTransform* prefix_extractor, const SliceTransform* prefix_extractor,
const Comparator* comparator, const Comparator* comparator,
const Slice* const const_ikey_ptr, bool* filter_checked, const Slice* const const_ikey_ptr, bool* filter_checked,
bool need_upper_bound_check, bool need_upper_bound_check, bool no_io,
BlockCacheLookupContext* lookup_context) override; BlockCacheLookupContext* lookup_context) override;
private: private: