Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up performance of range tombstones #3992

Closed
wants to merge 4 commits into from
Closed

Speed up performance of range tombstones #3992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d5eb148
Test internals of range collapsing
benesch Jun 13, 2018
4535268
Simplify range deletion collapsing
benesch Jun 13, 2018
214a176
Avoid unnecessary ifs in deletion collapsing
benesch Jun 13, 2018
825f0eb
Avoid unnecessary scans when collapsing tombstones
benesch Jun 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 31 additions & 31 deletions db/range_del_aggregator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,17 +282,13 @@ Status RangeDelAggregator::AddTombstone(RangeTombstone tombstone) {
new_range_dels_iter != new_range_dels.end()) {
const Slice *tombstone_map_iter_end = nullptr,
*new_range_dels_iter_end = nullptr;
if (tombstone_map_iter != tombstone_map.end()) {
auto next_tombstone_map_iter = std::next(tombstone_map_iter);
if (next_tombstone_map_iter != tombstone_map.end()) {
tombstone_map_iter_end = &next_tombstone_map_iter->first;
}
auto next_tombstone_map_iter = std::next(tombstone_map_iter);
if (next_tombstone_map_iter != tombstone_map.end()) {
tombstone_map_iter_end = &next_tombstone_map_iter->first;
}
if (new_range_dels_iter != new_range_dels.end()) {
auto next_new_range_dels_iter = std::next(new_range_dels_iter);
if (next_new_range_dels_iter != new_range_dels.end()) {
new_range_dels_iter_end = &next_new_range_dels_iter->start_key_;
}
auto next_new_range_dels_iter = std::next(new_range_dels_iter);
if (next_new_range_dels_iter != new_range_dels.end()) {
new_range_dels_iter_end = &next_new_range_dels_iter->start_key_;
}

// our positions in existing/new tombstone collections should always
Expand All @@ -307,24 +303,19 @@ Status RangeDelAggregator::AddTombstone(RangeTombstone tombstone) {

int new_to_old_start_cmp = icmp_.user_comparator()->Compare(
new_range_dels_iter->start_key_, tombstone_map_iter->first);
// nullptr end means extends infinitely rightwards, set new_to_old_end_cmp
// accordingly so we can use common code paths later.
int new_to_old_end_cmp;
if (new_range_dels_iter_end == nullptr &&
tombstone_map_iter_end == nullptr) {
new_to_old_end_cmp = 0;
} else if (new_range_dels_iter_end == nullptr) {
new_to_old_end_cmp = 1;
} else if (tombstone_map_iter_end == nullptr) {
new_to_old_end_cmp = -1;
} else {
// If we're looking at either the last new tombstone or the last existing
// tombstone, we can't compare against nullptr, but we know that the new
// tombstone logically ends before the existing tombstone.
int new_to_old_end_cmp = -1;
if (new_range_dels_iter_end != nullptr &&
tombstone_map_iter_end != nullptr) {
new_to_old_end_cmp = icmp_.user_comparator()->Compare(
*new_range_dels_iter_end, *tombstone_map_iter_end);
}

if (new_to_old_start_cmp < 0) {
// the existing one's left endpoint comes after, so raise/delete it if
// it's covered.
if (new_to_old_start_cmp <= 0) {
// the existing one's left endpoint comes after or coincides, so
// raise/delete the right endpoint if it's covered.
if (tombstone_map_iter->second.seq_ < new_range_dels_iter->seq_) {
untermed_seq = tombstone_map_iter->second.seq_;
if (tombstone_map_iter != tombstone_map.begin() &&
Expand All @@ -336,7 +327,7 @@ Status RangeDelAggregator::AddTombstone(RangeTombstone tombstone) {
tombstone_map_iter->second.seq_ = new_range_dels_iter->seq_;
}
}
} else if (new_to_old_start_cmp > 0) {
} else {
if (untermed_seq != kMaxSequenceNumber ||
tombstone_map_iter->second.seq_ < new_range_dels_iter->seq_) {
auto seq = tombstone_map_iter->second.seq_;
Expand All @@ -352,12 +343,6 @@ Status RangeDelAggregator::AddTombstone(RangeTombstone tombstone) {
new_range_dels_iter->seq_)));
untermed_seq = seq;
}
} else {
// their left endpoints coincide, so raise the existing one if needed
if (tombstone_map_iter->second.seq_ < new_range_dels_iter->seq_) {
untermed_seq = tombstone_map_iter->second.seq_;
tombstone_map_iter->second.seq_ = new_range_dels_iter->seq_;
}
}

// advance whichever one ends earlier, or both if their right endpoints
Expand Down Expand Up @@ -547,4 +532,19 @@ bool RangeDelAggregator::AddFile(uint64_t file_number) {
return rep_->added_files_.emplace(file_number).second;
}

RangeDelAggregator::CollapsedTombstoneMap
RangeDelAggregator::DumpCollapsedTombstones() {
if (rep_ == nullptr) {
return CollapsedTombstoneMap();
}
assert(collapse_deletions_);
assert(rep_->stripe_map_.size() == 2);
auto& tombstone_map = GetPositionalTombstoneMap(1 /* valid seqno */).raw_map;
CollapsedTombstoneMap out;
for (auto it = tombstone_map.begin(); it != tombstone_map.end(); ++it) {
out.emplace(it->first, it->second.seq_);
}
return out;
}

} // namespace rocksdb
7 changes: 7 additions & 0 deletions db/range_del_aggregator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ class RangeDelAggregator {
bool IsEmpty();
bool AddFile(uint64_t file_number);

// Only for testing. DumpCollapsedTombstones can be called only if
// collapse_deletions_ is true and only the default snapshot stripes are
// present.
typedef std::multimap<Slice, SequenceNumber, stl_wrappers::LessOfComparator>
CollapsedTombstoneMap;
CollapsedTombstoneMap DumpCollapsedTombstones();

private:
// Maps tombstone user start key -> tombstone object
typedef std::multimap<Slice, RangeTombstone, stl_wrappers::LessOfComparator>
Expand Down
87 changes: 64 additions & 23 deletions db/range_del_aggregator_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ enum Direction {
};

void VerifyRangeDels(const std::vector<RangeTombstone>& range_dels,
const std::vector<ExpectedPoint>& expected_points) {
const std::vector<ExpectedPoint>& expected_points,
const RangeDelAggregator::CollapsedTombstoneMap& expected_map) {
auto icmp = InternalKeyComparator(BytewiseComparator());
// Test same result regardless of which order the range deletions are added.
for (Direction dir : {kForward, kReverse}) {
Expand Down Expand Up @@ -61,6 +62,20 @@ void VerifyRangeDels(const std::vector<RangeTombstone>& range_dels,
RangeDelAggregator::RangePositioningMode::kForwardTraversal));
}
}

auto actual_map = range_del_agg.DumpCollapsedTombstones();
auto ite = expected_map.begin();
auto ita = actual_map.begin();
for (; ite != expected_map.end() && ita != actual_map.end(); ite++, ita++) {
ASSERT_EQ(ite->first.ToString(), ita->first.ToString());
ASSERT_EQ(ite->second, ita->second);
}
if (ite != expected_map.end()) {
ADD_FAILURE() << "tombstone map missing key: " << ite->first.ToString();
}
if (ita != actual_map.end()) {
ADD_FAILURE() << "tombstone map has extra key: " << ita->first.ToString();
}
}

RangeDelAggregator range_del_agg(icmp, {} /* snapshots */,
Expand All @@ -87,80 +102,105 @@ void VerifyRangeDels(const std::vector<RangeTombstone>& range_dels,

} // anonymous namespace

TEST_F(RangeDelAggregatorTest, Empty) { VerifyRangeDels({}, {{"a", 0}}); }
TEST_F(RangeDelAggregatorTest, Empty) {
VerifyRangeDels({},
{{"a", 0}},
{});
}

TEST_F(RangeDelAggregatorTest, SameStartAndEnd) {
VerifyRangeDels({{"a", "a", 5}}, {{" ", 0}, {"a", 0}, {"b", 0}});
VerifyRangeDels({{"a", "a", 5}},
{{" ", 0}, {"a", 0}, {"b", 0}},
{{"a", 5}, {"a", 0}});
}

TEST_F(RangeDelAggregatorTest, Single) {
VerifyRangeDels({{"a", "b", 10}}, {{" ", 0}, {"a", 10}, {"b", 0}});
VerifyRangeDels({{"a", "b", 10}},
{{" ", 0}, {"a", 10}, {"b", 0}},
{{"a", 10}, {"b", 0}});
}

TEST_F(RangeDelAggregatorTest, OverlapAboveLeft) {
VerifyRangeDels({{"a", "c", 10}, {"b", "d", 5}},
{{" ", 0}, {"a", 10}, {"c", 5}, {"d", 0}});
{{" ", 0}, {"a", 10}, {"c", 5}, {"d", 0}},
{{"a", 10}, {"c", 5}, {"d", 0}});
}

TEST_F(RangeDelAggregatorTest, OverlapAboveRight) {
VerifyRangeDels({{"a", "c", 5}, {"b", "d", 10}},
{{" ", 0}, {"a", 5}, {"b", 10}, {"d", 0}});
{{" ", 0}, {"a", 5}, {"b", 10}, {"d", 0}},
{{"a", 5}, {"b", 10}, {"d", 0}});
}

TEST_F(RangeDelAggregatorTest, OverlapAboveMiddle) {
VerifyRangeDels({{"a", "d", 5}, {"b", "c", 10}},
{{" ", 0}, {"a", 5}, {"b", 10}, {"c", 5}, {"d", 0}});
{{" ", 0}, {"a", 5}, {"b", 10}, {"c", 5}, {"d", 0}},
{{"a", 5}, {"b", 10}, {"c", 5}, {"d", 0}});
}

TEST_F(RangeDelAggregatorTest, OverlapFully) {
VerifyRangeDels({{"a", "d", 10}, {"b", "c", 5}},
{{" ", 0}, {"a", 10}, {"d", 0}});
{{" ", 0}, {"a", 10}, {"d", 0}},
{{"a", 10}, {"d", 0}});
}

