You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by aw...@apache.org on 2020/12/03 01:47:50 UTC
[kudu] branch branch-1.13.x updated: KUDU-3108: fix invalid memory
accesses in merge iterator
This is an automated email from the ASF dual-hosted git repository.
awong pushed a commit to branch branch-1.13.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/branch-1.13.x by this push:
new 4502d36 KUDU-3108: fix invalid memory accesses in merge iterator
4502d36 is described below
commit 4502d3651d40770bee51fd4704cf8e0a2fb1e0ea
Author: Andrew Wong <aw...@cloudera.com>
AuthorDate: Mon Nov 23 01:14:00 2020 -0800
KUDU-3108: fix invalid memory accesses in merge iterator
The 'hotmaxes_' heap and 'hot_' heap in the MergeIterator are meant to
represent the same set of iterator states currently deemed "hot" in the
optimal merge algorithm[1]. They used different ordering constraints to
allow constant time access to the smallest last row and the smallest
next row across all hot states respectively. However, we were using
pop() on both heaps when iterator states were no longer deemed hot,
incorrectly expecting that the call would remove the same iterator state
from both heaps.
In the case that this pop() was followed by the destruction of the
sub-iterator (e.g. if the iterator was fully exhausted), this would
leave an iterator state in the heaps that pointed at destructed state:
F1030 21:16:58.411253 40800 schema.h:706] Check failed: KeyEquals(*lhs.schema()) && KeyEquals(*rhs.schema())
*** Check failure stack trace: ***
*** Aborted at 1604117818 (unix time) try "date -d @1604117818" if you are using GNU date ***
PC: @ 0x7f701fcf11d7 __GI_raise
*** SIGABRT (@0x111700009efd) received by PID 40701 (TID 0x7f6ff0f47700) from PID 40701; stack trace: ***
@ 0x7f7026a70370 (unknown)
@ 0x7f701fcf11d7 __GI_raise
@ 0x7f701fcf28c8 __GI_abort
@ 0x7f70224377b9 google::logging_fail()
@ 0x7f7022438f8d google::LogMessage::Fail()
@ 0x7f702243aee3 google::LogMessage::SendToLog()
@ 0x7f7022438ae9 google::LogMessage::Flush()
@ 0x7f702243b86f google::LogMessageFatal::~LogMessageFatal()
@ 0x7f702cc99fbc kudu::Schema::Compare<>()
@ 0x7f7026167cfd kudu::MergeIterator::RefillHotHeap()
@ 0x7f7026167357 kudu::MergeIterator::AdvanceAndReheap()
@ 0x7f7026169617 kudu::MergeIterator::MaterializeOneRow()
@ 0x7f70261688e9 kudu::MergeIterator::NextBlock()
@ 0x7f702cbddd9b kudu::tablet::Tablet::Iterator::NextBlock()
@ 0x7f70317bcab3 kudu::tserver::TabletServiceImpl::HandleContinueScanRequest()
@ 0x7f70317bb857 kudu::tserver::TabletServiceImpl::HandleNewScanRequest()
@ 0x7f70317b464e kudu::tserver::TabletServiceImpl::Scan()
@ 0x7f702ddfd762 _ZZN4kudu7tserver21TabletServerServiceIfC1ERK13scoped_refptrINS_12MetricEntityEERKS2_INS_3rpc13ResultTrackerEEENKUlPKN6google8protobuf7MessageEPSE_PNS7_10RpcContextEE4_clESG_SH_SJ_
@ 0x7f702de0064d _ZNSt17_Function_handlerIFvPKN6google8protobuf7MessageEPS2_PN4kudu3rpc10RpcContextEEZNS6_7tserver21TabletServerServiceIfC1ERK13scoped_refptrINS6_12MetricEntityEERKSD_INS7_13ResultTrackerEEEUlS4_S5_S9_E4_E9_M_invokeERKSt9_Any_dataS4_S5_S9_
@ 0x7f702b4ddcc2 std::function<>::operator()()
@ 0x7f702b4dd6ed kudu::rpc::GeneratedServiceIf::Handle()
@ 0x7f702b4dfff8 kudu::rpc::ServicePool::RunThread()
@ 0x7f702b4de8c5 _ZZN4kudu3rpc11ServicePool4InitEiENKUlvE_clEv
@ 0x7f702b4e0337 _ZNSt17_Function_handlerIFvvEZN4kudu3rpc11ServicePool4InitEiEUlvE_E9_M_invokeERKSt9_Any_data
@ 0x7f7033524b9c std::function<>::operator()()
@ 0x7f70248227e0 kudu::Thread::SuperviseThread()
@ 0x7f7026a68dc5 start_thread
@ 0x7f701fdb376d __clone
Aborted
This patch removes the 'hotmaxes_' min heap in favor of a two-heap
variant of the merge algorithm that Adar described to me that uses the
last value in the top iterator in the hot heap. This is not as optimal
in terms of merge window size, but is still correct and avoids this bug.
I experimented with some other approaches, described below. I ran the
same test used in 1567dec086 (generic_iterators-test TestMerge and
TestMergeNonOverlapping with 10000 rows per list), averaged over five
runs:
a: An iteration of this patch that used std::set for hotmaxes, a call
to find() before calling Advance(), and a position-based erase()
after. The find() allowed for an erase() call that did not rely at
all on the comparator, which may have pointed at destructed state
following the call to Advance().
b: An iteration of this patch that used std::set for hotmaxes, a call
to a value-based erase() before calling Advance(). The call to
erase() before Advance() ensured Advance() calls never interfered
with our ability to compare and erase from hotmaxes, at the
potential cost of an extra insert if the iterator were still hot.
c: The original version that uses heap::pop() after calling Advance().
d: This patch, that doesn't use hotmaxes, and instead uses the last row
of the top value in the hot heap to define the upper bound of the
merge window.
Parameters | a | b | c | d
----------------------------+----------+----------+----------+---------
overlapping, 10 lists | 0.059s | 0.0744s | 0.0472s | 0.0478s
overlapping, 100 lists | 0.6726s | 0.8876s | 0.4938s | 0.491s
overlapping, 1000 lists | 15.5588s | 18.87s | 10.3554s | 10.157s
non-overlapping, 10 lists | 0.011s | 0.0114s | 0.0106s | 0.0092s
non-overlapping, 100 lists | 0.0786s | 0.0794s | 0.083s | 0.0682s
non-overlapping, 1000 lists | 0.7824s | 0.7346s | 0.7174s | 0.6884s
I also ran an ordered scan with `kudu perf tablet_scan` on a 65GiB
tablet hosted on a single disk with 1667 rowsets and an average rowset
height of five, averaged over six runs each (I omitted testing version b
since it was the worst-performing of the above):
Results | a | c | d
----------+----------+----------+----------
real time | 1070.69s | 1166.96s | 1048.57s
stdev | 19.48s | 25.14s | 19.54s
I didn't profile these runs in depth, but the measurements suggest that
the maintenance of the hotmaxes set may add overhead that isn't always
recouped by an optimally-sized merge window. I left a TODO to experiment
further with hotmaxes of different data structures (e.g. absl::btree
seems like a good candidate).
To exercise these codepaths more rigorously, I bumped fuzz-itest's
default keyspace size to 5. Some bugs (this one included) can only be
created when there are a mix of overlapping and non-overlapping rowsets,
which is impossible to achieve with the current keyspace size of 2.
[1] https://docs.google.com/document/d/1uP0ubjM6ulnKVCRrXtwT_dqrTWjF9tlFSRk0JN2e_O0/edit#
Change-Id: I8ec1cd3fd67ec4ea92a55b5b0ce555123748824d
Reviewed-on: http://gerrit.cloudera.org:8080/16777
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
(cherry picked from commit 12accbce319ff05164c2049f822c936659d5dd59)
Reviewed-on: http://gerrit.cloudera.org:8080/16796
---
src/kudu/common/generic_iterators.cc | 110 ++++++++++++++-----------------
src/kudu/integration-tests/fuzz-itest.cc | 22 ++++++-
src/kudu/tablet/diff_scan-test.cc | 62 +++++++++++++++++
3 files changed, 132 insertions(+), 62 deletions(-)
diff --git a/src/kudu/common/generic_iterators.cc b/src/kudu/common/generic_iterators.cc
index 390740a..c2c3851 100644
--- a/src/kudu/common/generic_iterators.cc
+++ b/src/kudu/common/generic_iterators.cc
@@ -192,7 +192,7 @@ class MergeIterState : public boost::intrusive::list_base_hook<> {
// Returns the schema from the underlying iterator.
const Schema& schema() const {
- return iwb_.iter->schema();
+ return DCHECK_NOTNULL(iwb_.iter)->schema();
}
// Pulls the next block from the underlying iterator.
@@ -221,7 +221,7 @@ class MergeIterState : public boost::intrusive::list_base_hook<> {
// iterators.
//
// Must already be Init'ed at MergeIterState construction time.
- IterWithBounds iwb_;
+ const IterWithBounds iwb_;
// Allocates memory for read_block_.
RowBlockMemory memory_;
@@ -291,7 +291,7 @@ Status MergeIterState::PullNextBlock() {
while (iwb_.iter->HasNext()) {
RETURN_NOT_OK(iwb_.iter->NextBlock(read_block_.get()));
- SelectionVector *selection = read_block_->selection_vector();
+ SelectionVector* selection = read_block_->selection_vector();
DCHECK_EQ(selection->nrows(), read_block_->nrows());
DCHECK_LE(selection->CountSelected(), read_block_->nrows());
size_t rows_valid = selection->CountSelected();
@@ -321,7 +321,7 @@ Status MergeIterState::PullNextBlock() {
LOG(FATAL) << "unreachable code"; // guaranteed by the short-circuit above
}
- // The underlying iterator is fully exhausted.
+ VLOG(1) << "Fully exhausted iter " << iwb_.iter->ToString();
next_row_idx_ = 0;
read_block_.reset();
return Status::OK();
@@ -347,10 +347,10 @@ Status MergeIterState::CopyBlock(RowBlock* dst, size_t dst_offset,
// An iterator which merges the results of other iterators, comparing
// based on keys.
//
-// Three different heaps are used to optimize the merge process. To explain how
-// it works, let's start with an explanation of a traditional heap-based merge:
-// there exist N sorted lists of elements and the goal is to produce a single
-// sorted list containing all of the elements.
+// Two heaps are used to optimize the merge process. To explain how it works,
+// let's start with an explanation of a traditional heap-based merge: there
+// exist N sorted lists of elements and the goal is to produce a single sorted
+// list containing all of the elements.
//
// To begin:
// - For each list, peek the first element into a per-list buffer.
@@ -428,22 +428,20 @@ Status MergeIterState::CopyBlock(RowBlock* dst, size_t dst_offset,
// output), or when a sub-iterator finishes its NEXT and needs to peek again.
//
// But which sub-iterators should be moved? To answer this question efficiently,
-// we need two more heaps:
-// - COLD: a min-heap for sub-iterators in the second set, ordered by each's
-// NEXT's lower bound. At any given time, the NEXT belonging to the top-most
-// sub-iterator in COLD is nearest the merge window.
-// - HOTMAXES: a min-heap for keys. Each entry corresponds to a sub-iterator
-// present in HOT, and specifically, to its NEXT's upper bound. At any given
-// time, the top-most key in HOTMAXES represents the end of the merge window.
+// we need another heap: we'll define a COLD min-heap for sub-iterators in the
+// second set, ordered by each's NEXT's lower bound. At any given time, the
+// NEXT belonging to the top-most sub-iterator in COLD is nearest the merge
+// window.
//
-// When the merge window's end has moved and we need to refill HOT, the top-most
-// sub-iterator in COLD is the best candidate. To figure out whether it should
-// be moved, we compare its NEXT's lower bound against the top-most key in
-// HOTMAXES: if the lower bound is less than or equal to the key, we move the
-// sub-iterator from COLD to HOT. On the flip side, when a sub-iterator from HOT
-// finishes its NEXT and peeks again, we also need to check whether it has
-// exited the merge window. The approach is similar: if its NEXT's lower bound
-// is greater than the top-most key in HOTMAXES, it's time to move it to COLD.
+// When the merge window's end has moved and we need to refill HOT, the
+// top-most sub-iterator in COLD is the best candidate. To figure out whether
+// it should be moved, we compare its NEXT's lower bound against the upper
+// bound in HOT's first iterator: if the lower bound is less than or equal to
+// the key, we move the sub-iterator from COLD to HOT. On the flip side, when a
+// sub-iterator from HOT finishes its NEXT and peeks again, we also need to
+// check whether it has exited the merge window. The approach is similar: if
+// its NEXT's lower bound is greater than the upper bound of HOT'S first
+// iterator, it's time to move it to COLD.
//
// There's one more piece to this puzzle: the absolute bounds that are known
// ahead of time are used as stand-ins for NEXT's lower and upper bounds. This
@@ -451,6 +449,15 @@ Status MergeIterState::CopyBlock(RowBlock* dst, size_t dst_offset,
// moves from COLD to HOT. After that, peeked memory must remain resident until
// the sub-iterator is fully exhausted.
//
+// TODO(awong): there is a variant of this algorithm that defines another
+// container to further optimize the size of the merge window: HOTMAXES, an
+// ordered set for sub-iterators in HOT, ordered by each sub-iterator's upper
+// bound. At any given time, the first iterator in HOTMAXES represents the
+// optimal end of the merge window, allowing us to move elements onto COLD via
+// comparison to the first value of HOTMAXES. Some experiments defined this
+// using std::set and found its maintenance sometimes takes more than it nets.
+// Experiment with a less memory-hungry data structure (maybe absl::btree?).
+//
// For another description of this algorithm including pseudocode, see
// https://docs.google.com/document/d/1uP0ubjM6ulnKVCRrXtwT_dqrTWjF9tlFSRk0JN2e_O0/edit#
class MergeIterator : public RowwiseIterator {
@@ -496,7 +503,7 @@ class MergeIterator : public RowwiseIterator {
Status InitSubIterators(ScanSpec *spec);
// Advances 'state' by 'num_rows_to_advance', destroying it and/or updating
- // the three heaps if necessary.
+ // 'hot_' and 'cold_' if necessary.
Status AdvanceAndReheap(MergeIterState* state, size_t num_rows_to_advance);
// Moves sub-iterators from cold_ to hot_ if they now overlap with the merge
@@ -543,18 +550,6 @@ class MergeIterator : public RowwiseIterator {
// MergeIterState's decoded bounds remain allocated for its lifetime.
RowBlockMemory decoded_bounds_memory_;
- // Min-heap that orders rows by their keys. A call to top() will yield the row
- // with the smallest key.
- struct RowComparator {
- bool operator()(const RowBlockRow& a, const RowBlockRow& b) const {
- // This is counter-intuitive, but it's because boost::heap defaults to
- // a max-heap; the comparator must be inverted to yield a min-heap.
- return a.schema()->Compare(a, b) > 0;
- }
- };
- typedef boost::heap::skew_heap<
- RowBlockRow, boost::heap::compare<RowComparator>> RowMinHeap;
-
// Min-heap that orders sub-iterators by their next row key. A call to top()
// will yield the sub-iterator with the smallest next row key.
struct MergeIterStateComparator {
@@ -565,16 +560,15 @@ class MergeIterator : public RowwiseIterator {
}
};
typedef boost::heap::skew_heap<
- MergeIterState*, boost::heap::compare<MergeIterStateComparator>> MergeStateMinHeap;
+ MergeIterState*, boost::heap::compare<MergeIterStateComparator>> MergeStateMinHeap;
- // The three heaps as described in the algorithm above.
- //
- // Note that the heaps do not "own" any of the objects they contain:
- // - The MergeIterStates in hot_ and cold_ are owned by states_.
- // - The data backing the rows in hotmaxes_ is owned by all states' read_block_.
+ // The min-heaps as described in the algorithm above.
//
- // Care must be taken to remove entries from the heaps when the corresponding
- // objects are destroyed.
+ // Note that none of these containers "own" the objects they contain: the
+ // MergeIterStates are all owned by states_. Care must be taken to remove
+ // entries from the containers in such a way that does not access the
+ // corresponding objects, even if they are destroyed (e.g. if an iterator
+ // state becomes fully exhausted while still in the containers).
//
// Boost offers a variety of different heap data structures[1]. Perf testing
// via generic_iterators-test (TestMerge and TestMergeNonOverlapping with
@@ -587,7 +581,6 @@ class MergeIterator : public RowwiseIterator {
// 1. https://www.boost.org/doc/libs/1_69_0/doc/html/heap/data_structures.html
MergeStateMinHeap hot_;
MergeStateMinHeap cold_;
- RowMinHeap hotmaxes_;
};
MergeIterator::MergeIterator(MergeIteratorOptions opts,
@@ -644,7 +637,7 @@ Status MergeIterator::Init(ScanSpec *spec) {
[](const MergeIterState& s) { return PREDICT_FALSE(s.IsFullyExhausted()); },
[](MergeIterState* s) { delete s; });
- // Establish the merge window and initialize the three heaps.
+ // Establish the merge window and initialize the ordered iterator containers.
for (auto& s : states_) {
cold_.push(&s);
}
@@ -680,27 +673,21 @@ Status MergeIterator::InitSubIterators(ScanSpec *spec) {
Status MergeIterator::AdvanceAndReheap(MergeIterState* state,
size_t num_rows_to_advance) {
- bool pulled_new_block;
+ DCHECK_EQ(state, hot_.top());
+ bool pulled_new_block = false;
RETURN_NOT_OK(state->Advance(num_rows_to_advance, &pulled_new_block));
hot_.pop();
- // Note that hotmaxes_ is not yet popped as it's not necessary to do so if the
- // merge window hasn't changed. Thus, we can avoid some work by deferring it
- // into the cases below.
-
if (state->IsFullyExhausted()) {
- hotmaxes_.pop();
DestroySubIterator(state);
// This sub-iterator's removal means the end of the merge window may have shifted.
RETURN_NOT_OK(RefillHotHeap());
} else if (pulled_new_block) {
- hotmaxes_.pop();
-
// This sub-iterator has a new block, which means the end of the merge window
- //may have shifted.
- if (!hotmaxes_.empty() &&
- schema_->Compare(hotmaxes_.top(), state->next_row()) < 0) {
+ // may have shifted.
+ if (!hot_.empty() &&
+ schema_->Compare(hot_.top()->last_row(), state->next_row()) < 0) {
// The new block lies beyond the new end of the merge window.
VLOG(2) << "Block finished, became cold: " << state->ToString();
cold_.push(state);
@@ -708,7 +695,6 @@ Status MergeIterator::AdvanceAndReheap(MergeIterState* state,
// The new block is still within the merge window.
VLOG(2) << "Block finished, still hot: " << state->ToString();
hot_.push(state);
- hotmaxes_.push(state->last_row());
}
RETURN_NOT_OK(RefillHotHeap());
} else {
@@ -722,8 +708,8 @@ Status MergeIterator::AdvanceAndReheap(MergeIterState* state,
Status MergeIterator::RefillHotHeap() {
VLOG(2) << "Refilling hot heap";
while (!cold_.empty() &&
- (hotmaxes_.empty() ||
- schema_->Compare(hotmaxes_.top(), cold_.top()->next_row()) >= 0)) {
+ (hot_.empty() ||
+ schema_->Compare(hot_.top()->last_row(), cold_.top()->next_row()) >= 0)) {
MergeIterState* warmest = cold_.top();
cold_.pop();
@@ -750,7 +736,6 @@ Status MergeIterator::RefillHotHeap() {
}
VLOG(2) << "Became hot: " << warmest->ToString();
hot_.push(warmest);
- hotmaxes_.push(warmest->last_row());
}
if (VLOG_IS_ON(2)) {
VLOG(2) << "Done refilling hot heap";
@@ -896,6 +881,9 @@ Status MergeIterator::MaterializeOneRow(RowBlock* dst, size_t* dst_row_idx) {
RETURN_NOT_OK(CopyRow(row_to_return_iter->next_row(), &dst_row, dst->arena()));
// Advance all matching sub-iterators and remove any that are exhausted.
+ // Since we're advancing iterators of the same row starting with hot_.top(),
+ // each of these calls is effectively being called on hot_.top(), since the
+ // reheaping will put each top iterator farther down the hot heap.
for (auto& s : smallest) {
RETURN_NOT_OK(AdvanceAndReheap(s, /*num_rows_to_advance=*/1));
}
diff --git a/src/kudu/integration-tests/fuzz-itest.cc b/src/kudu/integration-tests/fuzz-itest.cc
index 1ed8252..9f349aa 100644
--- a/src/kudu/integration-tests/fuzz-itest.cc
+++ b/src/kudu/integration-tests/fuzz-itest.cc
@@ -66,7 +66,7 @@
#include "kudu/util/test_macros.h"
#include "kudu/util/test_util.h"
-DEFINE_int32(keyspace_size, 2, "number of distinct primary keys to test with");
+DEFINE_int32(keyspace_size, 5, "number of distinct primary keys to test with");
DECLARE_bool(enable_maintenance_manager);
DECLARE_bool(scanner_allow_snapshot_scans_with_logical_timestamps);
DECLARE_bool(use_hybrid_clock);
@@ -1310,6 +1310,26 @@ TEST_F(FuzzTest, TestDiffScanRowLifespanInOneScanDRS) {
});
}
+// Regression test for KUDU-3108. Previously caused us to have divergent 'hot'
+// and 'hotmaxes' containers in the merge iterator, causing us to read invalid
+// state and crash.
+TEST_F(FuzzTest, Kudu3108) {
+ CreateTabletAndStartClusterWithSchema(CreateKeyValueTestSchema());
+ RunFuzzCase({
+ {TEST_INSERT, 1},
+ {TEST_FLUSH_OPS},
+ {TEST_FLUSH_TABLET},
+ {TEST_INSERT, 3},
+ {TEST_DELETE, 1},
+ {TEST_FLUSH_OPS},
+ {TEST_FLUSH_TABLET},
+ {TEST_INSERT, 1},
+ {TEST_INSERT, 0},
+ {TEST_FLUSH_OPS},
+ {TEST_DIFF_SCAN, 5, 12},
+ });
+}
+
} // namespace tablet
} // namespace kudu
diff --git a/src/kudu/tablet/diff_scan-test.cc b/src/kudu/tablet/diff_scan-test.cc
index a1db260..dc3edc6 100644
--- a/src/kudu/tablet/diff_scan-test.cc
+++ b/src/kudu/tablet/diff_scan-test.cc
@@ -22,6 +22,7 @@
#include <utility>
#include <vector>
+#include <boost/optional/optional.hpp>
#include <gtest/gtest.h>
#include "kudu/common/common.pb.h"
@@ -99,6 +100,12 @@ TEST_P(DiffScanTest, TestDiffScan) {
static const bool kIsDeletedDefault = false;
SchemaBuilder builder(tablet->metadata()->schema());
if (order_mode == ORDERED) {
+ // Define our diff scan to start from snap1.
+ // NOTE: it isn't critical to set this given the default is -Inf, but it
+ // can't hurt to specify one, given we expect it to be the common case with
+ // the backup jobs.
+ opts.snap_to_exclude = snap1;
+
// The merge iterator requires an IS_DELETED column when including deleted
// rows in order to support deduplication of the rows.
ASSERT_OK(builder.AddColumn("deleted", IS_DELETED,
@@ -140,5 +147,60 @@ TEST_P(DiffScanTest, TestDiffScan) {
}
}
+class OrderedDiffScanWithDeletesTest : public TabletTestBase<IntKeyTestSetup<INT64>> {
+ public:
+ OrderedDiffScanWithDeletesTest()
+ : Superclass(TabletHarness::Options::ClockType::HYBRID_CLOCK) {}
+
+ private:
+ using Superclass = TabletTestBase<IntKeyTestSetup<INT64>>;
+};
+
+// Regression test for KUDU-3108, wherein running the merge iterator on
+// overlapping rowsets could potentially lead to invalid memory access.
+TEST_F(OrderedDiffScanWithDeletesTest, TestKudu3108) {
+ auto tablet = this->tablet();
+ auto tablet_id = tablet->tablet_id();
+
+ LocalTabletWriter writer(tablet.get(), &client_schema_);
+ ASSERT_OK(InsertTestRow(&writer, 1, 1));
+ ASSERT_OK(tablet->Flush());
+
+ MvccSnapshot snap1(*tablet->mvcc_manager());
+ ASSERT_OK(DeleteTestRow(&writer, 1));
+ ASSERT_OK(InsertTestRow(&writer, 3, 1));
+ ASSERT_OK(tablet->Flush());
+
+ ASSERT_OK(InsertTestRow(&writer, 1, 1));
+ ASSERT_OK(InsertTestRow(&writer, 0, 1));
+ ASSERT_OK(tablet->mvcc_manager()->WaitForApplyingOpsToApply());
+ MvccSnapshot snap2(*tablet->mvcc_manager());
+
+ RowIteratorOptions opts;
+ opts.snap_to_exclude = snap1;
+ opts.snap_to_include = snap2;
+ opts.order = ORDERED;
+ opts.include_deleted_rows = true;
+ static const bool kIsDeletedDefault = false;
+ SchemaBuilder builder(tablet->metadata()->schema());
+ ASSERT_OK(builder.AddColumn("deleted", IS_DELETED,
+ /*is_nullable=*/ false,
+ /*read_default=*/ &kIsDeletedDefault,
+ /*write_default=*/ nullptr));
+ Schema projection = builder.BuildWithoutIds();
+ opts.projection = &projection;
+
+ // We should be able to iterate through the rows without issue.
+ unique_ptr<RowwiseIterator> row_iterator;
+ ASSERT_OK(tablet->NewRowIterator(std::move(opts),
+ &row_iterator));
+ ASSERT_TRUE(row_iterator);
+ ScanSpec spec;
+ ASSERT_OK(row_iterator->Init(&spec));
+ vector<string> rows;
+ ASSERT_OK(tablet::IterateToStringList(row_iterator.get(), &rows));
+ ASSERT_EQ(3, rows.size());
+}
+
} // namespace tablet
} // namespace kudu