From e0ad0f26b8cd54b7dce2d883f7d15068532f62e6 Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Thu, 14 Nov 2013 14:05:15 -0800 Subject: [PATCH] Fix bloom filters Summary: https://reviews.facebook.net/D13167 broke bloom filters. If filter is not in cache, we want to return true (safe thing). Am I right? Test Plan: when benchmarking https://reviews.facebook.net/D14031 I got different results when using bloom filters vs. when not using them. This fixed the issue. I will also be putting this change to the other diff, but that one will probably be in review for longer time. Reviewers: kailiu, dhruba, haobo Reviewed By: kailiu CC: leveldb Differential Revision: https://reviews.facebook.net/D14085 --- table/block_based_table_reader.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc index 54b75336c..4a45be635 100644 --- a/table/block_based_table_reader.cc +++ b/table/block_based_table_reader.cc @@ -904,7 +904,10 @@ bool BlockBasedTable::PrefixMayMatch(const Slice& internal_prefix) { if (!iiter->Valid()) { // we're past end of file - may_match = false; + // if it's incomplete, it means that we avoided I/O + // and we're not really sure that we're past the end + // of the file + may_match = iiter->status().IsIncomplete(); } else if (ExtractUserKey(iiter->key()).starts_with( ExtractUserKey(internal_prefix))) { // we need to check for this subtle case because our only @@ -930,7 +933,7 @@ bool BlockBasedTable::PrefixMayMatch(const Slice& internal_prefix) { assert(s.ok()); auto filter_entry = GetFilter(true /* no io */); may_match = - filter_entry.value != nullptr && + filter_entry.value == nullptr || filter_entry.value->PrefixMayMatch(handle.offset(), internal_prefix); filter_entry.Release(rep_->options.block_cache.get()); }