Skip to content

Commit

Permalink
dont skip IO for filter blocks
Browse files Browse the repository at this point in the history
Summary:
Based on my experience with linkbench, We should not skip loading bloom filter blocks when they are not available in block cache when using Iterator::Seek

Actually I am not sure why this behavior existed in the first place
Closes facebook#2255

Differential Revision: D5010721

Pulled By: maysamyabandeh

fbshipit-source-id: 0af545a06ac4baeecb248706ec34d009c2480ca4
  • Loading branch information
IslamAbdelRahman authored and facebook-github-bot committed May 9, 2017
1 parent 3f73d54 commit 4897eb2
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 25 deletions.
27 changes: 5 additions & 22 deletions db/db_bloom_filter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -798,36 +798,19 @@ TEST_P(BloomStatsTestWithParam, BloomStatsTestWithIter) {
ASSERT_OK(iter->status());
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(value1, iter->value().ToString());
if (partition_filters_) {
ASSERT_EQ(0, perf_context.bloom_sst_hit_count); // no_io
ASSERT_EQ(0, perf_context.bloom_sst_miss_count); // no_io
} else {
ASSERT_EQ(1, perf_context.bloom_sst_hit_count);
}
ASSERT_EQ(1, perf_context.bloom_sst_hit_count);

iter->Seek(key3);
ASSERT_OK(iter->status());
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(value3, iter->value().ToString());
if (partition_filters_) {
ASSERT_EQ(0, perf_context.bloom_sst_hit_count); // no_io
ASSERT_EQ(0, perf_context.bloom_sst_miss_count); // no_io
} else {
ASSERT_EQ(2, perf_context.bloom_sst_hit_count);
}
ASSERT_EQ(2, perf_context.bloom_sst_hit_count);

iter->Seek(key2);
ASSERT_OK(iter->status());
if (partition_filters_) {
// iter is still valid since filter did not reject the key2
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(0, perf_context.bloom_sst_hit_count); // no_io
ASSERT_EQ(0, perf_context.bloom_sst_miss_count); // no_io
} else {
ASSERT_TRUE(!iter->Valid());
ASSERT_EQ(1, perf_context.bloom_sst_miss_count);
ASSERT_EQ(2, perf_context.bloom_sst_hit_count);
}
ASSERT_TRUE(!iter->Valid());
ASSERT_EQ(1, perf_context.bloom_sst_miss_count);
ASSERT_EQ(2, perf_context.bloom_sst_hit_count);
}

INSTANTIATE_TEST_CASE_P(BloomStatsTestWithParam, BloomStatsTestWithParam,
Expand Down
5 changes: 2 additions & 3 deletions table/block_based_table_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1477,14 +1477,13 @@ bool BlockBasedTable::PrefixMayMatch(const Slice& internal_key) {
Status s;

// First, try check with full filter
const bool no_io = true;
auto filter_entry = GetFilter(no_io);
auto filter_entry = GetFilter();
FilterBlockReader* filter = filter_entry.value;
if (filter != nullptr) {
if (!filter->IsBlockBased()) {
const Slice* const const_ikey_ptr = &internal_key;
may_match =
filter->PrefixMayMatch(prefix, kNotValid, no_io, const_ikey_ptr);
filter->PrefixMayMatch(prefix, kNotValid, false, const_ikey_ptr);
} else {
InternalKey internal_key_prefix(prefix, kMaxSequenceNumber, kTypeValue);
auto internal_prefix = internal_key_prefix.Encode();
Expand Down

0 comments on commit 4897eb2

Please sign in to comment.