TEST_F(RangeDelAggregatorTest, OverlapPoint) {
VerifyRangeDels({{"a", "b", 5}, {"b", "c", 10}},
{{" ", 0}, {"a", 5}, {"b", 10}, {"c", 0}});
{{" ", 0}, {"a", 5}, {"b", 10}, {"c", 0}},
{{"a", 5}, {"b", 10}, {"c", 0}});
}

TEST_F(RangeDelAggregatorTest, SameStartKey) {
VerifyRangeDels({{"a", "c", 5}, {"a", "b", 10}},
{{" ", 0}, {"a", 10}, {"b", 5}, {"c", 0}});
{{" ", 0}, {"a", 10}, {"b", 5}, {"c", 0}},
{{"a", 10}, {"b", 5}, {"c", 0}});
}

TEST_F(RangeDelAggregatorTest, SameEndKey) {
VerifyRangeDels({{"a", "d", 5}, {"b", "d", 10}},
{{" ", 0}, {"a", 5}, {"b", 10}, {"d", 0}});
{{" ", 0}, {"a", 5}, {"b", 10}, {"d", 0}},
{{"a", 5}, {"b", 10}, {"d", 0}});
}

TEST_F(RangeDelAggregatorTest, GapsBetweenRanges) {
VerifyRangeDels({{"a", "b", 5}, {"c", "d", 10}, {"e", "f", 15}}, {{" ", 0},
{"a", 5},
{"b", 0},
{"c", 10},
{"d", 0},
{"da", 0},
{"e", 15},
{"f", 0}});
VerifyRangeDels({{"a", "b", 5}, {"c", "d", 10}, {"e", "f", 15}},
{{" ", 0},
{"a", 5},
{"b", 0},
{"c", 10},
{"d", 0},
{"da", 0},
{"e", 15},
{"f", 0}},
{{"a", 5},
{"b", 0},
{"c", 10},
{"d", 0},
{"e", 15},
{"f", 0}});
}

