From 616a1464eae3435486631499692414f3ef7b8298 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Wed, 25 Jan 2017 11:03:27 -0800 Subject: [PATCH] Fix DeleteRange including sentinels in output files Summary: when writing RangeDelAggregator::AddToBuilder, I forgot that there are sentinel tombstones in the middle of the interval map since gaps between real tombstones are represented with sentinels. blame: #1614 Closes https://github.com/facebook/rocksdb/pull/1804 Differential Revision: D4460426 Pulled By: ajkr fbshipit-source-id: 69444b5 --- db/db_range_del_test.cc | 22 ++++++++++++++++++++++ db/range_del_aggregator.cc | 7 ++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/db/db_range_del_test.cc b/db/db_range_del_test.cc index d46ee2c7b..8bd03fc9b 100644 --- a/db/db_range_del_test.cc +++ b/db/db_range_del_test.cc @@ -165,6 +165,28 @@ TEST_F(DBRangeDelTest, MaxCompactionBytesCutsOutputFiles) { db_->ReleaseSnapshot(snapshot); } +TEST_F(DBRangeDelTest, SentinelsOmittedFromOutputFile) { + // Regression test for bug where sentinel range deletions (i.e., ones with + // sequence number of zero) were included in output files. + // snapshot protects range tombstone from dropping due to becoming obsolete. + const Snapshot* snapshot = db_->GetSnapshot(); + + // gaps between ranges creates sentinels in our internal representation + std::vector> range_dels = {{"a", "b"}, {"c", "d"}, {"e", "f"}}; + for (const auto& range_del : range_dels) { + ASSERT_OK(db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), + range_del.first, range_del.second)); + } + ASSERT_OK(db_->Flush(FlushOptions())); + ASSERT_EQ(1, NumTableFilesAtLevel(0)); + + std::vector> files; + dbfull()->TEST_GetFilesMetaData(db_->DefaultColumnFamily(), &files); + ASSERT_GT(files[0][0].smallest_seqno, 0); + + db_->ReleaseSnapshot(snapshot); +} + TEST_F(DBRangeDelTest, FlushRangeDelsSameStartKey) { db_->Put(WriteOptions(), "b1", "val"); ASSERT_OK( diff --git a/db/range_del_aggregator.cc b/db/range_del_aggregator.cc index 27a680fea..e59f9dc28 100644 --- a/db/range_del_aggregator.cc +++ b/db/range_del_aggregator.cc @@ -420,9 +420,10 @@ void RangeDelAggregator::AddToBuilder( RangeTombstone tombstone; if (collapse_deletions_) { auto next_tombstone_map_iter = std::next(tombstone_map_iter); - if (next_tombstone_map_iter == stripe_map_iter->second.raw_map.end()) { - // it's the sentinel tombstone - break; + if (next_tombstone_map_iter == stripe_map_iter->second.raw_map.end() || + tombstone_map_iter->second.seq_ == 0) { + // it's a sentinel tombstone + continue; } tombstone.start_key_ = tombstone_map_iter->first; tombstone.end_key_ = next_tombstone_map_iter->first;