andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

fix assertion failure in Prev()

Browse Source 
Summary:
fix assertion failure in db_stress.
It happens because of prefix seek key is larger than merge iterator key when they have the same user key

Test Plan: ./db_stress --max_background_compactions=1 --max_write_buffer_number=3 --sync=0 --reopen=20 --write_buffer_size=33554432 --delpercent=5 --log2_keys_per_lock=10 --block_size=16384 --allow_concurrent_memtable_write=0 --test_batches_snapshots=0 --max_bytes_for_level_base=67108864 --progress_reports=0 --mmap_read=0 --writepercent=35 --disable_data_sync=0 --readpercent=50 --subcompactions=4 --ops_per_thread=20000000 --memtablerep=skip_list --prefix_size=0 --target_file_size_multiplier=1 --column_families=1 --threads=32 --disable_wal=0 --open_files=500000 --destroy_db_initially=0 --target_file_size_base=16777216 --nooverwritepercent=1 --iterpercent=10 --max_key=100000000 --prefixpercent=0 --use_clock_cache=false --kill_random_test=888887 --cache_size=1048576 --verify_checksum=1

Reviewers: sdong, andrewkr, yiwu, yhchiang

Reviewed By: yhchiang

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D65025
This commit is contained in:
Aaron Gao 2016-10-13 17:36:48 -07:00
parent b9311aa65c
commit 21e8daced5
4 changed files with 8 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
db/db_iter.cc
View File

@ -512,8 +512,7 @@ void DBIter::ReverseToForward() {
IterKey last_key;
last_key.SetInternalKey(ParsedInternalKey(
saved_key_.GetKey(), kMaxSequenceNumber, kValueTypeForSeek));
Slice db_iter_key = last_key.GetKey();
iter_->ResetPrefixSeekKey(&db_iter_key);
iter_->Seek(last_key.GetKey());
}
FindNextUserKey();
direction_ = kForward;
@ -527,8 +526,7 @@ void DBIter::ReverseToBackward() {
IterKey last_key;
last_key.SetInternalKey(
ParsedInternalKey(saved_key_.GetKey(), 0, kValueTypeForSeekForPrev));
Slice db_iter_key = last_key.GetKey();
iter_->ResetPrefixSeekKey(&db_iter_key);
iter_->SeekForPrev(last_key.GetKey());
}
if (current_entry_is_merged_) {
// Not placed in the same key. Need to call Prev() until finding the

7
db/db_iterator_test.cc
View File

@ -666,7 +666,8 @@ TEST_F(DBIteratorTest, IterMultiWithDelete) {
// TODO: merge operator does not support backward iteration yet
if (kPlainTableAllBytesPrefix != option_config_ &&
kBlockBasedTableWithWholeKeyHashIndex != option_config_ &&
kHashLinkList != option_config_) {
kHashLinkList != option_config_ &&
kHashSkipList != option_config_) { // doesn't support SeekToLast
iter->Prev();
ASSERT_EQ(IterStatus(iter), "ka->va");
}
@ -732,7 +733,7 @@ TEST_F(DBIteratorTest, IterWithSnapshot) {
// TODO: merge operator does not support backward iteration yet
if (kPlainTableAllBytesPrefix != option_config_ &&
kBlockBasedTableWithWholeKeyHashIndex != option_config_ &&
kHashLinkList != option_config_) {
kHashLinkList != option_config_ && kHashSkipList != option_config_) {
iter->Prev();
ASSERT_EQ(IterStatus(iter), "key4->val4");
iter->Prev();
@ -751,7 +752,7 @@ TEST_F(DBIteratorTest, IterWithSnapshot) {
// TODO(gzh): merge operator does not support backward iteration yet
if (kPlainTableAllBytesPrefix != option_config_ &&
kBlockBasedTableWithWholeKeyHashIndex != option_config_ &&
kHashLinkList != option_config_) {
kHashLinkList != option_config_ && kHashSkipList != option_config_) {
iter->SeekForPrev("key1");
ASSERT_EQ(IterStatus(iter), "key1->val1");
iter->Next();

6
table/internal_iterator.h
View File

@ -94,12 +94,6 @@ class InternalIterator : public Cleanable {
virtual Status GetProperty(std::string prop_name, std::string* prop) {
return Status::NotSupported("");
}
// Reset the key used for Seek() in merge iterator, especially for prefix
// seek mode
// When in prefix_seek_mode, there is inconsistency between db_iterator and
// merge iterator. This inconsistency can cause problem when do Seek() in
// merge iterator in prefix mode.
virtual void ResetPrefixSeekKey(const Slice* prefix_seek_key = nullptr) {}
protected:
void SeekForPrevImpl(const Slice& target, const Comparator* cmp) {

16
table/merger.cc
View File

@ -158,7 +158,7 @@ class MergingIterator : public InternalIterator {
ClearHeaps();
for (auto& child : children_) {
if (&child != current_) {
child.Seek(prefix_seek_key_ ? *prefix_seek_key_ : key());
child.Seek(key());
if (child.Valid() && comparator_->Equal(key(), child.key())) {
child.Next();
}
@ -204,7 +204,7 @@ class MergingIterator : public InternalIterator {
InitMaxHeap();
for (auto& child : children_) {
if (&child != current_) {
child.SeekForPrev(prefix_seek_key_ ? *prefix_seek_key_ : key());
child.SeekForPrev(key());
if (child.Valid() && comparator_->Equal(key(), child.key())) {
child.Prev();
}
@ -277,17 +277,6 @@ class MergingIterator : public InternalIterator {
current_->IsValuePinned();
}
virtual void ResetPrefixSeekKey(const Slice* db_iter_key) override {
if (db_iter_key == nullptr) {
prefix_seek_key_.reset();
return;
}
if (!prefix_seek_key_) {
prefix_seek_key_.reset(new std::string);
}
*prefix_seek_key_ = db_iter_key->ToString();
}
private:
// Clears heaps for both directions, used when changing direction or seeking
void ClearHeaps();
@ -314,7 +303,6 @@ class MergingIterator : public InternalIterator {
// forward. Lazily initialize it to save memory.
std::unique_ptr<MergerMaxIterHeap> maxHeap_;
PinnedIteratorsManager* pinned_iters_mgr_;
std::unique_ptr<std::string> prefix_seek_key_;
IteratorWrapper* CurrentForward() const {
assert(direction_ == kForward);