Skip to content

Commit

Permalink
Fix DeleteRange including sentinels in output files
Browse files Browse the repository at this point in the history
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: facebook#1614
Closes facebook#1804

Differential Revision: D4460426

Pulled By: ajkr

fbshipit-source-id: 69444b5
  • Loading branch information
ajkr committed Jan 27, 2017
1 parent 2412679 commit acf6d6b
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 3 deletions.
22 changes: 22 additions & 0 deletions db/db_range_del_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::pair<std::string, std::string>> 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<std::vector<FileMetaData>> 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(
Expand Down
7 changes: 4 additions & 3 deletions db/range_del_aggregator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit acf6d6b

Please sign in to comment.