Revert "Fixed the bug when both whole_key_filtering and prefix_extractor are set."

Summary:
This patch reverts commit 57605d7ef3 as it will
cause BlockBasedTableTest.NoopTransformSeek test crashes in some environment.

Test Plan: revert the patch

Reviewers: anthony, IslamAbdelRahman, rven, kradhakrishnan, sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D52623
main
Yueh-Hsuan Chiang 9 years ago
parent 57605d7ef3
commit 73c31377bb
  1. 44
      table/block_based_filter_block.cc
  2. 5
      table/block_based_filter_block.h
  3. 82
      table/table_test.cc

@ -19,6 +19,18 @@
namespace rocksdb {
namespace {
bool SamePrefix(const SliceTransform* prefix_extractor, const Slice& key1,
const Slice& key2) {
if (!prefix_extractor->InDomain(key1) && !prefix_extractor->InDomain(key2)) {
return true;
} else if (!prefix_extractor->InDomain(key1) ||
!prefix_extractor->InDomain(key2)) {
return false;
} else {
return (prefix_extractor->Transform(key1) ==
prefix_extractor->Transform(key2));
}
}
void AppendItem(std::string* props, const std::string& key,
const std::string& value) {
@ -65,9 +77,7 @@ BlockBasedFilterBlockBuilder::BlockBasedFilterBlockBuilder(
const BlockBasedTableOptions& table_opt)
: policy_(table_opt.filter_policy.get()),
prefix_extractor_(prefix_extractor),
whole_key_filtering_(table_opt.whole_key_filtering),
prev_prefix_start_(0),
prev_prefix_size_(0) {
whole_key_filtering_(table_opt.whole_key_filtering) {
assert(policy_);
}
@ -80,12 +90,13 @@ void BlockBasedFilterBlockBuilder::StartBlock(uint64_t block_offset) {
}
void BlockBasedFilterBlockBuilder::Add(const Slice& key) {
if (prefix_extractor_ && prefix_extractor_->InDomain(key)) {
AddPrefix(key);
}
added_to_start_ = 0;
if (whole_key_filtering_) {
AddKey(key);
added_to_start_ = 1;
}
if (prefix_extractor_ && prefix_extractor_->InDomain(key)) {
AddPrefix(key);
}
}
@ -99,16 +110,19 @@ inline void BlockBasedFilterBlockBuilder::AddKey(const Slice& key) {
inline void BlockBasedFilterBlockBuilder::AddPrefix(const Slice& key) {
// get slice for most recently added entry
Slice prev;
if (prev_prefix_size_ > 0) {
prev = Slice(entries_.data() + prev_prefix_start_, prev_prefix_size_);
if (start_.size() > added_to_start_) {
size_t prev_start = start_[start_.size() - 1 - added_to_start_];
const char* base = entries_.data() + prev_start;
size_t length = entries_.size() - prev_start;
prev = Slice(base, length);
}
Slice prefix = prefix_extractor_->Transform(key);
// insert prefix only when it's different from the previous prefix.
if (prev.size() == 0 || prefix != prev) {
// 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(prefix_extractor_, key, prev)) {
Slice prefix = prefix_extractor_->Transform(key);
start_.push_back(entries_.size());
prev_prefix_start_ = entries_.size();
prev_prefix_size_ = prefix.size();
entries_.append(prefix.data(), prefix.size());
}
}
@ -154,8 +168,6 @@ void BlockBasedFilterBlockBuilder::GenerateFilter() {
tmp_entries_.clear();
entries_.clear();
start_.clear();
prev_prefix_start_ = 0;
prev_prefix_size_ = 0;
}
BlockBasedFilterBlockReader::BlockBasedFilterBlockReader(

@ -55,12 +55,9 @@ class BlockBasedFilterBlockBuilder : public FilterBlockBuilder {
const SliceTransform* prefix_extractor_;
bool whole_key_filtering_;
size_t prev_prefix_start_; // the position of the last appended prefix
// to "entries_".
size_t prev_prefix_size_; // the length of the last appended prefix to
// "entries_".
std::string entries_; // Flattened entry contents
std::vector<size_t> start_; // Starting index in entries_ of each entry
uint32_t added_to_start_; // To indicate if key is added
std::string result_; // Filter data computed so far
std::vector<Slice> tmp_entries_; // policy_->CreateFilter() argument
std::vector<uint32_t> filter_offsets_;

@ -2281,88 +2281,6 @@ TEST_F(HarnessTest, FooterTests) {
}
}
class PrefixTest : public testing::Test {
public:
PrefixTest() : testing::Test() {}
~PrefixTest() {}
};
namespace {
// A simple PrefixExtractor that only works for test PrefixAndWholeKeyTest
class TestPrefixExtractor : public rocksdb::SliceTransform {
public:
~TestPrefixExtractor() override{};
const char* Name() const override { return "TestPrefixExtractor"; }
rocksdb::Slice Transform(const rocksdb::Slice& src) const override {
assert(IsValid(src));
return rocksdb::Slice(src.data(), 3);
}
bool InDomain(const rocksdb::Slice& src) const override {
assert(IsValid(src));
return true;
}
bool InRange(const rocksdb::Slice& dst) const override { return true; }
bool IsValid(const rocksdb::Slice& src) const {
if (src.size() != 4) {
return false;
}
if (src[0] != '[') {
return false;
}
if (src[1] < '0' || src[1] > '9') {
return false;
}
if (src[2] != ']') {
return false;
}
if (src[3] < '0' || src[3] > '9') {
return false;
}
return true;
}
};
} // namespace
TEST_F(PrefixTest, PrefixAndWholeKeyTest) {
rocksdb::Options options;
options.compaction_style = rocksdb::kCompactionStyleUniversal;
options.num_levels = 20;
options.create_if_missing = true;
options.optimize_filters_for_hits = false;
options.target_file_size_base = 268435456;
options.prefix_extractor = std::make_shared<TestPrefixExtractor>();
rocksdb::BlockBasedTableOptions bbto;
bbto.filter_policy.reset(rocksdb::NewBloomFilterPolicy(10));
bbto.block_size = 262144;
bbto.whole_key_filtering = true;
const std::string kDBPath = test::TmpDir() + "/prefix_test";
options.table_factory.reset(NewBlockBasedTableFactory(bbto));
DestroyDB(kDBPath, options);
rocksdb::DB* db;
ASSERT_OK(rocksdb::DB::Open(options, kDBPath, &db));
// Create a bunch of keys with 10 filters.
for (int i = 0; i < 10; i++) {
std::string prefix = "[" + std::to_string(i) + "]";
for (int j = 0; j < 10; j++) {
std::string key = prefix + std::to_string(j);
db->Put(rocksdb::WriteOptions(), key, "1");
}
}
// Trigger compaction.
db->CompactRange(CompactRangeOptions(), nullptr, nullptr);
delete db;
// In the second round, turn whole_key_filtering off and expect
// rocksdb still works.
}
} // namespace rocksdb
int main(int argc, char** argv) {

Loading…
Cancel
Save