Skip to content

Commit

Permalink
Extend PartialRangeTombstone to use InternalKeys
Browse files Browse the repository at this point in the history 
Change `RangeDelAggregator::GetTombstone,ShouldDeleteRange` to take
internal keys instead of user keys.
  • Loading branch information
@petermattis
petermattis committed Oct 16, 2018
1 parent a8370d4 commit 6f11959
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 95 deletions.
16 changes: 9 additions & 7 deletions db/dbformat.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -635,13 +635,15 @@ class PartialRangeTombstone {
PartialRangeTombstone()
: start_key_valid_(false), end_key_valid_(false), seq_(0) {}

PartialRangeTombstone(const Slice* sk, const Slice* ek, SequenceNumber sq)
PartialRangeTombstone(const ParsedInternalKey* sk,
const ParsedInternalKey* ek,
SequenceNumber sq)
: seq_(sq) {
SetStartKey(sk);
SetEndKey(ek);
}

void SetStartKey(const Slice* sk) {
void SetStartKey(const ParsedInternalKey* sk) {
if (sk != nullptr) {
start_key_ = *sk;
start_key_valid_ = true;
Expand All @@ -650,7 +652,7 @@ class PartialRangeTombstone {
}
}

void SetEndKey(const Slice* ek) {
void SetEndKey(const ParsedInternalKey* ek) {
if (ek != nullptr) {
end_key_ = *ek;
end_key_valid_ = true;
Expand All @@ -659,15 +661,15 @@ class PartialRangeTombstone {
}
}

const Slice* start_key() const {
const ParsedInternalKey* start_key() const {
return start_key_valid_ ? &start_key_ : nullptr;
}
const Slice* end_key() const { return end_key_valid_ ? &end_key_ : nullptr; }
const ParsedInternalKey* end_key() const { return end_key_valid_ ? &end_key_ : nullptr; }
SequenceNumber seq() const { return seq_; }

private:
Slice start_key_;
Slice end_key_;
ParsedInternalKey start_key_;
ParsedInternalKey end_key_;
bool start_key_valid_;
bool end_key_valid_;
SequenceNumber seq_;
Expand Down
23 changes: 15 additions & 8 deletions db/range_del_aggregator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,25 +298,25 @@ class CollapsedRangeDelMap : public RangeDelMap {
if (!ParseInternalKey(end, &parsed_end)) {
assert(false);
}
if (ucmp_->Compare(parsed_start.user_key, parsed_end.user_key) > 0) {
if (icmp_->Compare(parsed_start, parsed_end) > 0) {
return false;
}

auto iter = rep_.upper_bound(parsed_start.user_key);
auto iter = rep_.upper_bound(parsed_start);
if (iter == rep_.begin()) {
// before start of deletion intervals
return false;
}
--iter;
if (ucmp_->Compare(parsed_start.user_key, iter->first) < 0) {
if (icmp_->Compare(parsed_start, iter->first) < 0) {
assert(false);
return false;
}
// Loop looking for a tombstone that is older than the range sequence
// number, or we determine that our range is completely covered by newer
// tombstones.
for (; iter != rep_.end(); ++iter) {
if (ucmp_->Compare(parsed_end.user_key, iter->first) < 0) {
if (icmp_->Compare(parsed_end, iter->first) < 0) {
return true;
}
if (seqno >= iter->second) {
Expand All @@ -327,9 +327,16 @@ class CollapsedRangeDelMap : public RangeDelMap {
return false;
}

PartialRangeTombstone GetTombstone(const Slice& user_key,
PartialRangeTombstone GetTombstone(const Slice& key,
SequenceNumber seqno) override {
auto iter = rep_.upper_bound(user_key);
ParsedInternalKey parsed_key;
if (!ParseInternalKey(key, &parsed_key)) {
assert(false);
// Fail open.
return PartialRangeTombstone();
}

auto iter = rep_.upper_bound(parsed_key);
if (iter == rep_.begin()) {
// before start of deletion intervals
return PartialRangeTombstone(nullptr, &iter->first, 0);
Expand Down Expand Up @@ -583,7 +590,7 @@ bool RangeDelAggregator::ShouldDeleteRange(
return tombstone_map.ShouldDeleteRange(start, end, seqno);
}

PartialRangeTombstone RangeDelAggregator::GetTombstone(const Slice& user_key,
PartialRangeTombstone RangeDelAggregator::GetTombstone(const Slice& key,
SequenceNumber seqno) {
if (rep_ == nullptr) {
return PartialRangeTombstone();
Expand All @@ -592,7 +599,7 @@ PartialRangeTombstone RangeDelAggregator::GetTombstone(const Slice& user_key,
if (tombstone_map.IsEmpty()) {
return PartialRangeTombstone();
}
return tombstone_map.GetTombstone(user_key, seqno);
return tombstone_map.GetTombstone(key, seqno);
}

bool RangeDelAggregator::IsRangeOverlapped(const Slice& start,
Expand Down
12 changes: 6 additions & 6 deletions db/range_del_aggregator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class RangeDelMap {
RangeDelPositioningMode mode) = 0;
virtual bool ShouldDeleteRange(const Slice& start, const Slice& end,
SequenceNumber seqno) = 0;
virtual PartialRangeTombstone GetTombstone(const Slice& user_key,
virtual PartialRangeTombstone GetTombstone(const Slice& key,
SequenceNumber seqno) = 0;
virtual bool IsRangeOverlapped(const ParsedInternalKey& start,
const ParsedInternalKey& end) = 0;
Expand Down Expand Up @@ -171,11 +171,11 @@ class RangeDelAggregator {
bool ShouldDeleteRange(const Slice& start, const Slice& end,
SequenceNumber seqno);

// Get the range tombstone at the specified user_key and sequence number. A
// valid tombstone is always returned, though it may cover an empty range of
// keys or the sequence number may be 0 to indicate that no tombstone covers
// the specified key.
PartialRangeTombstone GetTombstone(const Slice& user_key,
// Get the range tombstone at the specified internal key and sequence
// number. A valid tombstone is always returned, though it may cover an
// empty range of keys or the sequence number may be 0 to indicate that no
// tombstone covers the specified key.
PartialRangeTombstone GetTombstone(const Slice& key,
SequenceNumber seqno);

// Checks whether range deletions cover any keys between `start` and `end`,
Expand Down
124 changes: 77 additions & 47 deletions db/range_del_aggregator_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,16 @@ void VerifyPartialTombstonesEq(const PartialRangeTombstone& a,
const PartialRangeTombstone& b) {
ASSERT_EQ(a.seq(), b.seq());
if (a.start_key() != nullptr) {
ASSERT_EQ(*a.start_key(), *b.start_key());
ASSERT_EQ(a.start_key()->user_key, b.start_key()->user_key);
ASSERT_EQ(a.start_key()->sequence, b.start_key()->sequence);
ASSERT_EQ(a.start_key()->type, b.start_key()->type);
} else {
ASSERT_EQ(b.start_key(), nullptr);
}
if (a.end_key() != nullptr) {
ASSERT_EQ(*a.end_key(), *b.end_key());
ASSERT_EQ(a.end_key()->user_key, b.end_key()->user_key);
ASSERT_EQ(a.end_key()->sequence, b.end_key()->sequence);
ASSERT_EQ(a.end_key()->type, b.end_key()->type);
} else {
ASSERT_EQ(b.end_key(), nullptr);
}
Expand Down Expand Up @@ -170,41 +174,28 @@ void VerifyRangeDels(
}
}

bool ShouldDeleteRange(const std::vector<RangeTombstone>& range_dels,
bool ShouldDeleteRange(const AddTombstonesArgs& range_dels,
const ExpectedRange& expected_range) {
RangeDelAggregator range_del_agg(bytewise_icmp, {} /* snapshots */, true);
std::vector<std::string> keys, values;
for (const auto& range_del : range_dels) {
auto key_and_value = range_del.Serialize();
keys.push_back(key_and_value.first.Encode().ToString());
values.push_back(key_and_value.second.ToString());
}
std::unique_ptr<test::VectorIterator> range_del_iter(
new test::VectorIterator(keys, values));
range_del_agg.AddTombstones(std::move(range_del_iter));
AddTombstones(&range_del_agg, range_dels.tombstones,
range_dels.smallest, range_dels.largest);

std::string begin, end;
AppendInternalKey(&begin, {expected_range.begin, expected_range.seq, kTypeValue});
AppendInternalKey(&end, {expected_range.end, expected_range.seq, kTypeValue});
return range_del_agg.ShouldDeleteRange(begin, end, expected_range.seq);
}

void VerifyGetTombstone(const std::vector<RangeTombstone>& range_dels,
void VerifyGetTombstone(const AddTombstonesArgs& range_dels,
const ExpectedPoint& expected_point,
const PartialRangeTombstone& expected_tombstone) {
RangeDelAggregator range_del_agg(bytewise_icmp, {} /* snapshots */, true);
ASSERT_TRUE(range_del_agg.IsEmpty());
std::vector<std::string> keys, values;
for (const auto& range_del : range_dels) {
auto key_and_value = range_del.Serialize();
keys.push_back(key_and_value.first.Encode().ToString());
values.push_back(key_and_value.second.ToString());
}
std::unique_ptr<test::VectorIterator> range_del_iter(
new test::VectorIterator(keys, values));
range_del_agg.AddTombstones(std::move(range_del_iter));
AddTombstones(&range_del_agg, range_dels.tombstones,
range_dels.smallest, range_dels.largest);

auto tombstone = range_del_agg.GetTombstone(expected_point.begin, expected_point.seq);
auto key = InternalKey(expected_point.begin, kMaxSequenceNumber, kTypeValue);
auto tombstone = range_del_agg.GetTombstone(key.Encode(), expected_point.seq);
VerifyPartialTombstonesEq(expected_tombstone, tombstone);
}

Expand Down Expand Up @@ -395,72 +386,111 @@ TEST_F(RangeDelAggregatorTest, MergingIteratorSeek) {
}

TEST_F(RangeDelAggregatorTest, ShouldDeleteRange) {
const InternalKey b8("b", 8, kTypeValue);
const InternalKey b9("b", 9, kTypeValue);
const InternalKey b10("b", 10, kTypeValue);
const InternalKey c9("c", 9, kTypeValue);
const InternalKey c10("c", 10, kTypeValue);

ASSERT_TRUE(ShouldDeleteRange(
{{"a", "c", 10}},
{{{"a", "c", 10}}},
{"a", "b", 9}));
ASSERT_TRUE(ShouldDeleteRange(
{{{"a", "c", 10}}, nullptr, &b9},
{"a", "b", 9}));
ASSERT_FALSE(ShouldDeleteRange(
{{{"a", "c", 10}}, nullptr, &b10},
{"a", "b", 9}));
ASSERT_TRUE(ShouldDeleteRange(
{{"a", "c", 10}},
{{{"a", "c", 10}}},
{"a", "a", 9}));
ASSERT_FALSE(ShouldDeleteRange(
{{"a", "c", 10}},
{{{"a", "c", 10}}},
{"b", "a", 9}));
ASSERT_FALSE(ShouldDeleteRange(
{{"a", "c", 10}},
{{{"a", "c", 10}}},
{"a", "b", 10}));
ASSERT_FALSE(ShouldDeleteRange(
{{"a", "c", 10}},
{{{"a", "c", 10}}},
{"a", "c", 9}));
ASSERT_FALSE(ShouldDeleteRange(
{{"b", "c", 10}},
{{{"b", "c", 10}}},
{"a", "b", 9}));
ASSERT_TRUE(ShouldDeleteRange(
{{"a", "b", 10}, {"b", "d", 20}},
{{{"a", "b", 10}, {"b", "d", 20}}},
{"a", "c", 9}));
ASSERT_TRUE(ShouldDeleteRange(
{{{"a", "b", 10}, {"b", "d", 20}}, nullptr, &c9},
{"a", "c", 9}));
ASSERT_FALSE(ShouldDeleteRange(
{{{"a", "b", 10}, {"b", "d", 20}}, nullptr, &c10},
{"a", "c", 9}));
ASSERT_TRUE(ShouldDeleteRange(
{{{"a", "b", 10}, {"b", "d", 20}}, &b9, nullptr},
{"b", "c", 9}));
ASSERT_FALSE(ShouldDeleteRange(
{{{"a", "b", 10}, {"b", "d", 20}}, &b8, nullptr},
{"b", "c", 9}));
ASSERT_FALSE(ShouldDeleteRange(
{{"a", "b", 10}, {"b", "d", 20}},
{{{"a", "b", 10}, {"b", "d", 20}}},
{"a", "c", 15}));
ASSERT_FALSE(ShouldDeleteRange(
{{"a", "b", 10}, {"c", "e", 20}},
{{{"a", "b", 10}, {"c", "e", 20}}},
{"a", "d", 9}));
ASSERT_TRUE(ShouldDeleteRange(
{{"a", "b", 10}, {"c", "e", 20}},
{{{"a", "b", 10}, {"c", "e", 20}}},
{"c", "d", 15}));
ASSERT_FALSE(ShouldDeleteRange(
{{"a", "b", 10}, {"c", "e", 20}},
{{{"a", "b", 10}, {"c", "e", 20}}},
{"c", "d", 20}));
}

TEST_F(RangeDelAggregatorTest, GetTombstone) {
Slice a = "a", b = "b", c = "c", d = "d", e = "e", h = "h";
VerifyGetTombstone({{"b", "d", 10}}, {"b", 9},
const ParsedInternalKey a = {"a", kMaxSequenceNumber, kMaxValue};
const ParsedInternalKey b = {"b", kMaxSequenceNumber, kMaxValue};
const ParsedInternalKey b10 = {"b", 10, kMaxValue};
const InternalKey ib10("b", 10, kTypeValue);
const ParsedInternalKey c = {"c", kMaxSequenceNumber, kMaxValue};
const ParsedInternalKey c8 = {"c", 8, kMaxValue};
const InternalKey ic9("c", 9, kTypeValue);
const ParsedInternalKey d = {"d", kMaxSequenceNumber, kMaxValue};
const ParsedInternalKey e = {"e", kMaxSequenceNumber, kMaxValue};
const ParsedInternalKey h = {"h", kMaxSequenceNumber, kMaxValue};
VerifyGetTombstone({{{"b", "d", 10}}}, {"b", 9},
PartialRangeTombstone(&b, &d, 10));
VerifyGetTombstone({{"b", "d", 10}}, {"b", 10},
VerifyGetTombstone({{{"b", "d", 10}}, nullptr, &ic9}, {"b", 9},
PartialRangeTombstone(&b, &c8, 10));
VerifyGetTombstone({{{"a", "d", 10}}, &ib10, nullptr}, {"c", 9},
PartialRangeTombstone(&b10, &d, 10));
VerifyGetTombstone({{{"b", "d", 10}}}, {"b", 10},
PartialRangeTombstone(&b, &d, 0));
VerifyGetTombstone({{"b", "d", 10}}, {"b", 20},
VerifyGetTombstone({{{"b", "d", 10}}}, {"b", 20},
PartialRangeTombstone(&b, &d, 0));
VerifyGetTombstone({{"b", "d", 10}}, {"a", 9},
VerifyGetTombstone({{{"b", "d", 10}}}, {"a", 9},
PartialRangeTombstone(nullptr, &b, 0));
VerifyGetTombstone({{"b", "d", 10}}, {"d", 9},
VerifyGetTombstone({{{"b", "d", 10}}}, {"d", 9},
PartialRangeTombstone(&d, nullptr, 0));
VerifyGetTombstone({{"a", "c", 10}, {"e", "h", 20}}, {"d", 9},
VerifyGetTombstone({{{"a", "c", 10}, {"e", "h", 20}}}, {"d", 9},
PartialRangeTombstone(&c, &e, 0));
VerifyGetTombstone({{"a", "c", 10}, {"e", "h", 20}}, {"b", 9},
VerifyGetTombstone({{{"a", "c", 10}, {"e", "h", 20}}}, {"b", 9},
PartialRangeTombstone(&a, &c, 10));
VerifyGetTombstone({{"a", "c", 10}, {"e", "h", 20}}, {"b", 10},
VerifyGetTombstone({{{"a", "c", 10}, {"e", "h", 20}}}, {"b", 10},
PartialRangeTombstone(&a, &c, 0));
VerifyGetTombstone({{"a", "c", 10}, {"e", "h", 20}}, {"e", 19},
VerifyGetTombstone({{{"a", "c", 10}, {"e", "h", 20}}}, {"e", 19},
PartialRangeTombstone(&e, &h, 20));
VerifyGetTombstone({{"a", "c", 10}, {"e", "h", 20}}, {"e", 20},
VerifyGetTombstone({{{"a", "c", 10}, {"e", "h", 20}}}, {"e", 20},
PartialRangeTombstone(&e, &h, 0));
}

TEST_F(RangeDelAggregatorTest, AddGetTombstoneInterleaved) {
RangeDelAggregator range_del_agg(bytewise_icmp, {} /* snapshots */,
true /* collapsed */);
AddTombstones(&range_del_agg, {{"b", "c", 10}});
auto tombstone = range_del_agg.GetTombstone("b", 5);
auto key = InternalKey("b", kMaxSequenceNumber, kTypeValue);
auto tombstone = range_del_agg.GetTombstone(key.Encode(), 5);
AddTombstones(&range_del_agg, {{"a", "d", 20}});
Slice b = "b", c = "c";
ParsedInternalKey b = {"b", kMaxSequenceNumber, kMaxValue};
ParsedInternalKey c {"c", kMaxSequenceNumber, kMaxValue};
VerifyPartialTombstonesEq(PartialRangeTombstone(&b, &c, 10), tombstone);
}

Expand Down
Loading

0 comments on commit 6f11959

Please sign in to comment.