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

Fix data block upper bound checking for iterator reseek case (#5883)

Browse Source 
Summary:
When an iterator reseek happens with the user specifying a new iterate_upper_bound in ReadOptions, and the new seek position is at the end of the same data block, the Seek() ends up using a stale value of data_block_within_upper_bound_ and may return incorrect results.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5883

Test Plan: Added a new test case DBIteratorTest.IterReseekNewUpperBound. Verified that it failed due to the assertion failure without the fix, and passes with the fix.

Differential Revision: D17752740

Pulled By: anand1976

fbshipit-source-id: f9b635ff5d6aeb0e1bef102cf8b2f900efd378e3
This commit is contained in:
anand76 2019-10-03 20:52:17 -07:00 committed by Facebook Github Bot
parent 9f54446525
commit 19a97dd139
2 changed files with 37 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
db/db_iterator_test.cc
View File

@ -183,6 +183,33 @@ TEST_P(DBIteratorTest, IterSeekBeforePrev) {
delete iter; delete iter;
} }
TEST_P(DBIteratorTest, IterReseekNewUpperBound) {
Random rnd(301);
Options options = CurrentOptions();
BlockBasedTableOptions table_options;
table_options.block_size = 1024;
table_options.block_size_deviation = 50;
options.table_factory.reset(NewBlockBasedTableFactory(table_options));
options.compression = kNoCompression;
Reopen(options);
ASSERT_OK(Put("a", RandomString(&rnd, 400)));
ASSERT_OK(Put("aabb", RandomString(&rnd, 400)));
ASSERT_OK(Put("aaef", RandomString(&rnd, 400)));
ASSERT_OK(Put("b", RandomString(&rnd, 400)));
dbfull()->Flush(FlushOptions());
ReadOptions opts;
Slice ub = Slice("aa");
opts.iterate_upper_bound = &ub;
auto iter = NewIterator(opts);
iter->Seek(Slice("a"));
ub = Slice("b");
iter->Seek(Slice("aabc"));
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(iter->key().ToString(), "aaef");
delete iter;
}
TEST_P(DBIteratorTest, IterSeekForPrevBeforeNext) { TEST_P(DBIteratorTest, IterSeekForPrevBeforeNext) {
ASSERT_OK(Put("a", "b")); ASSERT_OK(Put("a", "b"));
ASSERT_OK(Put("c", "d")); ASSERT_OK(Put("c", "d"));

11
table/block_based/block_based_table_reader.cc
View File

@ -2707,11 +2707,21 @@ void BlockBasedTableIterator<TBlockIter, TValue>::SeekImpl(
// Index contains the first key of the block, and it's >= target. // Index contains the first key of the block, and it's >= target.
// We can defer reading the block. // We can defer reading the block.
is_at_first_key_from_index_ = true; is_at_first_key_from_index_ = true;
// ResetDataIter() will invalidate block_iter_. Thus, there is no need to
// call CheckDataBlockWithinUpperBound() to check for iterate_upper_bound
// as that will be done later when the data block is actually read.
ResetDataIter(); ResetDataIter();
} else { } else {
// Need to use the data block. // Need to use the data block.
if (!same_block) { if (!same_block) {
InitDataBlock(); InitDataBlock();
} else {
// When the user does a reseek, the iterate_upper_bound might have
// changed. CheckDataBlockWithinUpperBound() needs to be called
// explicitly if the reseek ends up in the same data block.
// If the reseek ends up in a different block, InitDataBlock() will do
// the iterator upper bound check.
CheckDataBlockWithinUpperBound();
} }
if (target) { if (target) {
@ -2722,7 +2732,6 @@ void BlockBasedTableIterator<TBlockIter, TValue>::SeekImpl(
FindKeyForward(); FindKeyForward();
} }
CheckDataBlockWithinUpperBound();
CheckOutOfBound(); CheckOutOfBound();
if (target) { if (target) {