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

Revert two diffs related to DBIter::FindPrevUserKey()

Browse Source 
Summary:
This diff reverts the following two previous diffs related to
DBIter::FindPrevUserKey(), which makes db_stress unstable.
We should bake a better fix for this.

* "Fix a comparison in DBIter::FindPrevUserKey()"
  ec70fea4c4025351190eba7a02bd09bb5f083790.

* "Fixed endless loop in DBIter::FindPrevUserKey()"
  acee2b08a2d37154b8f9e2dc74b1966202c15ec5.

Test Plan: db_stress

Reviewers: anthony, igor, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D41301
(cherry picked from commit 685582a0b43cb19cb12bf7122657f9ea0f8c52c4)
This commit is contained in:
Yueh-Hsuan Chiang 2015-07-07 11:36:24 -07:00 committed by agiardullo
parent 718309432b
commit 0640c28723
4 changed files with 11 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
db/db_iter.cc
View File

@ -350,9 +350,6 @@ void DBIter::MergeValuesNewToOld() {
void DBIter::Prev() {
assert(valid_);
if (direction_ == kForward) {
if (!iter_->Valid()) {
iter_->SeekToLast();
}
FindPrevUserKey();
direction_ = kReverse;
}
@ -556,7 +553,7 @@ void DBIter::FindNextUserKey() {
ParsedInternalKey ikey;
FindParseableKey(&ikey, kForward);
while (iter_->Valid() &&
user_comparator_->Compare(ikey.user_key, saved_key_.GetKey()) <= 0) {
user_comparator_->Compare(ikey.user_key, saved_key_.GetKey()) != 0) {
iter_->Next();
FindParseableKey(&ikey, kForward);
}
@ -571,7 +568,7 @@ void DBIter::FindPrevUserKey() {
ParsedInternalKey ikey;
FindParseableKey(&ikey, kReverse);
while (iter_->Valid() &&
user_comparator_->Compare(ikey.user_key, saved_key_.GetKey()) >= 0) {
user_comparator_->Compare(ikey.user_key, saved_key_.GetKey()) == 0) {
if (num_skipped >= max_skip_) {
num_skipped = 0;
IterKey last_key;

8
db/db_iter_test.cc
View File

@ -1668,7 +1668,9 @@ TEST_F(DBIteratorTest, DBIterator8) {
ASSERT_EQ(db_iter->value().ToString(), "0");
}
TEST_F(DBIteratorTest, DBIterator9) {
// TODO(3.13): fix the issue of Seek() then Prev() which might not necessary
// return the biggest element smaller than the seek key.
TEST_F(DBIteratorTest, DISABLED_DBIterator9) {
Options options;
options.merge_operator = MergeOperators::CreateFromStringId("stringappend");
{
@ -1716,7 +1718,9 @@ TEST_F(DBIteratorTest, DBIterator9) {
}
}
TEST_F(DBIteratorTest, DBIterator10) {
// TODO(3.13): fix the issue of Seek() then Prev() which might not necessary
// return the biggest element smaller than the seek key.
TEST_F(DBIteratorTest, DISABLED_DBIterator10) {
Options options;
TestIterator* internal_iter = new TestIterator(BytewiseComparator());

4
db/db_test.cc
View File

@ -14111,7 +14111,9 @@ TEST_F(DBTest, RowCache) {
ASSERT_EQ(TestGetTickerCount(options, ROW_CACHE_MISS), 1);
}
TEST_F(DBTest, PrevAfterMerge) {
// TODO(3.13): fix the issue of Seek() + Prev() which might not necessary
// return the biggest key which is smaller than the seek key.
TEST_F(DBTest, DISABLED_PrevAfterMerge) {
Options options;
options.create_if_missing = true;
options.merge_operator = MergeOperators::CreatePutOperator();

13
table/merger.cc
View File

@ -208,19 +208,6 @@ class MergingIterator : public Iterator {
} else {
// Child has no entries >= key(). Position at last entry.
child.SeekToLast();
if (child.Valid() && comparator_->Compare(child.key(), key()) > 0) {
// Prefix bloom or prefix hash may return !Valid() if one of the
// following condition happens: 1. when prefix doesn't match.
// 2. Does not exist any row larger than the key within the prefix
// while SeekToLast() may return larger keys.
//
// Temporarily remove this child to avoid Prev() to return a key
// larger than the original keys. However, this can cause missing
// rows.
//
// TODO(3.13): need to fix.
continue;
}
}
if (child.Valid()) {
maxHeap_.push(&child);