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
main
Huachao Huang 7 years ago committed by Facebook Github Bot
parent 70282cf876
commit 7a6353bd1c
  1. 7
      table/block_based_filter_block.cc
  2. 2
      table/block_based_filter_block.h
  3. 2
      table/block_based_filter_block_test.cc
  4. 7
      table/block_based_table_builder.cc
  5. 1
      table/filter_block.h
  6. 1
      table/full_filter_block.h
  7. 2
      table/full_filter_block_test.cc
  8. 4
      table/partitioned_filter_block.cc
  9. 4
      table/partitioned_filter_block.h

@ -67,7 +67,8 @@ BlockBasedFilterBlockBuilder::BlockBasedFilterBlockBuilder(
prefix_extractor_(prefix_extractor), prefix_extractor_(prefix_extractor),
whole_key_filtering_(table_opt.whole_key_filtering), whole_key_filtering_(table_opt.whole_key_filtering),
prev_prefix_start_(0), prev_prefix_start_(0),
prev_prefix_size_(0) { prev_prefix_size_(0),
num_added_(0) {
assert(policy_); assert(policy_);
} }
@ -91,6 +92,7 @@ void BlockBasedFilterBlockBuilder::Add(const Slice& key) {
// Add key to filter if needed // Add key to filter if needed
inline void BlockBasedFilterBlockBuilder::AddKey(const Slice& key) { inline void BlockBasedFilterBlockBuilder::AddKey(const Slice& key) {
num_added_++;
start_.push_back(entries_.size()); start_.push_back(entries_.size());
entries_.append(key.data(), key.size()); entries_.append(key.data(), key.size());
} }
@ -106,10 +108,9 @@ inline void BlockBasedFilterBlockBuilder::AddPrefix(const Slice& key) {
Slice prefix = prefix_extractor_->Transform(key); Slice prefix = prefix_extractor_->Transform(key);
// insert prefix only when it's different from the previous prefix. // insert prefix only when it's different from the previous prefix.
if (prev.size() == 0 || prefix != prev) { if (prev.size() == 0 || prefix != prev) {
start_.push_back(entries_.size()); AddKey(prefix);
prev_prefix_start_ = entries_.size(); prev_prefix_start_ = entries_.size();
prev_prefix_size_ = prefix.size(); prev_prefix_size_ = prefix.size();
entries_.append(prefix.data(), prefix.size());
} }
} }

@ -41,6 +41,7 @@ class BlockBasedFilterBlockBuilder : public FilterBlockBuilder {
virtual bool IsBlockBased() override { return true; } virtual bool IsBlockBased() override { return true; }
virtual void StartBlock(uint64_t block_offset) override; virtual void StartBlock(uint64_t block_offset) override;
virtual void Add(const Slice& key) 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; virtual Slice Finish(const BlockHandle& tmp, Status* status) override;
using FilterBlockBuilder::Finish; using FilterBlockBuilder::Finish;
@ -65,6 +66,7 @@ class BlockBasedFilterBlockBuilder : public FilterBlockBuilder {
std::string result_; // Filter data computed so far std::string result_; // Filter data computed so far
std::vector<Slice> tmp_entries_; // policy_->CreateFilter() argument std::vector<Slice> tmp_entries_; // policy_->CreateFilter() argument
std::vector<uint32_t> filter_offsets_; std::vector<uint32_t> filter_offsets_;
size_t num_added_; // Number of keys added
// No copying allowed // No copying allowed
BlockBasedFilterBlockBuilder(const BlockBasedFilterBlockBuilder&); BlockBasedFilterBlockBuilder(const BlockBasedFilterBlockBuilder&);

@ -65,6 +65,7 @@ TEST_F(FilterBlockTest, EmptyBuilder) {
TEST_F(FilterBlockTest, SingleChunk) { TEST_F(FilterBlockTest, SingleChunk) {
BlockBasedFilterBlockBuilder builder(nullptr, table_options_); BlockBasedFilterBlockBuilder builder(nullptr, table_options_);
ASSERT_EQ(0, builder.NumAdded());
builder.StartBlock(100); builder.StartBlock(100);
builder.Add("foo"); builder.Add("foo");
builder.Add("bar"); builder.Add("bar");
@ -73,6 +74,7 @@ TEST_F(FilterBlockTest, SingleChunk) {
builder.Add("box"); builder.Add("box");
builder.StartBlock(300); builder.StartBlock(300);
builder.Add("hello"); builder.Add("hello");
ASSERT_EQ(5, builder.NumAdded());
BlockContents block(builder.Finish(), false, kNoCompression); BlockContents block(builder.Finish(), false, kNoCompression);
BlockBasedFilterBlockReader reader(nullptr, table_options_, true, BlockBasedFilterBlockReader reader(nullptr, table_options_, true,
std::move(block), nullptr); std::move(block), nullptr);

@ -650,8 +650,11 @@ Status BlockBasedTableBuilder::Finish() {
BlockHandle filter_block_handle, metaindex_block_handle, index_block_handle, BlockHandle filter_block_handle, metaindex_block_handle, index_block_handle,
compression_dict_block_handle, range_del_block_handle; compression_dict_block_handle, range_del_block_handle;
// Write filter block // 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(); Status s = Status::Incomplete();
while (s.IsIncomplete()) { while (s.IsIncomplete()) {
Slice filter_content = r->filter_builder->Finish(filter_block_handle, &s); Slice filter_content = r->filter_builder->Finish(filter_block_handle, &s);
@ -687,7 +690,7 @@ Status BlockBasedTableBuilder::Finish() {
} }
if (ok()) { if (ok()) {
if (r->filter_builder != nullptr) { if (!empty_filter_block) {
// Add mapping from "<filter_block_prefix>.Name" to location // Add mapping from "<filter_block_prefix>.Name" to location
// of filter data. // of filter data.
std::string key; std::string key;

@ -51,6 +51,7 @@ class FilterBlockBuilder {
virtual bool IsBlockBased() = 0; // If is blockbased filter virtual bool IsBlockBased() = 0; // If is blockbased filter
virtual void StartBlock(uint64_t block_offset) = 0; // Start new block 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 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 Slice Finish() { // Generate Filter
const BlockHandle empty_handle; const BlockHandle empty_handle;
Status dont_care_status; Status dont_care_status;

@ -45,6 +45,7 @@ class FullFilterBlockBuilder : public FilterBlockBuilder {
virtual bool IsBlockBased() override { return false; } virtual bool IsBlockBased() override { return false; }
virtual void StartBlock(uint64_t /*block_offset*/) override {} virtual void StartBlock(uint64_t /*block_offset*/) override {}
virtual void Add(const Slice& key) 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; virtual Slice Finish(const BlockHandle& tmp, Status* status) override;
using FilterBlockBuilder::Finish; using FilterBlockBuilder::Finish;

@ -163,11 +163,13 @@ TEST_F(FullFilterBlockTest, EmptyBuilder) {
TEST_F(FullFilterBlockTest, SingleChunk) { TEST_F(FullFilterBlockTest, SingleChunk) {
FullFilterBlockBuilder builder( FullFilterBlockBuilder builder(
nullptr, true, table_options_.filter_policy->GetFilterBitsBuilder()); nullptr, true, table_options_.filter_policy->GetFilterBitsBuilder());
ASSERT_EQ(0, builder.NumAdded());
builder.Add("foo"); builder.Add("foo");
builder.Add("bar"); builder.Add("bar");
builder.Add("box"); builder.Add("box");
builder.Add("box"); builder.Add("box");
builder.Add("hello"); builder.Add("hello");
ASSERT_EQ(5, builder.NumAdded());
Slice block = builder.Finish(); Slice block = builder.Finish();
FullFilterBlockReader reader( FullFilterBlockReader reader(
nullptr, true, block, nullptr, true, block,

@ -25,7 +25,8 @@ PartitionedFilterBlockBuilder::PartitionedFilterBlockBuilder(
filter_bits_builder), filter_bits_builder),
index_on_filter_block_builder_(index_block_restart_interval), index_on_filter_block_builder_(index_block_restart_interval),
p_index_builder_(p_index_builder), p_index_builder_(p_index_builder),
filters_in_partition_(0) { filters_in_partition_(0),
num_added_(0) {
filters_per_partition_ = filters_per_partition_ =
filter_bits_builder_->CalculateNumEntry(partition_size); filter_bits_builder_->CalculateNumEntry(partition_size);
} }
@ -53,6 +54,7 @@ void PartitionedFilterBlockBuilder::AddKey(const Slice& key) {
MaybeCutAFilterBlock(); MaybeCutAFilterBlock();
filter_bits_builder_->AddKey(key); filter_bits_builder_->AddKey(key);
filters_in_partition_++; filters_in_partition_++;
num_added_++;
} }
Slice PartitionedFilterBlockBuilder::Finish( Slice PartitionedFilterBlockBuilder::Finish(

@ -33,6 +33,8 @@ class PartitionedFilterBlockBuilder : public FullFilterBlockBuilder {
void AddKey(const Slice& key) override; void AddKey(const Slice& key) override;
size_t NumAdded() const override { return num_added_; }
virtual Slice Finish(const BlockHandle& last_partition_block_handle, virtual Slice Finish(const BlockHandle& last_partition_block_handle,
Status* status) override; Status* status) override;
@ -59,6 +61,8 @@ class PartitionedFilterBlockBuilder : public FullFilterBlockBuilder {
uint32_t filters_per_partition_; uint32_t filters_per_partition_;
// The current number of filters in the last partition // The current number of filters in the last partition
uint32_t filters_in_partition_; uint32_t filters_in_partition_;
// Number of keys added
size_t num_added_;
}; };
class PartitionedFilterBlockReader : public FilterBlockReader { class PartitionedFilterBlockReader : public FilterBlockReader {

Loading…
Cancel
Save