// Note the Cover* tests also test cases where tombstones are inserted under a
// larger one when VerifyRangeDels() runs them in reverse
TEST_F(RangeDelAggregatorTest, CoverMultipleFromLeft) {
VerifyRangeDels(
{{"b", "d", 5}, {"c", "f", 10}, {"e", "g", 15}, {"a", "f", 20}},
{{" ", 0}, {"a", 20}, {"f", 15}, {"g", 0}});
{{" ", 0}, {"a", 20}, {"f", 15}, {"g", 0}},
{{"a", 20}, {"f", 15}, {"g", 0}});
}

TEST_F(RangeDelAggregatorTest, CoverMultipleFromRight) {
VerifyRangeDels(
{{"b", "d", 5}, {"c", "f", 10}, {"e", "g", 15}, {"c", "h", 20}},
{{" ", 0}, {"b", 5}, {"c", 20}, {"h", 0}});
{{" ", 0}, {"b", 5}, {"c", 20}, {"h", 0}},
{{"b", 5}, {"c", 20}, {"h", 0}});
}

TEST_F(RangeDelAggregatorTest, CoverMultipleFully) {
VerifyRangeDels(
{{"b", "d", 5}, {"c", "f", 10}, {"e", "g", 15}, {"a", "h", 20}},
{{" ", 0}, {"a", 20}, {"h", 0}});
{{" ", 0}, {"a", 20}, {"h", 0}},
{{"a", 20}, {"h", 0}});
}

TEST_F(RangeDelAggregatorTest, AlternateMultipleAboveBelow) {
Expand All @@ -172,7 +212,8 @@ TEST_F(RangeDelAggregatorTest, AlternateMultipleAboveBelow) {
{"d", 10},
{"e", 20},
{"g", 5},
{"h", 0}});
{"h", 0}},
{{"a", 5}, {"b", 15}, {"d", 10}, {"e", 20}, {"g", 5}, {"h", 0}});
}

} // namespace rocksdb
Expand Down