rocksdb/table/block_based/partitioned_index_iterator.cc
sdong 712bc4b6a2 Fix regression bug in partitioned index reseek caused by #6531 (#6551)
Summary:
https://github.com/facebook/rocksdb/pull/6531 removed some code in partitioned index seek logic. By mistake the logic of storing previous index offset is removed, while the logic of using it is preserved, so that the code might use wrong value to determine reseeking condition.
This will trigger a bug, if following a Seek() not going to the last block, SeekToLast() is called, and then Seek() is called which should position the cursor to the block before SeekToLast().
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6551

Test Plan: Add a unit test that reproduces the bug. In the same unit test, also some reseek cases are covered to avoid regression.

Reviewed By: pdillinger

Differential Revision: D20493990

fbshipit-source-id: 3919aa4861c0481ec96844e053048da1a934b91d
2020-03-17 12:33:10 -07:00

164 lines
5.1 KiB
C++

// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
// This source code is licensed under both the GPLv2 (found in the
// COPYING file in the root directory) and Apache 2.0 License
// (found in the LICENSE.Apache file in the root directory).
//
// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. See the AUTHORS file for names of contributors.
#include "table/block_based/partitioned_index_iterator.h"
namespace ROCKSDB_NAMESPACE {
void ParititionedIndexIterator::Seek(const Slice& target) { SeekImpl(&target); }
void ParititionedIndexIterator::SeekToFirst() { SeekImpl(nullptr); }
void ParititionedIndexIterator::SeekImpl(const Slice* target) {
SavePrevIndexValue();
if (target) {
index_iter_->Seek(*target);
} else {
index_iter_->SeekToFirst();
}
if (!index_iter_->Valid()) {
ResetPartitionedIndexIter();
return;
}
InitPartitionedIndexBlock();
if (target) {
block_iter_.Seek(*target);
} else {
block_iter_.SeekToFirst();
}
FindKeyForward();
// We could check upper bound here, but that would be too complicated
// and checking index upper bound is less useful than for data blocks.
if (target) {
assert(!Valid() || (table_->get_rep()->index_key_includes_seq
? (icomp_.Compare(*target, key()) <= 0)
: (user_comparator_.Compare(ExtractUserKey(*target),
key()) <= 0)));
}
}
void ParititionedIndexIterator::SeekToLast() {
SavePrevIndexValue();
index_iter_->SeekToLast();
if (!index_iter_->Valid()) {
ResetPartitionedIndexIter();
return;
}
InitPartitionedIndexBlock();
block_iter_.SeekToLast();
FindKeyBackward();
}
void ParititionedIndexIterator::Next() {
assert(block_iter_points_to_real_block_);
block_iter_.Next();
FindKeyForward();
}
void ParititionedIndexIterator::Prev() {
assert(block_iter_points_to_real_block_);
block_iter_.Prev();
FindKeyBackward();
}
void ParititionedIndexIterator::InitPartitionedIndexBlock() {
BlockHandle partitioned_index_handle = index_iter_->value().handle;
if (!block_iter_points_to_real_block_ ||
partitioned_index_handle.offset() != prev_block_offset_ ||
// if previous attempt of reading the block missed cache, try again
block_iter_.status().IsIncomplete()) {
if (block_iter_points_to_real_block_) {
ResetPartitionedIndexIter();
}
auto* rep = table_->get_rep();
bool is_for_compaction =
lookup_context_.caller == TableReaderCaller::kCompaction;
// Prefetch additional data for range scans (iterators).
// Implicit auto readahead:
// Enabled after 2 sequential IOs when ReadOptions.readahead_size == 0.
// Explicit user requested readahead:
// Enabled from the very first IO when ReadOptions.readahead_size is set.
block_prefetcher_.PrefetchIfNeeded(rep, partitioned_index_handle,
read_options_.readahead_size,
is_for_compaction);
Status s;
table_->NewDataBlockIterator<IndexBlockIter>(
read_options_, partitioned_index_handle, &block_iter_,
BlockType::kIndex,
/*get_context=*/nullptr, &lookup_context_, s,
block_prefetcher_.prefetch_buffer(),
/*for_compaction=*/is_for_compaction);
block_iter_points_to_real_block_ = true;
// We could check upper bound here but it is complicated to reason about
// upper bound in index iterator. On the other than, in large scans, index
// iterators are moved much less frequently compared to data blocks. So
// the upper bound check is skipped for simplicity.
}
}
void ParititionedIndexIterator::FindKeyForward() {
// This method's code is kept short to make it likely to be inlined.
assert(block_iter_points_to_real_block_);
if (!block_iter_.Valid()) {
// This is the only call site of FindBlockForward(), but it's extracted into
// a separate method to keep FindKeyForward() short and likely to be
// inlined. When transitioning to a different block, we call
// FindBlockForward(), which is much longer and is probably not inlined.
FindBlockForward();
} else {
// This is the fast path that avoids a function call.
}
}
void ParititionedIndexIterator::FindBlockForward() {
// 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".
do {
if (!block_iter_.status().ok()) {
return;
}
ResetPartitionedIndexIter();
index_iter_->Next();
if (!index_iter_->Valid()) {
return;
}
InitPartitionedIndexBlock();
block_iter_.SeekToFirst();
} while (!block_iter_.Valid());
}
void ParititionedIndexIterator::FindKeyBackward() {
while (!block_iter_.Valid()) {
if (!block_iter_.status().ok()) {
return;
}
ResetPartitionedIndexIter();
index_iter_->Prev();
if (index_iter_->Valid()) {
InitPartitionedIndexBlock();
block_iter_.SeekToLast();
} else {
return;
}
}
}
} // namespace ROCKSDB_NAMESPACE