From 1d23b5c470844c1208301311f0889eca750431c0 Mon Sep 17 00:00:00 2001 From: Feng Zhu Date: Thu, 28 Aug 2014 17:06:29 -0700 Subject: [PATCH] remove_internal_filter_policy Summary: 1. remove class InternalFilterPolicy in db/dbformat.h 2. Transformation from internal key to user key is done in filter_block.cc 3. This is a preparation for patch D20979 Test Plan: make all check valgrind ./db_test Reviewers: igor, yhchiang, ljin, sdong Reviewed By: sdong Subscribers: leveldb Differential Revision: https://reviews.facebook.net/D22509 --- db/dbformat.cc | 20 -------------------- db/dbformat.h | 13 ------------- table/block_based_table_builder.cc | 2 +- table/block_based_table_factory.cc | 4 ---- table/block_based_table_reader.cc | 10 ++++------ table/filter_block.cc | 14 ++++---------- 6 files changed, 9 insertions(+), 54 deletions(-) diff --git a/db/dbformat.cc b/db/dbformat.cc index e53d16dc1..baeb86802 100644 --- a/db/dbformat.cc +++ b/db/dbformat.cc @@ -127,26 +127,6 @@ void InternalKeyComparator::FindShortSuccessor(std::string* key) const { } } -const char* InternalFilterPolicy::Name() const { - return user_policy_->Name(); -} - -void InternalFilterPolicy::CreateFilter(const Slice* keys, int n, - std::string* dst) const { - // We rely on the fact that the code in table.cc does not mind us - // adjusting keys[]. - Slice* mkey = const_cast(keys); - for (int i = 0; i < n; i++) { - mkey[i] = ExtractUserKey(keys[i]); - // TODO(sanjay): Suppress dups? - } - user_policy_->CreateFilter(keys, n, dst); -} - -bool InternalFilterPolicy::KeyMayMatch(const Slice& key, const Slice& f) const { - return user_policy_->KeyMayMatch(ExtractUserKey(key), f); -} - LookupKey::LookupKey(const Slice& user_key, SequenceNumber s) { size_t usize = user_key.size(); size_t needed = usize + 13; // A conservative estimate diff --git a/db/dbformat.h b/db/dbformat.h index 4dab5f196..eb5d8ed53 100644 --- a/db/dbformat.h +++ b/db/dbformat.h @@ -124,19 +124,6 @@ class InternalKeyComparator : public Comparator { int Compare(const ParsedInternalKey& a, const ParsedInternalKey& b) const; }; -// Filter policy wrapper that converts from internal keys to user keys -class InternalFilterPolicy : public FilterPolicy { - private: - std::shared_ptr shared_ptr_; - const FilterPolicy* const user_policy_; - public: - explicit InternalFilterPolicy(std::shared_ptr p) - : shared_ptr_(p), user_policy_(p.get()) {} - virtual const char* Name() const; - virtual void CreateFilter(const Slice* keys, int n, std::string* dst) const; - virtual bool KeyMayMatch(const Slice& key, const Slice& filter) const; -}; - // Modules in this directory should keep internal keys wrapped inside // the following class instead of plain strings so that we do not // incorrectly use string comparisons instead of an InternalKeyComparator. diff --git a/table/block_based_table_builder.cc b/table/block_based_table_builder.cc index 37b6f86fc..0e5ea0a69 100644 --- a/table/block_based_table_builder.cc +++ b/table/block_based_table_builder.cc @@ -492,7 +492,7 @@ void BlockBasedTableBuilder::Add(const Slice& key, const Slice& value) { } if (r->filter_block != nullptr) { - r->filter_block->AddKey(key); + r->filter_block->AddKey(ExtractUserKey(key)); } r->last_key.assign(key.data(), key.size()); diff --git a/table/block_based_table_factory.cc b/table/block_based_table_factory.cc index 0f7863e8d..de30fb383 100644 --- a/table/block_based_table_factory.cc +++ b/table/block_based_table_factory.cc @@ -38,10 +38,6 @@ BlockBasedTableFactory::BlockBasedTableFactory( table_options_.block_size_deviation > 100) { table_options_.block_size_deviation = 0; } - if (table_options_.filter_policy) { - auto* p = new InternalFilterPolicy(table_options_.filter_policy); - table_options_.filter_policy.reset(p); - } } Status BlockBasedTableFactory::NewTableReader( diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc index 3c0ef527e..0be38a1dc 100644 --- a/table/block_based_table_reader.cc +++ b/table/block_based_table_reader.cc @@ -1067,9 +1067,8 @@ bool BlockBasedTable::PrefixMayMatch(const Slice& internal_key) { s = handle.DecodeFrom(&handle_value); assert(s.ok()); auto filter_entry = GetFilter(true /* no io */); - may_match = - filter_entry.value == nullptr || - filter_entry.value->PrefixMayMatch(handle.offset(), internal_prefix); + may_match = filter_entry.value == nullptr || + filter_entry.value->PrefixMayMatch(handle.offset(), prefix); filter_entry.Release(rep_->table_options.block_cache.get()); } @@ -1105,9 +1104,8 @@ Status BlockBasedTable::Get( BlockHandle handle; bool may_not_exist_in_filter = - filter != nullptr && - handle.DecodeFrom(&handle_value).ok() && - !filter->KeyMayMatch(handle.offset(), key); + filter != nullptr && handle.DecodeFrom(&handle_value).ok() && + !filter->KeyMayMatch(handle.offset(), ExtractUserKey(key)); if (may_not_exist_in_filter) { // Not found diff --git a/table/filter_block.cc b/table/filter_block.cc index 8366db268..6b4ff1c10 100644 --- a/table/filter_block.cc +++ b/table/filter_block.cc @@ -71,20 +71,14 @@ void FilterBlockBuilder::AddKey(const Slice& key) { } // add prefix to filter if needed - if (prefix_extractor_ && prefix_extractor_->InDomain(ExtractUserKey(key))) { - // If prefix_extractor_, this filter_block layer assumes we only - // operate on internal keys. - Slice user_key = ExtractUserKey(key); + if (prefix_extractor_ && prefix_extractor_->InDomain(key)) { // this assumes prefix(prefix(key)) == prefix(key), as the last // entry in entries_ may be either a key or prefix, and we use // prefix(last entry) to get the prefix of the last key. - if (prev.size() == 0 || - !SamePrefix(user_key, ExtractUserKey(prev))) { - Slice prefix = prefix_extractor_->Transform(user_key); - InternalKey internal_prefix_tmp(prefix, 0, kTypeValue); - Slice internal_prefix = internal_prefix_tmp.Encode(); + if (prev.size() == 0 || !SamePrefix(key, prev)) { + Slice prefix = prefix_extractor_->Transform(key); start_.push_back(entries_.size()); - entries_.append(internal_prefix.data(), internal_prefix.size()); + entries_.append(prefix.data(), prefix.size()); } } }