From 68af7811eae8891b695a3113193186109d47a7c1 Mon Sep 17 00:00:00 2001 From: sdong Date: Wed, 4 Feb 2015 17:03:57 -0800 Subject: [PATCH] Remember whole key/prefix filtering on/off in SST file Summary: Remember whole key or prefix filtering on/off in SST files. If user opens the DB with a different setting that cannot be satisfied while reading the SST file, ignore the bloom filter. Test Plan: Add a unit test for it Reviewers: yhchiang, igor, rven Reviewed By: rven Subscribers: leveldb, dhruba Differential Revision: https://reviews.facebook.net/D32889 --- HISTORY.md | 1 + db/db_test.cc | 149 +++++++++++++++++++++++++ include/rocksdb/table.h | 4 + table/block_based_filter_block.cc | 5 +- table/block_based_filter_block.h | 1 + table/block_based_filter_block_test.cc | 15 ++- table/block_based_table_builder.cc | 22 +++- table/block_based_table_factory.cc | 6 + table/block_based_table_factory.h | 2 + table/block_based_table_reader.cc | 56 ++++++++-- table/full_filter_block.cc | 21 ++-- table/full_filter_block.h | 6 +- table/full_filter_block_test.cc | 28 +++-- 13 files changed, 266 insertions(+), 50 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index bef3e1ff1..b5718cfaa 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -14,6 +14,7 @@ * MemEnv (env that stores data in memory) is now available in default library build. You can create it by calling NewMemEnv(). * Add SliceTransform.SameResultWhenAppended() to help users determine it is safe to apply prefix bloom/hash. * Block based table now makes use of prefix bloom filter if it is a full fulter. +* Block based table remembers whether a whole key or prefix based bloom filter is supported in SST files. Do a sanity check when reading the file with users' configuration. ### Public API changes * Deprecated skip_log_error_on_recovery option diff --git a/db/db_test.cc b/db/db_test.cc index b4e0a46d0..0431964df 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -2018,6 +2018,155 @@ TEST(DBTest, GetFilterByPrefixBloom) { ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 2); } +TEST(DBTest, WholeKeyFilterProp) { + Options options = last_options_; + options.prefix_extractor.reset(NewFixedPrefixTransform(3)); + options.statistics = rocksdb::CreateDBStatistics(); + + BlockBasedTableOptions bbto; + bbto.filter_policy.reset(NewBloomFilterPolicy(10, false)); + bbto.whole_key_filtering = false; + options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + DestroyAndReopen(options); + + WriteOptions wo; + ReadOptions ro; + FlushOptions fo; + fo.wait = true; + std::string value; + + ASSERT_OK(dbfull()->Put(wo, "foobar", "foo")); + // Needs insert some keys to make sure files are not filtered out by key + // ranges. + ASSERT_OK(dbfull()->Put(wo, "aaa", "")); + ASSERT_OK(dbfull()->Put(wo, "zzz", "")); + dbfull()->Flush(fo); + + Reopen(options); + ASSERT_EQ("NOT_FOUND", Get("foo")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 0); + ASSERT_EQ("NOT_FOUND", Get("bar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); + ASSERT_EQ("foo", Get("foobar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); + + // Reopen with whole key filtering enabled and prefix extractor + // NULL. Bloom filter should be off for both of whole key and + // prefix bloom. + bbto.whole_key_filtering = true; + options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + options.prefix_extractor.reset(); + Reopen(options); + + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); + ASSERT_EQ("NOT_FOUND", Get("foo")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); + ASSERT_EQ("NOT_FOUND", Get("bar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); + ASSERT_EQ("foo", Get("foobar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); + // Write DB with only full key filtering. + ASSERT_OK(dbfull()->Put(wo, "foobar", "foo")); + // Needs insert some keys to make sure files are not filtered out by key + // ranges. + ASSERT_OK(dbfull()->Put(wo, "aaa", "")); + ASSERT_OK(dbfull()->Put(wo, "zzz", "")); + db_->CompactRange(nullptr, nullptr); + + // Reopen with both of whole key off and prefix extractor enabled. + // Still no bloom filter should be used. + options.prefix_extractor.reset(NewFixedPrefixTransform(3)); + bbto.whole_key_filtering = false; + options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + Reopen(options); + + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); + ASSERT_EQ("NOT_FOUND", Get("foo")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); + ASSERT_EQ("NOT_FOUND", Get("bar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); + ASSERT_EQ("foo", Get("foobar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); + + // Try to create a DB with mixed files: + ASSERT_OK(dbfull()->Put(wo, "foobar", "foo")); + // Needs insert some keys to make sure files are not filtered out by key + // ranges. + ASSERT_OK(dbfull()->Put(wo, "aaa", "")); + ASSERT_OK(dbfull()->Put(wo, "zzz", "")); + db_->CompactRange(nullptr, nullptr); + + options.prefix_extractor.reset(); + bbto.whole_key_filtering = true; + options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + Reopen(options); + + // Try to create a DB with mixed files. + ASSERT_OK(dbfull()->Put(wo, "barfoo", "bar")); + // In this case needs insert some keys to make sure files are + // not filtered out by key ranges. + ASSERT_OK(dbfull()->Put(wo, "aaa", "")); + ASSERT_OK(dbfull()->Put(wo, "zzz", "")); + Flush(); + + // Now we have two files: + // File 1: An older file with prefix bloom. + // File 2: A newer file with whole bloom filter. + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); + ASSERT_EQ("NOT_FOUND", Get("foo")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 2); + ASSERT_EQ("NOT_FOUND", Get("bar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 3); + ASSERT_EQ("foo", Get("foobar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 4); + ASSERT_EQ("bar", Get("barfoo")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 4); + + // Reopen with the same setting: only whole key is used + Reopen(options); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 4); + ASSERT_EQ("NOT_FOUND", Get("foo")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 5); + ASSERT_EQ("NOT_FOUND", Get("bar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 6); + ASSERT_EQ("foo", Get("foobar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 7); + ASSERT_EQ("bar", Get("barfoo")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 7); + + // Restart with both filters are allowed + options.prefix_extractor.reset(NewFixedPrefixTransform(3)); + bbto.whole_key_filtering = true; + options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + Reopen(options); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 7); + // File 1 will has it filtered out. + // File 2 will not, as prefix `foo` exists in the file. + ASSERT_EQ("NOT_FOUND", Get("foo")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 8); + ASSERT_EQ("NOT_FOUND", Get("bar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 10); + ASSERT_EQ("foo", Get("foobar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 11); + ASSERT_EQ("bar", Get("barfoo")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 11); + + // Restart with only prefix bloom is allowed. + options.prefix_extractor.reset(NewFixedPrefixTransform(3)); + bbto.whole_key_filtering = false; + options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + Reopen(options); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 11); + ASSERT_EQ("NOT_FOUND", Get("foo")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 11); + ASSERT_EQ("NOT_FOUND", Get("bar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 12); + ASSERT_EQ("foo", Get("foobar")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 12); + ASSERT_EQ("bar", Get("barfoo")); + ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 12); +} + TEST(DBTest, IterSeekBeforePrev) { ASSERT_OK(Put("a", "b")); ASSERT_OK(Put("c", "d")); diff --git a/include/rocksdb/table.h b/include/rocksdb/table.h index b67eeffef..655b3926e 100644 --- a/include/rocksdb/table.h +++ b/include/rocksdb/table.h @@ -147,6 +147,10 @@ struct BlockBasedTableOptions { struct BlockBasedTablePropertyNames { // value of this propertis is a fixed int32 number. static const std::string kIndexType; + // value is "1" for true and "0" for false. + static const std::string kWholeKeyFiltering; + // value is "1" for true and "0" for false. + static const std::string kPrefixFiltering; }; // Create default block based table factory. diff --git a/table/block_based_filter_block.cc b/table/block_based_filter_block.cc index 7037d85bc..8cf3a855c 100644 --- a/table/block_based_filter_block.cc +++ b/table/block_based_filter_block.cc @@ -171,10 +171,11 @@ void BlockBasedFilterBlockBuilder::GenerateFilter() { BlockBasedFilterBlockReader::BlockBasedFilterBlockReader( const SliceTransform* prefix_extractor, - const BlockBasedTableOptions& table_opt, BlockContents&& contents) + const BlockBasedTableOptions& table_opt, bool whole_key_filtering, + BlockContents&& contents) : policy_(table_opt.filter_policy.get()), prefix_extractor_(prefix_extractor), - whole_key_filtering_(table_opt.whole_key_filtering), + whole_key_filtering_(whole_key_filtering), data_(nullptr), offset_(nullptr), num_(0), diff --git a/table/block_based_filter_block.h b/table/block_based_filter_block.h index cf8c1b47c..d339ac68a 100644 --- a/table/block_based_filter_block.h +++ b/table/block_based_filter_block.h @@ -74,6 +74,7 @@ class BlockBasedFilterBlockReader : public FilterBlockReader { // REQUIRES: "contents" and *policy must stay live while *this is live. BlockBasedFilterBlockReader(const SliceTransform* prefix_extractor, const BlockBasedTableOptions& table_opt, + bool whole_key_filtering, BlockContents&& contents); virtual bool IsBlockBased() override { return true; } virtual bool KeyMayMatch(const Slice& key, diff --git a/table/block_based_filter_block_test.cc b/table/block_based_filter_block_test.cc index 28eea16ce..319e5920d 100644 --- a/table/block_based_filter_block_test.cc +++ b/table/block_based_filter_block_test.cc @@ -57,7 +57,8 @@ TEST(FilterBlockTest, EmptyBuilder) { BlockBasedFilterBlockBuilder builder(nullptr, table_options_); BlockContents block(builder.Finish(), false, kNoCompression); ASSERT_EQ("\\x00\\x00\\x00\\x00\\x0b", EscapeString(block.data)); - BlockBasedFilterBlockReader reader(nullptr, table_options_, std::move(block)); + BlockBasedFilterBlockReader reader(nullptr, table_options_, true, + std::move(block)); ASSERT_TRUE(reader.KeyMayMatch("foo", 0)); ASSERT_TRUE(reader.KeyMayMatch("foo", 100000)); } @@ -73,7 +74,8 @@ TEST(FilterBlockTest, SingleChunk) { builder.StartBlock(300); builder.Add("hello"); BlockContents block(builder.Finish(), false, kNoCompression); - BlockBasedFilterBlockReader reader(nullptr, table_options_, std::move(block)); + BlockBasedFilterBlockReader reader(nullptr, table_options_, true, + std::move(block)); ASSERT_TRUE(reader.KeyMayMatch("foo", 100)); ASSERT_TRUE(reader.KeyMayMatch("bar", 100)); ASSERT_TRUE(reader.KeyMayMatch("box", 100)); @@ -104,7 +106,8 @@ TEST(FilterBlockTest, MultiChunk) { builder.Add("hello"); BlockContents block(builder.Finish(), false, kNoCompression); - BlockBasedFilterBlockReader reader(nullptr, table_options_, std::move(block)); + BlockBasedFilterBlockReader reader(nullptr, table_options_, true, + std::move(block)); // Check first filter ASSERT_TRUE(reader.KeyMayMatch("foo", 0)); @@ -150,7 +153,7 @@ TEST(BlockBasedFilterBlockTest, BlockBasedEmptyBuilder) { BlockContents block(builder->Finish(), false, kNoCompression); ASSERT_EQ("\\x00\\x00\\x00\\x00\\x0b", EscapeString(block.data)); FilterBlockReader* reader = new BlockBasedFilterBlockReader( - nullptr, table_options_, std::move(block)); + nullptr, table_options_, true, std::move(block)); ASSERT_TRUE(reader->KeyMayMatch("foo", 0)); ASSERT_TRUE(reader->KeyMayMatch("foo", 100000)); @@ -171,7 +174,7 @@ TEST(BlockBasedFilterBlockTest, BlockBasedSingleChunk) { builder->Add("hello"); BlockContents block(builder->Finish(), false, kNoCompression); FilterBlockReader* reader = new BlockBasedFilterBlockReader( - nullptr, table_options_, std::move(block)); + nullptr, table_options_, true, std::move(block)); ASSERT_TRUE(reader->KeyMayMatch("foo", 100)); ASSERT_TRUE(reader->KeyMayMatch("bar", 100)); ASSERT_TRUE(reader->KeyMayMatch("box", 100)); @@ -207,7 +210,7 @@ TEST(BlockBasedFilterBlockTest, BlockBasedMultiChunk) { BlockContents block(builder->Finish(), false, kNoCompression); FilterBlockReader* reader = new BlockBasedFilterBlockReader( - nullptr, table_options_, std::move(block)); + nullptr, table_options_, true, std::move(block)); // Check first filter ASSERT_TRUE(reader->KeyMayMatch("foo", 0)); diff --git a/table/block_based_table_builder.cc b/table/block_based_table_builder.cc index 813f8a125..19a11312e 100644 --- a/table/block_based_table_builder.cc +++ b/table/block_based_table_builder.cc @@ -33,6 +33,7 @@ #include "table/block_builder.h" #include "table/filter_block.h" #include "table/block_based_filter_block.h" +#include "table/block_based_table_factory.h" #include "table/full_filter_block.h" #include "table/format.h" #include "table/meta_blocks.h" @@ -292,7 +293,8 @@ FilterBlockBuilder* CreateFilterBlockBuilder(const ImmutableCFOptions& opt, if (filter_bits_builder == nullptr) { return new BlockBasedFilterBlockBuilder(opt.prefix_extractor, table_opt); } else { - return new FullFilterBlockBuilder(opt.prefix_extractor, table_opt, + return new FullFilterBlockBuilder(opt.prefix_extractor, + table_opt.whole_key_filtering, filter_bits_builder); } } @@ -387,8 +389,11 @@ class BlockBasedTableBuilder::BlockBasedTablePropertiesCollector : public TablePropertiesCollector { public: explicit BlockBasedTablePropertiesCollector( - BlockBasedTableOptions::IndexType index_type) - : index_type_(index_type) {} + BlockBasedTableOptions::IndexType index_type, bool whole_key_filtering, + bool prefix_filtering) + : index_type_(index_type), + whole_key_filtering_(whole_key_filtering), + prefix_filtering_(prefix_filtering) {} virtual Status Add(const Slice& key, const Slice& value) { // Intentionally left blank. Have no interest in collecting stats for @@ -400,6 +405,10 @@ class BlockBasedTableBuilder::BlockBasedTablePropertiesCollector std::string val; PutFixed32(&val, static_cast(index_type_)); properties->insert({BlockBasedTablePropertyNames::kIndexType, val}); + properties->insert({BlockBasedTablePropertyNames::kWholeKeyFiltering, + whole_key_filtering_ ? kPropTrue : kPropFalse}); + properties->insert({BlockBasedTablePropertyNames::kPrefixFiltering, + prefix_filtering_ ? kPropTrue : kPropFalse}); return Status::OK(); } @@ -415,6 +424,8 @@ class BlockBasedTableBuilder::BlockBasedTablePropertiesCollector private: BlockBasedTableOptions::IndexType index_type_; + bool whole_key_filtering_; + bool prefix_filtering_; }; struct BlockBasedTableBuilder::Rep { @@ -473,7 +484,9 @@ struct BlockBasedTableBuilder::Rep { collector_factories->CreateTablePropertiesCollector()); } table_properties_collectors.emplace_back( - new BlockBasedTablePropertiesCollector(table_options.index_type)); + new BlockBasedTablePropertiesCollector( + table_options.index_type, table_options.whole_key_filtering, + _ioptions.prefix_extractor != nullptr)); } }; @@ -851,5 +864,4 @@ uint64_t BlockBasedTableBuilder::FileSize() const { const std::string BlockBasedTable::kFilterBlockPrefix = "filter."; const std::string BlockBasedTable::kFullFilterBlockPrefix = "fullfilter."; - } // namespace rocksdb diff --git a/table/block_based_table_factory.cc b/table/block_based_table_factory.cc index 17ee0b8cb..6a80559e2 100644 --- a/table/block_based_table_factory.cc +++ b/table/block_based_table_factory.cc @@ -158,8 +158,14 @@ TableFactory* NewBlockBasedTableFactory( const std::string BlockBasedTablePropertyNames::kIndexType = "rocksdb.block.based.table.index.type"; +const std::string BlockBasedTablePropertyNames::kWholeKeyFiltering = + "rocksdb.block.based.table.whole.key.filtering"; +const std::string BlockBasedTablePropertyNames::kPrefixFiltering = + "rocksdb.block.based.table.prefix.filtering"; const std::string kHashIndexPrefixesBlock = "rocksdb.hashindex.prefixes"; const std::string kHashIndexPrefixesMetadataBlock = "rocksdb.hashindex.metadata"; +const std::string kPropTrue = "1"; +const std::string kPropFalse = "0"; } // namespace rocksdb diff --git a/table/block_based_table_factory.h b/table/block_based_table_factory.h index 674289779..aad9053c6 100644 --- a/table/block_based_table_factory.h +++ b/table/block_based_table_factory.h @@ -59,5 +59,7 @@ class BlockBasedTableFactory : public TableFactory { extern const std::string kHashIndexPrefixesBlock; extern const std::string kHashIndexPrefixesMetadataBlock; +extern const std::string kPropTrue; +extern const std::string kPropFalse; } // namespace rocksdb diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc index f03ab2b4b..abda6b8bf 100644 --- a/table/block_based_table_reader.cc +++ b/table/block_based_table_reader.cc @@ -27,6 +27,7 @@ #include "table/block.h" #include "table/filter_block.h" #include "table/block_based_filter_block.h" +#include "table/block_based_table_factory.h" #include "table/full_filter_block.h" #include "table/block_hash_index.h" #include "table/block_prefix_index.h" @@ -324,7 +325,9 @@ struct BlockBasedTable::Rep { env_options(_env_options), table_options(_table_opt), filter_policy(_table_opt.filter_policy.get()), - internal_comparator(_internal_comparator) {} + internal_comparator(_internal_comparator), + whole_key_filtering(_table_opt.whole_key_filtering), + prefix_filtering(true) {} const ImmutableCFOptions& ioptions; const EnvOptions& env_options; @@ -349,6 +352,8 @@ struct BlockBasedTable::Rep { std::shared_ptr table_properties; BlockBasedTableOptions::IndexType index_type; bool hash_index_allow_collision; + bool whole_key_filtering; + bool prefix_filtering; // TODO(kailiu) It is very ugly to use internal key in table, since table // module should not be relying on db module. However to make things easier // and compatible with existing code, we introduce a wrapper that allows @@ -427,6 +432,27 @@ void BlockBasedTable::GenerateCachePrefix(Cache* cc, } } +namespace { +// Return True if table_properties has `user_prop_name` has a `true` value +// or it doesn't contain this property (for backward compatible). +bool IsFeatureSupported(const TableProperties& table_properties, + const std::string& user_prop_name, Logger* info_log) { + auto& props = table_properties.user_collected_properties; + auto pos = props.find(user_prop_name); + // Older version doesn't have this value set. Skip this check. + if (pos != props.end()) { + if (pos->second == kPropFalse) { + return false; + } else if (pos->second != kPropTrue) { + Log(InfoLogLevel::WARN_LEVEL, info_log, + "Property %s has invalidate value %s", user_prop_name.c_str(), + pos->second.c_str()); + } + } + return true; +} +} // namespace + Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions, const EnvOptions& env_options, const BlockBasedTableOptions& table_options, @@ -496,6 +522,17 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions, "Cannot find Properties block from file."); } + // Determine whether whole key filtering is supported. + if (rep->table_properties) { + rep->whole_key_filtering &= + IsFeatureSupported(*(rep->table_properties), + BlockBasedTablePropertyNames::kWholeKeyFiltering, + rep->ioptions.info_log); + rep->prefix_filtering &= IsFeatureSupported( + *(rep->table_properties), + BlockBasedTablePropertyNames::kPrefixFiltering, rep->ioptions.info_log); + } + // Will use block cache for index/filter blocks access? if (table_options.cache_index_and_filter_blocks) { assert(table_options.block_cache != nullptr); @@ -748,16 +785,15 @@ FilterBlockReader* BlockBasedTable::ReadFilter( assert(rep->filter_policy); if (kFilterBlockPrefix == prefix) { return new BlockBasedFilterBlockReader( - rep->ioptions.prefix_extractor, rep->table_options, - std::move(block)); + rep->prefix_filtering ? rep->ioptions.prefix_extractor : nullptr, + rep->table_options, rep->whole_key_filtering, std::move(block)); } else if (kFullFilterBlockPrefix == prefix) { auto filter_bits_reader = rep->filter_policy-> GetFilterBitsReader(block.data); if (filter_bits_reader != nullptr) { - return new FullFilterBlockReader(rep->ioptions.prefix_extractor, - rep->table_options, - std::move(block), - filter_bits_reader); + return new FullFilterBlockReader( + rep->prefix_filtering ? rep->ioptions.prefix_extractor : nullptr, + rep->whole_key_filtering, std::move(block), filter_bits_reader); } } else { assert(false); @@ -1403,9 +1439,9 @@ Status BlockBasedTable::DumpTable(WritableFile* out_file) { BlockContents block; if (ReadBlockContents(rep_->file.get(), rep_->footer, ReadOptions(), handle, &block, rep_->ioptions.env, false).ok()) { - rep_->filter.reset( - new BlockBasedFilterBlockReader(rep_->ioptions.prefix_extractor, - table_options, std::move(block))); + rep_->filter.reset(new BlockBasedFilterBlockReader( + rep_->ioptions.prefix_extractor, table_options, + table_options.whole_key_filtering, std::move(block))); } } } diff --git a/table/full_filter_block.cc b/table/full_filter_block.cc index 4113ec57a..b3afdac03 100644 --- a/table/full_filter_block.cc +++ b/table/full_filter_block.cc @@ -12,11 +12,10 @@ namespace rocksdb { FullFilterBlockBuilder::FullFilterBlockBuilder( - const SliceTransform* prefix_extractor, - const BlockBasedTableOptions& table_opt, + const SliceTransform* prefix_extractor, bool whole_key_filtering, FilterBitsBuilder* filter_bits_builder) : prefix_extractor_(prefix_extractor), - whole_key_filtering_(table_opt.whole_key_filtering), + whole_key_filtering_(whole_key_filtering), num_added_(0) { assert(filter_bits_builder != nullptr); filter_bits_builder_.reset(filter_bits_builder); @@ -53,22 +52,20 @@ Slice FullFilterBlockBuilder::Finish() { } FullFilterBlockReader::FullFilterBlockReader( - const SliceTransform* prefix_extractor, - const BlockBasedTableOptions& table_opt, const Slice& contents, - FilterBitsReader* filter_bits_reader) + const SliceTransform* prefix_extractor, bool whole_key_filtering, + const Slice& contents, FilterBitsReader* filter_bits_reader) : prefix_extractor_(prefix_extractor), - whole_key_filtering_(table_opt.whole_key_filtering), + whole_key_filtering_(whole_key_filtering), contents_(contents) { assert(filter_bits_reader != nullptr); filter_bits_reader_.reset(filter_bits_reader); } FullFilterBlockReader::FullFilterBlockReader( - const SliceTransform* prefix_extractor, - const BlockBasedTableOptions& table_opt, BlockContents&& contents, - FilterBitsReader* filter_bits_reader) - : FullFilterBlockReader(prefix_extractor, table_opt, contents.data, - filter_bits_reader) { + const SliceTransform* prefix_extractor, bool whole_key_filtering, + BlockContents&& contents, FilterBitsReader* filter_bits_reader) + : FullFilterBlockReader(prefix_extractor, whole_key_filtering, + contents.data, filter_bits_reader) { block_contents_ = std::move(contents); } diff --git a/table/full_filter_block.h b/table/full_filter_block.h index 6d6294cf2..1ecc07a01 100644 --- a/table/full_filter_block.h +++ b/table/full_filter_block.h @@ -36,7 +36,7 @@ class FilterBitsReader; class FullFilterBlockBuilder : public FilterBlockBuilder { public: explicit FullFilterBlockBuilder(const SliceTransform* prefix_extractor, - const BlockBasedTableOptions& table_opt, + bool whole_key_filtering, FilterBitsBuilder* filter_bits_builder); // bits_builder is created in filter_policy, it should be passed in here // directly. and be deleted here @@ -73,11 +73,11 @@ class FullFilterBlockReader : public FilterBlockReader { // REQUIRES: "contents" and filter_bits_reader must stay live // while *this is live. explicit FullFilterBlockReader(const SliceTransform* prefix_extractor, - const BlockBasedTableOptions& table_opt, + bool whole_key_filtering, const Slice& contents, FilterBitsReader* filter_bits_reader); explicit FullFilterBlockReader(const SliceTransform* prefix_extractor, - const BlockBasedTableOptions& table_opt, + bool whole_key_filtering, BlockContents&& contents, FilterBitsReader* filter_bits_reader); diff --git a/table/full_filter_block_test.cc b/table/full_filter_block_test.cc index 7adb5f08d..1226242fc 100644 --- a/table/full_filter_block_test.cc +++ b/table/full_filter_block_test.cc @@ -103,27 +103,29 @@ class PluginFullFilterBlockTest { }; TEST(PluginFullFilterBlockTest, PluginEmptyBuilder) { - FullFilterBlockBuilder builder(nullptr, table_options_, - table_options_.filter_policy->GetFilterBitsBuilder()); + FullFilterBlockBuilder builder( + nullptr, true, table_options_.filter_policy->GetFilterBitsBuilder()); Slice block = builder.Finish(); ASSERT_EQ("", EscapeString(block)); - FullFilterBlockReader reader(nullptr, table_options_, block, + FullFilterBlockReader reader( + nullptr, true, block, table_options_.filter_policy->GetFilterBitsReader(block)); // Remain same symantic with blockbased filter ASSERT_TRUE(reader.KeyMayMatch("foo")); } TEST(PluginFullFilterBlockTest, PluginSingleChunk) { - FullFilterBlockBuilder builder(nullptr, table_options_, - table_options_.filter_policy->GetFilterBitsBuilder()); + FullFilterBlockBuilder builder( + nullptr, true, table_options_.filter_policy->GetFilterBitsBuilder()); builder.Add("foo"); builder.Add("bar"); builder.Add("box"); builder.Add("box"); builder.Add("hello"); Slice block = builder.Finish(); - FullFilterBlockReader reader(nullptr, table_options_, block, + FullFilterBlockReader reader( + nullptr, true, block, table_options_.filter_policy->GetFilterBitsReader(block)); ASSERT_TRUE(reader.KeyMayMatch("foo")); ASSERT_TRUE(reader.KeyMayMatch("bar")); @@ -146,27 +148,29 @@ class FullFilterBlockTest { }; TEST(FullFilterBlockTest, EmptyBuilder) { - FullFilterBlockBuilder builder(nullptr, table_options_, - table_options_.filter_policy->GetFilterBitsBuilder()); + FullFilterBlockBuilder builder( + nullptr, true, table_options_.filter_policy->GetFilterBitsBuilder()); Slice block = builder.Finish(); ASSERT_EQ("", EscapeString(block)); - FullFilterBlockReader reader(nullptr, table_options_, block, + FullFilterBlockReader reader( + nullptr, true, block, table_options_.filter_policy->GetFilterBitsReader(block)); // Remain same symantic with blockbased filter ASSERT_TRUE(reader.KeyMayMatch("foo")); } TEST(FullFilterBlockTest, SingleChunk) { - FullFilterBlockBuilder builder(nullptr, table_options_, - table_options_.filter_policy->GetFilterBitsBuilder()); + FullFilterBlockBuilder builder( + nullptr, true, table_options_.filter_policy->GetFilterBitsBuilder()); builder.Add("foo"); builder.Add("bar"); builder.Add("box"); builder.Add("box"); builder.Add("hello"); Slice block = builder.Finish(); - FullFilterBlockReader reader(nullptr, table_options_, block, + FullFilterBlockReader reader( + nullptr, true, block, table_options_.filter_policy->GetFilterBitsReader(block)); ASSERT_TRUE(reader.KeyMayMatch("foo")); ASSERT_TRUE(reader.KeyMayMatch("bar"));