mirror of https://github.com/facebook/rocksdb.git
ForwardIterator seek bugfix
Summary: If `NeedToSeekImmutable()` returns false, `SeekInternal()` won't reset the contents of `immutable_min_heap_`. However, since it calls `UpdateCurrent()` unconditionally, if `current_` is one of immutable iterators (previously popped from `immutable_min_heap_`), `UpdateCurrent()` will overwrite it. As a result, if old `current_` in fact pointed to the smallest entry, forward iterator will skip some records. Fix implemented in this diff pushes `current_` back to `immutable_min_heap_` before calling `UpdateCurrent()`. Test Plan: New unit test (courtesy of @lovro): $ ROCKSDB_TESTS=TailingIteratorSeekToSame ./db_test Reviewers: igor, dhruba, haobo, ljin Reviewed By: ljin Subscribers: lovro, leveldb Differential Revision: https://reviews.facebook.net/D19653
This commit is contained in:
parent
a51fbf5f3f
commit
3b97ee96c4
|
@ -6939,6 +6939,41 @@ TEST(DBTest, TailingIteratorIncomplete) {
|
|||
ASSERT_TRUE(iter->Valid() || iter->status().IsIncomplete());
|
||||
}
|
||||
|
||||
TEST(DBTest, TailingIteratorSeekToSame) {
|
||||
Options options = CurrentOptions();
|
||||
options.compaction_style = kCompactionStyleUniversal;
|
||||
options.write_buffer_size = 1000;
|
||||
CreateAndReopenWithCF({"pikachu"}, &options);
|
||||
|
||||
ReadOptions read_options;
|
||||
read_options.tailing = true;
|
||||
|
||||
const int NROWS = 10000;
|
||||
// Write rows with keys 00000, 00002, 00004 etc.
|
||||
for (int i = 0; i < NROWS; ++i) {
|
||||
char buf[100];
|
||||
snprintf(buf, sizeof(buf), "%05d", 2*i);
|
||||
std::string key(buf);
|
||||
std::string value("value");
|
||||
ASSERT_OK(db_->Put(WriteOptions(), key, value));
|
||||
}
|
||||
|
||||
std::unique_ptr<Iterator> iter(db_->NewIterator(read_options));
|
||||
// Seek to 00001. We expect to find 00002.
|
||||
std::string start_key = "00001";
|
||||
iter->Seek(start_key);
|
||||
ASSERT_TRUE(iter->Valid());
|
||||
|
||||
std::string found = iter->key().ToString();
|
||||
ASSERT_EQ("00002", found);
|
||||
|
||||
// Now seek to the same key. The iterator should remain in the same
|
||||
// position.
|
||||
iter->Seek(found);
|
||||
ASSERT_TRUE(iter->Valid());
|
||||
ASSERT_EQ(found, iter->key().ToString());
|
||||
}
|
||||
|
||||
TEST(DBTest, BlockBasedTablePrefixIndexTest) {
|
||||
// create a DB with block prefix index
|
||||
BlockBasedTableOptions table_options;
|
||||
|
|
|
@ -292,6 +292,9 @@ void ForwardIterator::SeekInternal(const Slice& internal_key,
|
|||
prev_key_.SetKey(internal_key);
|
||||
is_prev_set_ = true;
|
||||
}
|
||||
} else if (current_ && current_ != mutable_iter_) {
|
||||
// current_ is one of immutable iterators, push it back to the heap
|
||||
immutable_min_heap_.push(current_);
|
||||
}
|
||||
|
||||
UpdateCurrent();
|
||||
|
|
Loading…
Reference in New Issue