Skip to content

Commit

Permalink
Remember whole key/prefix filtering on/off in SST file
Browse files Browse the repository at this point in the history
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
  • Loading branch information
siying committed Feb 11, 2015
1 parent fd5970b commit 68af781
Show file tree
Hide file tree
Showing 13 changed files with 266 additions and 50 deletions.
1 change: 1 addition & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
149 changes: 149 additions & 0 deletions db/db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
4 changes: 4 additions & 0 deletions include/rocksdb/table.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions table/block_based_filter_block.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions table/block_based_filter_block.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 9 additions & 6 deletions table/block_based_filter_block_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));

Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand Down
22 changes: 17 additions & 5 deletions table/block_based_table_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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
Expand All @@ -400,6 +405,10 @@ class BlockBasedTableBuilder::BlockBasedTablePropertiesCollector
std::string val;
PutFixed32(&val, static_cast<uint32_t>(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();
}

Expand All @@ -415,6 +424,8 @@ class BlockBasedTableBuilder::BlockBasedTablePropertiesCollector

private:
BlockBasedTableOptions::IndexType index_type_;
bool whole_key_filtering_;
bool prefix_filtering_;
};

struct BlockBasedTableBuilder::Rep {
Expand Down Expand Up @@ -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));
}
};

Expand Down Expand Up @@ -851,5 +864,4 @@ uint64_t BlockBasedTableBuilder::FileSize() const {

const std::string BlockBasedTable::kFilterBlockPrefix = "filter.";
const std::string BlockBasedTable::kFullFilterBlockPrefix = "fullfilter.";

} // namespace rocksdb
6 changes: 6 additions & 0 deletions table/block_based_table_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions table/block_based_table_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading

0 comments on commit 68af781

Please sign in to comment.