andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Store internal keys in TombstoneMap

Browse Source 
Summary:
This fixes a correctness issue where ranges with same begin key would overwrite each other.

This diff uses InternalKey as TombstoneMap's key such that all tombstones have unique keys even when their start keys overlap. We also update TombstoneMap to use an internal key comparator.

End-to-end tests pass and are here (https://gist.github.com/ajkr/851ffe4c1b8a15a68d33025be190a7d9) but cannot be included yet since the DeleteRange() API is yet to be checked in. Note both tests failed before this fix.
Closes https://github.com/facebook/rocksdb/pull/1484

Differential Revision: D4155248

Pulled By: ajkr

fbshipit-source-id: 304b4b9
This commit is contained in:
Andrew Kryczka 2016-11-09 15:00:16 -08:00 committed by Facebook Github Bot
parent a9fb346e4a
commit 4e20c5da20
3 changed files with 8 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
db/range_del_aggregator.cc
View File

@ -15,9 +15,8 @@ RangeDelAggregator::RangeDelAggregator(
: icmp_(icmp) { : icmp_(icmp) {
pinned_iters_mgr_.StartPinning(); pinned_iters_mgr_.StartPinning();
for (auto snapshot : snapshots) { for (auto snapshot : snapshots) {
stripe_map_.emplace( stripe_map_.emplace(snapshot,
snapshot, TombstoneMap(stl_wrappers::LessOfComparator(&icmp_)));
TombstoneMap(stl_wrappers::LessOfComparator(icmp_.user_comparator())));
} }
// Data newer than any snapshot falls in this catch-all stripe // Data newer than any snapshot falls in this catch-all stripe
stripe_map_.emplace(kMaxSequenceNumber, TombstoneMap()); stripe_map_.emplace(kMaxSequenceNumber, TombstoneMap());
@ -87,8 +86,7 @@ Status RangeDelAggregator::AddTombstones(InternalIterator* input, bool arena) {
} }
RangeTombstone tombstone(parsed_key, input->value()); RangeTombstone tombstone(parsed_key, input->value());
auto& tombstone_map = GetTombstoneMap(tombstone.seq_); auto& tombstone_map = GetTombstoneMap(tombstone.seq_);
tombstone_map.emplace(tombstone.start_key_.ToString(), tombstone_map.emplace(input->key(), std::move(tombstone));
std::move(tombstone));
input->Next(); input->Next();
} }
return Status::OK(); return Status::OK();

4
db/range_del_aggregator.h
View File

@ -74,8 +74,8 @@ class RangeDelAggregator {
bool IsEmpty(); bool IsEmpty();
private: private:
// Maps tombstone start key -> tombstone object // Maps tombstone internal start key -> tombstone object
typedef std::map<std::string, RangeTombstone, stl_wrappers::LessOfComparator> typedef std::map<Slice, RangeTombstone, stl_wrappers::LessOfComparator>
TombstoneMap; TombstoneMap;
// Maps snapshot seqnum -> map of tombstones that fall in that stripe, i.e., // Maps snapshot seqnum -> map of tombstones that fall in that stripe, i.e.,
// their seqnums are greater than the next smaller snapshot's seqnum. // their seqnums are greater than the next smaller snapshot's seqnum.

3
util/kv_map.h
View File

@ -22,6 +22,9 @@ struct LessOfComparator {
bool operator()(const std::string& a, const std::string& b) const { bool operator()(const std::string& a, const std::string& b) const {
return cmp->Compare(Slice(a), Slice(b)) < 0; return cmp->Compare(Slice(a), Slice(b)) < 0;
} }
bool operator()(const Slice& a, const Slice& b) const {
return cmp->Compare(a, b) < 0;
}
const Comparator* cmp; const Comparator* cmp;
}; };