From f8f6983f8978969802d56c231b5af7b5c427488b Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Wed, 1 Aug 2018 12:04:17 -0700 Subject: [PATCH] Skip range deletions at seqno zero when collapsing (#4216) Summary: `CollapsedRangeDelMap` internally uses seqno zero as a sentinel value to denote a gap between range tombstones or the end of range tombstones. It therefore expects to never have consecutive sentinel tombstones. However, since `DeleteRange` is now supported in `SstFileWriter`, an ingested file may contain range tombstones, and that ingested file may be assigned global seqno zero. When such tombstones are added to the collapsed map, they resemble sentinel tombstones due to having seqno zero. Then, the invariant mentioned above about never having consecutive sentinel tombstones can be violated. The symptom of this violation was dereferencing the `end()` iterator (#4204). The fix in this PR is to not add range tombstones with seqno zero to the collapsed map. They're not needed anyways since they can't possibly cover anything (in case of a key and a range tombstone with the same seqno, the key is visible). Pull Request resolved: https://github.com/facebook/rocksdb/pull/4216 Differential Revision: D9121716 Pulled By: ajkr fbshipit-source-id: f5b78a70bea9527354603ea7ac8542a7e2b6a210 --- db/range_del_aggregator.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/db/range_del_aggregator.cc b/db/range_del_aggregator.cc index 6be02de49..c6b32d621 100644 --- a/db/range_del_aggregator.cc +++ b/db/range_del_aggregator.cc @@ -155,6 +155,9 @@ class CollapsedRangeDelMap : public RangeDelMap { } RangeTombstone Tombstone() const override { + assert(Valid()); + assert(std::next(iter_) != rep_.end()); + assert(iter_->second != 0); RangeTombstone tombstone; tombstone.start_key_ = iter_->first; tombstone.end_key_ = std::next(iter_)->first; @@ -232,7 +235,8 @@ class CollapsedRangeDelMap : public RangeDelMap { } void AddTombstone(RangeTombstone t) { - if (ucmp_->Compare(t.start_key_, t.end_key_) >= 0) { + if (ucmp_->Compare(t.start_key_, t.end_key_) >= 0 || + t.seq_ == 0) { // The tombstone covers no keys. Nothing to do. return; }