Skip to content

Commit 4255d49

Browse files
jaykoreanfacebook-github-bot
authored andcommitted
Add reverse order support to MultiScan (#14876)
Summary: Add reverse scan support to `MultiScanArgs`/`MultiScan` so callers can iterate bounded scan ranges in descending order. Plumb reverse scan state through `DBIter`, `BlockBasedTableIterator`, and `MultiScanIndexIterator`, including reverse prefetch windowing and async I/O support. Update `db_bench` `multiscanrandom` to honor `--reverse_iterator` and report rows scanned so reverse MultiScan and iterator baselines can be compared on row throughput. Add `db_stress --multiscan_reverse` coverage so the prepared MultiScan path can be stressed with `SeekForPrev()`/`Prev()` against a control iterator. Also fix the table child iterator path to accept reverse `SeekForPrev()` targets above a file's prepared ranges. `MergingIterator` and `LevelIterator` can legally seek every child to the current range limit; a child file may only have lower prepared ranges, so treating that as invalid caused intermittent `SeekForPrev target is outside prepared ranges` exceptions in the benchmark. Differential Revision: D109357730
1 parent 27b4864 commit 4255d49

17 files changed

Lines changed: 1159 additions & 385 deletions

db/db_iter.cc

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1921,6 +1921,9 @@ Status DBIter::ValidateScanOptions(const MultiScanArgs& multiscan_opts) const {
19211921

19221922
const std::vector<ScanOptions>& scan_opts = multiscan_opts.GetScanRanges();
19231923
const bool has_limit = scan_opts.front().range.limit.has_value();
1924+
if (multiscan_opts.reverse && !has_limit) {
1925+
return Status::InvalidArgument("Reverse MultiScan requires limit");
1926+
}
19241927
if (!has_limit && scan_opts.size() > 1) {
19251928
return Status::InvalidArgument("Scan has no upper bound");
19261929
}
@@ -2004,6 +2007,12 @@ void DBIter::Seek(const Slice& target) {
20042007
StopWatch sw(clock_, statistics_, DB_SEEK);
20052008

20062009
if (scan_opts_.has_value()) {
2010+
if (scan_opts_->reverse) {
2011+
status_ = Status::InvalidArgument("Seek called on reverse MultiScan");
2012+
valid_ = false;
2013+
return;
2014+
}
2015+
20072016
// Validate the seek target is as expected in the previously prepared range
20082017
auto const& scan_ranges = scan_opts_.value().GetScanRanges();
20092018
if (scan_index_ >= scan_ranges.size()) {
@@ -2110,6 +2119,61 @@ void DBIter::SeekForPrev(const Slice& target) {
21102119
PERF_CPU_TIMER_GUARD(iter_seek_cpu_nanos, clock_);
21112120
StopWatch sw(clock_, statistics_, DB_SEEK);
21122121

2122+
if (scan_opts_.has_value()) {
2123+
if (!scan_opts_->reverse) {
2124+
status_ =
2125+
Status::InvalidArgument("SeekForPrev called on forward MultiScan");
2126+
valid_ = false;
2127+
return;
2128+
}
2129+
2130+
auto const& scan_ranges = scan_opts_.value().GetScanRanges();
2131+
if (scan_index_ >= scan_ranges.size()) {
2132+
status_ = Status::InvalidArgument(
2133+
"SeekForPrev called after exhausting all of the scan ranges");
2134+
valid_ = false;
2135+
return;
2136+
}
2137+
2138+
auto const& range = scan_ranges[scan_index_];
2139+
auto const& limit = range.range.limit;
2140+
assert(limit.has_value());
2141+
if (!limit.has_value() ||
2142+
user_comparator_.CompareWithoutTimestamp(target, *limit) != 0) {
2143+
status_ = Status::InvalidArgument(
2144+
"SeekForPrev target does not match the limit of the next prepared "
2145+
"range at index " +
2146+
std::to_string(scan_index_));
2147+
valid_ = false;
2148+
return;
2149+
}
2150+
2151+
auto const& start = range.range.start;
2152+
assert(start.has_value());
2153+
if (iterate_lower_bound_ == nullptr ||
2154+
user_comparator_.CompareWithoutTimestamp(start.value(),
2155+
*iterate_lower_bound_) != 0) {
2156+
status_ = Status::InvalidArgument(
2157+
"Lower bound is not set to the same start value of the next "
2158+
"prepared range at index " +
2159+
std::to_string(scan_index_));
2160+
valid_ = false;
2161+
return;
2162+
}
2163+
if (iterate_upper_bound_ == nullptr ||
2164+
user_comparator_.CompareWithoutTimestamp(limit.value(),
2165+
*iterate_upper_bound_) != 0) {
2166+
status_ = Status::InvalidArgument(
2167+
"Upper bound is not set to the same limit value of the next "
2168+
"prepared range at index " +
2169+
std::to_string(scan_index_));
2170+
valid_ = false;
2171+
return;
2172+
}
2173+
2174+
scan_index_++;
2175+
}
2176+
21132177
if (has_trace_state_) {
21142178
// TODO: What do we do if this returns an error?
21152179
Slice lower_bound, upper_bound;

db/db_iterator_test.cc

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5418,6 +5418,98 @@ TEST_P(DBMultiScanIteratorTest, AsyncPrefetchAcrossMultipleFiles) {
54185418
iter.reset();
54195419
}
54205420

5421+
TEST_P(DBMultiScanIteratorTest, ReversePrefetchAcrossMultipleRanges) {
5422+
auto options = CurrentOptions();
5423+
options.target_file_size_base = 1 << 15; // 32KiB
5424+
options.compaction_style = kCompactionStyleUniversal;
5425+
options.num_levels = 50;
5426+
options.compression = kNoCompression;
5427+
DestroyAndReopen(options);
5428+
5429+
Random rnd(303);
5430+
for (int i = 0; i < 1000; ++i) {
5431+
ASSERT_OK(Put(Key(i), rnd.RandomString(1 << 10)));
5432+
}
5433+
ASSERT_OK(Flush());
5434+
ASSERT_OK(db_->CompactRange({}, nullptr, nullptr));
5435+
ASSERT_GT(NumTableFilesAtLevel(49), 3);
5436+
5437+
ReadOptions ro;
5438+
ro.fill_cache = GetParam();
5439+
ColumnFamilyHandle* cfh = dbfull()->DefaultColumnFamily();
5440+
auto tracking_dispatcher = std::make_shared<TrackingIODispatcher>();
5441+
5442+
MultiScanArgs scan_options(BytewiseComparator());
5443+
scan_options.use_async_io = false;
5444+
scan_options.reverse = true;
5445+
scan_options.io_dispatcher = tracking_dispatcher;
5446+
std::vector<std::string> key_ranges(
5447+
{Key(100), Key(200), Key(500), Key(600), Key(800), Key(900)});
5448+
scan_options.insert(key_ranges[0], key_ranges[1]);
5449+
scan_options.insert(key_ranges[2], key_ranges[3]);
5450+
scan_options.insert(key_ranges[4], key_ranges[5]);
5451+
5452+
std::unique_ptr<MultiScan> iter =
5453+
dbfull()->NewMultiScan(ro, cfh, scan_options);
5454+
ASSERT_NE(iter, nullptr);
5455+
5456+
std::vector<std::string> actual_keys;
5457+
try {
5458+
for (auto range : *iter) {
5459+
for (auto it : range) {
5460+
actual_keys.push_back(it.first.ToString());
5461+
}
5462+
}
5463+
} catch (MultiScanException& ex) {
5464+
FAIL() << "Iterator returned status " << ex.what();
5465+
} catch (std::logic_error& ex) {
5466+
FAIL() << "Iterator returned logic error " << ex.what();
5467+
}
5468+
5469+
std::vector<std::string> expected_keys;
5470+
for (int i = 199; i >= 100; --i) {
5471+
expected_keys.push_back(Key(i));
5472+
}
5473+
for (int i = 599; i >= 500; --i) {
5474+
expected_keys.push_back(Key(i));
5475+
}
5476+
for (int i = 899; i >= 800; --i) {
5477+
expected_keys.push_back(Key(i));
5478+
}
5479+
ASSERT_EQ(actual_keys, expected_keys);
5480+
ASSERT_GT(tracking_dispatcher->GetReadSets().size(), 0);
5481+
}
5482+
5483+
TEST_P(DBMultiScanIteratorTest, ReverseRequiresLimits) {
5484+
auto options = CurrentOptions();
5485+
options.compression = kNoCompression;
5486+
DestroyAndReopen(options);
5487+
5488+
ASSERT_OK(Put("a", "va"));
5489+
ASSERT_OK(Put("b", "vb"));
5490+
ASSERT_OK(Flush());
5491+
5492+
ReadOptions ro;
5493+
ro.fill_cache = GetParam();
5494+
ColumnFamilyHandle* cfh = dbfull()->DefaultColumnFamily();
5495+
5496+
MultiScanArgs scan_options(BytewiseComparator());
5497+
scan_options.reverse = true;
5498+
scan_options.insert("a");
5499+
scan_options.insert("b", "c");
5500+
5501+
std::unique_ptr<MultiScan> iter =
5502+
dbfull()->NewMultiScan(ro, cfh, scan_options);
5503+
ASSERT_NE(iter, nullptr);
5504+
try {
5505+
(void)iter->begin();
5506+
FAIL() << "Expected reverse MultiScan without an upper bound to fail";
5507+
} catch (MultiScanException& ex) {
5508+
ASSERT_NOK(ex.status());
5509+
ASSERT_TRUE(ex.status().IsInvalidArgument());
5510+
}
5511+
}
5512+
54215513
// Wrapper filesystem that does not support async IO.
54225514
// Used to verify that MultiScan gracefully falls back to sync IO.
54235515
class NoAsyncIOFS : public FileSystemWrapper {

db/multi_scan.cc

Lines changed: 103 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,56 @@
33
// COPYING file in the root directory) and Apache 2.0 License
44
// (found in the LICENSE.Apache file in the root directory).
55

6+
#include <string>
7+
#include <vector>
8+
69
#include "file/file_util.h"
710
#include "rocksdb/db.h"
811
#include "rocksdb/file_system.h"
12+
#include "rocksdb/iterator.h"
913

1014
namespace ROCKSDB_NAMESPACE {
1115

1216
using MultiScanIterator = MultiScan::MultiScanIterator;
1317

18+
namespace {
19+
20+
Status ValidateScanOptionsForMultiScan(const MultiScanArgs& multiscan_opts) {
21+
if (multiscan_opts.empty()) {
22+
return Status::InvalidArgument("Empty MultiScanArgs");
23+
}
24+
25+
const Comparator* comparator = multiscan_opts.GetComparator();
26+
if (comparator == nullptr) {
27+
return Status::InvalidArgument("MultiScanArgs requires comparator");
28+
}
29+
30+
const std::vector<ScanOptions>& scan_opts = multiscan_opts.GetScanRanges();
31+
for (size_t i = 0; i < scan_opts.size(); ++i) {
32+
const auto& scan_range = scan_opts[i].range;
33+
if (!scan_range.start.has_value()) {
34+
return Status::InvalidArgument("Scan has no start key at index " +
35+
std::to_string(i));
36+
}
37+
if (multiscan_opts.reverse && !scan_range.limit.has_value()) {
38+
return Status::InvalidArgument(
39+
"Reverse MultiScan requires limit at index " + std::to_string(i));
40+
}
41+
if (scan_range.limit.has_value() &&
42+
comparator->CompareWithoutTimestamp(
43+
scan_range.start.value(), /*a_has_ts=*/false,
44+
scan_range.limit.value(), /*b_has_ts=*/false) >= 0) {
45+
return Status::InvalidArgument(
46+
"Scan start key is large or equal than limit at index " +
47+
std::to_string(i));
48+
}
49+
}
50+
51+
return Status::OK();
52+
}
53+
54+
} // namespace
55+
1456
MultiScan::MultiScan(const ReadOptions& read_options,
1557
const MultiScanArgs& scan_opts, DB* db,
1658
ColumnFamilyHandle* cfh)
@@ -28,14 +70,28 @@ MultiScan::MultiScan(const ReadOptions& read_options,
2870
}()),
2971
db_(db),
3072
cfh_(cfh) {
73+
Status validate_status = ValidateScanOptionsForMultiScan(scan_opts_);
74+
if (!validate_status.ok()) {
75+
db_iter_.reset(NewErrorIterator(validate_status));
76+
return;
77+
}
78+
3179
bool slow_path = false;
32-
// Setup read_options with iterate_uuper_bound based on the first scan.
33-
// Subsequent scans will update and allocate a new DB iterator as necessary
34-
if (scan_opts_.GetScanRanges()[0].range.limit) {
35-
upper_bound_ = *scan_opts_.GetScanRanges()[0].range.limit;
36-
read_options_.iterate_upper_bound = &upper_bound_;
37-
} else {
38-
read_options_.iterate_upper_bound = nullptr;
80+
// Setup read_options bounds based on the first scan. Subsequent scans will
81+
// update the bounds and allocate a new DB iterator as necessary.
82+
{
83+
const ScanOptions& first_scan = scan_opts_.GetScanRanges()[0];
84+
if (scan_opts_.reverse) {
85+
lower_bound_ = *first_scan.range.start;
86+
read_options_.iterate_lower_bound = &lower_bound_;
87+
upper_bound_ = *first_scan.range.limit;
88+
read_options_.iterate_upper_bound = &upper_bound_;
89+
} else if (first_scan.range.limit) {
90+
upper_bound_ = *first_scan.range.limit;
91+
read_options_.iterate_upper_bound = &upper_bound_;
92+
} else {
93+
read_options_.iterate_upper_bound = nullptr;
94+
}
3995
}
4096
for (const auto& opts : scan_opts_.GetScanRanges()) {
4197
// Check that all the ScanOptions either specify an upper bound or not. If
@@ -54,36 +110,55 @@ MultiScan::MultiScan(const ReadOptions& read_options,
54110
}
55111
}
56112

113+
void MultiScanIterator::SeekCurrentRange() {
114+
const ScanOptions& scan_opt = scan_opts_[idx_];
115+
if (scan_opt.range.limit) {
116+
*upper_bound_ = *scan_opt.range.limit;
117+
read_options_.iterate_upper_bound = upper_bound_;
118+
} else {
119+
read_options_.iterate_upper_bound = nullptr;
120+
}
121+
122+
if (reverse_) {
123+
*lower_bound_ = *scan_opt.range.start;
124+
read_options_.iterate_lower_bound = lower_bound_;
125+
assert(scan_opt.range.limit.has_value());
126+
db_iter_->SeekForPrev(*scan_opt.range.limit);
127+
} else {
128+
db_iter_->Seek(*scan_opt.range.start);
129+
}
130+
}
131+
57132
MultiScanIterator& MultiScanIterator::operator++() {
58133
status_ = db_iter_->status();
59134
if (!status_.ok()) {
60135
throw MultiScanException(status_);
61136
}
62137

138+
if (!valid_) {
139+
throw std::logic_error("Incrementing exhausted MultiScan iterator");
140+
}
141+
++idx_;
63142
if (idx_ >= scan_opts_.size()) {
64-
throw std::logic_error("Index out of range");
143+
valid_ = false;
144+
return *this;
65145
}
66-
idx_++;
67-
if (idx_ < scan_opts_.size()) {
68-
// Check if we need to update read_options_
69-
if (scan_opts_[idx_].range.limit.has_value() !=
70-
(read_options_.iterate_upper_bound != nullptr)) {
71-
if (scan_opts_[idx_].range.limit) {
72-
*upper_bound_ = *scan_opts_[idx_].range.limit;
73-
read_options_.iterate_upper_bound = upper_bound_;
74-
} else {
75-
read_options_.iterate_upper_bound = nullptr;
76-
}
77-
db_iter_.reset(db_->NewIterator(read_options_, cfh_));
78-
scan_.Reset(db_iter_.get());
79-
} else if (scan_opts_[idx_].range.limit) {
80-
*upper_bound_ = *scan_opts_[idx_].range.limit;
81-
}
82-
db_iter_->Seek(*scan_opts_[idx_].range.start);
83-
status_ = db_iter_->status();
84-
if (!status_.ok()) {
85-
throw MultiScanException(status_);
146+
147+
// Check if we need to update read_options_
148+
if (scan_opts_[idx_].range.limit.has_value() !=
149+
(read_options_.iterate_upper_bound != nullptr)) {
150+
if (scan_opts_[idx_].range.limit) {
151+
read_options_.iterate_upper_bound = upper_bound_;
152+
} else {
153+
read_options_.iterate_upper_bound = nullptr;
86154
}
155+
db_iter_.reset(db_->NewIterator(read_options_, cfh_));
156+
scan_.Reset(db_iter_.get(), reverse_);
157+
}
158+
SeekCurrentRange();
159+
status_ = db_iter_->status();
160+
if (!status_.ok()) {
161+
throw MultiScanException(status_);
87162
}
88163
return *this;
89164
}

db_stress_tool/db_stress_common.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,7 @@ DECLARE_uint32(min_tombstones_for_range_conversion);
470470
DECLARE_uint32(ingest_wbwi_one_in);
471471
DECLARE_bool(universal_reduce_file_locking);
472472
DECLARE_bool(use_multiscan);
473+
DECLARE_bool(multiscan_reverse);
473474
DECLARE_bool(multiscan_use_async_io);
474475
DECLARE_bool(read_scoped_block_buffer_provider);
475476
DECLARE_uint64(multiscan_max_prefetch_memory_bytes);

db_stress_tool/db_stress_gflags.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1723,6 +1723,10 @@ DEFINE_bool(
17231723
DEFINE_bool(use_multiscan, false,
17241724
"If set, use the batched MultiScan API for scans.");
17251725

1726+
DEFINE_bool(multiscan_reverse, false,
1727+
"If set with use_multiscan, scan each MultiScan range in reverse "
1728+
"using SeekForPrev and Prev.");
1729+
17261730
DEFINE_bool(multiscan_use_async_io, false,
17271731
"If set, enable async_io for MultiScan operations.");
17281732

0 commit comments

Comments
 (0)