revert Prev() in MergingIterator to use previous code in non-prefix-seek mode

Summary: Siying suggested to keep old code for normal mode prev() for safety

Test Plan: make check -j64

Reviewers: yiwu, andrewkr, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D65439
main
Aaron Gao 8 years ago
parent 24495186da
commit 9de2f75216
  1. 5
      db/db_impl.cc
  2. 43
      table/merger.cc
  3. 6
      table/merger.h

@ -3856,7 +3856,10 @@ InternalIterator* DBImpl::NewInternalIterator(const ReadOptions& read_options,
InternalIterator* internal_iter; InternalIterator* internal_iter;
assert(arena != nullptr); assert(arena != nullptr);
// Need to create internal iterator from the arena. // Need to create internal iterator from the arena.
MergeIteratorBuilder merge_iter_builder(&cfd->internal_comparator(), arena); MergeIteratorBuilder merge_iter_builder(
&cfd->internal_comparator(), arena,
!read_options.total_order_seek &&
cfd->ioptions()->prefix_extractor != nullptr);
// Collect iterator for mutable mem // Collect iterator for mutable mem
merge_iter_builder.AddIterator( merge_iter_builder.AddIterator(
super_version->mem->NewIterator(read_options, arena)); super_version->mem->NewIterator(read_options, arena));

@ -36,12 +36,13 @@ const size_t kNumIterReserve = 4;
class MergingIterator : public InternalIterator { class MergingIterator : public InternalIterator {
public: public:
MergingIterator(const Comparator* comparator, InternalIterator** children, MergingIterator(const Comparator* comparator, InternalIterator** children,
int n, bool is_arena_mode) int n, bool is_arena_mode, bool prefix_seek_mode)
: is_arena_mode_(is_arena_mode), : is_arena_mode_(is_arena_mode),
comparator_(comparator), comparator_(comparator),
current_(nullptr), current_(nullptr),
direction_(kForward), direction_(kForward),
minHeap_(comparator_), minHeap_(comparator_),
prefix_seek_mode_(prefix_seek_mode),
pinned_iters_mgr_(nullptr) { pinned_iters_mgr_(nullptr) {
children_.resize(n); children_.resize(n);
for (int i = 0; i < n; i++) { for (int i = 0; i < n; i++) {
@ -204,9 +205,23 @@ class MergingIterator : public InternalIterator {
InitMaxHeap(); InitMaxHeap();
for (auto& child : children_) { for (auto& child : children_) {
if (&child != current_) { if (&child != current_) {
child.SeekForPrev(key()); if (!prefix_seek_mode_) {
if (child.Valid() && comparator_->Equal(key(), child.key())) { child.Seek(key());
child.Prev(); if (child.Valid()) {
// Child is at first entry >= key(). Step back one to be < key()
TEST_SYNC_POINT_CALLBACK("MergeIterator::Prev:BeforePrev",
&child);
child.Prev();
} else {
// Child has no entries >= key(). Position at last entry.
TEST_SYNC_POINT("MergeIterator::Prev:BeforeSeekToLast");
child.SeekToLast();
}
} else {
child.SeekForPrev(key());
if (child.Valid() && comparator_->Equal(key(), child.key())) {
child.Prev();
}
} }
} }
if (child.Valid()) { if (child.Valid()) {
@ -214,6 +229,13 @@ class MergingIterator : public InternalIterator {
} }
} }
direction_ = kReverse; direction_ = kReverse;
if (!prefix_seek_mode_) {
// Note that we don't do assert(current_ == CurrentReverse()) here
// because it is possible to have some keys larger than the seek-key
// inserted between Seek() and SeekToLast(), which makes current_ not
// equal to CurrentReverse().
current_ = CurrentReverse();
}
// The loop advanced all non-current children to be < key() so current_ // The loop advanced all non-current children to be < key() so current_
// should still be strictly the smallest key. // should still be strictly the smallest key.
assert(current_ == CurrentReverse()); assert(current_ == CurrentReverse());
@ -299,6 +321,8 @@ class MergingIterator : public InternalIterator {
}; };
Direction direction_; Direction direction_;
MergerMinIterHeap minHeap_; MergerMinIterHeap minHeap_;
bool prefix_seek_mode_;
// Max heap is used for reverse iteration, which is way less common than // Max heap is used for reverse iteration, which is way less common than
// forward. Lazily initialize it to save memory. // forward. Lazily initialize it to save memory.
std::unique_ptr<MergerMaxIterHeap> maxHeap_; std::unique_ptr<MergerMaxIterHeap> maxHeap_;
@ -331,7 +355,7 @@ void MergingIterator::InitMaxHeap() {
InternalIterator* NewMergingIterator(const Comparator* cmp, InternalIterator* NewMergingIterator(const Comparator* cmp,
InternalIterator** list, int n, InternalIterator** list, int n,
Arena* arena) { Arena* arena, bool prefix_seek_mode) {
assert(n >= 0); assert(n >= 0);
if (n == 0) { if (n == 0) {
return NewEmptyInternalIterator(arena); return NewEmptyInternalIterator(arena);
@ -339,19 +363,20 @@ InternalIterator* NewMergingIterator(const Comparator* cmp,
return list[0]; return list[0];
} else { } else {
if (arena == nullptr) { if (arena == nullptr) {
return new MergingIterator(cmp, list, n, false); return new MergingIterator(cmp, list, n, false, prefix_seek_mode);
} else { } else {
auto mem = arena->AllocateAligned(sizeof(MergingIterator)); auto mem = arena->AllocateAligned(sizeof(MergingIterator));
return new (mem) MergingIterator(cmp, list, n, true); return new (mem) MergingIterator(cmp, list, n, true, prefix_seek_mode);
} }
} }
} }
MergeIteratorBuilder::MergeIteratorBuilder(const Comparator* comparator, MergeIteratorBuilder::MergeIteratorBuilder(const Comparator* comparator,
Arena* a) Arena* a, bool prefix_seek_mode)
: first_iter(nullptr), use_merging_iter(false), arena(a) { : first_iter(nullptr), use_merging_iter(false), arena(a) {
auto mem = arena->AllocateAligned(sizeof(MergingIterator)); auto mem = arena->AllocateAligned(sizeof(MergingIterator));
merge_iter = new (mem) MergingIterator(comparator, nullptr, 0, true); merge_iter =
new (mem) MergingIterator(comparator, nullptr, 0, true, prefix_seek_mode);
} }
void MergeIteratorBuilder::AddIterator(InternalIterator* iter) { void MergeIteratorBuilder::AddIterator(InternalIterator* iter) {

@ -28,7 +28,8 @@ class Arena;
// REQUIRES: n >= 0 // REQUIRES: n >= 0
extern InternalIterator* NewMergingIterator(const Comparator* comparator, extern InternalIterator* NewMergingIterator(const Comparator* comparator,
InternalIterator** children, int n, InternalIterator** children, int n,
Arena* arena = nullptr); Arena* arena = nullptr,
bool prefix_seek_mode = false);
class MergingIterator; class MergingIterator;
@ -37,7 +38,8 @@ class MergeIteratorBuilder {
public: public:
// comparator: the comparator used in merging comparator // comparator: the comparator used in merging comparator
// arena: where the merging iterator needs to be allocated from. // arena: where the merging iterator needs to be allocated from.
explicit MergeIteratorBuilder(const Comparator* comparator, Arena* arena); explicit MergeIteratorBuilder(const Comparator* comparator, Arena* arena,
bool prefix_seek_mode = false);
~MergeIteratorBuilder() {} ~MergeIteratorBuilder() {}
// Add iter to the merging iterator. // Add iter to the merging iterator.

Loading…
Cancel
Save