Fix crash with memtable prefix bloom and key out of prefix extractor domain (#5190)

Summary:
Before using prefix extractor `InDomain()` should be check. All uses in memtable.cc didn't check `InDomain()`.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5190

Differential Revision: D14923773

Pulled By: miasantreble

fbshipit-source-id: b3ad60bcca5f3a1a2b929a6eb34b0b7ba6326f04
This commit is contained in:
yiwu-arbug 2019-04-12 17:03:08 -07:00 committed by Facebook Github Bot
parent d655a3aab7
commit cca141ecf8
3 changed files with 32 additions and 6 deletions

View file

@ -8,10 +8,12 @@
### Public API Change
* Change the behavior of OptimizeForPointLookup(): move away from hash-based block-based-table index, and use whole key memtable filtering.
* Change the behavior of OptimizeForSmallDb(): use a 16MB block cache, put index and filter blocks into it, and cost the memtable size to it. DBOptions.OptimizeForSmallDb() and ColumnFamilyOptions.OptimizeForSmallDb() start to take an optional cache object.
### Bug Fixes
* Fix a bug in 2PC where a sequence of txn prepare, memtable flush, and crash could result in losing the prepared transaction.
* Fix a bug in Encryption Env which could cause encrypted files to be read beyond file boundaries.
* Fix a race condition between WritePrepared::Get and ::Put with duplicate keys.
* Fix crash when memtable prefix bloom is enabled and read/write a key out of domain of prefix extractor.
## 6.1.0 (3/27/2019)
### New Features

View file

@ -836,6 +836,25 @@ TEST_F(DBBloomFilterTest, MemtableWholeKeyBloomFilter) {
ASSERT_EQ(1, get_perf_context()->bloom_memtable_hit_count);
}
TEST_F(DBBloomFilterTest, MemtablePrefixBloomOutOfDomain) {
constexpr size_t kPrefixSize = 8;
const std::string kKey = "key";
assert(kKey.size() < kPrefixSize);
Options options = CurrentOptions();
options.prefix_extractor.reset(NewFixedPrefixTransform(kPrefixSize));
options.memtable_prefix_bloom_size_ratio = 0.25;
Reopen(options);
ASSERT_OK(Put(kKey, "v"));
ASSERT_EQ("v", Get(kKey));
std::unique_ptr<Iterator> iter(dbfull()->NewIterator(ReadOptions()));
iter->Seek(kKey);
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(kKey, iter->key());
iter->SeekForPrev(kKey);
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(kKey, iter->key());
}
#ifndef ROCKSDB_LITE
class BloomStatsTestWithParam
: public DBBloomFilterTest,

View file

@ -318,8 +318,9 @@ class MemTableIterator : public InternalIterator {
PERF_COUNTER_ADD(seek_on_memtable_count, 1);
if (bloom_) {
// iterator should only use prefix bloom filter
if (!bloom_->MayContain(
prefix_extractor_->Transform(ExtractUserKey(k)))) {
Slice user_key(ExtractUserKey(k));
if (prefix_extractor_->InDomain(user_key) &&
!bloom_->MayContain(prefix_extractor_->Transform(user_key))) {
PERF_COUNTER_ADD(bloom_memtable_miss_count, 1);
valid_ = false;
return;
@ -334,8 +335,9 @@ class MemTableIterator : public InternalIterator {
PERF_TIMER_GUARD(seek_on_memtable_time);
PERF_COUNTER_ADD(seek_on_memtable_count, 1);
if (bloom_) {
if (!bloom_->MayContain(
prefix_extractor_->Transform(ExtractUserKey(k)))) {
Slice user_key(ExtractUserKey(k));
if (prefix_extractor_->InDomain(user_key) &&
!bloom_->MayContain(prefix_extractor_->Transform(user_key))) {
PERF_COUNTER_ADD(bloom_memtable_miss_count, 1);
valid_ = false;
return;
@ -518,7 +520,8 @@ bool MemTable::Add(SequenceNumber s, ValueType type,
std::memory_order_relaxed);
}
if (bloom_filter_ && prefix_extractor_) {
if (bloom_filter_ && prefix_extractor_ &&
prefix_extractor_->InDomain(key)) {
bloom_filter_->Add(prefix_extractor_->Transform(key));
}
if (bloom_filter_ && moptions_.memtable_whole_key_filtering) {
@ -551,7 +554,8 @@ bool MemTable::Add(SequenceNumber s, ValueType type,
post_process_info->num_deletes++;
}
if (bloom_filter_ && prefix_extractor_) {
if (bloom_filter_ && prefix_extractor_ &&
prefix_extractor_->InDomain(key)) {
bloom_filter_->AddConcurrently(prefix_extractor_->Transform(key));
}
if (bloom_filter_ && moptions_.memtable_whole_key_filtering) {
@ -771,6 +775,7 @@ bool MemTable::Get(const LookupKey& key, std::string* value, Status* s,
} else {
assert(prefix_extractor_);
may_contain =
!prefix_extractor_->InDomain(user_key) ||
bloom_filter_->MayContain(prefix_extractor_->Transform(user_key));
}
}