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

Fix bug when seeking backward against an out-of-bound iterator (#4187)

Browse Source 
Summary:
92ee3350e0 introduces an out-of-bound check in BlockBasedTableIterator::Valid(). However, this flag is not reset when re-seeking in backward direction. This caused the iterator to be invalide by mistake. Fix it by always resetting the out-of-bound flag in every seek.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/4187

Differential Revision: D8996600

Pulled By: siying

fbshipit-source-id: b6235ea614f71381e50e7904c4fb036300604ac1
This commit is contained in:
Siying Dong 2018-07-25 17:03:39 -07:00 committed by Facebook Github Bot
parent 18f538038a
commit 2a81633da2
2 changed files with 30 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
db/db_iterator_test.cc
View File

@ -2413,6 +2413,30 @@ TEST_P(DBIteratorTest, NonBlockingIterationBugRepro) {
EXPECT_TRUE(iter->status().IsIncomplete()); EXPECT_TRUE(iter->status().IsIncomplete());
} }
TEST_P(DBIteratorTest, SeekBackwardAfterOutOfUpperBound) {
Put("a", "");
Put("b", "");
Flush();
ReadOptions ropt;
Slice ub = "b";
ropt.iterate_upper_bound = &ub;
std::unique_ptr<Iterator> it(dbfull()->NewIterator(ropt));
it->SeekForPrev("a");
ASSERT_TRUE(it->Valid());
ASSERT_OK(it->status());
ASSERT_EQ("a", it->key().ToString());
it->Next();
ASSERT_FALSE(it->Valid());
ASSERT_OK(it->status());
it->SeekForPrev("a");
ASSERT_OK(it->status());
ASSERT_TRUE(it->Valid());
ASSERT_EQ("a", it->key().ToString());
}
INSTANTIATE_TEST_CASE_P(DBIteratorTestInstance, DBIteratorTest, INSTANTIATE_TEST_CASE_P(DBIteratorTestInstance, DBIteratorTest,
testing::Values(true, false)); testing::Values(true, false));

7
table/block_based_table_reader.cc
View File

@ -1991,6 +1991,7 @@ bool BlockBasedTable::PrefixMayMatch(
template <class TBlockIter> template <class TBlockIter>
void BlockBasedTableIterator<TBlockIter>::Seek(const Slice& target) { void BlockBasedTableIterator<TBlockIter>::Seek(const Slice& target) {
is_out_of_bound_ = false;
if (!CheckPrefixMayMatch(target)) { if (!CheckPrefixMayMatch(target)) {
ResetDataIter(); ResetDataIter();
return; return;
@ -2020,6 +2021,7 @@ void BlockBasedTableIterator<TBlockIter>::Seek(const Slice& target) {
template <class TBlockIter> template <class TBlockIter>
void BlockBasedTableIterator<TBlockIter>::SeekForPrev(const Slice& target) { void BlockBasedTableIterator<TBlockIter>::SeekForPrev(const Slice& target) {
is_out_of_bound_ = false;
if (!CheckPrefixMayMatch(target)) { if (!CheckPrefixMayMatch(target)) {
ResetDataIter(); ResetDataIter();
return; return;
@ -2062,6 +2064,7 @@ void BlockBasedTableIterator<TBlockIter>::SeekForPrev(const Slice& target) {
template <class TBlockIter> template <class TBlockIter>
void BlockBasedTableIterator<TBlockIter>::SeekToFirst() { void BlockBasedTableIterator<TBlockIter>::SeekToFirst() {
is_out_of_bound_ = false;
SavePrevIndexValue(); SavePrevIndexValue();
index_iter_->SeekToFirst(); index_iter_->SeekToFirst();
if (!index_iter_->Valid()) { if (!index_iter_->Valid()) {
@ -2075,6 +2078,7 @@ void BlockBasedTableIterator<TBlockIter>::SeekToFirst() {
template <class TBlockIter> template <class TBlockIter>
void BlockBasedTableIterator<TBlockIter>::SeekToLast() { void BlockBasedTableIterator<TBlockIter>::SeekToLast() {
is_out_of_bound_ = false;
SavePrevIndexValue(); SavePrevIndexValue();
index_iter_->SeekToLast(); index_iter_->SeekToLast();
if (!index_iter_->Valid()) { if (!index_iter_->Valid()) {
@ -2153,7 +2157,7 @@ void BlockBasedTableIterator<TBlockIter>::InitDataBlock() {
template <class TBlockIter> template <class TBlockIter>
void BlockBasedTableIterator<TBlockIter>::FindKeyForward() { void BlockBasedTableIterator<TBlockIter>::FindKeyForward() {
is_out_of_bound_ = false; assert(!is_out_of_bound_);
// TODO the while loop inherits from two-level-iterator. We don't know // TODO the while loop inherits from two-level-iterator. We don't know
// whether a block can be empty so it can be replaced by an "if". // whether a block can be empty so it can be replaced by an "if".
while (!block_iter_.Valid()) { while (!block_iter_.Valid()) {
@ -2193,6 +2197,7 @@ void BlockBasedTableIterator<TBlockIter>::FindKeyForward() {
template <class TBlockIter> template <class TBlockIter>
void BlockBasedTableIterator<TBlockIter>::FindKeyBackward() { void BlockBasedTableIterator<TBlockIter>::FindKeyBackward() {
assert(!is_out_of_bound_);
while (!block_iter_.Valid()) { while (!block_iter_.Valid()) {
if (!block_iter_.status().ok()) { if (!block_iter_.status().ok()) {
return; return;