Skip to content

Commit

Permalink
Add a test for tailing_iterator
Browse files Browse the repository at this point in the history
Summary:
A bug that tailingIterator->Seek(target) skips records.

I think the bug is in the SeekInternal starting at lines 387:
search_left_bound > search_right_bound
There are only 2 cases this can happen:
(1) target key is smaller than left most file
(2) target key is larger than right most file

The comment is wrong, there is another possibility that at the higher level there is a big gap such that the file in the lower level fits completely in the gap and then
indexer->GetNextLevelIndex returns search_left_bound > search_right_bound I think pointing on the files after and before the gap.
details: #1372

fixed this bug with test case added.
Closes #1436

Reviewed By: IslamAbdelRahman

Differential Revision: D4099313

Pulled By: lightmark

fbshipit-source-id: 6a675b3
  • Loading branch information
lightmark authored and Facebook Github Bot committed Oct 29, 2016
1 parent 04751d5 commit b50a81a
Showing 1 changed file with 53 additions and 0 deletions.
53 changes: 53 additions & 0 deletions db/db_tailing_iter_test.cc
Expand Up @@ -428,6 +428,59 @@ TEST_F(DBTestTailingIterator, TailingIteratorUpperBound) {
ASSERT_EQ(0, immutable_seeks);
}

TEST_F(DBTestTailingIterator, TailingIteratorGap) {
// level 1: [20, 25] [35, 40]
// level 2: [10 - 15] [45 - 50]
// level 3: [20, 30, 40]
// Previously there is a bug in tailing_iterator that if there is a gap in
// lower level, the key will be skipped if it is within the range between
// the largest key of index n file and the smallest key of index n+1 file
// if both file fit in that gap. In this example, 25 < key < 35
// https://github.com/facebook/rocksdb/issues/1372
CreateAndReopenWithCF({"pikachu"}, CurrentOptions());

ReadOptions read_options;
read_options.tailing = true;

ASSERT_OK(Put(1, "20", "20"));
ASSERT_OK(Put(1, "30", "30"));
ASSERT_OK(Put(1, "40", "40"));
ASSERT_OK(Flush(1));
MoveFilesToLevel(3, 1);

ASSERT_OK(Put(1, "10", "10"));
ASSERT_OK(Put(1, "15", "15"));
ASSERT_OK(Flush(1));
ASSERT_OK(Put(1, "45", "45"));
ASSERT_OK(Put(1, "50", "50"));
ASSERT_OK(Flush(1));
MoveFilesToLevel(2, 1);

ASSERT_OK(Put(1, "20", "20"));
ASSERT_OK(Put(1, "25", "25"));
ASSERT_OK(Flush(1));
ASSERT_OK(Put(1, "35", "35"));
ASSERT_OK(Put(1, "40", "40"));
ASSERT_OK(Flush(1));
MoveFilesToLevel(1, 1);

ColumnFamilyMetaData meta;
db_->GetColumnFamilyMetaData(handles_[1], &meta);

std::unique_ptr<Iterator> it(db_->NewIterator(read_options, handles_[1]));
it->Seek("30");
ASSERT_TRUE(it->Valid());
ASSERT_EQ("30", it->key().ToString());

it->Next();
ASSERT_TRUE(it->Valid());
ASSERT_EQ("35", it->key().ToString());

it->Next();
ASSERT_TRUE(it->Valid());
ASSERT_EQ("40", it->key().ToString());
}

TEST_F(DBTestTailingIterator, ManagedTailingIteratorSingle) {
ReadOptions read_options;
read_options.tailing = true;
Expand Down

0 comments on commit b50a81a

Please sign in to comment.