diff --git a/HISTORY.md b/HISTORY.md index b9b6998c6f5ea6148b6021f676ff41b995e69ced..b3c2ef14ac26c9fa7bcbe41b5a244cc7c31ad057 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -24,6 +24,7 @@ ### Bug Fixes * Fix a bug in WAL replay of secondary instance by skipping write batches with older sequence numbers than the current last sequence number. +* Fix flush's/compaction's merge processing logic which allowed `Put`s covered by range tombstones to reappear. Note `Put`s may exist even if the user only ever called `Merge()` due to an internal conversion during compaction to the bottommost level. ## 6.2.0 (4/30/2019) ### New Features diff --git a/db/db_range_del_test.cc b/db/db_range_del_test.cc index 16d682fc0835bec5a071e4d4f6f5d1148d50cd17..e58095b2d9216557bfd12d49fae93ce4f35b750a 100644 --- a/db/db_range_del_test.cc +++ b/db/db_range_del_test.cc @@ -491,6 +491,30 @@ TEST_F(DBRangeDelTest, CompactionRemovesCoveredMergeOperands) { ASSERT_EQ(expected, actual); } +TEST_F(DBRangeDelTest, PutDeleteRangeMergeFlush) { + // Test the sequence of operations: (1) Put, (2) DeleteRange, (3) Merge, (4) + // Flush. The `CompactionIterator` previously had a bug where we forgot to + // check for covering range tombstones when processing the (1) Put, causing + // it to reappear after the flush. + Options opts = CurrentOptions(); + opts.merge_operator = MergeOperators::CreateUInt64AddOperator(); + Reopen(opts); + + std::string val; + PutFixed64(&val, 1); + ASSERT_OK(db_->Put(WriteOptions(), "key", val)); + ASSERT_OK(db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), + "key", "key_")); + ASSERT_OK(db_->Merge(WriteOptions(), "key", val)); + ASSERT_OK(db_->Flush(FlushOptions())); + + ReadOptions read_opts; + std::string expected, actual; + ASSERT_OK(db_->Get(read_opts, "key", &actual)); + PutFixed64(&expected, 1); + ASSERT_EQ(expected, actual); +} + // NumTableFilesAtLevel() is not supported in ROCKSDB_LITE #ifndef ROCKSDB_LITE TEST_F(DBRangeDelTest, ObsoleteTombstoneCleanup) { diff --git a/db/merge_helper.cc b/db/merge_helper.cc index 4a4d2fb714e314bf5dc9151854036fb50ae75ab1..b5ae924ffc607b699ae9d9ff6c3cfb93e143c0ff 100644 --- a/db/merge_helper.cc +++ b/db/merge_helper.cc @@ -201,7 +201,15 @@ Status MergeHelper::MergeUntil(InternalIterator* iter, // want. Also if we're in compaction and it's a put, it would be nice to // run compaction filter on it. const Slice val = iter->value(); - const Slice* val_ptr = (kTypeValue == ikey.type) ? &val : nullptr; + const Slice* val_ptr; + if (kTypeValue == ikey.type && + (range_del_agg == nullptr || + !range_del_agg->ShouldDelete( + ikey, RangeDelPositioningMode::kForwardTraversal))) { + val_ptr = &val; + } else { + val_ptr = nullptr; + } std::string merge_result; s = TimedFullMerge(user_merge_operator_, ikey.user_key, val_ptr, merge_context_.GetOperands(), &merge_result, logger_,