From 7a6353bd1c516fe3f7118248c77035697c5ac247 Mon Sep 17 00:00:00 2001 From: Huachao Huang Date: Wed, 21 Mar 2018 22:56:48 -0700 Subject: [PATCH] Ignore empty filter block when data block is empty Summary: Close https://github.com/facebook/rocksdb/issues/3592 Closes https://github.com/facebook/rocksdb/pull/3614 Differential Revision: D7291706 Pulled By: ajkr fbshipit-source-id: 9dd8f40bd7716588e1e3fd6be0c2bc2766861f8c --- table/block_based_filter_block.cc | 7 ++++--- table/block_based_filter_block.h | 2 ++ table/block_based_filter_block_test.cc | 2 ++ table/block_based_table_builder.cc | 7 +++++-- table/filter_block.h | 1 + table/full_filter_block.h | 1 + table/full_filter_block_test.cc | 2 ++ table/partitioned_filter_block.cc | 4 +++- table/partitioned_filter_block.h | 4 ++++ 9 files changed, 24 insertions(+), 6 deletions(-) diff --git a/table/block_based_filter_block.cc b/table/block_based_filter_block.cc index 2121ff828..780f22195 100644 --- a/table/block_based_filter_block.cc +++ b/table/block_based_filter_block.cc @@ -67,7 +67,8 @@ BlockBasedFilterBlockBuilder::BlockBasedFilterBlockBuilder( prefix_extractor_(prefix_extractor), whole_key_filtering_(table_opt.whole_key_filtering), prev_prefix_start_(0), - prev_prefix_size_(0) { + prev_prefix_size_(0), + num_added_(0) { assert(policy_); } @@ -91,6 +92,7 @@ void BlockBasedFilterBlockBuilder::Add(const Slice& key) { // Add key to filter if needed inline void BlockBasedFilterBlockBuilder::AddKey(const Slice& key) { + num_added_++; start_.push_back(entries_.size()); entries_.append(key.data(), key.size()); } @@ -106,10 +108,9 @@ inline void BlockBasedFilterBlockBuilder::AddPrefix(const Slice& key) { Slice prefix = prefix_extractor_->Transform(key); // insert prefix only when it's different from the previous prefix. if (prev.size() == 0 || prefix != prev) { - start_.push_back(entries_.size()); + AddKey(prefix); prev_prefix_start_ = entries_.size(); prev_prefix_size_ = prefix.size(); - entries_.append(prefix.data(), prefix.size()); } } diff --git a/table/block_based_filter_block.h b/table/block_based_filter_block.h index 52b79fea5..3bfb3b24a 100644 --- a/table/block_based_filter_block.h +++ b/table/block_based_filter_block.h @@ -41,6 +41,7 @@ class BlockBasedFilterBlockBuilder : public FilterBlockBuilder { virtual bool IsBlockBased() override { return true; } virtual void StartBlock(uint64_t block_offset) override; virtual void Add(const Slice& key) override; + virtual size_t NumAdded() const override { return num_added_; } virtual Slice Finish(const BlockHandle& tmp, Status* status) override; using FilterBlockBuilder::Finish; @@ -65,6 +66,7 @@ class BlockBasedFilterBlockBuilder : public FilterBlockBuilder { std::string result_; // Filter data computed so far std::vector tmp_entries_; // policy_->CreateFilter() argument std::vector filter_offsets_; + size_t num_added_; // Number of keys added // No copying allowed BlockBasedFilterBlockBuilder(const BlockBasedFilterBlockBuilder&); diff --git a/table/block_based_filter_block_test.cc b/table/block_based_filter_block_test.cc index f666ba252..dece461e3 100644 --- a/table/block_based_filter_block_test.cc +++ b/table/block_based_filter_block_test.cc @@ -65,6 +65,7 @@ TEST_F(FilterBlockTest, EmptyBuilder) { TEST_F(FilterBlockTest, SingleChunk) { BlockBasedFilterBlockBuilder builder(nullptr, table_options_); + ASSERT_EQ(0, builder.NumAdded()); builder.StartBlock(100); builder.Add("foo"); builder.Add("bar"); @@ -73,6 +74,7 @@ TEST_F(FilterBlockTest, SingleChunk) { builder.Add("box"); builder.StartBlock(300); builder.Add("hello"); + ASSERT_EQ(5, builder.NumAdded()); BlockContents block(builder.Finish(), false, kNoCompression); BlockBasedFilterBlockReader reader(nullptr, table_options_, true, std::move(block), nullptr); diff --git a/table/block_based_table_builder.cc b/table/block_based_table_builder.cc index eaeca52ea..9908257b6 100644 --- a/table/block_based_table_builder.cc +++ b/table/block_based_table_builder.cc @@ -650,8 +650,11 @@ Status BlockBasedTableBuilder::Finish() { BlockHandle filter_block_handle, metaindex_block_handle, index_block_handle, compression_dict_block_handle, range_del_block_handle; + // Write filter block - if (ok() && r->filter_builder != nullptr) { + bool empty_filter_block = (r->filter_builder == nullptr || + r->filter_builder->NumAdded() == 0); + if (ok() && !empty_filter_block) { Status s = Status::Incomplete(); while (s.IsIncomplete()) { Slice filter_content = r->filter_builder->Finish(filter_block_handle, &s); @@ -687,7 +690,7 @@ Status BlockBasedTableBuilder::Finish() { } if (ok()) { - if (r->filter_builder != nullptr) { + if (!empty_filter_block) { // Add mapping from ".Name" to location // of filter data. std::string key; diff --git a/table/filter_block.h b/table/filter_block.h index feaf0c9f0..97f493fb7 100644 --- a/table/filter_block.h +++ b/table/filter_block.h @@ -51,6 +51,7 @@ class FilterBlockBuilder { virtual bool IsBlockBased() = 0; // If is blockbased filter virtual void StartBlock(uint64_t block_offset) = 0; // Start new block filter virtual void Add(const Slice& key) = 0; // Add a key to current filter + virtual size_t NumAdded() const = 0; // Number of keys added Slice Finish() { // Generate Filter const BlockHandle empty_handle; Status dont_care_status; diff --git a/table/full_filter_block.h b/table/full_filter_block.h index e161d079e..6aec5530a 100644 --- a/table/full_filter_block.h +++ b/table/full_filter_block.h @@ -45,6 +45,7 @@ class FullFilterBlockBuilder : public FilterBlockBuilder { virtual bool IsBlockBased() override { return false; } virtual void StartBlock(uint64_t /*block_offset*/) override {} virtual void Add(const Slice& key) override; + virtual size_t NumAdded() const override { return num_added_; } virtual Slice Finish(const BlockHandle& tmp, Status* status) override; using FilterBlockBuilder::Finish; diff --git a/table/full_filter_block_test.cc b/table/full_filter_block_test.cc index 5fbda4c6f..2e73612d1 100644 --- a/table/full_filter_block_test.cc +++ b/table/full_filter_block_test.cc @@ -163,11 +163,13 @@ TEST_F(FullFilterBlockTest, EmptyBuilder) { TEST_F(FullFilterBlockTest, SingleChunk) { FullFilterBlockBuilder builder( nullptr, true, table_options_.filter_policy->GetFilterBitsBuilder()); + ASSERT_EQ(0, builder.NumAdded()); builder.Add("foo"); builder.Add("bar"); builder.Add("box"); builder.Add("box"); builder.Add("hello"); + ASSERT_EQ(5, builder.NumAdded()); Slice block = builder.Finish(); FullFilterBlockReader reader( nullptr, true, block, diff --git a/table/partitioned_filter_block.cc b/table/partitioned_filter_block.cc index 2f7c67a99..146f3b3e4 100644 --- a/table/partitioned_filter_block.cc +++ b/table/partitioned_filter_block.cc @@ -25,7 +25,8 @@ PartitionedFilterBlockBuilder::PartitionedFilterBlockBuilder( filter_bits_builder), index_on_filter_block_builder_(index_block_restart_interval), p_index_builder_(p_index_builder), - filters_in_partition_(0) { + filters_in_partition_(0), + num_added_(0) { filters_per_partition_ = filter_bits_builder_->CalculateNumEntry(partition_size); } @@ -53,6 +54,7 @@ void PartitionedFilterBlockBuilder::AddKey(const Slice& key) { MaybeCutAFilterBlock(); filter_bits_builder_->AddKey(key); filters_in_partition_++; + num_added_++; } Slice PartitionedFilterBlockBuilder::Finish( diff --git a/table/partitioned_filter_block.h b/table/partitioned_filter_block.h index 1a00a86e6..fb7d7cd10 100644 --- a/table/partitioned_filter_block.h +++ b/table/partitioned_filter_block.h @@ -33,6 +33,8 @@ class PartitionedFilterBlockBuilder : public FullFilterBlockBuilder { void AddKey(const Slice& key) override; + size_t NumAdded() const override { return num_added_; } + virtual Slice Finish(const BlockHandle& last_partition_block_handle, Status* status) override; @@ -59,6 +61,8 @@ class PartitionedFilterBlockBuilder : public FullFilterBlockBuilder { uint32_t filters_per_partition_; // The current number of filters in the last partition uint32_t filters_in_partition_; + // Number of keys added + size_t num_added_; }; class PartitionedFilterBlockReader : public FilterBlockReader {