diff --git a/db/builder.cc b/db/builder.cc index b7d3b6259b1..e9336a68d6c 100644 --- a/db/builder.cc +++ b/db/builder.cc @@ -162,6 +162,13 @@ Status BuildTable( context.reason = tboptions.reason; compaction_filter = ioptions.compaction_filter_factory->CreateCompactionFilter(context); + CompactionFilter::MergeOrder order; + s = GetCompactionFilterMergeOrder( + compaction_filter.get(), ioptions.compaction_filter_factory.get(), + tboptions.reason, &order); + if (!s.ok()) { + return s; + } if (compaction_filter != nullptr && !compaction_filter->IgnoreSnapshots()) { s.PermitUncheckedError(); @@ -217,7 +224,8 @@ Status BuildTable( auto ucmp = tboptions.internal_comparator.user_comparator(); MergeHelper merge( env, ucmp, ioptions.merge_operator.get(), compaction_filter.get(), - ioptions.logger, true /* internal key corruption is not ok */, + CompactionFilter::MergeOrder::kFilterBeforeMerge, ioptions.logger, + true /* internal key corruption is not ok */, snapshots.empty() ? 0 : snapshots.back(), snapshot_checker); std::unique_ptr blob_file_builder( diff --git a/db/compaction/compaction_iterator.cc b/db/compaction/compaction_iterator.cc index 7df7f46eded..5d6d21b4c2c 100644 --- a/db/compaction/compaction_iterator.cc +++ b/db/compaction/compaction_iterator.cc @@ -185,6 +185,9 @@ CompactionIterator::CompactionIterator( blob_file_builder_(blob_file_builder), compaction_(std::move(compaction)), compaction_filter_(compaction_filter), + merge_order_(merge_helper + ? merge_helper->merge_order() + : CompactionFilter::MergeOrder::kFilterBeforeMerge), filter_supports_v4_( compaction_filter_ ? compaction_filter_->SupportsFilterV4() : false), shutting_down_(shutting_down), @@ -217,6 +220,14 @@ CompactionIterator::CompactionIterator( level_(compaction_ == nullptr ? 0 : compaction_->level()), preserve_seqno_after_(preserve_seqno_min.value_or(earliest_snapshot)) { assert(snapshots_ != nullptr); + if (compaction_filter_ && + merge_order_ == CompactionFilter::MergeOrder::kMergeBeforeFilter && + (snapshot_checker_ != nullptr || timestamp_size_ != 0 || + compaction_filter_->IsStackedBlobDbInternalCompactionFilter())) { + status_ = Status::NotSupported( + "Merge-before-filter does not support SnapshotChecker, timestamps, " + "or stacked BlobDB"); + } assert(preserve_seqno_after_ <= earliest_snapshot_); TEST_SYNC_POINT_CALLBACK( @@ -282,6 +293,9 @@ void CompactionIterator::SeekToFirst() { } void CompactionIterator::SeekToFirstWithoutPreparingOutput() { + if (!status_.ok()) { + return; + } NextFromInput(); } @@ -293,6 +307,9 @@ void CompactionIterator::Next() { void CompactionIterator::NextWithoutPreparingOutput() { NextImpl(); } void CompactionIterator::NextImpl() { + if (!status_.ok()) { + return; + } output_prepared_ = false; // If there is a merge output, return it before continuing to process the // input. @@ -300,7 +317,12 @@ void CompactionIterator::NextImpl() { merge_out_iter_.Next(); // Check if we returned all records of the merge output. - if (merge_out_iter_.Valid()) { + if (merge_out_iter_.Valid() && + merge_order_ == CompactionFilter::MergeOrder::kMergeBeforeFilter) { + if (!SelectMergeOutput(ValidContext::kMerge1) && status_.ok()) { + NextFromInput(); + } + } else if (merge_out_iter_.Valid()) { key_ = merge_out_iter_.key(); value_ = merge_out_iter_.value(); Status s = ParseInternalKey(key_, &ikey_, allow_data_in_errors_); @@ -370,20 +392,23 @@ void CompactionIterator::PrepareOutputIfNeeded() { } bool CompactionIterator::InvokeFilterIfNeeded(bool* need_skip, - Slice* skip_until) { + Slice* skip_until, + bool merge_output) { if (!compaction_filter_) { return true; } if (ikey_.type != kTypeValue && ikey_.type != kTypeBlobIndex && - ikey_.type != kTypeWideColumnEntity) { + ikey_.type != kTypeWideColumnEntity && + !(merge_output && ikey_.type == kTypeMerge)) { return true; } CompactionFilter::Decision decision = CompactionFilter::Decision::kUndetermined; CompactionFilter::ValueType value_type = - ikey_.type == kTypeValue ? CompactionFilter::ValueType::kValue + ikey_.type == kTypeValue ? CompactionFilter::ValueType::kValue + : ikey_.type == kTypeMerge ? CompactionFilter::ValueType::kMergeOperand : ikey_.type == kTypeBlobIndex ? CompactionFilter::ValueType::kBlobIndex : CompactionFilter::ValueType::kWideColumnEntity; @@ -616,6 +641,45 @@ bool CompactionIterator::InvokeFilterIfNeeded(bool* need_skip, return false; } + if (merge_order_ == CompactionFilter::MergeOrder::kMergeBeforeFilter) { + const bool operand = ikey_.type == kTypeMerge; + switch (decision) { + case CompactionFilter::Decision::kKeep: + case CompactionFilter::Decision::kRemove: + case CompactionFilter::Decision::kChangeValue: + case CompactionFilter::Decision::kRemoveAndSkipUntil: + break; + case CompactionFilter::Decision::kPurge: + if (!operand) { + // Use ordinary removal for both input values and merge results, so + // later compactions apply the same policy. SingleDelete cannot be + // mixed with Merge or ordinary Delete on affected keys. + decision = CompactionFilter::Decision::kRemove; + break; + } + [[fallthrough]]; + case CompactionFilter::Decision::kChangeWideColumnEntity: + if (!operand) { + break; + } + // Partial results and untouched operands must remain valid merge + // operands. Neither SingleDelete nor an entity replacement represents + // an operand; kRemove drops its effect and kChangeValue replaces it. + [[fallthrough]]; + case CompactionFilter::Decision::kChangeBlobIndex: + case CompactionFilter::Decision::kIOError: + case CompactionFilter::Decision::kUndetermined: + // kChangeBlobIndex/kIOError belong to the old stacked BlobDB protocol. + // Integrated BlobDB changes use kChangeValue and report resolver errors + // through Status. kUndetermined is only a FilterBlobByKey fallback, + // never a final FilterV2/V3/V4 decision. + status_ = Status::NotSupported( + "Invalid decision for merge-before-filter value type"); + validity_info_.Invalidate(); + return false; + } + } + if (decision == CompactionFilter::Decision::kRemoveAndSkipUntil && cmp_->Compare(*compaction_filter_skip_until_.rep(), ikey_.user_key) <= 0) { @@ -625,6 +689,11 @@ bool CompactionIterator::InvokeFilterIfNeeded(bool* need_skip, } if (decision == CompactionFilter::Decision::kRemove) { + if (ikey_.type == kTypeMerge) { + ++iter_stats_.num_record_drop_user; + validity_info_.Invalidate(); + return true; + } // convert the current key to a delete; key_ is pointing into // current_key_ at this point, so updating current_key_ updates key() ikey_.type = kTypeDeletion; @@ -641,7 +710,7 @@ bool CompactionIterator::InvokeFilterIfNeeded(bool* need_skip, value_.clear(); iter_stats_.num_record_drop_user++; } else if (decision == CompactionFilter::Decision::kChangeValue) { - if (ikey_.type != kTypeValue) { + if (ikey_.type != kTypeValue && ikey_.type != kTypeMerge) { ikey_.type = kTypeValue; current_key_.UpdateInternalKey(ikey_.sequence, kTypeValue); } @@ -723,6 +792,221 @@ bool CompactionIterator::InvokeFilterIfNeeded(bool* need_skip, return true; } +bool CompactionIterator::PrefilterUncombinedOutputs(uint8_t valid_context) { + uncombined_prefiltered_ = true; + uncombined_decisions_.clear(); + uncombined_index_ = 0; + MergeOutputIterator it(merge_helper_); + for (it.SeekToFirst(); it.Valid(); it.Next()) { + key_ = it.key(); + value_ = it.value(); + status_ = ParseInternalKey(key_, &ikey_, allow_data_in_errors_); + if (!status_.ok()) { + validity_info_.Invalidate(); + merge_out_iter_.Invalidate(); + return false; + } + current_key_.UpdateInternalKey(ikey_.sequence, ikey_.type); + key_ = current_key_.GetInternalKey(); + ikey_.user_key = current_key_.GetUserKey(); + entity_deserialized_ = false; + blob_value_.Reset(); + validity_info_.SetValid(valid_context); + UncombinedDecision decision; + if (merge_helper_->OperandEligible(ikey_.sequence)) { + bool need_skip = false; + Slice skip_until; + if (!InvokeFilterIfNeeded(&need_skip, &skip_until, true)) { + merge_out_iter_.Invalidate(); + return false; + } + if (need_skip) { + // Newer operands of this batch were not output yet, so the skip drops + // the whole batch. + merge_out_iter_.Invalidate(); + validity_info_.Invalidate(); + pinned_iters_mgr_.ReleasePinnedData(); + SkipUntil(skip_until); + return false; + } + decision.keep = Valid(); + if (decision.keep && value_.data() == compaction_filter_value_.data()) { + decision.changed = true; + decision.value = compaction_filter_value_; + } + } + uncombined_decisions_.push_back(std::move(decision)); + } + return true; +} + +bool CompactionIterator::SelectMergeOutput(uint8_t valid_context) { + const bool prefiltered = + compaction_filter_ != nullptr && + merge_helper_->outcome() == MergeHelper::Outcome::kUncombined; + if (prefiltered && !uncombined_prefiltered_ && + !PrefilterUncombinedOutputs(valid_context)) { + return false; + } + while (merge_out_iter_.Valid() && !IsShuttingDown() && + !IsPausingManualCompaction()) { + key_ = merge_out_iter_.key(); + value_ = merge_out_iter_.value(); + status_ = ParseInternalKey(key_, &ikey_, allow_data_in_errors_); + if (!status_.ok()) { + validity_info_.Invalidate(); + merge_out_iter_.Invalidate(); + return false; + } + current_key_.UpdateInternalKey(ikey_.sequence, ikey_.type); + key_ = current_key_.GetInternalKey(); + ikey_.user_key = current_key_.GetUserKey(); + entity_deserialized_ = false; + blob_value_.Reset(); + validity_info_.SetValid(valid_context); + if (prefiltered) { + if (uncombined_index_ < uncombined_decisions_.size()) { + const UncombinedDecision& decision = + uncombined_decisions_[uncombined_index_]; + if (!decision.keep) { + validity_info_.Invalidate(); + } else if (decision.changed) { + value_ = decision.value; + } + } + ++uncombined_index_; + } else if (merge_helper_->aggregate_eligible() && compaction_filter_) { + bool need_skip = false; + Slice skip_until; + if (!InvokeFilterIfNeeded(&need_skip, &skip_until, true)) { + merge_out_iter_.Invalidate(); + return false; + } + if (need_skip) { + merge_out_iter_.Invalidate(); + validity_info_.Invalidate(); + pinned_iters_mgr_.ReleasePinnedData(); + SkipUntil(skip_until); + return false; + } + if (Valid() && ikey_.type == kTypeDeletion) { + SequenceNumber prev_snapshot = 0; + current_user_key_sequence_ = ikey_.sequence; + current_user_key_snapshot_ = + visible_at_tip_ + ? earliest_snapshot_ + : findEarliestVisibleSnapshot(ikey_.sequence, &prev_snapshot); + ProcessDeletion(prev_snapshot, true); + } + } + if (Valid()) { + return true; + } + merge_out_iter_.Next(); + } + if (merge_until_status_.IsMergeInProgress()) { + has_current_user_key_ = false; + } + pinned_iters_mgr_.ReleasePinnedData(); + return false; +} + +bool CompactionIterator::ProcessDeletion(SequenceNumber prev_snapshot, + bool input_already_advanced) { + if (compaction_ != nullptr && + (ikey_.type == kTypeDeletion || + (ikey_.type == kTypeDeletionWithTimestamp && + cmp_with_history_ts_low_ < 0)) && + !compaction_->allow_ingest_behind() && + DefinitelyInSnapshot(ikey_.sequence, earliest_snapshot_) && + compaction_->KeyNotExistsBeyondOutputLevel(ikey_.user_key, + &level_ptrs_)) { + // TODO(noetzli): This is the only place where we use compaction_ + // (besides the constructor). We should probably get rid of this + // dependency and find a way to do similar filtering during flushes. + // + // For this user key: + // (1) there is no data in higher levels + // (2) data in lower levels will have larger sequence numbers + // (3) data in layers that are being compacted here and have + // smaller sequence numbers will be dropped in the next + // few iterations of this loop (by rule (A) above). + // Therefore this deletion marker is obsolete and can be dropped. + // + // Note: Dropping this Delete will not affect TransactionDB + // write-conflict checking since it is earlier than any snapshot. + // + // It seems that we can also drop deletion later than earliest snapshot + // given that: + // (1) The deletion is earlier than earliest_write_conflict_snapshot, and + // (2) No value exist earlier than the deletion. + // + // Note also that a deletion marker of type kTypeDeletionWithTimestamp + // will be treated as a different user key unless the timestamp is older + // than *full_history_ts_low_. + validity_info_.Invalidate(); + ++iter_stats_.num_record_drop_obsolete; + if (!bottommost_level_) { + ++iter_stats_.num_optimized_del_drop_obsolete; + } + if (!input_already_advanced) { + AdvanceInputIterWithoutOutput(); + } + } else if ((ikey_.type == kTypeDeletion || + (ikey_.type == kTypeDeletionWithTimestamp && + cmp_with_history_ts_low_ < 0)) && + bottommost_level_) { + assert(compaction_); + assert(!compaction_->allow_ingest_behind()); // bottommost_level_ is true + // Handle the case where we have a delete key at the bottom most level + // We can skip outputting the key iff there are no subsequent puts for + // this key + assert(compaction_->KeyNotExistsBeyondOutputLevel(ikey_.user_key, + &level_ptrs_)); + validity_info_.Invalidate(); + ParsedInternalKey next_ikey; + if (!input_already_advanced) { + AdvanceInputIterWithoutOutput(); + } +#ifndef NDEBUG + const Compaction* c = + compaction_ ? compaction_->real_compaction() : nullptr; +#endif + TEST_SYNC_POINT_CALLBACK( + "CompactionIterator::NextFromInput:BottommostDelete:1", + const_cast(c)); + // Skip over all versions of this key that happen to occur in the same + // snapshot range as the delete. + // + // Note that a deletion marker of type kTypeDeletionWithTimestamp will be + // considered to have a different user key unless the timestamp is older + // than *full_history_ts_low_. + // + // Range tombstone start keys are skipped as they are not "real" keys. + while (!IsPausingManualCompaction() && !IsShuttingDown() && + input_.Valid() && + (ParseInternalKey(input_.key(), &next_ikey, allow_data_in_errors_) + .ok()) && + cmp_->EqualWithoutTimestamp(ikey_.user_key, next_ikey.user_key) && + (prev_snapshot == 0 || input_.IsDeleteRangeSentinelKey() || + DefinitelyNotInSnapshot(next_ikey.sequence, prev_snapshot))) { + AdvanceInputIterWithoutOutput(); + } + // If you find you still need to output a row with this key, we need to + // output the delete too + if (input_.Valid() && + (ParseInternalKey(input_.key(), &next_ikey, allow_data_in_errors_) + .ok()) && + cmp_->EqualWithoutTimestamp(ikey_.user_key, next_ikey.user_key)) { + validity_info_.SetValid(ValidContext::kKeepDel); + at_next_ = true; + } + } else { + return false; + } + return true; +} + void CompactionIterator::NextFromInput() { at_next_ = false; validity_info_.Invalidate(); @@ -839,7 +1123,7 @@ void CompactionIterator::NextFromInput() { // Apply the compaction filter to the first committed version of the user // key. if (current_key_committed_ && - !InvokeFilterIfNeeded(&need_skip, &skip_until)) { + !InvokeFilterIfNeeded(&need_skip, &skip_until, false)) { break; } } else { @@ -861,7 +1145,7 @@ void CompactionIterator::NextFromInput() { // Apply the compaction filter to the first committed version of the // user key. if (current_key_committed_ && - !InvokeFilterIfNeeded(&need_skip, &skip_until)) { + !InvokeFilterIfNeeded(&need_skip, &skip_until, false)) { break; } } @@ -1182,88 +1466,10 @@ void CompactionIterator::NextFromInput() { ++iter_stats_.num_record_drop_hidden; AdvanceInputIterWithoutOutput(); - } else if (compaction_ != nullptr && - (ikey_.type == kTypeDeletion || - (ikey_.type == kTypeDeletionWithTimestamp && - cmp_with_history_ts_low_ < 0)) && - !compaction_->allow_ingest_behind() && - DefinitelyInSnapshot(ikey_.sequence, earliest_snapshot_) && - compaction_->KeyNotExistsBeyondOutputLevel(ikey_.user_key, - &level_ptrs_)) { - // TODO(noetzli): This is the only place where we use compaction_ - // (besides the constructor). We should probably get rid of this - // dependency and find a way to do similar filtering during flushes. - // - // For this user key: - // (1) there is no data in higher levels - // (2) data in lower levels will have larger sequence numbers - // (3) data in layers that are being compacted here and have - // smaller sequence numbers will be dropped in the next - // few iterations of this loop (by rule (A) above). - // Therefore this deletion marker is obsolete and can be dropped. - // - // Note: Dropping this Delete will not affect TransactionDB - // write-conflict checking since it is earlier than any snapshot. - // - // It seems that we can also drop deletion later than earliest snapshot - // given that: - // (1) The deletion is earlier than earliest_write_conflict_snapshot, and - // (2) No value exist earlier than the deletion. - // - // Note also that a deletion marker of type kTypeDeletionWithTimestamp - // will be treated as a different user key unless the timestamp is older - // than *full_history_ts_low_. - ++iter_stats_.num_record_drop_obsolete; - if (!bottommost_level_) { - ++iter_stats_.num_optimized_del_drop_obsolete; - } - AdvanceInputIterWithoutOutput(); } else if ((ikey_.type == kTypeDeletion || (ikey_.type == kTypeDeletionWithTimestamp && cmp_with_history_ts_low_ < 0)) && - bottommost_level_) { - assert(compaction_); - assert(!compaction_->allow_ingest_behind()); // bottommost_level_ is true - // Handle the case where we have a delete key at the bottom most level - // We can skip outputting the key iff there are no subsequent puts for - // this key - assert(compaction_->KeyNotExistsBeyondOutputLevel(ikey_.user_key, - &level_ptrs_)); - ParsedInternalKey next_ikey; - AdvanceInputIterWithoutOutput(); -#ifndef NDEBUG - const Compaction* c = - compaction_ ? compaction_->real_compaction() : nullptr; -#endif - TEST_SYNC_POINT_CALLBACK( - "CompactionIterator::NextFromInput:BottommostDelete:1", - const_cast(c)); - // Skip over all versions of this key that happen to occur in the same - // snapshot range as the delete. - // - // Note that a deletion marker of type kTypeDeletionWithTimestamp will be - // considered to have a different user key unless the timestamp is older - // than *full_history_ts_low_. - // - // Range tombstone start keys are skipped as they are not "real" keys. - while (!IsPausingManualCompaction() && !IsShuttingDown() && - input_.Valid() && - (ParseInternalKey(input_.key(), &next_ikey, allow_data_in_errors_) - .ok()) && - cmp_->EqualWithoutTimestamp(ikey_.user_key, next_ikey.user_key) && - (prev_snapshot == 0 || input_.IsDeleteRangeSentinelKey() || - DefinitelyNotInSnapshot(next_ikey.sequence, prev_snapshot))) { - AdvanceInputIterWithoutOutput(); - } - // If you find you still need to output a row with this key, we need to - // output the delete too - if (input_.Valid() && - (ParseInternalKey(input_.key(), &next_ikey, allow_data_in_errors_) - .ok()) && - cmp_->EqualWithoutTimestamp(ikey_.user_key, next_ikey.user_key)) { - validity_info_.SetValid(ValidContext::kKeepDel); - at_next_ = true; - } + ProcessDeletion(prev_snapshot, false)) { } else if (ikey_.type == kTypeValuePreferredSeqno && DefinitelyInSnapshot(ikey_.sequence, earliest_snapshot_) && (bottommost_level_ || @@ -1344,31 +1550,39 @@ void CompactionIterator::NextFromInput() { allow_data_in_errors_, blob_fetcher_.get(), full_history_ts_low_, prefetch_buffers_.get(), &iter_stats_); merge_out_iter_.SeekToFirst(); + uncombined_prefiltered_ = false; if (!merge_until_status_.ok() && !merge_until_status_.IsMergeInProgress()) { status_ = merge_until_status_; return; } else if (merge_out_iter_.Valid()) { - // NOTE: key, value, and ikey_ refer to old entries. - // These will be correctly set below. - key_ = merge_out_iter_.key(); - value_ = merge_out_iter_.value(); - pik_status = ParseInternalKey(key_, &ikey_, allow_data_in_errors_); - // MergeUntil stops when it encounters a corrupt key and does not - // include them in the result, so we expect the keys here to valid. - if (!pik_status.ok()) { - ROCKS_LOG_FATAL( - info_log_, "Invalid key %s in compaction. %s", - allow_data_in_errors_ ? key_.ToString(true).c_str() : "hidden", - pik_status.getState()); - assert(false); + if (merge_order_ == CompactionFilter::MergeOrder::kMergeBeforeFilter) { + SelectMergeOutput(ValidContext::kMerge2); + if (!status_.ok()) { + return; + } + } else { + // NOTE: key, value, and ikey_ refer to old entries. + // These will be correctly set below. + key_ = merge_out_iter_.key(); + value_ = merge_out_iter_.value(); + pik_status = ParseInternalKey(key_, &ikey_, allow_data_in_errors_); + // MergeUntil stops when it encounters a corrupt key and does not + // include them in the result, so we expect the keys here to valid. + if (!pik_status.ok()) { + ROCKS_LOG_FATAL( + info_log_, "Invalid key %s in compaction. %s", + allow_data_in_errors_ ? key_.ToString(true).c_str() : "hidden", + pik_status.getState()); + assert(false); + } + // Keep current_key_ in sync. + current_key_.UpdateInternalKey(ikey_.sequence, ikey_.type); + key_ = current_key_.GetInternalKey(); + ikey_.user_key = current_key_.GetUserKey(); + validity_info_.SetValid(ValidContext::kMerge2); } - // Keep current_key_ in sync. - current_key_.UpdateInternalKey(ikey_.sequence, ikey_.type); - key_ = current_key_.GetInternalKey(); - ikey_.user_key = current_key_.GetUserKey(); - validity_info_.SetValid(ValidContext::kMerge2); } else { // all merge operands were filtered out. reset the user key, since the // batch consumed by the merge operator should not shadow any keys diff --git a/db/compaction/compaction_iterator.h b/db/compaction/compaction_iterator.h index 25ab3a8f630..efe3841a923 100644 --- a/db/compaction/compaction_iterator.h +++ b/db/compaction/compaction_iterator.h @@ -448,7 +448,12 @@ class CompactionIterator { // Invoke compaction filter if needed. // Return true on success, false on failures (e.g.: kIOError). - bool InvokeFilterIfNeeded(bool* need_skip, Slice* skip_until); + bool InvokeFilterIfNeeded(bool* need_skip, Slice* skip_until, + bool merge_output); + bool SelectMergeOutput(uint8_t valid_context); + bool PrefilterUncombinedOutputs(uint8_t valid_context); + bool ProcessDeletion(SequenceNumber prev_snapshot, + bool input_already_advanced); // Given a sequence number, return the sequence number of the // earliest snapshot that this sequence number is visible in. @@ -509,6 +514,7 @@ class CompactionIterator { BlobFileBuilder* blob_file_builder_; std::unique_ptr compaction_; const CompactionFilter* compaction_filter_; + const CompactionFilter::MergeOrder merge_order_; const bool filter_supports_v4_; const std::atomic* shutting_down_; const std::atomic& manual_compaction_canceled_; @@ -601,6 +607,17 @@ class CompactionIterator { bool clear_and_output_next_key_ = false; MergeOutputIterator merge_out_iter_; + // Merge-before-filter decisions for the current batch of uncombined + // operands. They are made before any operand is output, so + // kRemoveAndSkipUntil drops the whole batch, as in the legacy order. + struct UncombinedDecision { + bool keep = true; + bool changed = false; + std::string value; + }; + std::vector uncombined_decisions_; + size_t uncombined_index_ = 0; + bool uncombined_prefiltered_ = false; Status merge_until_status_; // PinnedIteratorsManager used to pin input_ Iterator blocks while reading // merge operands and then releasing them after consuming them. diff --git a/db/compaction/compaction_iterator_test.cc b/db/compaction/compaction_iterator_test.cc index 01f1aadf32d..578a94cbcf2 100644 --- a/db/compaction/compaction_iterator_test.cc +++ b/db/compaction/compaction_iterator_test.cc @@ -19,6 +19,7 @@ #include "file/filename.h" #include "options/cf_options.h" #include "port/port.h" +#include "test_util/merge_filter_test_util.h" #include "test_util/testharness.h" #include "test_util/testutil.h" #include "util/string_util.h" @@ -286,10 +287,18 @@ class CompactionIteratorTest : public testing::TestWithParam { snapshot_checker_.reset( new TestSnapshotChecker(last_committed_sequence, snapshot_map_)); } - merge_helper_.reset( - new MergeHelper(Env::Default(), cmp_, merge_op, filter, nullptr, false, - 0 /*latest_snapshot*/, snapshot_checker_.get(), - 0 /*level*/, nullptr /*statistics*/, &shutting_down_)); + CompactionFilter::MergeOrder merge_order = + filter ? filter->GetMergeOrder() + : CompactionFilter::MergeOrder::kFilterBeforeMerge; + SequenceNumber latest_merge_snapshot = + merge_order == CompactionFilter::MergeOrder::kMergeBeforeFilter && + !snapshots_.empty() + ? snapshots_.back() + : 0; + merge_helper_.reset(new MergeHelper( + Env::Default(), cmp_, merge_op, filter, merge_order, nullptr, false, + latest_merge_snapshot, snapshot_checker_.get(), 0 /*level*/, + nullptr /*statistics*/, &shutting_down_)); if (c_iter_) { // Since iter_ is still used in ~CompactionIterator(), we call @@ -342,7 +351,8 @@ class CompactionIteratorTest : public testing::TestWithParam { c_iter_->SeekToFirst(); for (size_t i = 0; i < expected_keys.size(); i++) { std::string info = "i = " + std::to_string(i); - ASSERT_TRUE(c_iter_->Valid()) << info; + ASSERT_TRUE(c_iter_->Valid()) + << info << " " << c_iter_->status().ToString(); ASSERT_OK(c_iter_->status()) << info; ASSERT_EQ(expected_keys[i], c_iter_->key().ToString()) << info; ASSERT_EQ(expected_values[i], c_iter_->value().ToString()) << info; @@ -372,6 +382,498 @@ class CompactionIteratorTest : public testing::TestWithParam { FakeCompaction* compaction_proxy_; }; +TEST_P(CompactionIteratorTest, MergeBeforeFilterFullOrder) { + if (GetParam()) { + return; + } + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + RunTest({test::KeyStr("a", 5, kTypeMerge), test::KeyStr("a", 4, kTypeMerge), + test::KeyStr("a", 3, kTypeValue), test::KeyStr("b", 2, kTypeValue)}, + {"C", "B", "A", "next"}, + {test::KeyStr("a", 5, kTypeValue), test::KeyStr("b", 2, kTypeValue)}, + {"ABC", "next"}, kMaxSequenceNumber, &merge, &filter); + ASSERT_EQ( + (std::vector{"full:a:ABC", "value:a:ABC", "value:b:next"}), + trace.Events()); + ASSERT_EQ(MergeHelper::Outcome::kFull, merge_helper_->outcome()); + ASSERT_TRUE(merge_helper_->aggregate_eligible()); +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterPartialAndDecline) { + if (GetParam()) { + return; + } + for (bool partial_success : {true, false}) { + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + merge.partial_success = partial_success; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.operand_decision = CompactionFilter::Decision::kChangeValue; + filter.replacement = "replacement"; + const std::vector inputs{test::KeyStr("a", 5, kTypeMerge), + test::KeyStr("a", 4, kTypeMerge)}; + RunTest(inputs, {"B", "A"}, + partial_success ? std::vector{inputs[0]} : inputs, + partial_success + ? std::vector{"replacement"} + : std::vector{"replacement", "replacement"}, + kMaxSequenceNumber, &merge, &filter); + ASSERT_EQ(partial_success ? MergeHelper::Outcome::kPartial + : MergeHelper::Outcome::kUncombined, + merge_helper_->outcome()); + ASSERT_EQ(partial_success + ? (std::vector{"partial:a:AB", "operand:a:AB"}) + : (std::vector{"partial:a:AB", "operand:a:B", + "operand:a:A"}), + trace.Events()); + } +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterSingleOperand) { + if (GetParam()) { + return; + } + for (bool allow_single : {false, true}) { + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + merge.allow_single = allow_single; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + RunTest({test::KeyStr("a", 5, kTypeMerge)}, {"A"}, + {test::KeyStr("a", 5, kTypeMerge)}, {"A"}, kMaxSequenceNumber, + &merge, &filter); + ASSERT_EQ(allow_single + ? (std::vector{"partial:a:A", "operand:a:A"}) + : (std::vector{"operand:a:A"}), + trace.Events()); + } +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterFullRemovalCursor) { + if (GetParam()) { + return; + } + for (bool bottommost : {false, true}) { + for (bool snapshot : {false, true}) { + for (CompactionFilter::Decision decision : + {CompactionFilter::Decision::kRemove, + CompactionFilter::Decision::kPurge}) { + ClearSnapshots(); + if (snapshot) { + AddSnapshot(1); + } + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.action_key = "a"; + filter.value_decision = decision; + InitIterators( + {test::KeyStr("a", 3, kTypeMerge), test::KeyStr("a", 2, kTypeValue), + test::KeyStr("a", 1, kTypeValue), + test::KeyStr("b", 4, kTypeValue)}, + {"M", "base", "old", "next"}, {}, {}, 4, kMaxSequenceNumber, &merge, + &filter, bottommost); + c_iter_->SeekToFirst(); + if (!bottommost || snapshot) { + ASSERT_TRUE(c_iter_->Valid()); + ASSERT_EQ(kTypeDeletion, c_iter_->ikey().type); + ASSERT_EQ(3, c_iter_->ikey().sequence); + c_iter_->Next(); + } + if (snapshot) { + ASSERT_TRUE(c_iter_->Valid()); + ASSERT_EQ("old", c_iter_->value()); + ASSERT_EQ(kTypeValue, c_iter_->ikey().type); + c_iter_->Next(); + } + ASSERT_TRUE(c_iter_->Valid()); + ASSERT_EQ("b", c_iter_->ikey().user_key); + ASSERT_EQ("next", c_iter_->value()); + c_iter_->Next(); + ASSERT_OK(c_iter_->status()); + ASSERT_FALSE(c_iter_->Valid()); + ASSERT_EQ(1, c_iter_->iter_stats().num_record_drop_user); + ASSERT_EQ((std::vector{"full:a:baseM", "value:a:baseM", + "value:b:next"}), + trace.Events()); + } + } + } +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterPartialRemovalPreservesBase) { + if (GetParam()) { + return; + } + AddSnapshot(2); + for (bool partial_success : {true, false}) { + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + merge.partial_success = partial_success; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.operand_decision = CompactionFilter::Decision::kRemove; + RunTest({test::KeyStr("a", 4, kTypeMerge), test::KeyStr("a", 3, kTypeMerge), + test::KeyStr("a", 2, kTypeValue)}, + {"B", "A", "base"}, {test::KeyStr("a", 2, kTypeValue)}, {"base"}, + kMaxSequenceNumber, &merge, &filter); + ASSERT_EQ(partial_success ? 1 : 2, + c_iter_->iter_stats().num_record_drop_user); + ASSERT_EQ(partial_success + ? (std::vector{"partial:a:AB", "operand:a:AB", + "value:a:base"}) + : (std::vector{"partial:a:AB", "operand:a:B", + "operand:a:A", "value:a:base"}), + trace.Events()); + } +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterInvalidOperandDecision) { + if (GetParam()) { + return; + } + for (CompactionFilter::Decision decision : + {CompactionFilter::Decision::kPurge, + CompactionFilter::Decision::kChangeWideColumnEntity, + CompactionFilter::Decision::kUndetermined, + CompactionFilter::Decision::kChangeBlobIndex, + CompactionFilter::Decision::kIOError}) { + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.operand_decision = decision; + InitIterators({test::KeyStr("a", 5, kTypeMerge)}, {"A"}, {}, {}, 5, + kMaxSequenceNumber, &merge, &filter); + c_iter_->SeekToFirst(); + ASSERT_TRUE(c_iter_->status().IsNotSupported()); + ASSERT_FALSE(c_iter_->Valid()); + ASSERT_EQ(0, c_iter_->iter_stats().num_record_drop_user); + ASSERT_EQ((std::vector{"operand:a:A"}), trace.Events()); + } +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterSingleDeleteAndCancellation) { + if (GetParam()) { + return; + } + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + RunTest({test::KeyStr("a", 5, kTypeSingleDeletion), + test::KeyStr("a", 4, kTypeValue)}, + {"", "A"}, {}, {}, kMaxSequenceNumber, &merge, &filter); + ASSERT_TRUE(trace.Events().empty()); + for (ValueType type : {kTypeMerge, kTypeDeletion}) { + InitIterators( + {test::KeyStr("a", 5, kTypeSingleDeletion), test::KeyStr("a", 4, type)}, + {"", "A"}, {}, {}, 5, kMaxSequenceNumber, &merge, &filter); + c_iter_->SeekToFirst(); + if (type == kTypeDeletion) { + ASSERT_TRUE(c_iter_->status().IsCorruption()); + } else { + // Preserve the existing mismatch accounting; this is not valid history. + ASSERT_OK(c_iter_->status()); + ASSERT_EQ(1, c_iter_->iter_stats().num_single_del_mismatch); + } + ASSERT_FALSE(c_iter_->Valid()); + } + merge.partial_success = false; + InitIterators( + {test::KeyStr("a", 5, kTypeMerge), test::KeyStr("a", 4, kTypeMerge)}, + {"B", "A"}, {}, {}, 5, kMaxSequenceNumber, &merge, &filter); + c_iter_->SeekToFirst(); + ASSERT_TRUE(c_iter_->Valid()); + shutting_down_.store(true); + c_iter_->Next(); + ASSERT_TRUE(c_iter_->status().IsShutdownInProgress()); + ASSERT_FALSE(c_iter_->Valid()); + // The whole uncombined batch is filtered before its first operand is output. + ASSERT_EQ( + (std::vector{"partial:a:AB", "operand:a:B", "operand:a:A"}), + trace.Events()); +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterStackedFilterRejected) { + class StackedFilter : public test::RecordingMergeFilter { + public: + explicit StackedFilter(test::MergeFilterTrace* trace) + : RecordingMergeFilter(MergeOrder::kMergeBeforeFilter, trace) {} + bool IsStackedBlobDbInternalCompactionFilter() const override { + return true; + } + }; + test::MergeFilterTrace trace; + StackedFilter filter(&trace); + test::RecordingMergeOperator merge(&trace); + InitIterators({test::KeyStr("a", 5, kTypeMerge)}, {"A"}, {}, {}, 5, + kMaxSequenceNumber, &merge, &filter); + c_iter_->SeekToFirst(); + ASSERT_TRUE(c_iter_->status().IsNotSupported()); + ASSERT_FALSE(c_iter_->Valid()); + ASSERT_TRUE(trace.Events().empty()); +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterDeletionAndExhaustedHistory) { + if (GetParam()) { + return; + } + for (bool deletion_base : {false, true}) { + for (bool remove : {false, true}) { + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.action_key = "a"; + filter.value_decision = remove ? CompactionFilter::Decision::kPurge + : CompactionFilter::Decision::kKeep; + std::vector keys{test::KeyStr("a", 5, kTypeMerge)}; + std::vector values{"A"}; + if (deletion_base) { + keys.emplace_back(test::KeyStr("a", 4, kTypeDeletion)); + values.emplace_back(""); + } + keys.emplace_back(test::KeyStr("b", 3, kTypeValue)); + values.emplace_back("next"); + InitIterators(keys, values, {}, {}, 5, kMaxSequenceNumber, &merge, + &filter, + /*bottommost_level=*/true); + c_iter_->SeekToFirst(); + if (!remove) { + ASSERT_TRUE(c_iter_->Valid()); + ASSERT_EQ("A", c_iter_->value()); + ASSERT_EQ(kTypeValue, c_iter_->ikey().type); + ASSERT_EQ(0, c_iter_->ikey().sequence); + c_iter_->Next(); + } + ASSERT_TRUE(c_iter_->Valid()); + ASSERT_EQ("b", c_iter_->ikey().user_key); + c_iter_->Next(); + ASSERT_OK(c_iter_->status()); + ASSERT_FALSE(c_iter_->Valid()); + ASSERT_EQ(remove ? 1 : 0, c_iter_->iter_stats().num_record_drop_user); + ASSERT_EQ( + (std::vector{"full:a:A", "value:a:A", "value:b:next"}), + trace.Events()); + } + } +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterSequenceZero) { + if (GetParam()) { + return; + } + for (bool full : {false, true}) { + for (bool partial_success : {false, true}) { + for (bool all_zero : {false, true}) { + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + merge.partial_success = partial_success; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + std::vector keys{ + test::KeyStr("a", all_zero ? 0 : 5, kTypeMerge), + test::KeyStr("a", 0, kTypeMerge)}; + std::vector values{"B", "A"}; + if (full) { + keys.emplace_back(test::KeyStr("a", 0, kTypeValue)); + values.emplace_back("base"); + } + InitIterators(keys, values, {}, {}, 5, kMaxSequenceNumber, &merge, + &filter); + c_iter_->SeekToFirst(); + std::vector output_keys; + std::vector output_values; + while (c_iter_->Valid()) { + output_keys.push_back(c_iter_->key().ToString()); + output_values.push_back(c_iter_->value().ToString()); + c_iter_->Next(); + } + ASSERT_OK(c_iter_->status()); + ASSERT_FALSE(merge_helper_->aggregate_eligible()); + std::vector expected{full ? "full:a:baseAB" + : "partial:a:AB"}; + if (!full && !partial_success && !all_zero) { + expected.emplace_back("operand:a:B"); + } + ASSERT_EQ(expected, trace.Events()); + if (full) { + ASSERT_EQ(1, output_keys.size()); + trace.Clear(); + RunTest(output_keys, output_values, output_keys, output_values, + kMaxSequenceNumber, &merge, &filter); + ASSERT_EQ((std::vector{"value:a:baseAB"}), + trace.Events()); + } else if (partial_success) { + ASSERT_EQ(test::KeyStr("a", all_zero ? 0 : 5, kTypeMerge), + output_keys[0]); + } + } + } + } +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterProtectedAggregate) { + if (GetParam()) { + return; + } + AddSnapshot(6); + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.operand_decision = CompactionFilter::Decision::kRemove; + RunTest({test::KeyStr("a", 5, kTypeMerge), test::KeyStr("a", 4, kTypeMerge)}, + {"B", "A"}, {test::KeyStr("a", 5, kTypeMerge)}, {"AB"}, + kMaxSequenceNumber, &merge, &filter); + ASSERT_FALSE(merge_helper_->aggregate_eligible()); + ASSERT_EQ((std::vector{"partial:a:AB"}), trace.Events()); + // A stored partial operand can become eligible in a later, explicit pass. + ClearSnapshots(); + trace.Clear(); + RunTest({test::KeyStr("a", 5, kTypeMerge)}, {"AB"}, {}, {}, + kMaxSequenceNumber, &merge, &filter); + ASSERT_EQ((std::vector{"operand:a:AB"}), trace.Events()); +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterRangeAndSkips) { + if (GetParam()) { + return; + } + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + InitIterators( + {test::KeyStr("a", 5, kTypeMerge), test::KeyStr("a", 3, kTypeMerge), + test::KeyStr("b", 2, kTypeValue)}, + {"new", "covered", "next"}, {test::KeyStr("a", 4, kTypeRangeDeletion)}, + {"b"}, 5, kMaxSequenceNumber, &merge, &filter); + c_iter_->SeekToFirst(); + ASSERT_TRUE(c_iter_->Valid()); + ASSERT_EQ("new", c_iter_->value()); + c_iter_->Next(); + ASSERT_TRUE(c_iter_->Valid()); + ASSERT_EQ("b", c_iter_->ikey().user_key); + c_iter_->Next(); + ASSERT_OK(c_iter_->status()); + ASSERT_EQ((std::vector{"operand:a:new", "value:b:next"}), + trace.Events()); + for (bool valid_skip : {false, true}) { + trace.Clear(); + merge.partial_success = false; + filter.action_key = "a"; + filter.operand_decision = CompactionFilter::Decision::kRemoveAndSkipUntil; + filter.skip_target = valid_skip ? "b+" : "a"; + const std::vector keys{ + test::KeyStr("a", 5, kTypeMerge), test::KeyStr("a", 4, kTypeMerge), + test::KeyStr("b", 3, kTypeValue), test::KeyStr("c", 2, kTypeValue)}; + RunTest(keys, {"B", "A", "skipped", "next"}, + valid_skip ? std::vector{keys[3]} : keys, + valid_skip ? std::vector{"next"} + : std::vector{"B", "A", "skipped", "next"}, + kMaxSequenceNumber, &merge, &filter); + ASSERT_EQ(valid_skip ? (std::vector{ + "partial:a:AB", "operand:a:B", "value:c:next"}) + : (std::vector{ + "partial:a:AB", "operand:a:B", "operand:a:A", + "value:b:skipped", "value:c:next"}), + trace.Events()); + ASSERT_EQ(0, c_iter_->iter_stats().num_record_drop_user); + } +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterSkipOnOlderOperandDropsBatch) { + if (GetParam()) { + return; + } + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + merge.partial_success = false; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.action_value = "A"; + filter.operand_decision = CompactionFilter::Decision::kRemoveAndSkipUntil; + filter.skip_target = "b"; + const std::vector keys{test::KeyStr("a", 5, kTypeMerge), + test::KeyStr("a", 4, kTypeMerge), + test::KeyStr("c", 2, kTypeValue)}; + // The skip on the older operand also drops the newer kept operand. + RunTest(keys, {"B", "A", "next"}, {keys[2]}, {"next"}, kMaxSequenceNumber, + &merge, &filter); + ASSERT_EQ((std::vector{"partial:a:AB", "operand:a:B", + "operand:a:A", "value:c:next"}), + trace.Events()); + ASSERT_EQ(0, c_iter_->iter_stats().num_record_drop_user); +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterFullReplacementAndFailure) { + if (GetParam()) { + return; + } + for (CompactionFilter::Decision decision : + {CompactionFilter::Decision::kChangeValue, + CompactionFilter::Decision::kChangeWideColumnEntity}) { + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.value_decision = decision; + filter.replacement = "changed"; + filter.replacement_columns = {{"extra", "payload"}, {"", "changed"}}; + std::string expected = "changed"; + ValueType type = kTypeValue; + if (decision == CompactionFilter::Decision::kChangeWideColumnEntity) { + type = kTypeWideColumnEntity; + expected.clear(); + ASSERT_OK(WideColumnSerialization::Serialize( + {{"", "changed"}, {"extra", "payload"}}, expected)); + } + RunTest( + {test::KeyStr("a", 5, kTypeMerge), test::KeyStr("a", 4, kTypeValue)}, + {"B", "A"}, {test::KeyStr("a", 5, type)}, {expected}, + kMaxSequenceNumber, &merge, &filter); + ASSERT_EQ((std::vector{"full:a:AB", "value:a:AB"}), + trace.Events()); + } + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + merge.fail_full = true; + merge.failure_scope = MergeOperator::OpFailureScope::kMustMerge; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.operand_decision = CompactionFilter::Decision::kChangeValue; + filter.replacement = "changed"; + RunTest({test::KeyStr("a", 5, kTypeMerge), test::KeyStr("a", 4, kTypeValue)}, + {"B", "A"}, + {test::KeyStr("a", 5, kTypeMerge), test::KeyStr("a", 4, kTypeValue)}, + {"changed", "A"}, kMaxSequenceNumber, &merge, &filter); + ASSERT_EQ((std::vector{"full:a:AB", "operand:a:B", "value:a:A"}), + trace.Events()); +} + +TEST_P(CompactionIteratorTest, MergeBeforeFilterSnapshotCheckerRejected) { + if (!GetParam()) { + return; + } + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + InitIterators({test::KeyStr("a", 5, kTypeMerge)}, {"A"}, {}, {}, 5, + kMaxSequenceNumber, &merge, &filter); + c_iter_->SeekToFirst(); + ASSERT_TRUE(c_iter_->status().IsNotSupported()); + ASSERT_FALSE(c_iter_->Valid()); + ASSERT_TRUE(trace.Events().empty()); +} + // It is possible that the output of the compaction iterator is empty even if // the input is not. TEST_P(CompactionIteratorTest, EmptyResult) { @@ -1428,6 +1930,30 @@ class CompactionIteratorWithAllowIngestBehindTest // When allow_ingest_behind is set, compaction iterator is not targeting // the bottommost level since there is no guarantee there won't be further // data ingested under the compaction output in future. +TEST_P(CompactionIteratorWithAllowIngestBehindTest, + MergeBeforeFilterRetainsDelete) { + if (GetParam()) { + return; + } + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.action_key = "a"; + filter.value_decision = CompactionFilter::Decision::kPurge; + RunTest( + {test::KeyStr("a", 4, kTypeMerge), test::KeyStr("a", 3, kTypeValue), + test::KeyStr("b", 2, kTypeValue)}, + {"B", "A", "next"}, + {test::KeyStr("a", 4, kTypeDeletion), test::KeyStr("b", 2, kTypeValue)}, + {"", "next"}, kMaxSequenceNumber, &merge, &filter, + /*bottommost_level=*/true, kMaxSequenceNumber, + /*key_not_exists_beyond_output_level=*/true); + ASSERT_EQ( + (std::vector{"full:a:AB", "value:a:AB", "value:b:next"}), + trace.Events()); +} + TEST_P(CompactionIteratorWithAllowIngestBehindTest, NoConvertToPutAtBottom) { std::shared_ptr merge_op = MergeOperators::CreateStringAppendOperator(); @@ -1463,6 +1989,19 @@ class CompactionIteratorTsGcTest : public CompactionIteratorTest { : CompactionIteratorTest(test::BytewiseComparatorWithU64TsWrapper()) {} }; +TEST_P(CompactionIteratorTsGcTest, MergeBeforeFilterTimestampRejected) { + test::MergeFilterTrace trace; + test::RecordingMergeOperator merge(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + InitIterators({test::KeyStr(10002, "a", 5, kTypeMerge)}, {"A"}, {}, {}, 5, + kMaxSequenceNumber, &merge, &filter); + c_iter_->SeekToFirst(); + ASSERT_TRUE(c_iter_->status().IsNotSupported()); + ASSERT_FALSE(c_iter_->Valid()); + ASSERT_TRUE(trace.Events().empty()); +} + TEST_P(CompactionIteratorTsGcTest, NoKeyEligibleForGC) { constexpr char user_key[][2] = {{'a', '\0'}, {'b', '\0'}}; const std::vector input_keys = { @@ -1934,11 +2473,12 @@ class WideColumnEntityBlobExtractionTest : public testing::Test { std::atomic shutting_down{false}; const std::atomic manual_compaction_canceled{false}; - MergeHelper merge_helper(Env::Default(), cmp, nullptr /*merge_op*/, - nullptr /*filter*/, nullptr /*db_options*/, false, - 0 /*latest_snapshot*/, - nullptr /*snapshot_checker*/, 0 /*level*/, - nullptr /*statistics*/, &shutting_down); + MergeHelper merge_helper( + Env::Default(), cmp, nullptr /*merge_op*/, nullptr /*filter*/, + CompactionFilter::MergeOrder::kFilterBeforeMerge, + nullptr /*db_options*/, false, 0 /*latest_snapshot*/, + nullptr /*snapshot_checker*/, 0 /*level*/, nullptr /*statistics*/, + &shutting_down); iter->SeekToFirst(); CompactionIterator c_iter( @@ -2377,11 +2917,12 @@ class WideColumnEntityBlobGCTest : public testing::Test { std::atomic shutting_down{false}; const std::atomic manual_compaction_canceled{false}; - MergeHelper merge_helper(Env::Default(), cmp, nullptr /*merge_op*/, - nullptr /*filter*/, nullptr /*db_options*/, false, - 0 /*latest_snapshot*/, - nullptr /*snapshot_checker*/, 0 /*level*/, - nullptr /*statistics*/, &shutting_down); + MergeHelper merge_helper( + Env::Default(), cmp, nullptr /*merge_op*/, nullptr /*filter*/, + CompactionFilter::MergeOrder::kFilterBeforeMerge, + nullptr /*db_options*/, false, 0 /*latest_snapshot*/, + nullptr /*snapshot_checker*/, 0 /*level*/, nullptr /*statistics*/, + &shutting_down); // Create compaction proxy. Note: blob GC is disabled since we don't have // a real Version object. These tests verify that entities with blob indices @@ -2817,8 +3358,10 @@ TEST_F(WideColumnEntityBlobGCTest, std::atomic shutting_down{false}; const std::atomic manual_compaction_canceled{false}; - MergeHelper merge_helper(Env::Default(), cmp, nullptr, nullptr, nullptr, - false, 0, nullptr, 0, nullptr, &shutting_down); + MergeHelper merge_helper(Env::Default(), cmp, nullptr, nullptr, + CompactionFilter::MergeOrder::kFilterBeforeMerge, + nullptr, false, 0, nullptr, 0, nullptr, + &shutting_down); auto fake_compaction = std::make_unique(); diff --git a/db/compaction/compaction_job.cc b/db/compaction/compaction_job.cc index 3483fe18dc4..89087381062 100644 --- a/db/compaction/compaction_job.cc +++ b/db/compaction/compaction_job.cc @@ -1569,8 +1569,32 @@ void CompactionJob::NotifyOnSubcompactionCompleted( bool CompactionJob::ShouldUseLocalCompaction(SubcompactionState* sub_compact) { if (db_options_.compaction_service) { + const ImmutableCFOptions& options = + sub_compact->compaction->column_family_data()->ioptions(); + const CompactionFilterFactory* factory = nullptr; + if (options.compaction_filter == nullptr && + options.compaction_filter_factory && + options.compaction_filter_factory->ShouldFilterTableFileCreation( + TableFileCreationReason::kCompaction)) { + factory = options.compaction_filter_factory.get(); + } + CompactionFilter::MergeOrder order; + Status s = GetCompactionFilterMergeOrder( + options.compaction_filter, factory, + TableFileCreationReason::kCompaction, &order); + if (!s.ok()) { + sub_compact->status = s; + return false; + } + if (order == CompactionFilter::MergeOrder::kMergeBeforeFilter && + !db_options_.compaction_service->SupportsMergeBeforeFilter()) { + TEST_SYNC_POINT( + "CompactionJob::ShouldUseLocalCompaction:MergeBeforeFilterLocal"); + sub_compact->compaction_job_stats.is_remote_compaction = false; + return true; + } CompactionServiceJobStatus comp_status = - ProcessKeyValueCompactionWithCompactionService(sub_compact); + ProcessKeyValueCompactionWithCompactionService(sub_compact, order); if (comp_status != CompactionServiceJobStatus::kUseLocal) { return false; } @@ -1602,7 +1626,8 @@ Status CompactionJob::SetupAndValidateCompactionFilter( SubcompactionState* sub_compact, const CompactionFilter* configured_compaction_filter, const CompactionFilter*& compaction_filter, - std::unique_ptr& compaction_filter_from_factory) { + std::unique_ptr& compaction_filter_from_factory, + CompactionFilter::MergeOrder& merge_order) { compaction_filter = configured_compaction_filter; if (compaction_filter == nullptr) { @@ -1611,13 +1636,40 @@ Status CompactionJob::SetupAndValidateCompactionFilter( compaction_filter = compaction_filter_from_factory.get(); } + const CompactionFilterFactory* factory = + configured_compaction_filter == nullptr + ? sub_compact->compaction->column_family_data() + ->ioptions() + .compaction_filter_factory.get() + : nullptr; + Status s = GetCompactionFilterMergeOrder(compaction_filter, factory, + TableFileCreationReason::kCompaction, + &merge_order); + if (!s.ok()) { + return s; + } + if (compaction_filter == nullptr) { + merge_order = CompactionFilter::MergeOrder::kFilterBeforeMerge; + } + if (compaction_filter && + merge_order == CompactionFilter::MergeOrder::kMergeBeforeFilter && + (job_context_->snapshot_checker != nullptr || + sub_compact->compaction->column_family_data() + ->user_comparator() + ->timestamp_size() != 0 || + compaction_filter->IsStackedBlobDbInternalCompactionFilter())) { + return Status::NotSupported( + "Merge-before-filter does not support SnapshotChecker, timestamps, " + "or stacked BlobDB"); + } + if (compaction_filter != nullptr && !compaction_filter->IgnoreSnapshots()) { return Status::NotSupported( "CompactionFilter::IgnoreSnapshots() = false is not supported " "anymore."); } - return Status::OK(); + return ValidateCompactionFilterMergeOrder(compaction_filter, merge_order); } void CompactionJob::InitializeReadOptionsAndBoundaries( @@ -2128,9 +2180,10 @@ void CompactionJob::ProcessLocalKeyValueCompaction( ColumnFamilyData* cfd = sub_compact->compaction->column_family_data(); const CompactionFilter* compaction_filter; std::unique_ptr compaction_filter_from_factory = nullptr; + CompactionFilter::MergeOrder merge_order; Status filter_status = SetupAndValidateCompactionFilter( sub_compact, cfd->ioptions().compaction_filter, compaction_filter, - compaction_filter_from_factory); + compaction_filter_from_factory, merge_order); if (!filter_status.ok()) { sub_compact->status = filter_status; return; @@ -2161,7 +2214,7 @@ void CompactionJob::ProcessLocalKeyValueCompaction( MergeHelper merge( env_, cfd->user_comparator(), cfd->ioptions().merge_operator.get(), - compaction_filter, db_options_.info_log.get(), + compaction_filter, merge_order, db_options_.info_log.get(), false /* internal key corruption is expected */, job_context_->GetLatestSnapshotSequence(), job_context_->snapshot_checker, compact_->compaction->level(), db_options_.stats); diff --git a/db/compaction/compaction_job.h b/db/compaction/compaction_job.h index 92b1078d7ff..e690723949c 100644 --- a/db/compaction/compaction_job.h +++ b/db/compaction/compaction_job.h @@ -348,7 +348,8 @@ class CompactionJob { uint64_t num_input_range_del); CompactionServiceJobStatus ProcessKeyValueCompactionWithCompactionService( - SubcompactionState* sub_compact); + SubcompactionState* sub_compact, + CompactionFilter::MergeOrder merge_order); struct CompactionIOStatsSnapshot { PerfLevel prev_perf_level = PerfLevel::kEnableTime; @@ -402,7 +403,8 @@ class CompactionJob { SubcompactionState* sub_compact, const CompactionFilter* configured_compaction_filter, const CompactionFilter*& compaction_filter, - std::unique_ptr& compaction_filter_from_factory); + std::unique_ptr& compaction_filter_from_factory, + CompactionFilter::MergeOrder& merge_order); void InitializeReadOptionsAndBoundaries( size_t ts_sz, ReadOptions& read_options, SubcompactionKeyBoundaries& boundaries); @@ -594,6 +596,14 @@ class CompactionJob { // True if this job builds output files as a remote (offloaded) // CompactionService worker. Overridden by CompactionServiceCompactionJob. virtual bool IsRemoteCompaction() const { return false; } + // Checks the actual filter order against what the job requires before any + // input is processed. A remote worker compares it with the primary's + // request and records the acknowledgment or decline in its result. + virtual Status ValidateCompactionFilterMergeOrder( + const CompactionFilter* /*compaction_filter*/, + CompactionFilter::MergeOrder /*merge_order*/) { + return Status::OK(); + } // The rate limiter priority (io_priority) is determined dynamically here. // The Compaction Read and Write priorities are the same for different // scenarios, such as write stalled. @@ -651,6 +661,7 @@ class CompactionJob { // Zero means the worker did not advertise support. constexpr uint64_t kCompactionServiceBlobVersion = 1; constexpr uint64_t kCompactionServiceBlobGCCutoffVersion = 1; +constexpr uint64_t kCompactionServiceMergeBeforeFilterVersion = 1; // CompactionServiceInput is used the pass compaction information between two // db instances. It contains the information needed to do a compaction. It @@ -659,6 +670,11 @@ constexpr uint64_t kCompactionServiceBlobGCCutoffVersion = 1; struct CompactionServiceInput { std::string cf_name; + // Requests merge-before-filter. Old workers ignore this field and return no + // applied-order acknowledgment, so the primary falls back to local compaction + // before importing their output. + bool merge_before_filter = false; + // Snapshot sequence numbers in strictly increasing order. std::vector snapshots; @@ -793,6 +809,10 @@ struct CompactionServiceOutputFile { // permission is able to install the result to the DB. struct CompactionServiceResult { Status status; + // Nonzero only after validating the actual worker filter/factory order. + uint64_t merge_before_filter_version = 0; + // The worker declined an order mismatch before processing input records. + bool merge_order_mismatch = false; std::vector output_files; // Blob metadata generated by the remote compaction. Blob file additions @@ -882,6 +902,9 @@ class CompactionServiceCompactionJob : private CompactionJob { std::optional GetBlobGarbageCollectionCutoffFileNumber() const override; bool IsRemoteCompaction() const override { return true; } + Status ValidateCompactionFilterMergeOrder( + const CompactionFilter* compaction_filter, + CompactionFilter::MergeOrder merge_order) override; // Specific the compaction output path, otherwise it uses default DB path const std::string output_path_; diff --git a/db/compaction/compaction_service_job.cc b/db/compaction/compaction_service_job.cc index dbba0b965c2..5c4a04d6d7e 100644 --- a/db/compaction/compaction_service_job.cc +++ b/db/compaction/compaction_service_job.cc @@ -110,13 +110,15 @@ Status ValidateRemoteBlobFile(FileSystem* fs, const FileOptions& file_options, CompactionServiceJobStatus CompactionJob::ProcessKeyValueCompactionWithCompactionService( - SubcompactionState* sub_compact) { + SubcompactionState* sub_compact, CompactionFilter::MergeOrder merge_order) { assert(sub_compact); assert(sub_compact->compaction); assert(db_options_.compaction_service); const Compaction* const compaction = sub_compact->compaction; CompactionServiceInput compaction_input; + compaction_input.merge_before_filter = + merge_order == CompactionFilter::MergeOrder::kMergeBeforeFilter; compaction_input.output_level = compaction->output_level(); compaction_input.db_id = db_id_; @@ -295,6 +297,7 @@ CompactionJob::ProcessKeyValueCompactionWithCompactionService( compaction->output_level()); info.use_separate_remote_compaction_pool = compaction->use_separate_remote_compaction_pool(); + info.merge_before_filter = compaction_input.merge_before_filter; CompactionServiceScheduleResponse response = db_options_.compaction_service->Schedule(info, compaction_input_binary); switch (response.status) { @@ -396,6 +399,31 @@ CompactionJob::ProcessKeyValueCompactionWithCompactionService( s = CompactionServiceResult::Read(compaction_result_binary, &compaction_result); + if (s.ok() && compaction_result.merge_order_mismatch && + (compaction_status == CompactionServiceJobStatus::kSuccess || + compaction_status == CompactionServiceJobStatus::kFailure)) { + if (!compaction_result.status.IsNotSupported() || + !compaction_result.output_files.empty() || + !compaction_result.blob_file_additions.empty() || + !compaction_result.blob_file_garbages.empty() || + compaction_result.stats.num_input_records != 0 || + compaction_result.merge_before_filter_version != 0) { + sub_compact->status = + Status::Corruption("Invalid remote compaction filter order decline"); + db_options_.compaction_service->OnInstallation( + response.scheduled_job_id, CompactionServiceJobStatus::kFailure); + return CompactionServiceJobStatus::kFailure; + } + ROCKS_LOG_WARN(db_options_.info_log, + "[%s] [JOB %d] Worker declined compaction filter order; " + "falling back to local compaction", + compaction->column_family_data()->GetName().c_str(), + job_id_); + db_options_.compaction_service->OnInstallation( + response.scheduled_job_id, CompactionServiceJobStatus::kUseLocal); + return CompactionServiceJobStatus::kUseLocal; + } + if (compaction_status == CompactionServiceJobStatus::kFailure) { if (s.ok()) { if (compaction_result.status.ok()) { @@ -472,6 +500,21 @@ CompactionJob::ProcessKeyValueCompactionWithCompactionService( return CompactionServiceJobStatus::kFailure; } + const uint64_t expected_merge_version = + compaction_input.merge_before_filter + ? kCompactionServiceMergeBeforeFilterVersion + : 0; + if (compaction_result.merge_before_filter_version != expected_merge_version) { + ROCKS_LOG_WARN(db_options_.info_log, + "[%s] [JOB %d] Declining remote compaction filter order " + "acknowledgment; falling back to local compaction", + compaction->column_family_data()->GetName().c_str(), + job_id_); + db_options_.compaction_service->OnInstallation( + response.scheduled_job_id, CompactionServiceJobStatus::kUseLocal); + return CompactionServiceJobStatus::kUseLocal; + } + auto decline_blob_result = [&](const std::string& reason) { RecordTick(stats_, REMOTE_COMPACT_BLOB_RESULT_DECLINED_COUNT); ROCKS_LOG_WARN(db_options_.info_log, @@ -1151,6 +1194,25 @@ void CompactionServiceCompactionJob::Prepare( compaction_progress_writer); } +Status CompactionServiceCompactionJob::ValidateCompactionFilterMergeOrder( + const CompactionFilter* compaction_filter, + CompactionFilter::MergeOrder merge_order) { + // Without an active filter, both orders produce the same output. + const bool actual_merge_before_filter = + compaction_filter == nullptr + ? compaction_input_.merge_before_filter + : merge_order == CompactionFilter::MergeOrder::kMergeBeforeFilter; + if (actual_merge_before_filter != compaction_input_.merge_before_filter) { + compaction_result_->merge_order_mismatch = true; + return Status::NotSupported( + "Remote compaction filter order does not match the primary request"); + } + compaction_result_->merge_before_filter_version = + actual_merge_before_filter ? kCompactionServiceMergeBeforeFilterVersion + : 0; + return Status::OK(); +} + Status CompactionServiceCompactionJob::Run() { AutoThreadOperationStageUpdater stage_updater( ThreadStatus::STAGE_COMPACTION_RUN); @@ -1375,6 +1437,9 @@ static std::unordered_map cfd_type_info = { }; static std::unordered_map cs_input_type_info = { + {"merge_before_filter", + {offsetof(struct CompactionServiceInput, merge_before_filter), + OptionType::kBoolean}}, {"cf_name", {offsetof(struct CompactionServiceInput, cf_name), OptionType::kEncodedString}}, @@ -1888,6 +1953,12 @@ static std::unordered_map }; static std::unordered_map cs_result_type_info = { + {"merge_before_filter_version", + {offsetof(struct CompactionServiceResult, merge_before_filter_version), + OptionType::kUInt64T}}, + {"merge_order_mismatch", + {offsetof(struct CompactionServiceResult, merge_order_mismatch), + OptionType::kBoolean}}, {"status", {offsetof(struct CompactionServiceResult, status), OptionType::kCustomizable, OptionVerificationType::kNormal, @@ -1979,6 +2050,7 @@ Status CompactionServiceInput::Read(const std::string& data_str, } auto format_version = DecodeFixed32(data_str.data()); if (format_version == kOptionsString) { + obj->merge_before_filter = false; obj->input_blob_files.clear(); obj->has_blob_gc_cutoff_file_number = false; obj->blob_gc_cutoff_file_number = 0; @@ -2030,6 +2102,8 @@ Status CompactionServiceResult::Read(const std::string& data_str, } auto format_version = DecodeFixed32(data_str.data()); if (format_version == kOptionsString) { + obj->merge_before_filter_version = 0; + obj->merge_order_mismatch = false; obj->blob_file_additions.clear(); obj->blob_file_garbages.clear(); obj->blob_compaction_version = 0; diff --git a/db/compaction/compaction_service_test.cc b/db/compaction/compaction_service_test.cc index 5a320cc0d0e..bc1df581446 100644 --- a/db/compaction/compaction_service_test.cc +++ b/db/compaction/compaction_service_test.cc @@ -38,6 +38,7 @@ #include "rocksdb/utilities/options_util.h" #include "table/block_based/block_based_table_factory.h" #include "table/unique_id_impl.h" +#include "test_util/merge_filter_test_util.h" #include "test_util/mock_time_env.h" #include "util/cast_util.h" #include "util/defer.h" @@ -326,6 +327,21 @@ class MyTestCompactionService : public CompactionService { const char* Name() const override { return kClassName(); } + bool SupportsMergeBeforeFilter() const override { + return supports_merge_before_filter_; + } + + void SetSupportsMergeBeforeFilter(bool value) { + supports_merge_before_filter_ = value; + } + + void SetWorkerCompactionFilter( + const CompactionFilter* filter, + std::shared_ptr factory) { + options_.compaction_filter = filter; + options_.compaction_filter_factory = std::move(factory); + } + CompactionServiceScheduleResponse Schedule( const CompactionServiceJobInfo& info, const std::string& compaction_service_input) override { @@ -564,6 +580,7 @@ class MyTestCompactionService : public CompactionService { private: std::atomic_int compaction_num_{0}; Options options_; + bool supports_merge_before_filter_ = false; CompactionServiceJobInfo start_info_; CompactionServiceJobInfo wait_info_; bool is_override_start_status_ = false; @@ -748,6 +765,521 @@ TEST_F(CompactionServiceTest, RemotePoolPropertyDoesNotCreatePool) { SyncPoint::GetInstance()->ClearAllCallBacks(); } +TEST_F(CompactionServiceTest, MergeBeforeFilterRemoteFullResults) { + for (bool use_factory : {false, true}) { + for (bool blobs : {false, true}) { + for (CompactionFilter::Decision decision : + {CompactionFilter::Decision::kKeep, + CompactionFilter::Decision::kChangeValue, + CompactionFilter::Decision::kRemove, + CompactionFilter::Decision::kPurge}) { + if (use_factory && + decision == CompactionFilter::Decision::kChangeValue) { + continue; + } + SCOPED_TRACE(blobs); + SCOPED_TRACE(use_factory); + SCOPED_TRACE(static_cast(decision)); + test::MergeFilterTrace trace; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.replacement = "replacement"; + auto factory = std::make_shared( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.enable_blob_files = blobs; + options.min_blob_size = 1; + options.merge_operator = + std::make_shared(&trace); + if (use_factory) { + options.compaction_filter_factory = factory; + } else { + options.compaction_filter = &filter; + } + ReopenWithCompactionService(&options); + GetCompactionService()->SetSupportsMergeBeforeFilter(true); + ASSERT_OK(Put("a", "older")); + ASSERT_OK(Flush()); + const Snapshot* snapshot = db_->GetSnapshot(); + MoveFilesToLevel(2); + const int previous_jobs = GetCompactionService()->GetCompactionNum(); + filter.value_decision = decision; + factory->value_decision = decision; + ASSERT_OK(Put("a", "base")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "M")); + ASSERT_OK(Flush()); + trace.Clear(); + std::vector files; + db_->GetLiveFilesMetaData(&files); + std::vector inputs; + for (const auto& file : files) { + if (file.level == 0) { + inputs.push_back(file.relative_filename); + } + } + CompactionOptions compact; + ASSERT_OK(db_->CompactFiles(compact, inputs, 1)); + ASSERT_EQ(previous_jobs + 1, + GetCompactionService()->GetCompactionNum()); + ASSERT_TRUE(GetCompactionService() + ->GetCompactionInfoForStart() + .merge_before_filter); + ASSERT_EQ((std::vector{"full:a:baseM", "value:a:baseM"}), + trace.Events()); + CompactionServiceResult result; + GetCompactionService()->GetResult(&result); + ASSERT_OK(result.status); + ASSERT_EQ(kCompactionServiceMergeBeforeFilterVersion, + result.merge_before_filter_version); + ASSERT_FALSE(result.merge_order_mismatch); + const std::string expected = + decision == CompactionFilter::Decision::kKeep ? "baseM" + : decision == CompactionFilter::Decision::kChangeValue + ? "replacement" + : "NOT_FOUND"; + ASSERT_EQ(expected, Get("a")); + ReadOptions read; + read.snapshot = snapshot; + std::string old_value; + ASSERT_OK(db_->Get(read, "a", &old_value)); + ASSERT_EQ("older", old_value); + db_->ReleaseSnapshot(snapshot); + } + } + } +} + +TEST_F(CompactionServiceTest, MergeBeforeFilterRemoteAcknowledgmentFallback) { + for (uint64_t acknowledgment : {uint64_t{0}, uint64_t{2}}) { + test::MergeFilterTrace trace; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.value_decision = CompactionFilter::Decision::kPurge; + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.merge_operator = + std::make_shared(&trace); + options.compaction_filter = &filter; + ReopenWithCompactionService(&options); + GetCompactionService()->SetSupportsMergeBeforeFilter(true); + GetCompactionService()->SetWaitResultMutator( + [acknowledgment](std::string* serialized) { + CompactionServiceResult result; + ASSERT_OK(CompactionServiceResult::Read(*serialized, &result)); + ASSERT_OK(result.status); + result.merge_before_filter_version = acknowledgment; + serialized->clear(); + ASSERT_OK(result.Write(serialized)); + }); + ASSERT_OK(Put("a", "base")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "M")); + ASSERT_OK(Flush()); + trace.Clear(); + ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr)); + ASSERT_EQ("NOT_FOUND", Get("a")); + ASSERT_EQ(1, GetCompactionService()->GetCompactionNum()); + ASSERT_EQ(1, GetCompactionService()->GetUseLocalInstallationCount()); + ASSERT_EQ((std::vector{"full:a:baseM", "value:a:baseM", + "full:a:baseM", "value:a:baseM"}), + trace.Events()); + } +} + +TEST_F(CompactionServiceTest, MergeBeforeFilterRemotePartialAndDeclined) { + for (bool partial_success : {false, true}) { + for (CompactionFilter::Decision decision : + {CompactionFilter::Decision::kKeep, + CompactionFilter::Decision::kChangeValue, + CompactionFilter::Decision::kRemove}) { + SCOPED_TRACE(partial_success); + SCOPED_TRACE(static_cast(decision)); + test::MergeFilterTrace trace; + auto merge = std::make_shared(&trace); + merge->partial_success = partial_success; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.operand_decision = decision; + filter.replacement = "X"; + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.compaction_filter = &filter; + options.merge_operator = merge; + ReopenWithCompactionService(&options); + GetCompactionService()->SetSupportsMergeBeforeFilter(true); + ASSERT_OK(Put("a", "base")); + ASSERT_OK(Flush()); + MoveFilesToLevel(2); + ASSERT_OK(Merge("a", "A")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "B")); + ASSERT_OK(Flush()); + trace.Clear(); + std::vector files; + db_->GetLiveFilesMetaData(&files); + std::vector inputs; + for (const auto& file : files) { + if (file.level == 0) { + inputs.push_back(file.relative_filename); + } + } + CompactionOptions compact; + ASSERT_OK(db_->CompactFiles(compact, inputs, 1)); + ASSERT_EQ(0, GetCompactionService()->GetUseLocalInstallationCount()); + ASSERT_EQ(partial_success + ? (std::vector{"partial:a:AB", "operand:a:AB"}) + : (std::vector{"partial:a:AB", "operand:a:B", + "operand:a:A"}), + trace.Events()); + const std::string expected = + decision == CompactionFilter::Decision::kKeep ? "baseAB" + : decision == CompactionFilter::Decision::kChangeValue + ? (partial_success ? "baseX" : "baseXX") + : "base"; + ASSERT_EQ(expected, Get("a")); + } + } +} + +TEST_F(CompactionServiceTest, MergeBeforeFilterRemoteErrorsDoNotFallback) { + for (bool factory_mismatch : {false, true}) { + test::MergeFilterTrace trace; + auto merge = std::make_shared(&trace); + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + auto factory = std::make_shared( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.merge_operator = merge; + options.compaction_filter = &filter; + ReopenWithCompactionService(&options); + GetCompactionService()->SetSupportsMergeBeforeFilter(true); + if (factory_mismatch) { + factory->instance_order = + CompactionFilter::MergeOrder::kFilterBeforeMerge; + GetCompactionService()->SetWorkerCompactionFilter(nullptr, factory); + } else { + merge->fail_full = true; + merge->failure_scope = MergeOperator::OpFailureScope::kTryMerge; + } + ASSERT_OK(Put("a", "base")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "M")); + ASSERT_OK(Flush()); + trace.Clear(); + Status status = db_->CompactRange(CompactRangeOptions(), nullptr, nullptr); + ASSERT_FALSE(status.ok()); + ASSERT_EQ(0, GetCompactionService()->GetUseLocalInstallationCount()); + CompactionServiceResult result; + GetCompactionService()->GetResult(&result); + ASSERT_FALSE(result.status.ok()); + ASSERT_FALSE(result.merge_order_mismatch); + ASSERT_TRUE(result.output_files.empty()); + if (factory_mismatch) { + ASSERT_TRUE(status.IsInvalidArgument()); + ASSERT_TRUE(trace.Events().empty()); + } + } +} + +TEST_F(CompactionServiceTest, MergeBeforeFilterRemoteInvalidDecline) { + test::MergeFilterTrace trace; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.compaction_filter = &filter; + options.merge_operator = + std::make_shared(&trace); + ReopenWithCompactionService(&options); + GetCompactionService()->SetSupportsMergeBeforeFilter(true); + GetCompactionService()->SetWaitResultMutator([](std::string* serialized) { + CompactionServiceResult result; + ASSERT_OK(CompactionServiceResult::Read(*serialized, &result)); + ASSERT_OK(result.status); + result.status = Status::IOError("Injected worker error"); + result.merge_order_mismatch = true; + result.merge_before_filter_version = 0; + result.stats.num_input_records = 0; + result.output_files.clear(); + serialized->clear(); + ASSERT_OK(result.Write(serialized)); + }); + ASSERT_OK(Put("a", "base")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "M")); + ASSERT_OK(Flush()); + Status status = db_->CompactRange(CompactRangeOptions(), nullptr, nullptr); + ASSERT_TRUE(status.IsCorruption()); + ASSERT_EQ("baseM", Get("a")); + ASSERT_EQ(0, GetCompactionService()->GetUseLocalInstallationCount()); +} + +TEST_F(CompactionServiceTest, MergeBeforeFilterRemoteWorkerOrderMismatch) { + for (bool new_order_primary : {false, true}) { + test::MergeFilterTrace trace; + test::RecordingMergeFilter legacy( + CompactionFilter::MergeOrder::kFilterBeforeMerge, &trace); + test::RecordingMergeFilter newer( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.merge_operator = + std::make_shared(&trace); + options.compaction_filter = new_order_primary ? &newer : &legacy; + ReopenWithCompactionService(&options); + GetCompactionService()->SetSupportsMergeBeforeFilter(true); + GetCompactionService()->SetWorkerCompactionFilter( + new_order_primary ? &legacy : &newer, nullptr); + ASSERT_OK(Put("a", "base")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "M")); + ASSERT_OK(Flush()); + trace.Clear(); + ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr)); + ASSERT_EQ("baseM", Get("a")); + ASSERT_EQ(1, GetCompactionService()->GetUseLocalInstallationCount()); + CompactionServiceResult result; + GetCompactionService()->GetResult(&result); + ASSERT_TRUE(result.status.IsNotSupported()); + ASSERT_TRUE(result.merge_order_mismatch); + ASSERT_TRUE(result.output_files.empty()); + ASSERT_EQ(0, result.stats.num_input_records); + ASSERT_EQ(0, GetCompactorStatistics()->getTickerCount(COMPACT_WRITE_BYTES)); + } +} + +TEST_F(CompactionServiceTest, MergeBeforeFilterNullWorkerFilterRunsRemotely) { + test::MergeFilterTrace trace; + auto factory = std::make_shared( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + factory->return_null = true; + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.merge_operator = + std::make_shared(&trace); + options.compaction_filter_factory = factory; + ReopenWithCompactionService(&options); + GetCompactionService()->SetSupportsMergeBeforeFilter(true); + GetCompactionService()->SetWorkerCompactionFilter(nullptr, factory); + ASSERT_OK(Put("a", "base")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "M")); + ASSERT_OK(Flush()); + ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr)); + ASSERT_EQ("baseM", Get("a")); + // Without a filter both orders give the same output, so the worker accepts. + ASSERT_EQ(1, GetCompactionService()->GetCompactionNum()); + ASSERT_EQ(0, GetCompactionService()->GetUseLocalInstallationCount()); + CompactionServiceResult result; + GetCompactionService()->GetResult(&result); + ASSERT_OK(result.status); + ASSERT_FALSE(result.merge_order_mismatch); + ASSERT_EQ(kCompactionServiceMergeBeforeFilterVersion, + result.merge_before_filter_version); + for (const std::string& event : trace.Events()) { + ASSERT_EQ(0, event.rfind("full:", 0)) << event; + } +} + +TEST_F(CompactionServiceTest, MergeBeforeFilterRemoteEntityAndStoredPurge) { + for (bool blobs : {false, true}) { + test::MergeFilterTrace trace; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.enable_blob_files = blobs; + options.min_blob_size = 0; + options.compaction_filter = &filter; + options.merge_operator = + std::make_shared(&trace); + ReopenWithCompactionService(&options); + GetCompactionService()->SetSupportsMergeBeforeFilter(true); + ASSERT_OK(db_->PutEntity(WriteOptions(), db_->DefaultColumnFamily(), "a", + {{"", "A"}, {"extra", "payload"}})); + ASSERT_OK(Put("b", "base")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "B")); + ASSERT_OK(Merge("b", "operand")); + ASSERT_OK(Flush()); + trace.Clear(); + std::vector files; + db_->GetLiveFilesMetaData(&files); + std::vector inputs; + inputs.reserve(files.size()); + for (const auto& file : files) { + inputs.push_back(file.relative_filename); + } + ASSERT_OK(db_->CompactFiles(CompactionOptions(), inputs, 1)); + ASSERT_EQ( + (std::vector{"full:a:AB", "entity:a:AB", + "full:b:baseoperand", "value:b:baseoperand"}), + trace.Events()); + PinnableWideColumns columns; + ASSERT_OK(db_->GetEntity(ReadOptions(), db_->DefaultColumnFamily(), "a", + &columns)); + ASSERT_EQ((WideColumns{{"", "AB"}, {"extra", "payload"}}), + columns.columns()); + filter.value_decision = CompactionFilter::Decision::kPurge; + filter.blob_decision = CompactionFilter::Decision::kPurge; + trace.Clear(); + files.clear(); + inputs.clear(); + db_->GetLiveFilesMetaData(&files); + inputs.reserve(files.size()); + for (const auto& file : files) { + inputs.push_back(file.relative_filename); + } + ASSERT_OK(db_->CompactFiles(CompactionOptions(), inputs, 2)); + ASSERT_EQ("NOT_FOUND", Get("a")); + ASSERT_EQ("NOT_FOUND", Get("b")); + ASSERT_EQ(2, GetCompactionService()->GetCompactionNum()); + ASSERT_EQ(0, GetCompactionService()->GetUseLocalInstallationCount()); + if (blobs) { + ASSERT_GT(filter.blob_key_calls.load(), 0); + } + CompactRangeOptions compact; + compact.bottommost_level_compaction = BottommostLevelCompaction::kForce; + ASSERT_OK(db_->CompactRange(compact, nullptr, nullptr)); + ASSERT_EQ("NOT_FOUND", Get("a")); + ASSERT_EQ("NOT_FOUND", Get("b")); + } +} + +TEST_F(CompactionServiceTest, MergeBeforeFilterRemoteWireVersions) { + for (bool new_order : {false, true}) { + CompactionServiceInput input; + input.merge_before_filter = new_order; + std::string serialized; + ASSERT_OK(input.Write(&serialized)); + // Both orders use the existing format; old workers ignore the new field. + ASSERT_EQ(1, DecodeFixed32(serialized.data())); + CompactionServiceInput decoded; + ASSERT_OK(CompactionServiceInput::Read(serialized, &decoded)); + ASSERT_EQ(new_order, decoded.merge_before_filter); + } + { + // A request from an old primary has no field and decodes as legacy order. + CompactionServiceInput decoded; + decoded.merge_before_filter = true; + std::string legacy(sizeof(uint32_t), '\0'); + EncodeFixed32(&legacy[0], 1); + legacy += "output_level=0;"; + ASSERT_OK(CompactionServiceInput::Read(legacy, &decoded)); + ASSERT_FALSE(decoded.merge_before_filter); + } + CompactionServiceResult decoded; + decoded.merge_before_filter_version = 1; + decoded.merge_order_mismatch = true; + std::string legacy(sizeof(uint32_t), '\0'); + EncodeFixed32(&legacy[0], 1); + legacy += "output_level=0;"; + ASSERT_OK(CompactionServiceResult::Read(legacy, &decoded)); + ASSERT_OK(decoded.status); + ASSERT_EQ(0, decoded.merge_before_filter_version); + ASSERT_FALSE(decoded.merge_order_mismatch); +} + +TEST_F(CompactionServiceTest, MergeBeforeFilterLocalSubcompactions) { + class CompletionCounter : public EventListener { + public: + void OnSubcompactionCompleted(const SubcompactionJobInfo& info) override { + EXPECT_FALSE(info.stats.is_remote_compaction); + completions.fetch_add(1); + } + std::atomic completions{0}; + }; + for (bool use_factory : {false, true}) { + test::MergeFilterTrace trace; + test::RecordingMergeFilter direct( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + auto factory = std::make_shared( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + factory->value_decision = CompactionFilter::Decision::kPurge; + auto listener = std::make_shared(); + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.max_subcompactions = 4; + options.target_file_size_base = 32 << 10; + options.compression = kNoCompression; + options.merge_operator = + std::make_shared(&trace); + options.listeners.push_back(listener); + if (use_factory) { + options.compaction_filter_factory = factory; + } else { + options.compaction_filter = &direct; + } + ReopenWithCompactionService(&options); + constexpr int kKeys = 4096; + const std::string base(1024, 'A'); + for (int i = 0; i < kKeys; ++i) { + ASSERT_OK(Put(Key(i), base)); + } + ASSERT_OK(Flush()); + for (int i = 0; i < kKeys; ++i) { + ASSERT_OK(Merge(Key(i), "B")); + } + ASSERT_OK(Flush()); + ASSERT_EQ(0, factory->creations.load()); + trace.Clear(); + std::vector files; + db_->GetLiveFilesMetaData(&files); + std::vector inputs; + for (const auto& file : files) { + inputs.push_back(file.relative_filename); + } + std::atomic local_routes{0}; + SyncPoint::GetInstance()->SetCallBack( + "CompactionJob::ShouldUseLocalCompaction:MergeBeforeFilterLocal", + [&](void*) { local_routes.fetch_add(1); }); + SyncPoint::GetInstance()->EnableProcessing(); + CompactionOptions compact; + compact.max_subcompactions = 4; + ASSERT_OK(db_->CompactFiles(compact, inputs, 1)); + const uint64_t subcompactions = listener->completions.load(); + ASSERT_GT(subcompactions, 1); + ASSERT_EQ(use_factory ? subcompactions : 0, factory->creations.load()); + ASSERT_EQ(0, GetCompactionService()->GetCompactionNum()); + ASSERT_EQ(0, GetCompactorStatistics()->getTickerCount(COMPACT_WRITE_BYTES)); + if (use_factory) { + ASSERT_EQ(0, GetPrimaryStatistics()->getTickerCount(COMPACT_WRITE_BYTES)); + } else { + ASSERT_GT(GetPrimaryStatistics()->getTickerCount(COMPACT_WRITE_BYTES), 0); + } + std::map full_positions; + size_t values = 0; + const std::vector events = trace.Events(); + for (size_t i = 0; i < events.size(); ++i) { + const size_t colon = events[i].find(':'); + const size_t last_colon = events[i].find(':', colon + 1); + const std::string key = + events[i].substr(colon + 1, last_colon - colon - 1); + if (events[i].compare(0, colon, "full") == 0) { + ASSERT_TRUE(full_positions.emplace(key, i).second); + } else { + ASSERT_EQ("value", events[i].substr(0, colon)); + ASSERT_EQ(1, full_positions.count(key)); + ASSERT_LT(full_positions.at(key), i); + ++values; + } + } + ASSERT_EQ(kKeys, full_positions.size()); + ASSERT_EQ(kKeys, values); + for (int i = 0; i < kKeys; ++i) { + ASSERT_EQ(use_factory ? "NOT_FOUND" : base + "B", Get(Key(i))); + } + ASSERT_EQ(subcompactions, local_routes.load()); + SyncPoint::GetInstance()->DisableProcessing(); + SyncPoint::GetInstance()->ClearAllCallBacks(); + Close(); + } +} + TEST_F(CompactionServiceTest, BasicCompactions) { Options options = CurrentOptions(); options.max_open_files = -1; diff --git a/db/db_compaction_filter_test.cc b/db/db_compaction_filter_test.cc index f7060f814e9..41db7e4ddcd 100644 --- a/db/db_compaction_filter_test.cc +++ b/db/db_compaction_filter_test.cc @@ -8,7 +8,12 @@ // found in the LICENSE file. See the AUTHORS file for names of contributors. #include "db/db_test_util.h" +#include "options/options_helper.h" #include "port/stack_trace.h" +#include "rocksdb/utilities/transaction_db.h" +#include "test_util/merge_filter_test_util.h" +#include "util/defer.h" +#include "utilities/fault_injection_env.h" namespace ROCKSDB_NAMESPACE { @@ -23,6 +28,17 @@ class DBTestCompactionFilter : public DBTestBase { public: DBTestCompactionFilter() : DBTestBase("db_compaction_filter_test", /*env_do_fsync=*/true) {} + + Status CompactAllMergeFilterFiles() { + std::vector files; + db_->GetLiveFilesMetaData(&files); + std::vector names; + names.reserve(files.size()); + for (const auto& file : files) { + names.push_back(file.relative_filename); + } + return db_->CompactFiles(CompactionOptions(), names, 1); + } }; // Param variant of DBTestBase::ChangeCompactOptions @@ -1048,6 +1064,561 @@ TEST_F(DBTestCompactionFilter, DropKeyWithSingleDelete) { } } +TEST_F(DBTestCompactionFilter, MergeBeforeFilterModeDeclarations) { + using Order = CompactionFilter::MergeOrder; + test::MergeFilterTrace trace; + test::RecordingMergeFilter legacy(Order::kFilterBeforeMerge, &trace); + test::RecordingMergeFilter post(Order::kMergeBeforeFilter, &trace); + test::RecordingMergeFilterFactory legacy_factory(Order::kFilterBeforeMerge, + &trace); + test::RecordingMergeFilterFactory post_factory(Order::kMergeBeforeFilter, + &trace); + for (int i = 0; i < 3; ++i) { + ASSERT_EQ(Order::kFilterBeforeMerge, legacy.GetMergeOrder()); + ASSERT_EQ(Order::kMergeBeforeFilter, post.GetMergeOrder()); + ASSERT_EQ(Order::kMergeBeforeFilter, post_factory.GetMergeOrder()); + } + Order order; + ASSERT_OK(GetCompactionFilterMergeOrder( + nullptr, nullptr, TableFileCreationReason::kCompaction, &order)); + ASSERT_EQ(Order::kFilterBeforeMerge, order); + ASSERT_OK(GetCompactionFilterMergeOrder( + &post, &post_factory, TableFileCreationReason::kCompaction, &order)); + ASSERT_EQ(Order::kMergeBeforeFilter, order); + ASSERT_TRUE( + GetCompactionFilterMergeOrder( + &legacy, &post_factory, TableFileCreationReason::kCompaction, &order) + .IsInvalidArgument()); + ASSERT_TRUE( + GetCompactionFilterMergeOrder( + &post, &legacy_factory, TableFileCreationReason::kCompaction, &order) + .IsInvalidArgument()); + for (TableFileCreationReason reason : + {TableFileCreationReason::kFlush, TableFileCreationReason::kRecovery, + TableFileCreationReason::kMisc, + TableFileCreationReason::kSstFileWriter}) { + ASSERT_OK(GetCompactionFilterMergeOrder(&legacy, &legacy_factory, reason, + &order)); + ASSERT_TRUE( + GetCompactionFilterMergeOrder(&post, &post_factory, reason, &order) + .IsInvalidArgument()); + } +} + +TEST_F(DBTestCompactionFilter, MergeBeforeFilterNullFactory) { + test::MergeFilterTrace trace; + auto factory = std::make_shared( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + factory->return_null = true; + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.compaction_filter_factory = factory; + Reopen(options); + ASSERT_OK(Put("a", "value")); + ASSERT_OK(Flush()); + ASSERT_OK(CompactAllMergeFilterFiles()); + ASSERT_EQ("value", Get("a")); + ASSERT_EQ(1, factory->creations.load()); + ASSERT_TRUE(trace.Events().empty()); + Close(); +} + +TEST_F(DBTestCompactionFilter, MergeBeforeFilterSharedCreationRuntimeGuard) { + for (bool recovery : {false, true}) { + test::MergeFilterTrace trace; + auto factory = std::make_shared( + CompactionFilter::MergeOrder::kFilterBeforeMerge, &trace); + factory->filter_other_reason = true; + factory->other_reason = recovery ? TableFileCreationReason::kRecovery + : TableFileCreationReason::kFlush; + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.compaction_filter_factory = factory; + DestroyAndReopen(options); + ASSERT_OK(Put("a", "durable")); + factory->instance_order = CompactionFilter::MergeOrder::kMergeBeforeFilter; + ASSERT_OK(ValidateCompactionFilterOptions(options)); + if (recovery) { + Close(); + ASSERT_TRUE(TryReopen(options).IsInvalidArgument()); + } else { + ASSERT_TRUE(Flush().IsInvalidArgument()); + ASSERT_EQ(0, NumTableFilesAtLevel(0)); + ASSERT_EQ("durable", Get("a")); + Close(); + } + ASSERT_EQ(1, factory->creations.load()); + ASSERT_TRUE(trace.Events().empty()); + factory->instance_order = CompactionFilter::MergeOrder::kFilterBeforeMerge; + Reopen(options); + ASSERT_EQ("durable", Get("a")); + Close(); + } +} + +TEST_F(DBTestCompactionFilter, MergeBeforeFilterRepairRuntimeGuard) { + test::MergeFilterTrace trace; + auto factory = std::make_shared( + CompactionFilter::MergeOrder::kFilterBeforeMerge, &trace); + factory->filter_other_reason = true; + factory->other_reason = TableFileCreationReason::kRecovery; + factory->instance_order = CompactionFilter::MergeOrder::kMergeBeforeFilter; + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + DestroyAndReopen(options); + ASSERT_OK(Put("sst", "preserved")); + ASSERT_OK(Flush()); + ASSERT_OK(Put("wal", "not-installed")); + Close(); + options.compaction_filter_factory = factory; + ASSERT_OK(ValidateCompactionFilterOptions(options)); + // Repair retains its best-effort policy: a failed WAL conversion is logged + // and archived, while existing SST files are still recovered. + ASSERT_OK(RepairDB(dbname_, options)); + ASSERT_EQ(1, factory->creations.load()); + // The filter was created but never saw a key, and the WAL data is absent. + ASSERT_TRUE(trace.Events().empty()); + options.compaction_filter_factory.reset(); + Reopen(options); + ASSERT_EQ("preserved", Get("sst")); + ASSERT_EQ("NOT_FOUND", Get("wal")); + ASSERT_EQ(1, TotalTableFiles()); + Close(); +} + +TEST_F(DBTestCompactionFilter, MergeBeforeFilterRepeatedPurgeAndRollback) { + test::MergeFilterTrace trace; + test::RecordingMergeFilter post( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + test::RecordingMergeFilter legacy( + CompactionFilter::MergeOrder::kFilterBeforeMerge, &trace); + auto merge = std::make_shared(&trace); + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.merge_operator = merge; + options.compaction_filter = &post; + Reopen(options); + ASSERT_OK(Put("a", "A")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "B")); + ASSERT_OK(Flush()); + trace.Clear(); + ASSERT_OK(CompactAllMergeFilterFiles()); + ASSERT_EQ((std::vector{"full:a:AB", "value:a:AB"}), + trace.Events()); + ASSERT_EQ("AB", Get("a")); + const Snapshot* snapshot = db_->GetSnapshot(); + ASSERT_OK(Merge("a", "C")); + ASSERT_OK(Flush()); + trace.Clear(); + ASSERT_OK(CompactAllMergeFilterFiles()); + ASSERT_EQ((std::vector{"operand:a:C", "value:a:AB"}), + trace.Events()); + ASSERT_EQ("AB", Get("a", snapshot)); + db_->ReleaseSnapshot(snapshot); + trace.Clear(); + ASSERT_OK(CompactAllMergeFilterFiles()); + ASSERT_EQ((std::vector{"full:a:ABC", "value:a:ABC"}), + trace.Events()); + trace.Clear(); + post.value_decision = CompactionFilter::Decision::kPurge; + ASSERT_OK(CompactAllMergeFilterFiles()); + ASSERT_EQ("NOT_FOUND", Get("a")); + ASSERT_EQ((std::vector{"value:a:ABC"}), trace.Events()); + // Startup selection changes ordering, retaining ordinary removal for history. + Close(); + legacy.value_decision = CompactionFilter::Decision::kRemove; + options.compaction_filter = &legacy; + Reopen(options); + ASSERT_EQ("NOT_FOUND", Get("a")); + ASSERT_OK(Put("a", "new")); + ASSERT_OK(Flush()); + ASSERT_OK(CompactAllMergeFilterFiles()); + ASSERT_EQ("NOT_FOUND", Get("a")); + Close(); +} + +TEST_F(DBTestCompactionFilter, + MergeBeforeFilterRemovalOutsideInputsAndExactSkip) { + for (bool skip : {false, true}) { + test::MergeFilterTrace trace; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + filter.action_key = "a"; + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.compaction_filter = &filter; + options.merge_operator = + std::make_shared(&trace); + DestroyAndReopen(options); + ASSERT_OK(Put("a", "older")); + ASSERT_OK(Flush()); + MoveFilesToLevel(options.num_levels - 1); + const Snapshot* snapshot = db_->GetSnapshot(); + ASSERT_OK(Put("a", "base")); + ASSERT_OK(Put("b", "middle")); + ASSERT_OK(Put("c", "next")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "operand")); + ASSERT_OK(Flush()); + filter.value_decision = + skip ? CompactionFilter::Decision::kRemoveAndSkipUntil + : CompactionFilter::Decision::kPurge; + filter.skip_target = "c"; + trace.Clear(); + std::vector files; + db_->GetLiveFilesMetaData(&files); + std::vector inputs; + for (const auto& file : files) { + if (file.level == 0) { + inputs.push_back(file.relative_filename); + } + } + ASSERT_EQ(2, inputs.size()); + ASSERT_OK(db_->CompactFiles(CompactionOptions(), inputs, 1)); + ASSERT_EQ(skip ? "older" : "NOT_FOUND", Get("a")); + ASSERT_EQ("older", Get("a", snapshot)); + ASSERT_EQ(skip ? "NOT_FOUND" : "middle", Get("b")); + ASSERT_EQ("next", Get("c")); + ASSERT_EQ( + skip ? (std::vector{"full:a:baseoperand", + "value:a:baseoperand", "value:c:next"}) + : (std::vector{"full:a:baseoperand", + "value:a:baseoperand", + "value:b:middle", "value:c:next"}), + trace.Events()); + db_->ReleaseSnapshot(snapshot); + if (!skip) { + CompactRangeOptions compact; + compact.bottommost_level_compaction = BottommostLevelCompaction::kForce; + ASSERT_OK(db_->CompactRange(compact, nullptr, nullptr)); + ASSERT_EQ("NOT_FOUND", Get("a")); + } + Close(); + Reopen(options); + ASSERT_EQ(skip ? "older" : "NOT_FOUND", Get("a")); + Close(); + } +} + +TEST_F(DBTestCompactionFilter, MergeBeforeFilterWideEntityAndBlobBase) { + for (bool blobs : {false, true}) { + test::MergeFilterTrace trace; + test::RecordingMergeFilter filter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + auto merge = std::make_shared(&trace); + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.compaction_filter = &filter; + options.merge_operator = merge; + options.enable_blob_files = blobs; + options.min_blob_size = 0; + DestroyAndReopen(options); + ASSERT_OK(db_->PutEntity(WriteOptions(), db_->DefaultColumnFamily(), "a", + {{"", "A"}, {"extra", "payload"}})); + ASSERT_OK(Put("b", "base")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "B")); + ASSERT_OK(Merge("b", "operand")); + ASSERT_OK(Flush()); + trace.Clear(); + ASSERT_OK(CompactAllMergeFilterFiles()); + ASSERT_EQ( + (std::vector{"full:a:AB", "entity:a:AB", + "full:b:baseoperand", "value:b:baseoperand"}), + trace.Events()); + PinnableWideColumns columns; + ASSERT_OK(db_->GetEntity(ReadOptions(), db_->DefaultColumnFamily(), "a", + &columns)); + ASSERT_EQ((WideColumns{{"", "AB"}, {"extra", "payload"}}), + columns.columns()); + ASSERT_EQ("baseoperand", Get("b")); + filter.value_decision = CompactionFilter::Decision::kPurge; + filter.blob_decision = CompactionFilter::Decision::kPurge; + trace.Clear(); + ASSERT_OK(CompactAllMergeFilterFiles()); + ASSERT_EQ("NOT_FOUND", Get("a")); + ASSERT_EQ("NOT_FOUND", Get("b")); + if (blobs) { + ASSERT_GT(filter.blob_key_calls.load(), 0); + } + Close(); + Reopen(options); + ASSERT_EQ("NOT_FOUND", Get("a")); + ASSERT_EQ("NOT_FOUND", Get("b")); + Close(); + } +} + +TEST_F(DBTestCompactionFilter, MergeBeforeFilterBlobReadFailures) { + class ResolvingFilter : public test::RecordingMergeFilter { + public: + explicit ResolvingFilter(test::MergeFilterTrace* trace) + : RecordingMergeFilter(MergeOrder::kMergeBeforeFilter, trace) {} + + Decision FilterV4( + int level, const Slice& key, ValueType type, const Slice* value, + const WideColumns* columns, std::string* new_value, + std::vector>* new_columns, + std::string* skip_until, + WideColumnBlobResolver* resolver) const override { + if (resolver != nullptr) { + Slice resolved; + Status s = resolver->ResolveColumn(0, &resolved); + if (!s.ok()) { + resolve_failed.store(true); + return Decision::kKeep; + } + } + return RecordingMergeFilter::FilterV4(level, key, type, value, columns, + new_value, new_columns, skip_until, + resolver); + } + + mutable std::atomic resolve_failed{false}; + }; + + for (int history : {0, 1, 2}) { + FaultInjectionTestEnv fault_env(env_); + test::MergeFilterTrace trace; + ResolvingFilter filter(&trace); + Defer cleanup([&]() { + SyncPoint::GetInstance()->DisableProcessing(); + SyncPoint::GetInstance()->ClearAllCallBacks(); + fault_env.SetFilesystemActive(true); + Close(); + }); + Options options = CurrentOptions(); + options.env = &fault_env; + options.disable_auto_compactions = true; + options.compaction_filter = &filter; + options.merge_operator = + std::make_shared(&trace); + options.enable_blob_files = true; + options.min_blob_size = 0; + options.blob_compaction_readahead_size = 0; + DestroyAndReopen(options); + if (history == 0) { + ASSERT_OK(Put("a", "base")); + } else { + ASSERT_OK(db_->PutEntity(WriteOptions(), db_->DefaultColumnFamily(), "a", + {{"", "base"}, {"extra", "payload"}})); + } + ASSERT_OK(Flush()); + if (history != 2) { + ASSERT_OK(Merge("a", "operand")); + ASSERT_OK(Flush()); + } + trace.Clear(); + std::vector before; + db_->GetLiveFilesMetaData(&before); + SyncPoint::GetInstance()->SetCallBack( + "BlobFileReader::GetBlob:ReadFromFile", [&](void*) { + fault_env.SetFilesystemActive(false, Status::IOError("blob read")); + }); + SyncPoint::GetInstance()->EnableProcessing(); + Status s = CompactAllMergeFilterFiles(); + SyncPoint::GetInstance()->DisableProcessing(); + SyncPoint::GetInstance()->ClearAllCallBacks(); + fault_env.SetFilesystemActive(true); + ASSERT_TRUE(s.IsIOError()) << s.ToString(); + ASSERT_TRUE(trace.Events().empty()); + ASSERT_EQ(history == 2, filter.resolve_failed.load()); + std::vector after; + db_->GetLiveFilesMetaData(&after); + ASSERT_EQ(before.size(), after.size()); + for (size_t i = 0; i < before.size(); ++i) { + ASSERT_EQ(before[i].file_number, after[i].file_number); + } + ASSERT_EQ(history == 2 ? "base" : "baseoperand", Get("a")); + Close(); + Reopen(options); + ASSERT_EQ(history == 2 ? "base" : "baseoperand", Get("a")); + } +} + +TEST_F(DBTestCompactionFilter, MergeBeforeFilterEarlyFactoryValidation) { + for (TableFileCreationReason reason : + {TableFileCreationReason::kFlush, TableFileCreationReason::kRecovery, + TableFileCreationReason::kMisc, + TableFileCreationReason::kSstFileWriter}) { + test::MergeFilterTrace trace; + auto factory = std::make_shared( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + factory->filter_other_reason = true; + factory->other_reason = reason; + test::RecordingMergeFilter direct( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + Options invalid = CurrentOptions(); + invalid.compaction_filter_factory = factory; + invalid.compaction_filter = &direct; + ASSERT_TRUE(ValidateCompactionFilterOptions(invalid).IsInvalidArgument()); + ColumnFamilyHandle* handle = nullptr; + ASSERT_TRUE(db_->CreateColumnFamily(invalid, "rejected", &handle) + .IsInvalidArgument()); + ASSERT_EQ(nullptr, handle); + ASSERT_EQ(0, factory->creations.load()); + std::vector handles; + Status batch = db_->CreateColumnFamilies( + {ColumnFamilyDescriptor("accepted", CurrentOptions()), + ColumnFamilyDescriptor("rejected", invalid)}, + &handles); + ASSERT_TRUE(batch.IsInvalidArgument()); + ASSERT_EQ(1, handles.size()); + ASSERT_EQ("accepted", handles[0]->GetName()); + ASSERT_OK(db_->DropColumnFamily(handles[0])); + ASSERT_OK(db_->DestroyColumnFamilyHandle(handles[0])); + Close(); + ASSERT_TRUE(TryReopen(invalid).IsInvalidArgument()); + ASSERT_EQ(0, factory->creations.load()); + ASSERT_TRUE(trace.Events().empty()); + Reopen(CurrentOptions()); + } +} + +TEST_F(DBTestCompactionFilter, MergeBeforeFilterUnsupportedConfigsFailEarly) { + test::MergeFilterTrace trace; + test::RecordingMergeFilter post( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + test::RecordingMergeFilter legacy( + CompactionFilter::MergeOrder::kFilterBeforeMerge, &trace); + // User-defined timestamps. + Options ts_options = CurrentOptions(); + ts_options.comparator = test::BytewiseComparatorWithU64TsWrapper(); + ts_options.compaction_filter = &post; + ASSERT_TRUE(ValidateCompactionFilterOptions(ts_options).IsNotSupported()); + ColumnFamilyHandle* handle = nullptr; + ASSERT_TRUE( + db_->CreateColumnFamily(ts_options, "ts", &handle).IsNotSupported()); + ASSERT_EQ(nullptr, handle); + ts_options.compaction_filter = &legacy; + ASSERT_OK(ValidateCompactionFilterOptions(ts_options)); + // WritePrepared and WriteUnprepared transactions use a SnapshotChecker. + const std::string path = test::PerThreadDBPath(env_, "mbf_txn"); + for (TxnDBWritePolicy policy : + {TxnDBWritePolicy::WRITE_COMMITTED, TxnDBWritePolicy::WRITE_PREPARED, + TxnDBWritePolicy::WRITE_UNPREPARED}) { + SCOPED_TRACE(static_cast(policy)); + Options options = CurrentOptions(); + options.create_if_missing = true; + options.compaction_filter = &post; + TransactionDBOptions txn_options; + txn_options.write_policy = policy; + ASSERT_OK(DestroyDB(path, options)); + TransactionDB* txn_db = nullptr; + Status s = TransactionDB::Open(options, txn_options, path, &txn_db); + if (policy == TxnDBWritePolicy::WRITE_COMMITTED) { + ASSERT_OK(s); + delete txn_db; + continue; + } + ASSERT_TRUE(s.IsNotSupported()) << s.ToString(); + ASSERT_EQ(nullptr, txn_db); + // Creating a merge-before-filter CF in such a DB fails too. + options.compaction_filter = &legacy; + ASSERT_OK(TransactionDB::Open(options, txn_options, path, &txn_db)); + Options cf_options = options; + cf_options.compaction_filter = &post; + ASSERT_TRUE(txn_db->CreateColumnFamily(cf_options, "post", &handle) + .IsNotSupported()); + ASSERT_EQ(nullptr, handle); + ASSERT_OK(txn_db->Put(WriteOptions(), "a", "value")); + delete txn_db; + } + ASSERT_OK(DestroyDB(path, CurrentOptions())); +} + +TEST_F(DBTestCompactionFilter, + MergeBeforeFilterSkipOnOlderOperandMatchesLegacy) { + for (CompactionFilter::MergeOrder order : + {CompactionFilter::MergeOrder::kFilterBeforeMerge, + CompactionFilter::MergeOrder::kMergeBeforeFilter}) { + SCOPED_TRACE(static_cast(order)); + test::MergeFilterTrace trace; + test::RecordingMergeFilter filter(order, &trace); + filter.action_value = "M1"; + filter.operand_decision = CompactionFilter::Decision::kRemoveAndSkipUntil; + filter.skip_target = "b"; + auto merge = std::make_shared(&trace); + merge->partial_success = false; + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.compaction_filter = &filter; + options.merge_operator = merge; + DestroyAndReopen(options); + ASSERT_OK(Put("a", "base")); + ASSERT_OK(Flush()); + MoveFilesToLevel(options.num_levels - 1); + ASSERT_OK(Merge("a", "M1")); + ASSERT_OK(Flush()); + ASSERT_OK(Merge("a", "M2")); + ASSERT_OK(Flush()); + std::vector files; + db_->GetLiveFilesMetaData(&files); + std::vector inputs; + for (const auto& file : files) { + if (file.level == 0) { + inputs.push_back(file.relative_filename); + } + } + ASSERT_EQ(2, inputs.size()); + ASSERT_OK(db_->CompactFiles(CompactionOptions(), inputs, 1)); + // Both orders drop every operand of the batch, exposing the older base. + ASSERT_EQ("base", Get("a")); + Close(); + } +} + +TEST_F(DBTestCompactionFilter, MergeBeforeFilterRuntimeDeclarationAndErrors) { + for (bool paranoid : {false, true}) { + test::MergeFilterTrace trace; + auto factory = std::make_shared( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace); + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.paranoid_checks = paranoid; + options.merge_operator = + std::make_shared(&trace); + options.compaction_filter_factory = factory; + DestroyAndReopen(options); + ASSERT_OK(Merge("a", "A")); + ASSERT_OK(Flush()); + trace.Clear(); + factory->instance_order = CompactionFilter::MergeOrder::kFilterBeforeMerge; + CompactRangeOptions compact; + compact.bottommost_level_compaction = BottommostLevelCompaction::kForce; + compact.change_level = true; + compact.target_level = options.num_levels - 1; + ASSERT_TRUE( + db_->CompactRange(compact, nullptr, nullptr).IsInvalidArgument()); + ASSERT_EQ(1, factory->creations.load()); + ASSERT_TRUE(trace.Events().empty()); + ASSERT_EQ("A", Get("a")); + Close(); + factory->instance_order = CompactionFilter::MergeOrder::kMergeBeforeFilter; + Reopen(options); + ASSERT_OK(db_->CompactRange(compact, nullptr, nullptr)); + factory->operand_decision = CompactionFilter::Decision::kPurge; + // Keep the base in a lower level outside this compaction's inputs. + ASSERT_OK(Merge("a", "B")); + ASSERT_OK(Flush()); + std::vector files; + db_->GetLiveFilesMetaData(&files); + ASSERT_FALSE(files.empty()); + const LiveFileMetaData* newest = &files.front(); + for (const auto& file : files) { + if (file.file_number > newest->file_number) { + newest = &file; + } + } + Status invalid = + db_->CompactFiles(CompactionOptions(), {newest->relative_filename}, 1); + ASSERT_TRUE(invalid.IsNotSupported()) << invalid.ToString(); + ASSERT_EQ("AB", Get("a")); + Close(); + } +} + } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) { diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index 32abb7fe8ec..ef2e5070ff6 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -5016,8 +5016,15 @@ Status DBImpl::CreateColumnFamiliesImpl( "Column family name cannot be the empty string"); break; } - preparation_status = - ColumnFamilyData::ValidateOptions(db_options, column_family.options); + preparation_status = ValidateCompactionFilterOptions(column_family.options); + if (preparation_status.ok() && use_custom_gc_ && + DeclaresMergeBeforeFilter(column_family.options)) { + preparation_status = MergeBeforeFilterTransactionError(); + } + if (preparation_status.ok()) { + preparation_status = + ColumnFamilyData::ValidateOptions(db_options, column_family.options); + } if (preparation_status.ok()) { for (const auto& cf_path : column_family.options.cf_paths) { preparation_status = env_->CreateDirIfMissing(cf_path.path); @@ -5092,7 +5099,13 @@ Status DBImpl::CreateColumnFamilyWithPreparedOptions( } } - Status s; + Status s = ValidateCompactionFilterOptions(sanitized_cf_options); + if (!s.ok()) { + return s; + } + if (use_custom_gc_ && DeclaresMergeBeforeFilter(sanitized_cf_options)) { + return MergeBeforeFilterTransactionError(); + } SuperVersionContext sv_context(/* create_superversion */ true); { InstrumentedMutexLock l(&mutex_); diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc index 1eb3586e03f..55c039e138e 100644 --- a/db/db_impl/db_impl_open.cc +++ b/db/db_impl/db_impl_open.cc @@ -2697,7 +2697,17 @@ Status DBImpl::Open(const DBOptions& db_options, const std::string& dbname, const WriteOptions write_options(Env::IOActivity::kDBOpen); const ReadOptions read_options(Env::IOActivity::kDBOpen); - Status s = ValidateOptionsByTable(db_options, column_families); + Status s; + for (const ColumnFamilyDescriptor& cf : column_families) { + s = ValidateCompactionFilterOptions(cf.options); + if (!s.ok()) { + return s; + } + if (seq_per_batch && DeclaresMergeBeforeFilter(cf.options)) { + return MergeBeforeFilterTransactionError(); + } + } + s = ValidateOptionsByTable(db_options, column_families); if (!s.ok()) { return s; } diff --git a/db/flush_job.cc b/db/flush_job.cc index fd8425ff4c2..53e7aef24a7 100644 --- a/db/flush_job.cc +++ b/db/flush_job.cc @@ -35,6 +35,7 @@ #include "monitoring/perf_context_imp.h" #include "monitoring/statistics_impl.h" #include "monitoring/thread_status_util.h" +#include "options/options_helper.h" #include "port/port.h" #include "rocksdb/db.h" #include "rocksdb/env.h" @@ -493,11 +494,20 @@ Status FlushJob::MemPurge() { ctx.reason = TableFileCreationReason::kFlush; compaction_filter = ioptions.compaction_filter_factory->CreateCompactionFilter(ctx); - if (compaction_filter != nullptr && + CompactionFilter::MergeOrder order; + s = GetCompactionFilterMergeOrder( + compaction_filter.get(), ioptions.compaction_filter_factory.get(), + TableFileCreationReason::kFlush, &order); + if (s.ok() && compaction_filter != nullptr && !compaction_filter->IgnoreSnapshots()) { s = Status::NotSupported( "CompactionFilter::IgnoreSnapshots() = false is not supported " "anymore."); + } + if (!s.ok()) { + // Destroy the application filter before restoring the caller's lock. + compaction_filter.reset(); + db_mutex_->Lock(); return s; } } @@ -515,6 +525,7 @@ Status FlushJob::MemPurge() { assert(env); MergeHelper merge(env, (cfd_->internal_comparator()).user_comparator(), (ioptions.merge_operator).get(), compaction_filter.get(), + CompactionFilter::MergeOrder::kFilterBeforeMerge, ioptions.logger, true /* internal key corruption is not ok */, job_context_->GetLatestSnapshotSequence(), diff --git a/db/flush_job_test.cc b/db/flush_job_test.cc index 0384807b1b7..f6a3074d360 100644 --- a/db/flush_job_test.cc +++ b/db/flush_job_test.cc @@ -185,6 +185,103 @@ class FlushJobTest : public FlushJobTestBase { BytewiseComparator()) {} }; +class FlushJobMergeFilterGuardTest : public FlushJobTest { + public: + class Filter : public CompactionFilter { + public: + Filter(InstrumentedMutex* mutex, std::atomic* destructions) + : mutex_(mutex), destructions_(destructions) {} + ~Filter() override { + const bool unlocked = mutex_->TryLock(); + EXPECT_TRUE(unlocked); + if (unlocked) { + mutex_->Unlock(); + } + destructions_->fetch_add(1); + } + const char* Name() const override { return "FlushJobMergeFilterGuard"; } + MergeOrder GetMergeOrder() const override { + const bool unlocked = mutex_->TryLock(); + EXPECT_TRUE(unlocked); + if (unlocked) { + mutex_->Unlock(); + } + return MergeOrder::kMergeBeforeFilter; + } + + private: + InstrumentedMutex* const mutex_; + std::atomic* const destructions_; + }; + + class Factory : public CompactionFilterFactory { + public: + Factory(InstrumentedMutex* mutex, std::atomic* destructions) + : mutex_(mutex), destructions_(destructions) {} + const char* Name() const override { + return "FlushJobMergeFilterGuardFactory"; + } + bool ShouldFilterTableFileCreation( + TableFileCreationReason reason) const override { + return reason == TableFileCreationReason::kFlush; + } + std::unique_ptr CreateCompactionFilter( + const CompactionFilter::Context& /*context*/) override { + creations.fetch_add(1); + return std::make_unique(mutex_, destructions_); + } + std::atomic creations{0}; + + private: + InstrumentedMutex* const mutex_; + std::atomic* const destructions_; + }; + + void SetUp() override { + factory_ = std::make_shared(&mutex_, &destructions_); + cf_options_.compaction_filter_factory = factory_; + cf_options_.experimental_mempurge_threshold = 2.0; + FlushJobTest::SetUp(); + } + + std::shared_ptr factory_; + std::atomic destructions_{0}; +}; + +TEST_F(FlushJobMergeFilterGuardTest, MergeBeforeFilterMemPurgeReacquiresMutex) { + JobContext context(0); + ColumnFamilyData* cfd = versions_->GetColumnFamilySet()->GetDefault(); + MemTable* mem = cfd->ConstructNewMemtable(cfd->GetLatestMutableCFOptions(), + kMaxSequenceNumber); + mem->Ref(); + ASSERT_OK(mem->Add(1, kTypeValue, "a", "value", nullptr)); + mem->ConstructFragmentedRangeTombstones(); + autovector to_delete; + cfd->imm()->Add(mem, &to_delete); + for (ReadOnlyMemTable* old : to_delete) { + delete old; + } + EventLogger logger(db_options_.info_log.get()); + context.InitSnapshotContext(nullptr, nullptr, kMaxSequenceNumber, {}); + FlushJob job(dbname_, cfd, db_options_, cfd->GetLatestMutableCFOptions(), + kMaxSequenceNumber, env_options_, versions_.get(), &mutex_, + &shutting_down_, &context, FlushReason::kWriteBufferFull, + nullptr, nullptr, nullptr, kNoCompression, + db_options_.statistics.get(), &logger, true, true, true, + Env::Priority::USER, nullptr, empty_seqno_to_time_mapping_); + { + InstrumentedMutexLock lock(&mutex_); + job.PickMemTable(); + ASSERT_TRUE(job.Run().IsInvalidArgument()); + mutex_.AssertHeld(); + } + // MemPurge rejects first, then the existing SST fallback also rejects. + ASSERT_EQ(2, factory_->creations.load()); + ASSERT_EQ(2, destructions_.load()); + ASSERT_EQ(0, cfd->current()->storage_info()->NumLevelFiles(0)); + context.Clean(); +} + TEST_F(FlushJobTest, Empty) { JobContext job_context(0); auto cfd = versions_->GetColumnFamilySet()->GetDefault(); diff --git a/db/merge_helper.cc b/db/merge_helper.cc index 7affb365c9f..fd19d6925fa 100644 --- a/db/merge_helper.cc +++ b/db/merge_helper.cc @@ -31,6 +31,7 @@ namespace ROCKSDB_NAMESPACE { MergeHelper::MergeHelper(Env* env, const Comparator* user_comparator, const MergeOperator* user_merge_operator, const CompactionFilter* compaction_filter, + CompactionFilter::MergeOrder merge_order, Logger* logger, bool assert_valid_internal_key, SequenceNumber latest_snapshot, const SnapshotChecker* snapshot_checker, int level, @@ -41,6 +42,7 @@ MergeHelper::MergeHelper(Env* env, const Comparator* user_comparator, user_comparator_(user_comparator), user_merge_operator_(user_merge_operator), compaction_filter_(compaction_filter), + merge_order_(merge_order), shutting_down_(shutting_down), logger_(logger), assert_valid_internal_key_(assert_valid_internal_key), @@ -292,6 +294,8 @@ Status MergeHelper::MergeUntil(InternalIterator* iter, keys_.clear(); merge_context_.Clear(); has_compaction_filter_skip_until_ = false; + outcome_ = Outcome::kUncombined; + aggregate_eligible_ = true; assert(user_merge_operator_); assert(user_comparator_); const size_t ts_sz = user_comparator_->timestamp_size(); @@ -488,6 +492,7 @@ Status MergeHelper::MergeUntil(InternalIterator* iter, // We store the result in keys_.back() and operands_.back() // if nothing went wrong (i.e.: no operand corruption on disk) if (s.ok()) { + outcome_ = Outcome::kFull; // The original key encountered original_key = std::move(keys_.back()); @@ -528,7 +533,8 @@ Status MergeHelper::MergeUntil(InternalIterator* iter, // it out, no matter what compaction filter says // 2) it's not filtered by a compaction filter CompactionFilter::Decision filter = - ikey.sequence <= latest_snapshot_ + merge_order_ == CompactionFilter::MergeOrder::kMergeBeforeFilter || + ikey.sequence <= latest_snapshot_ ? CompactionFilter::Decision::kKeep : FilterMerge(orig_ikey.user_key, value_slice); // FIXME: should also check for kRemove here @@ -540,6 +546,7 @@ Status MergeHelper::MergeUntil(InternalIterator* iter, } if (filter == CompactionFilter::Decision::kKeep || filter == CompactionFilter::Decision::kChangeValue) { + aggregate_eligible_ &= OperandEligible(ikey.sequence); if (original_key_is_iter) { // this is just an optimization that saves us one memcpy keys_.emplace_front(original_key); @@ -630,6 +637,7 @@ Status MergeHelper::MergeUntil(InternalIterator* iter, &merge_result, /* result_operand */ nullptr, &merge_result_type); if (s.ok()) { + outcome_ = Outcome::kFull; // The original key encountered // We are certain that keys_ is not empty here (see assertions couple of // lines before). @@ -677,6 +685,7 @@ Status MergeHelper::MergeUntil(InternalIterator* iter, s = Status::Corruption("PartialMerge result exceeds 4GB limit"); return s; } + outcome_ = Outcome::kPartial; merge_context_.Clear(); merge_context_.PushOperand(merge_result); keys_.erase(keys_.begin(), keys_.end() - 1); diff --git a/db/merge_helper.h b/db/merge_helper.h index 621facb9b95..9c9588faf23 100644 --- a/db/merge_helper.h +++ b/db/merge_helper.h @@ -36,7 +36,8 @@ class MergeHelper { public: MergeHelper(Env* env, const Comparator* user_comparator, const MergeOperator* user_merge_operator, - const CompactionFilter* compaction_filter, Logger* logger, + const CompactionFilter* compaction_filter, + CompactionFilter::MergeOrder merge_order, Logger* logger, bool assert_valid_internal_key, SequenceNumber latest_snapshot, const SnapshotChecker* snapshot_checker = nullptr, int level = 0, Statistics* stats = nullptr, @@ -208,6 +209,14 @@ class MergeHelper { // So keys().back() was the first key seen by iterator. // TODO: Re-style this comment to be like the first one const std::deque& keys() const { return keys_; } + + enum class Outcome { kUncombined, kFull, kPartial }; + Outcome outcome() const { return outcome_; } + bool aggregate_eligible() const { return aggregate_eligible_; } + CompactionFilter::MergeOrder merge_order() const { return merge_order_; } + bool OperandEligible(SequenceNumber seq) const { + return seq > latest_snapshot_; + } const std::vector& values() const { return merge_context_.GetOperands(); } @@ -234,6 +243,9 @@ class MergeHelper { const Comparator* user_comparator_; const MergeOperator* user_merge_operator_; const CompactionFilter* compaction_filter_; + const CompactionFilter::MergeOrder merge_order_; + Outcome outcome_ = Outcome::kUncombined; + bool aggregate_eligible_ = true; const std::atomic* shutting_down_; Logger* logger_; bool assert_valid_internal_key_; // enforce no internal key corruption? @@ -304,6 +316,10 @@ class MergeOutputIterator { void SeekToFirst(); // Advances to the next record in the output. void Next(); + void Invalidate() { + it_keys_ = merge_helper_->keys().rend(); + it_values_ = merge_helper_->values().rend(); + } Slice key() { return Slice(*it_keys_); } Slice value() { return Slice(*it_values_); } diff --git a/db/merge_helper_test.cc b/db/merge_helper_test.cc index e4fd40ba5af..60203467293 100644 --- a/db/merge_helper_test.cc +++ b/db/merge_helper_test.cc @@ -11,6 +11,7 @@ #include "db/dbformat.h" #include "rocksdb/comparator.h" +#include "test_util/merge_filter_test_util.h" #include "test_util/testharness.h" #include "test_util/testutil.h" #include "util/coding.h" @@ -29,9 +30,13 @@ class MergeHelperTest : public testing::Test { SequenceNumber latest_snapshot = 0) { iter_.reset(new VectorIterator(ks_, vs_, &icmp_)); iter_->SeekToFirst(); - merge_helper_.reset(new MergeHelper(env_, icmp_.user_comparator(), - merge_op_.get(), filter_.get(), nullptr, - false, latest_snapshot)); + CompactionFilter* filter = + order_filter_ ? order_filter_.get() : filter_.get(); + merge_helper_.reset(new MergeHelper( + env_, icmp_.user_comparator(), merge_op_.get(), filter, + filter ? filter->GetMergeOrder() + : CompactionFilter::MergeOrder::kFilterBeforeMerge, + nullptr, false, latest_snapshot)); return merge_helper_->MergeUntil( iter_.get(), nullptr /* range_del_agg */, stop_before, at_bottom, false /* allow_data_in_errors */, nullptr /* blob_fetcher */, @@ -58,8 +63,56 @@ class MergeHelperTest : public testing::Test { std::vector ks_; std::vector vs_; std::unique_ptr filter_; + std::unique_ptr order_filter_; }; +TEST_F(MergeHelperTest, MergeBeforeFilterOutcomeAndReset) { + test::MergeFilterTrace trace; + merge_op_ = std::make_shared(&trace); + order_filter_.reset(new test::RecordingMergeFilter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace)); + AddKeyVal("a", 6, kTypeMerge, "C"); + AddKeyVal("a", 5, kTypeMerge, "B"); + AddKeyVal("a", 4, kTypeValue, "A"); + AddKeyVal("b", 3, kTypeMerge, "B"); + AddKeyVal("b", 0, kTypeMerge, "A"); + ASSERT_OK(Run(0, false)); + ASSERT_EQ(MergeHelper::Outcome::kFull, merge_helper_->outcome()); + ASSERT_TRUE(merge_helper_->aggregate_eligible()); + ASSERT_EQ((std::vector{"full:a:ABC"}), trace.Events()); + ASSERT_TRUE(merge_helper_ + ->MergeUntil(iter_.get(), nullptr, 0, false, false, nullptr, + nullptr, nullptr, nullptr) + .IsMergeInProgress()); + ASSERT_EQ(MergeHelper::Outcome::kPartial, merge_helper_->outcome()); + ASSERT_FALSE(merge_helper_->aggregate_eligible()); + ASSERT_EQ("AB", merge_helper_->values()[0]); + ASSERT_EQ((std::vector{"full:a:ABC", "partial:b:AB"}), + trace.Events()); +} + +TEST_F(MergeHelperTest, MergeBeforeFilterBottomMergeAndFailure) { + for (bool fail_full : {false, true}) { + test::MergeFilterTrace trace; + auto merge = std::make_shared(&trace); + merge->fail_full = fail_full; + merge->failure_scope = MergeOperator::OpFailureScope::kMustMerge; + merge_op_ = merge; + order_filter_.reset(new test::RecordingMergeFilter( + CompactionFilter::MergeOrder::kMergeBeforeFilter, &trace)); + ks_.clear(); + vs_.clear(); + AddKeyVal("a", 0, kTypeMerge, "A"); + Status s = Run(0, true); + ASSERT_TRUE(fail_full ? s.IsMergeInProgress() : s.ok()); + ASSERT_EQ(fail_full ? MergeHelper::Outcome::kUncombined + : MergeHelper::Outcome::kFull, + merge_helper_->outcome()); + ASSERT_FALSE(merge_helper_->aggregate_eligible()); + ASSERT_EQ((std::vector{"full:a:A"}), trace.Events()); + } +} + // If MergeHelper encounters a new key on the last level, we know that // the key has no more history and it can merge keys. TEST_F(MergeHelperTest, MergeAtBottomSuccess) { diff --git a/db/repair.cc b/db/repair.cc index 37c01bc2a7b..25af90fbf0e 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -503,6 +503,10 @@ class Repairer { BlobFileCreationReason::kRecovery, nullptr /* seqno_to_time_mapping */, nullptr /* event_logger */, 0 /* job_id */, nullptr /* table_properties */, write_hint); + // BuildTable() folds I/O failures into its Status, but it can return + // early (for example, on an invalid compaction filter declaration) + // before reading `io_s`. + io_s.PermitUncheckedError(); ROCKS_LOG_INFO(db_options_.info_log, "Log #%" PRIu64 ": %d ops saved to Table #%" PRIu64 " %s", log, counter, meta.fd.GetNumber(), diff --git a/db_stress_tool/db_stress_common.h b/db_stress_tool/db_stress_common.h index 325f68879f1..cc521db59be 100644 --- a/db_stress_tool/db_stress_common.h +++ b/db_stress_tool/db_stress_common.h @@ -82,6 +82,9 @@ using GFLAGS_NAMESPACE::SetUsageMessage; DECLARE_uint64(seed); DECLARE_bool(read_only); +DECLARE_bool(merge_before_compaction_filter); +DECLARE_uint32(merge_before_filter_change_one_in); +DECLARE_bool(merge_before_filter_remove_deleted); DECLARE_int64(max_key); DECLARE_double(hot_key_alpha); DECLARE_int32(max_key_len); diff --git a/db_stress_tool/db_stress_compaction_filter.h b/db_stress_tool/db_stress_compaction_filter.h index bcda3e72ba5..7ec9bce58bd 100644 --- a/db_stress_tool/db_stress_compaction_filter.h +++ b/db_stress_tool/db_stress_compaction_filter.h @@ -15,66 +15,216 @@ namespace ROCKSDB_NAMESPACE { +// Counts merge-before-filter decisions by value type, so a run's log shows +// which paths were exercised. +struct DbStressMergeFilterCounters { + enum Kind { kKeep, kChange, kRemove, kSkip, kNumKinds }; + static constexpr const char* kKindNames[kNumKinds] = {"keep", "change", + "remove", "skip"}; + // Index 0 is kValue (including full merge results), 1 is kMergeOperand + // (partial merge results and uncombined operands). + std::atomic counts[2][kNumKinds] = {}; + + void Record(CompactionFilter::ValueType value_type, Kind kind) { + const int type = + value_type == CompactionFilter::ValueType::kMergeOperand ? 1 : 0; + counts[type][kind].fetch_add(1, std::memory_order_relaxed); + } + + void Print() const { + for (int type = 0; type < 2; ++type) { + fprintf(stdout, "Merge-before-filter %s decisions:", + type == 0 ? "value" : "operand"); + for (int kind = 0; kind < kNumKinds; ++kind) { + fprintf(stdout, " %s=%" PRIu64, kKindNames[kind], + counts[type][kind].load(std::memory_order_relaxed)); + } + fprintf(stdout, "\n"); + } + } +}; + +inline DbStressMergeFilterCounters& GetDbStressMergeFilterCounters() { + static DbStressMergeFilterCounters counters; + return counters; +} + // DbStressCompactionFilter is safe to use with db_stress as it does not perform // any mutation. It only makes `kRemove` decisions for keys that are already // non-existent according to the `SharedState`. +// +// With merge-before-filter, every decision leaves the expected state valid: +// `kChangeValue` rewrites the same bytes, and `kRemove` / +// `kRemoveAndSkipUntil` only apply to deleted keys when +// merge_before_filter_remove_deleted is set (which requires no snapshots). +// Skips cover only the current key and only in full compactions, so no older +// version outside the compaction can reappear. The filter also checks that +// every value it sees is a well-formed stress value for its key. // TODO: add follow-up stress coverage for wide-column blob entities through // both FilterV3 compatibility and FilterV4 lazy-resolution paths, including // blob direct write configurations. The current stress filter only exercises // FilterV2 behavior. class DbStressCompactionFilter : public CompactionFilter { public: - DbStressCompactionFilter(SharedState* state, int cf_id) - : state_(state), cf_id_(cf_id) {} + DbStressCompactionFilter(SharedState* state, int cf_id, + bool is_full_compaction) + : state_(state), + cf_id_(cf_id), + is_full_compaction_(is_full_compaction), + merge_order_(FLAGS_merge_before_compaction_filter + ? MergeOrder::kMergeBeforeFilter + : MergeOrder::kFilterBeforeMerge) {} + + MergeOrder GetMergeOrder() const override { return merge_order_; } - Decision FilterV2(int /*level*/, const Slice& key, ValueType /*value_type*/, - const Slice& /*existing_value*/, std::string* /*new_value*/, - std::string* /*skip_until*/) const override { + Decision FilterV2(int /*level*/, const Slice& key, ValueType value_type, + const Slice& existing_value, std::string* new_value, + std::string* skip_until) const override { + if (merge_order_ == MergeOrder::kMergeBeforeFilter) { + return FilterMergeBeforeFilter(key, value_type, existing_value, new_value, + skip_until); + } if (state_ == nullptr) { return Decision::kKeep; } + uint64_t key_num = 0; + if (!ParseKey(key, &key_num)) { + return Decision::kKeep; + } + bool key_exists = false; + bool allow_overwrite = false; + if (!CheckKey(key_num, &key_exists, &allow_overwrite)) { + return Decision::kKeep; + } + if (!key_exists) { + return allow_overwrite ? Decision::kRemove : Decision::kPurge; + } + return Decision::kKeep; + } + + const char* Name() const override { return "DbStressCompactionFilter"; } + + private: + static bool ParseKey(const Slice& key, uint64_t* key_num) { if (key.empty() || ('0' <= key[0] && key[0] <= '9')) { // It is likely leftover from a test_batches_snapshots run. Below this // conditional, the test_batches_snapshots key format is not handled // properly. Just keep it to be safe. - return Decision::kKeep; - } - uint64_t key_num = 0; - { - Slice ukey_without_ts = key; - assert(ukey_without_ts.size() >= FLAGS_user_timestamp_size); - ukey_without_ts.remove_suffix(FLAGS_user_timestamp_size); - [[maybe_unused]] bool ok = - GetIntVal(ukey_without_ts.ToString(), &key_num); - assert(ok); + return false; } + Slice ukey_without_ts = key; + assert(ukey_without_ts.size() >= FLAGS_user_timestamp_size); + ukey_without_ts.remove_suffix(FLAGS_user_timestamp_size); + [[maybe_unused]] bool ok = GetIntVal(ukey_without_ts.ToString(), key_num); + assert(ok); + return true; + } + + // Returns false if the key lock is busy, in which case the caller keeps the + // record. + bool CheckKey(uint64_t key_num, bool* key_exists, + bool* allow_overwrite) const { port::Mutex* key_mutex = state_->GetMutexForKey(cf_id_, key_num); if (!key_mutex->TryLock()) { - return Decision::kKeep; + return false; } // Reaching here means we acquired the lock. key_mutex->AssertHeld(); - bool key_exists = state_->Exists(cf_id_, key_num); - const bool allow_overwrite = state_->AllowsOverwrite(key_num); - + *key_exists = state_->Exists(cf_id_, key_num); + *allow_overwrite = state_->AllowsOverwrite(key_num); key_mutex->Unlock(); + return true; + } - if (!key_exists) { - return allow_overwrite ? Decision::kRemove : Decision::kPurge; + // Values are GenerateValue() output: a 4-byte value base followed by bytes + // derived from it. Merge results and partial merge results keep this format + // because the stress merge operators return the newest operand. + static bool IsWellFormedValue(const Slice& value) { + if (value.empty()) { + // test_ingest_standalone_range_deletion ingests empty values that a + // range deletion in the same ingestion always covers. + return true; } - return Decision::kKeep; + if (value.size() < sizeof(uint32_t)) { + return false; + } + const uint32_t value_base = GetValueBase(value); + const size_t expected_size = + ((value_base % kRandomValueMaxFactor) + 1) * FLAGS_value_size_mult; + if (value.size() != expected_size) { + return false; + } + for (size_t i = sizeof(uint32_t); i < value.size(); ++i) { + if (value[i] != static_cast(value_base ^ i)) { + return false; + } + } + return true; } - const char* Name() const override { return "DbStressCompactionFilter"; } + Decision FilterMergeBeforeFilter(const Slice& key, ValueType value_type, + const Slice& existing_value, + std::string* new_value, + std::string* skip_until) const { + DbStressMergeFilterCounters& counters = GetDbStressMergeFilterCounters(); + uint64_t key_num = 0; + if (!ParseKey(key, &key_num)) { + counters.Record(value_type, DbStressMergeFilterCounters::kKeep); + return Decision::kKeep; + } + if (!IsWellFormedValue(existing_value)) { + fprintf(stderr, "Merge-before-filter got a malformed %s for key %s: %s\n", + value_type == ValueType::kMergeOperand ? "operand" : "value", + key.ToString(true).c_str(), + existing_value.ToString(true).c_str()); + std::abort(); + } + if (FLAGS_merge_before_filter_remove_deleted && state_ != nullptr) { + bool key_exists = false; + bool allow_overwrite = false; + if (CheckKey(key_num, &key_exists, &allow_overwrite) && !key_exists) { + if (is_full_compaction_) { + // Every version of this key is in the compaction, so dropping all of + // them leaves the key deleted. + *skip_until = key.ToString(); + skip_until->push_back('\0'); + counters.Record(value_type, DbStressMergeFilterCounters::kSkip); + return Decision::kRemoveAndSkipUntil; + } + counters.Record(value_type, DbStressMergeFilterCounters::kRemove); + return Decision::kRemove; + } + } + if (FLAGS_merge_before_filter_change_one_in > 0 && + !existing_value.empty() && + (GetValueBase(existing_value) + key_num) % + FLAGS_merge_before_filter_change_one_in == + 0) { + new_value->assign(existing_value.data(), existing_value.size()); + counters.Record(value_type, DbStressMergeFilterCounters::kChange); + return Decision::kChangeValue; + } + counters.Record(value_type, DbStressMergeFilterCounters::kKeep); + return Decision::kKeep; + } - private: SharedState* const state_; const int cf_id_; + const bool is_full_compaction_; + const MergeOrder merge_order_; }; class DbStressCompactionFilterFactory : public CompactionFilterFactory { public: - DbStressCompactionFilterFactory() : state_(nullptr) {} + DbStressCompactionFilterFactory() + : state_(nullptr), + merge_order_(FLAGS_merge_before_compaction_filter + ? CompactionFilter::MergeOrder::kMergeBeforeFilter + : CompactionFilter::MergeOrder::kFilterBeforeMerge) {} + + CompactionFilter::MergeOrder GetMergeOrder() const override { + return merge_order_; + } void SetSharedState(SharedState* state) { MutexLock state_mutex_guard(&state_mutex_); @@ -84,8 +234,15 @@ class DbStressCompactionFilterFactory : public CompactionFilterFactory { std::unique_ptr CreateCompactionFilter( const CompactionFilter::Context& context) override { MutexLock state_mutex_guard(&state_mutex_); - return std::unique_ptr( - new DbStressCompactionFilter(state_, context.column_family_id)); + if (merge_order_ == CompactionFilter::MergeOrder::kMergeBeforeFilter && + context.reason != TableFileCreationReason::kCompaction) { + fprintf(stderr, + "Merge-before-filter factory called for a non-compaction " + "table file creation\n"); + std::abort(); + } + return std::unique_ptr(new DbStressCompactionFilter( + state_, context.column_family_id, context.is_full_compaction)); } const char* Name() const override { @@ -95,6 +252,7 @@ class DbStressCompactionFilterFactory : public CompactionFilterFactory { private: port::Mutex state_mutex_; SharedState* state_; + const CompactionFilter::MergeOrder merge_order_; }; } // namespace ROCKSDB_NAMESPACE diff --git a/db_stress_tool/db_stress_compaction_service.h b/db_stress_tool/db_stress_compaction_service.h index fef085f4041..bace95830da 100644 --- a/db_stress_tool/db_stress_compaction_service.h +++ b/db_stress_tool/db_stress_compaction_service.h @@ -28,6 +28,7 @@ class DbStressCompactionService : public CompactionService { static const char* kClassName() { return "DbStressCompactionService"; } const char* Name() const override { return kClassName(); } + bool SupportsMergeBeforeFilter() const override { return true; } static constexpr uint64_t kWaitIntervalInMicros = 10 * 1000; // 10ms static constexpr const char* kTempOutputDirectoryPrefix = "tmp_output_"; @@ -66,6 +67,8 @@ class DbStressCompactionService : public CompactionService { CompactionServiceResult::Read(serialized, &result).ok()) { DestroyDir(Env::Default(), result.output_path).PermitUncheckedError(); } + // The primary handles the result status; this callback only owns cleanup. + result.status.PermitUncheckedError(); shared_->RemoveRemoteCompactionResult(scheduled_job_id); } diff --git a/db_stress_tool/db_stress_gflags.cc b/db_stress_tool/db_stress_gflags.cc index 6b8edd3e80e..e15be59be3f 100644 --- a/db_stress_tool/db_stress_gflags.cc +++ b/db_stress_tool/db_stress_gflags.cc @@ -1426,6 +1426,21 @@ DEFINE_bool(enable_compaction_filter, false, "If true, configures a compaction filter that returns a kRemove " "decision for deleted keys."); +DEFINE_bool(merge_before_compaction_filter, false, + "Make the compaction filter declare merge-before-filter order. " + "Requires enable_compaction_filter. Its decisions never change " + "what a reader sees."); + +DEFINE_uint32(merge_before_filter_change_one_in, 0, + "With merge_before_compaction_filter, return kChangeValue with " + "the same bytes for about 1 in N records. 0 disables it."); + +DEFINE_bool(merge_before_filter_remove_deleted, false, + "With merge_before_compaction_filter, remove records of keys that " + "are deleted in the expected state, and skip them in full " + "compactions. Requires the same no-snapshot settings as the " + "default-order compaction filter."); + DEFINE_bool(paranoid_file_checks, true, "After writing every SST file, reopen it and read all the keys " "and validate checksums"); diff --git a/db_stress_tool/db_stress_test_base.cc b/db_stress_tool/db_stress_test_base.cc index df4687884cf..6133ac5e263 100644 --- a/db_stress_tool/db_stress_test_base.cc +++ b/db_stress_tool/db_stress_test_base.cc @@ -2070,6 +2070,9 @@ std::string StressTest::DebugString(const Slice& value, } void StressTest::PrintStatistics() { + if (FLAGS_merge_before_compaction_filter) { + GetDbStressMergeFilterCounters().Print(); + } // Print statistics from the DB instance instead of global dbstats if (db_) { auto stats = db_->GetOptions().statistics; diff --git a/db_stress_tool/db_stress_tool.cc b/db_stress_tool/db_stress_tool.cc index 482be883d5a..38fae20bb56 100644 --- a/db_stress_tool/db_stress_tool.cc +++ b/db_stress_tool/db_stress_tool.cc @@ -690,7 +690,25 @@ int db_stress_tool(int argc, char** argv) { "true."); } } - if ((FLAGS_enable_compaction_filter || FLAGS_inplace_update_support) && + if (FLAGS_merge_before_compaction_filter && + (!FLAGS_enable_compaction_filter || FLAGS_use_txn || + FLAGS_user_timestamp_size != 0)) { + return ReturnValidationError( + "merge_before_compaction_filter requires enable_compaction_filter, " + "no transactions, and no user-defined timestamps"); + } + if ((FLAGS_merge_before_filter_change_one_in > 0 || + FLAGS_merge_before_filter_remove_deleted) && + !FLAGS_merge_before_compaction_filter) { + return ReturnValidationError( + "merge_before_filter_change_one_in and " + "merge_before_filter_remove_deleted require " + "merge_before_compaction_filter"); + } + if (((FLAGS_enable_compaction_filter && + (!FLAGS_merge_before_compaction_filter || + FLAGS_merge_before_filter_remove_deleted)) || + FLAGS_inplace_update_support) && (FLAGS_acquire_snapshot_one_in > 0 || FLAGS_compact_range_one_in > 0 || FLAGS_iterpercent > 0 || FLAGS_prefixpercent > 0 || FLAGS_test_batches_snapshots || FLAGS_test_cf_consistency || diff --git a/include/rocksdb/compaction_filter.h b/include/rocksdb/compaction_filter.h index e3b6fca185f..1bf151ec0b5 100644 --- a/include/rocksdb/compaction_filter.h +++ b/include/rocksdb/compaction_filter.h @@ -125,6 +125,56 @@ class WideColumnBlobResolver { // including data loss, unreported corruption, deadlocks, and more. class CompactionFilter : public Customizable { public: + // Determines how the compaction filter interacts with the merge operator + // during compaction. + enum class MergeOrder { + // (Default) Merge operands are offered to the filter individually before + // the merge operator combines them. Results of merging are not offered + // to the filter in the same compaction. + kFilterBeforeMerge, + // The merge operator runs first and the filter sees its results. + // Merging only combines a key's entries that are among the inputs of the + // current compaction (and not across snapshot boundaries); data in files + // outside the compaction is not visible to the merge or the filter. So: + // * If the inputs include the key's base (a Put or a tombstone), or the + // compaction is bottommost and no older entries for the key remain, + // the full merge result is offered as kValue or kWideColumnEntity. + // * Otherwise, the key's operands in the inputs are offered as + // kMergeOperand: combined into one by PartialMergeMulti when possible, + // or individually if partial merge declines or there is only one. + // The base and any older operands outside the compaction are not seen. + // Results changed by the filter are not merged again in the same + // compaction. Only applies to compaction (not flush or recovery). + // Filters that implement only FilterV2 or FilterV3 also work. + kMergeBeforeFilter, + }; + + // Returns the merge order this filter requires. Must return the same value + // for the lifetime of the filter. Can be called concurrently and without + // the DB mutex, so it must be cheap, must not block, and must not call + // into the DB. + // + // Notes on kMergeBeforeFilter: + // * A merge result is offered to the filter only if none of its operands + // is visible to any snapshot. Plain values are still offered regardless + // of snapshots, as with kFilterBeforeMerge. + // * kPurge on a value or entity is treated as kRemove (ordinary Delete), + // because SingleDelete is incompatible with Merge history. Do not use + // SingleDelete on affected keys, even after switching back to + // kFilterBeforeMerge. + // * kPurge and kChangeWideColumnEntity are not supported for kMergeOperand + // and fail the compaction. + // * How operands are grouped into partial results can differ between + // compactions, and SST files keep no record of earlier groupings, so + // filter decisions must be correct for any grouping. + // * Not supported with user-defined timestamps, WritePrepared or + // WriteUnprepared transactions (SnapshotChecker), or stacked BlobDB. + // DB open and column family creation reject these configurations. + // * Filter wrappers must forward this value from the wrapped filter. + virtual MergeOrder GetMergeOrder() const { + return MergeOrder::kFilterBeforeMerge; + } + // Value type of the key-value passed to the compaction filter's // FilterV2/V3/V4 methods. enum ValueType { @@ -263,8 +313,10 @@ class CompactionFilter : public Customizable { // The application can inspect the existing value of the key and make decision // based on it. // - // Key-Values that are results of merge operation during table file creation - // are not passed into this function. Currently, when you have a mix of Put()s + // With the default merge order, key-values that are results of merge + // operations during table file creation are not passed into this function. + // kMergeBeforeFilter offers eligible full results through FilterV4, which + // can delegate here. With the default order and a mix of Put()s // and Merge()s on a same key, we only guarantee to process the merge operands // through the `CompactionFilter`s. Put()s might be processed, or might not. // @@ -480,6 +532,22 @@ class CompactionFilter : public Customizable { // including data loss, unreported corruption, deadlocks, and more. class CompactionFilterFactory : public Customizable { public: + // Returns the merge order of filters created by this factory. Every + // non-null filter it creates must return the same value from + // CompactionFilter::GetMergeOrder(); otherwise the compaction fails with + // InvalidArgument. A null filter means no filtering for that compaction, + // which is allowed with either order. Same stability and concurrency + // requirements as CompactionFilter::GetMergeOrder(). + // + // With kMergeBeforeFilter, ShouldFilterTableFileCreation() must return + // false for every reason other than kCompaction (checked at DB open and + // column family creation). Flush, recovery, and other table file creation + // do not run this order, because they can see only part of a key's history + // and would offer the filter results that differ from compaction. + virtual CompactionFilter::MergeOrder GetMergeOrder() const { + return CompactionFilter::MergeOrder::kFilterBeforeMerge; + } + ~CompactionFilterFactory() override = default; static const char* Type() { return "CompactionFilterFactory"; } static Status CreateFromString( diff --git a/include/rocksdb/options.h b/include/rocksdb/options.h index 3fc057a645e..81accaaa958 100644 --- a/include/rocksdb/options.h +++ b/include/rocksdb/options.h @@ -498,6 +498,11 @@ struct CompactionServiceJobInfo { // updates. bool use_separate_remote_compaction_pool = false; + // True if the compaction filter uses + // CompactionFilter::MergeOrder::kMergeBeforeFilter, so the worker must + // support that order. + bool merge_before_filter = false; + CompactionServiceJobInfo() {} CompactionServiceJobInfo(std::string db_name_, std::string db_id_, std::string db_session_id_, uint32_t cf_id_, @@ -543,6 +548,15 @@ class CompactionService : public Customizable { // Returns the name of this compaction service. const char* Name() const override = 0; + // Returns true if RocksDB may send compactions whose filter uses + // CompactionFilter::MergeOrder::kMergeBeforeFilter to this service. A + // worker that cannot honor the order (for example, an older RocksDB + // version) causes a local fallback after the remote attempt, before any + // output is imported. With the default (false), such compactions run + // locally. Called without the DB mutex; must be cheap, must not block, and + // must not call into the DB. + virtual bool SupportsMergeBeforeFilter() const { return false; } + // Schedule compaction to be processed remotely. virtual CompactionServiceScheduleResponse Schedule( const CompactionServiceJobInfo& /*info*/, diff --git a/options/options_helper.cc b/options/options_helper.cc index 466705a0afa..7ab17285e96 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -30,6 +30,91 @@ #include "util/string_util.h" namespace ROCKSDB_NAMESPACE { +namespace { +// Merge-before-filter runs only in compaction; flush, recovery and other table +// creation paths do not run the merge-before-filter logic. No default case, so +// that -Wswitch flags a new reason, which must also be added to +// kAllTableFileCreationReasons. +bool SupportsMergeBeforeFilter(TableFileCreationReason reason) { + switch (reason) { + case TableFileCreationReason::kCompaction: + return true; + case TableFileCreationReason::kFlush: + case TableFileCreationReason::kRecovery: + case TableFileCreationReason::kMisc: + case TableFileCreationReason::kSstFileWriter: + return false; + } + return false; +} + +constexpr TableFileCreationReason kAllTableFileCreationReasons[] = { + TableFileCreationReason::kFlush, TableFileCreationReason::kCompaction, + TableFileCreationReason::kRecovery, TableFileCreationReason::kMisc, + TableFileCreationReason::kSstFileWriter}; +} // namespace + +Status GetCompactionFilterMergeOrder(const CompactionFilter* filter, + const CompactionFilterFactory* factory, + TableFileCreationReason reason, + CompactionFilter::MergeOrder* order) { + using Order = CompactionFilter::MergeOrder; + const Order filter_order = + filter ? filter->GetMergeOrder() : Order::kFilterBeforeMerge; + const Order factory_order = + factory ? factory->GetMergeOrder() : Order::kFilterBeforeMerge; + if (factory && filter && filter_order != factory_order) { + return Status::InvalidArgument( + "Compaction filter/factory merge order mismatch"); + } + *order = factory ? factory_order : filter_order; + if (*order == Order::kMergeBeforeFilter && + !SupportsMergeBeforeFilter(reason)) { + return Status::InvalidArgument( + "Merge-before-filter is supported only during compaction"); + } + return Status::OK(); +} + +bool DeclaresMergeBeforeFilter(const ColumnFamilyOptions& cf_opts) { + using Order = CompactionFilter::MergeOrder; + return (cf_opts.compaction_filter && + cf_opts.compaction_filter->GetMergeOrder() == + Order::kMergeBeforeFilter) || + (cf_opts.compaction_filter_factory && + cf_opts.compaction_filter_factory->GetMergeOrder() == + Order::kMergeBeforeFilter); +} + +Status MergeBeforeFilterTransactionError() { + return Status::NotSupported( + "Merge-before-filter does not support WritePrepared or WriteUnprepared " + "transactions"); +} + +Status ValidateCompactionFilterOptions(const ColumnFamilyOptions& cf_opts) { + if (!DeclaresMergeBeforeFilter(cf_opts)) { + return Status::OK(); + } + const CompactionFilterFactory* factory = + cf_opts.compaction_filter_factory.get(); + if (factory && factory->GetMergeOrder() == + CompactionFilter::MergeOrder::kMergeBeforeFilter) { + for (TableFileCreationReason reason : kAllTableFileCreationReasons) { + if (!SupportsMergeBeforeFilter(reason) && + factory->ShouldFilterTableFileCreation(reason)) { + return Status::InvalidArgument( + "Merge-before-filter factory selects non-compaction creation"); + } + } + } + if (cf_opts.comparator && cf_opts.comparator->timestamp_size() != 0) { + return Status::NotSupported( + "Merge-before-filter does not support user-defined timestamps"); + } + return Status::OK(); +} + ConfigOptions::ConfigOptions() : registry(ObjectRegistry::NewInstance()) { env = Env::Default(); } diff --git a/options/options_helper.h b/options/options_helper.h index 23cf30ec107..bc840648492 100644 --- a/options/options_helper.h +++ b/options/options_helper.h @@ -11,6 +11,7 @@ #include #include "rocksdb/advanced_options.h" +#include "rocksdb/compaction_filter.h" #include "rocksdb/options.h" #include "rocksdb/status.h" #include "rocksdb/table.h" @@ -49,6 +50,16 @@ inline bool HasTableFilter(const ReadOptions& read_options) { Status ValidateOptions(const DBOptions& db_opts, const ColumnFamilyOptions& cf_opts); +// Configuration queries: callers must not hold the DB mutex. +Status GetCompactionFilterMergeOrder(const CompactionFilter* filter, + const CompactionFilterFactory* factory, + TableFileCreationReason reason, + CompactionFilter::MergeOrder* order); +Status ValidateCompactionFilterOptions(const ColumnFamilyOptions& cf_opts); +// Whether the direct filter or the factory declares merge-before-filter. +bool DeclaresMergeBeforeFilter(const ColumnFamilyOptions& cf_opts); +Status MergeBeforeFilterTransactionError(); + DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options, const MutableDBOptions& mutable_db_options); // Overwrites `options` diff --git a/test_util/merge_filter_test_util.h b/test_util/merge_filter_test_util.h new file mode 100644 index 00000000000..b96f9226ddc --- /dev/null +++ b/test_util/merge_filter_test_util.h @@ -0,0 +1,200 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. +// This source code is licensed under both the GPLv2 (found in the +// COPYING file in the root directory) and Apache 2.0 License +// (found in the LICENSE.Apache file in the root directory). + +#pragma once + +#include +#include +#include + +#include "rocksdb/compaction_filter.h" +#include "rocksdb/merge_operator.h" +#include "util/mutexlock.h" + +namespace ROCKSDB_NAMESPACE { +namespace test { + +class MergeFilterTrace { + public: + void Add(const std::string& event) { + MutexLock lock(&mutex_); + events_.push_back(event); + } + + std::vector Events() const { + MutexLock lock(&mutex_); + return events_; + } + + void Clear() { + MutexLock lock(&mutex_); + events_.clear(); + } + + private: + mutable port::Mutex mutex_; + std::vector events_; +}; + +class RecordingMergeFilter : public CompactionFilter { + public: + RecordingMergeFilter(MergeOrder order, MergeFilterTrace* trace) + : order_(order), trace_(trace) {} + + MergeOrder GetMergeOrder() const override { return order_; } + const char* Name() const override { return "RecordingMergeFilter"; } + + Decision FilterV4( + int /*level*/, const Slice& key, ValueType type, const Slice* value, + const WideColumns* columns, std::string* new_value, + std::vector>* new_columns, + std::string* skip_until, + WideColumnBlobResolver* resolver) const override { + std::string payload = value ? value->ToString() : ""; + std::string kind = "value"; + if (type == ValueType::kMergeOperand) { + kind = "operand"; + } else if (type == ValueType::kWideColumnEntity) { + kind = resolver ? "lazy_entity" : "entity"; + for (const auto& column : *columns) { + if (column.name().empty()) { + payload = column.value().ToString(); + } + } + } + trace_->Add(kind + ":" + key.ToString() + ":" + payload); + if ((!action_key.empty() && key != action_key) || + (!action_value.empty() && payload != action_value)) { + return Decision::kKeep; + } + Decision decision = + type == ValueType::kMergeOperand ? operand_decision : value_decision; + if (decision == Decision::kChangeValue) { + *new_value = replacement; + } + if (decision == Decision::kChangeWideColumnEntity && new_columns) { + *new_columns = replacement_columns; + } + *skip_until = skip_target; + return decision; + } + + bool SupportsFilterV4() const override { return true; } + + Decision FilterBlobByKey(int /*level*/, const Slice& key, + std::string* /*new_value*/, + std::string* /*skip_until*/) const override { + blob_key_calls.fetch_add(1); + return action_key.empty() || key == action_key ? blob_decision + : Decision::kUndetermined; + } + + // Configure actions only between completed jobs; the order is immutable. + Decision value_decision = Decision::kKeep; + Decision operand_decision = Decision::kKeep; + Decision blob_decision = Decision::kUndetermined; + std::string action_key; + std::string action_value; + std::string replacement; + std::string skip_target; + std::vector> replacement_columns; + mutable std::atomic blob_key_calls{0}; + + private: + const MergeOrder order_; + MergeFilterTrace* const trace_; +}; + +class RecordingMergeOperator : public MergeOperator { + public: + explicit RecordingMergeOperator(MergeFilterTrace* trace) : trace_(trace) {} + const char* Name() const override { return "RecordingMergeOperator"; } + + bool FullMergeV2(const MergeOperationInput& input, + MergeOperationOutput* output) const override { + std::string result = + input.existing_value ? input.existing_value->ToString() : ""; + for (const Slice& operand : input.operand_list) { + result += operand.ToString(); + } + trace_->Add("full:" + input.key.ToString() + ":" + result); + if (fail_full) { + output->op_failure_scope = failure_scope; + return false; + } + output->new_value = result; + return true; + } + + bool PartialMergeMulti(const Slice& key, const std::deque& operands, + std::string* result, + Logger* /*logger*/) const override { + std::string combined; + for (const Slice& operand : operands) { + combined += operand.ToString(); + } + trace_->Add("partial:" + key.ToString() + ":" + combined); + if (!partial_success) { + return false; + } + *result = combined; + return true; + } + + bool AllowSingleOperand() const override { return allow_single; } + + bool partial_success = true; + bool allow_single = false; + bool fail_full = false; + OpFailureScope failure_scope = OpFailureScope::kTryMerge; + + private: + MergeFilterTrace* const trace_; +}; + +class RecordingMergeFilterFactory : public CompactionFilterFactory { + public: + RecordingMergeFilterFactory(CompactionFilter::MergeOrder order, + MergeFilterTrace* trace) + : instance_order(order), order_(order), trace_(trace) {} + + const char* Name() const override { return "RecordingMergeFilterFactory"; } + CompactionFilter::MergeOrder GetMergeOrder() const override { return order_; } + + bool ShouldFilterTableFileCreation( + TableFileCreationReason reason) const override { + return reason == TableFileCreationReason::kCompaction || + (filter_other_reason && reason == other_reason); + } + + std::unique_ptr CreateCompactionFilter( + const CompactionFilter::Context& /*context*/) override { + creations.fetch_add(1); + if (return_null) { + return nullptr; + } + auto filter = + std::make_unique(instance_order, trace_); + filter->operand_decision = operand_decision; + filter->value_decision = value_decision; + return filter; + } + + CompactionFilter::MergeOrder instance_order; + bool filter_other_reason = false; + TableFileCreationReason other_reason = TableFileCreationReason::kFlush; + bool return_null = false; + CompactionFilter::Decision operand_decision = + CompactionFilter::Decision::kKeep; + CompactionFilter::Decision value_decision = CompactionFilter::Decision::kKeep; + std::atomic creations{0}; + + private: + const CompactionFilter::MergeOrder order_; + MergeFilterTrace* const trace_; +}; + +} // namespace test +} // namespace ROCKSDB_NAMESPACE diff --git a/tools/db_bench_tool.cc b/tools/db_bench_tool.cc index b932c2b63a7..a97527e4fa1 100644 --- a/tools/db_bench_tool.cc +++ b/tools/db_bench_tool.cc @@ -964,6 +964,12 @@ DEFINE_bool(read_cache_direct_read, true, DEFINE_bool(use_keep_filter, false, "Whether to use a noop compaction filter"); +DEFINE_bool(merge_before_compaction_filter, false, + "Use merge-before-filter order for merge_filter_benchmark."); +DEFINE_string(merge_filter_benchmark, "", + "Compaction filter for merge benchmarks: v2_keep, v4_keep, or " + "remove_operands (removes about 90% of operand keys)."); + static bool ValidateCacheNumshardbits(const char* flagname, int32_t value) { if (value >= 20) { fprintf(stderr, "Invalid value for --%s: %d, must be < 20\n", flagname, @@ -4178,6 +4184,72 @@ class Benchmark { const char* Name() const override { return "KeepFilter"; } }; + class MergeBenchmarkFilter : public CompactionFilter { + public: + MergeBenchmarkFilter(MergeOrder order, bool remove_operands) + : order_(order), remove_operands_(remove_operands) {} + + MergeOrder GetMergeOrder() const override { return order_; } + const char* Name() const override { return "MergeBenchmarkFilterV2"; } + + Decision FilterV2(int /*level*/, const Slice& key, ValueType type, + const Slice& /*value*/, std::string* /*new_value*/, + std::string* /*skip_until*/) const override { + return remove_operands_ && type == ValueType::kMergeOperand && + XXH64(key.data(), key.size(), 301) % 10 != 0 + ? Decision::kRemove + : Decision::kKeep; + } + + private: + const MergeOrder order_; + const bool remove_operands_; + }; + + class MergeBenchmarkFilterV4 : public MergeBenchmarkFilter { + public: + explicit MergeBenchmarkFilterV4(MergeOrder order) + : MergeBenchmarkFilter(order, false) {} + + const char* Name() const override { return "MergeBenchmarkFilterV4"; } + bool SupportsFilterV4() const override { return true; } + + Decision FilterV4( + int /*level*/, const Slice& /*key*/, ValueType /*type*/, + const Slice* /*value*/, const WideColumns* /*columns*/, + std::string* /*new_value*/, + std::vector>* /*new_columns*/, + std::string* /*skip_until*/, + WideColumnBlobResolver* /*resolver*/) const override { + return Decision::kKeep; + } + }; + + class MergeBenchmarkFilterFactory : public CompactionFilterFactory { + public: + MergeBenchmarkFilterFactory(CompactionFilter::MergeOrder order, + const std::string& style) + : order_(order), style_(style) {} + + CompactionFilter::MergeOrder GetMergeOrder() const override { + return order_; + } + const char* Name() const override { return "MergeBenchmarkFilterFactory"; } + + std::unique_ptr CreateCompactionFilter( + const CompactionFilter::Context& /*context*/) override { + if (style_ == "v4_keep") { + return std::make_unique(order_); + } + return std::make_unique( + order_, style_ == "remove_operands"); + } + + private: + const CompactionFilter::MergeOrder order_; + const std::string style_; + }; + static std::shared_ptr GetCacheAllocator() { std::shared_ptr allocator; @@ -6509,6 +6581,34 @@ class Benchmark { } } + if (!FLAGS_merge_filter_benchmark.empty()) { + if (FLAGS_merge_filter_benchmark != "v2_keep" && + FLAGS_merge_filter_benchmark != "v4_keep" && + FLAGS_merge_filter_benchmark != "remove_operands") { + fprintf(stderr, "Invalid merge_filter_benchmark\n"); + db_bench_exit(1); + } + if (options.compaction_filter || + (options.compaction_filter_factory && + !options.compaction_filter_factory->IsInstanceOf( + "MergeBenchmarkFilterFactory"))) { + fprintf(stderr, + "merge_filter_benchmark requires no configured filter\n"); + db_bench_exit(1); + } + const CompactionFilter::MergeOrder order = + FLAGS_merge_before_compaction_filter + ? CompactionFilter::MergeOrder::kMergeBeforeFilter + : CompactionFilter::MergeOrder::kFilterBeforeMerge; + options.compaction_filter_factory = + std::make_shared( + order, FLAGS_merge_filter_benchmark); + fprintf(stdout, "Merge benchmark filter=%s order=%s\n", + FLAGS_merge_filter_benchmark.c_str(), + FLAGS_merge_before_compaction_filter ? "merge-before-filter" + : "legacy"); + } + if (FLAGS_num_multi_db <= 1) { OpenDb(options, hooks, FLAGS_db, &db_); } else { diff --git a/tools/db_crashtest.py b/tools/db_crashtest.py index b6eb7d21850..0816f470906 100644 --- a/tools/db_crashtest.py +++ b/tools/db_crashtest.py @@ -314,6 +314,9 @@ def apply_random_seed_per_iteration(): "num_dbs": 1, "enable_pipelined_write": lambda: random.randint(0, 1), "enable_compaction_filter": lambda: random.choice([0, 0, 0, 1]), + "merge_before_compaction_filter": lambda: random.randint(0, 1), + "merge_before_filter_change_one_in": lambda: random.choice([0, 1, 10, 100]), + "merge_before_filter_remove_deleted": lambda: random.randint(0, 1), "enable_compaction_on_deletion_trigger": lambda: random.choice([0, 0, 0, 1]), # `inplace_update_support` is incompatible with a wide range of features. # The current sanitization process is not sufficient to reliably handle @@ -1412,6 +1415,14 @@ def apply_liveness_remote_defaults(params, args): def finalize_and_sanitize(src_params): dest_params = {k: v() if callable(v) else v for (k, v) in src_params.items()} apply_cache_and_write_buffer_size_multiplier(dest_params) + # Merge-before-filter is only interesting with merges, so use them in about + # 3/4 of those runs. Later rules can still turn merges off. + if ( + dest_params.get("enable_compaction_filter", 0) == 1 + and dest_params.get("merge_before_compaction_filter", 0) == 1 + and dest_params.get("use_merge", 0) == 0 + ): + dest_params["use_merge"] = random.randint(0, 1) if is_release_mode(): dest_params["read_fault_one_in"] = 0 if dest_params.get("compression_max_dict_bytes") == 0: @@ -1884,10 +1895,30 @@ def finalize_and_sanitize(src_params): dest_params["index_block_search_type"] = 0 # TrieIndexFactory requires BytewiseComparator. dest_params["use_trie_index"] = 0 + # Select the new-order stress filter only with a compatible active filter; + # other profiles keep their existing filter policy and + # unsupported-configuration coverage. if ( - dest_params.get("enable_compaction_filter", 0) == 1 - or dest_params.get("inplace_update_support", 0) == 1 + dest_params.get("enable_compaction_filter", 0) != 1 + or dest_params.get("use_txn", 0) == 1 + or dest_params.get("use_optimistic_txn", 0) == 1 + or dest_params.get("test_multi_ops_txns", 0) == 1 + or dest_params.get("user_timestamp_size", 0) > 0 + or dest_params.get("use_blob_db", 0) == 1 ): + dest_params["merge_before_compaction_filter"] = 0 + if dest_params.get("merge_before_compaction_filter", 0) == 0: + dest_params["merge_before_filter_change_one_in"] = 0 + dest_params["merge_before_filter_remove_deleted"] = 0 + # Removing records (either order) can change what a snapshot reads. The + # new-order filter without removal never changes what a reader sees. + if ( + dest_params.get("enable_compaction_filter", 0) == 1 + and ( + dest_params.get("merge_before_compaction_filter", 0) == 0 + or dest_params.get("merge_before_filter_remove_deleted", 0) == 1 + ) + ) or dest_params.get("inplace_update_support", 0) == 1: # Compaction filter, inplace update support are incompatible with snapshots. Need to avoid taking # snapshots, as well as avoid operations that use snapshots for # verification. diff --git a/tools/db_crashtest_test.py b/tools/db_crashtest_test.py index 017e0965b35..781ce182b9f 100644 --- a/tools/db_crashtest_test.py +++ b/tools/db_crashtest_test.py @@ -130,6 +130,74 @@ def build_mode_args(self, test_type="liveness", **overrides): args.update(overrides) return SimpleNamespace(**args) + def test_merge_before_filter_profile_selection_and_snapshots(self): + db_crashtest = self.load_db_crashtest() + compatible = { + "enable_compaction_filter": 1, + "merge_before_compaction_filter": 1, + "merge_before_filter_remove_deleted": 0, + "test_batches_snapshots": 0, + "sync_fault_injection": 0, + "disable_wal": 0, + "manual_wal_flush_one_in": 0, + "inplace_update_support": 0, + "enable_blob_direct_write": 0, + "remote_compaction_worker_threads": 0, + "ingest_external_file_atomic_replace_one_in": 0, + "use_txn": 0, + "use_optimistic_txn": 0, + "test_multi_ops_txns": 0, + "user_timestamp_size": 0, + "persist_user_defined_timestamps": 1, + "use_blob_db": 0, + "acquire_snapshot_one_in": 100, + "compact_range_one_in": 100, + "prefix_size": -1, + "prefixpercent": 0, + "iterpercent": 10, + } + for overrides in ({}, {"merge_before_compaction_filter": 0}): + params = self.build_params(db_crashtest.default_params, compatible) + params.update(overrides) + finalized = db_crashtest.finalize_and_sanitize(params) + post = finalized["merge_before_compaction_filter"] + self.assertEqual( + compatible["acquire_snapshot_one_in"] if post else 0, + finalized["acquire_snapshot_one_in"], + ) + self.assertEqual(10 if post else 0, finalized["iterpercent"]) + command, _ = db_crashtest.gen_cmd(finalized, []) + self.assertIn(f"--merge_before_compaction_filter={post}", command) + if not post: + self.assertEqual(0, finalized["merge_before_filter_remove_deleted"]) + self.assertEqual(0, finalized["merge_before_filter_change_one_in"]) + # Removing records can change what a snapshot reads, so the remove/skip + # mode uses the same no-snapshot settings as the default-order filter. + params = self.build_params(db_crashtest.default_params, compatible) + params["merge_before_filter_remove_deleted"] = 1 + finalized = db_crashtest.finalize_and_sanitize(params) + self.assertEqual(1, finalized["merge_before_compaction_filter"]) + self.assertEqual(1, finalized["merge_before_filter_remove_deleted"]) + self.assertEqual(0, finalized["acquire_snapshot_one_in"]) + self.assertEqual(0, finalized["compact_range_one_in"]) + self.assertEqual(0, finalized["iterpercent"]) + for name in ( + "enable_compaction_filter", + "use_txn", + "use_optimistic_txn", + "test_multi_ops_txns", + "user_timestamp_size", + "use_blob_db", + "test_batches_snapshots", + "sync_fault_injection", + "disable_wal", + ): + with self.subTest(name=name): + params = self.build_params(db_crashtest.default_params, compatible) + params[name] = 0 if name == "enable_compaction_filter" else 1 + finalized = db_crashtest.finalize_and_sanitize(params) + self.assertEqual(0, finalized["merge_before_compaction_filter"]) + def test_stress_cmd_env_defaults_tsan_suppressions(self): os.environ.pop(_TSAN_OPTIONS_ENV_VAR, None) db_crashtest = self.load_db_crashtest() diff --git a/unreleased_history/bug_fixes/mempurge_filter_error_relock.md b/unreleased_history/bug_fixes/mempurge_filter_error_relock.md new file mode 100644 index 00000000000..8b335781511 --- /dev/null +++ b/unreleased_history/bug_fixes/mempurge_filter_error_relock.md @@ -0,0 +1 @@ +Fixed a bug where MemPurge returned with the DB mutex unlocked when the compaction filter created for flush did not ignore snapshots. diff --git a/unreleased_history/new_features/merge_before_compaction_filter.md b/unreleased_history/new_features/merge_before_compaction_filter.md new file mode 100644 index 00000000000..497aaba710a --- /dev/null +++ b/unreleased_history/new_features/merge_before_compaction_filter.md @@ -0,0 +1 @@ +Added `CompactionFilter::GetMergeOrder()` and `CompactionFilterFactory::GetMergeOrder()` to optionally run the merge operator before the compaction filter during compaction, so that the filter sees merge results. The default behavior is unchanged. diff --git a/utilities/blob_db/blob_compaction_filter.h b/utilities/blob_db/blob_compaction_filter.h index 1c55a53c246..e3a10aafc4c 100644 --- a/utilities/blob_db/blob_compaction_filter.h +++ b/utilities/blob_db/blob_compaction_filter.h @@ -44,6 +44,11 @@ class BlobIndexCompactionFilterBase : public LayeredCompactionFilterBase { ~BlobIndexCompactionFilterBase() override; + MergeOrder GetMergeOrder() const override { + return user_comp_filter() ? user_comp_filter()->GetMergeOrder() + : MergeOrder::kFilterBeforeMerge; + } + // Filter expired blob indexes regardless of snapshots. bool IgnoreSnapshots() const override { return true; } diff --git a/utilities/blob_db/blob_db_impl.cc b/utilities/blob_db/blob_db_impl.cc index 9bb30871fe8..f1848766bec 100644 --- a/utilities/blob_db/blob_db_impl.cc +++ b/utilities/blob_db/blob_db_impl.cc @@ -25,6 +25,7 @@ #include "monitoring/instrumented_mutex.h" #include "monitoring/statistics_impl.h" #include "monitoring/thread_status_util.h" +#include "options/options_helper.h" #include "rocksdb/convenience.h" #include "rocksdb/env.h" #include "rocksdb/iterator.h" @@ -135,6 +136,26 @@ Status BlobDBImpl::Open(std::vector* handles) { return Status::NotSupported("No blob directory in options"); } + // Validate the user declarations before the internal wrappers hide them. + Status s = ValidateCompactionFilterOptions(cf_options_); + if (!s.ok()) { + return s; + } + CompactionFilter::MergeOrder order; + s = GetCompactionFilterMergeOrder( + cf_options_.compaction_filter, + cf_options_.compaction_filter + ? nullptr + : cf_options_.compaction_filter_factory.get(), + TableFileCreationReason::kCompaction, &order); + if (!s.ok()) { + return s; + } + if (order == CompactionFilter::MergeOrder::kMergeBeforeFilter) { + return Status::NotSupported( + "Merge-before-filter is not supported by stacked BlobDB"); + } + // Temporarily disable compactions in the base DB during open; save the user // defined value beforehand so we can restore it once BlobDB is initialized. // Note: this is only needed if garbage collection is enabled. @@ -144,8 +165,6 @@ Status BlobDBImpl::Open(std::vector* handles) { cf_options_.disable_auto_compactions = true; } - Status s; - // Create info log. if (db_options_.info_log == nullptr) { s = CreateLoggerFromOptions(dbname_, db_options_, &db_options_.info_log); diff --git a/utilities/blob_db/blob_db_test.cc b/utilities/blob_db/blob_db_test.cc index 20f208ca322..ffd25f5255d 100644 --- a/utilities/blob_db/blob_db_test.cc +++ b/utilities/blob_db/blob_db_test.cc @@ -6,6 +6,7 @@ #include "utilities/blob_db/blob_db.h" #include +#include #include #include #include @@ -958,6 +959,156 @@ TEST_F(BlobDBTest, OutOfSpace) { ASSERT_TRUE(s.IsNoSpace()); } +namespace { +class BlobDBMergeOrderFilter : public CompactionFilter { + public: + explicit BlobDBMergeOrderFilter(MergeOrder order) : order_(order) {} + MergeOrder GetMergeOrder() const override { return order_; } + const char* Name() const override { return "BlobDBMergeOrderFilter"; } + Decision FilterV2(int /*level*/, const Slice& key, ValueType /*type*/, + const Slice& /*value*/, std::string* /*new_value*/, + std::string* /*skip_until*/) const override { + calls.fetch_add(1); + return key == "drop" ? Decision::kPurge : Decision::kKeep; + } + mutable std::atomic calls{0}; + + private: + const MergeOrder order_; +}; + +class BlobDBMergeOrderFactory : public CompactionFilterFactory { + public: + explicit BlobDBMergeOrderFactory(CompactionFilter::MergeOrder order) + : instance_order(order), order_(order) {} + CompactionFilter::MergeOrder GetMergeOrder() const override { return order_; } + const char* Name() const override { return "BlobDBMergeOrderFactory"; } + std::unique_ptr CreateCompactionFilter( + const CompactionFilter::Context& /*context*/) override { + creations.fetch_add(1); + return std::make_unique(instance_order); + } + CompactionFilter::MergeOrder instance_order; + std::atomic creations{0}; + + private: + const CompactionFilter::MergeOrder order_; +}; +} // namespace + +TEST_F(BlobDBTest, MergeBeforeFilterRealStackedOpen) { + using Order = CompactionFilter::MergeOrder; + const Order legacy = Order::kFilterBeforeMerge; + const Order post = Order::kMergeBeforeFilter; + struct Case { + Order direct_order; + Order factory_order; + bool direct; + bool factory; + bool not_supported; + }; + const Case cases[] = { + {legacy, legacy, false, false, false}, + {legacy, legacy, true, false, false}, + {legacy, legacy, false, true, false}, + {post, legacy, true, false, true}, + {legacy, post, false, true, true}, + {post, legacy, true, true, true}, + {legacy, post, true, true, false}, + }; + for (bool gc : {false, true}) { + for (const Case& c : cases) { + SCOPED_TRACE(gc); + SCOPED_TRACE(testing::PrintToString(c.direct_order)); + SCOPED_TRACE(testing::PrintToString(c.factory_order)); + BlobDBMergeOrderFilter direct(c.direct_order); + auto factory = std::make_shared(c.factory_order); + BlobDBOptions bdb_options; + bdb_options.disable_background_tasks = true; + bdb_options.enable_garbage_collection = gc; + Options options; + options.disable_auto_compactions = true; + options.compaction_filter = c.direct ? &direct : nullptr; + options.compaction_filter_factory = c.factory ? factory : nullptr; + Status s = TryOpen(bdb_options, options); + if (c.not_supported) { + ASSERT_TRUE(s.IsNotSupported()) << s.ToString(); + ASSERT_EQ(nullptr, blob_db_); + ASSERT_TRUE(Env::Default()->FileExists(dbname_).IsNotFound()); + ASSERT_EQ(0, direct.calls.load()); + ASSERT_EQ(0, factory->creations.load()); + continue; + } + ASSERT_OK(s); + ASSERT_OK(Put("a", "older")); + ASSERT_OK(Put("keep", "older")); + ASSERT_OK(blob_db_->Flush(FlushOptions())); + CompactRangeOptions move; + move.change_level = true; + move.target_level = 2; + ASSERT_OK(blob_db_->CompactRange(move, nullptr, nullptr)); + ASSERT_OK(Put("drop", "value")); + ASSERT_OK(Put("keep", "value")); + ASSERT_OK(blob_db_->Flush(FlushOptions())); + std::vector files; + blob_db_->GetRootDB()->GetLiveFilesMetaData(&files); + std::vector inputs; + for (const auto& file : files) { + if (file.level == 0) { + inputs.push_back(file.relative_filename); + } + } + ASSERT_FALSE(inputs.empty()); + ASSERT_OK( + blob_db_->GetRootDB()->CompactFiles(CompactionOptions(), inputs, 1)); + if (c.direct || c.factory) { + VerifyDB({{"a", "older"}, {"keep", "value"}}); + std::vector versions; + ASSERT_OK(GetAllKeyVersions(blob_db_->GetRootDB(), "drop", "droq", 10, + &versions)); + ASSERT_EQ(1, versions.size()); + ASSERT_EQ(kTypeSingleDeletion, versions.front().type); + } else { + VerifyDB({{"a", "older"}, {"drop", "value"}, {"keep", "value"}}); + } + if (c.direct) { + ASSERT_GT(direct.calls.load(), 0); + } + Destroy(); + } + } +} + +TEST_F(BlobDBTest, MergeBeforeFilterRealStackedRuntimeMismatch) { + for (bool gc : {false, true}) { + SCOPED_TRACE(gc); + auto factory = std::make_shared( + CompactionFilter::MergeOrder::kFilterBeforeMerge); + factory->instance_order = CompactionFilter::MergeOrder::kMergeBeforeFilter; + BlobDBOptions bdb_options; + bdb_options.disable_background_tasks = true; + bdb_options.enable_garbage_collection = gc; + Options options; + options.disable_auto_compactions = true; + options.compaction_filter_factory = factory; + Open(bdb_options, options); + ASSERT_OK(Put("drop", "value")); + ASSERT_OK(blob_db_->Flush(FlushOptions())); + std::vector before; + blob_db_->GetRootDB()->GetLiveFilesMetaData(&before); + ASSERT_TRUE(blob_db_->CompactRange(CompactRangeOptions(), nullptr, nullptr) + .IsInvalidArgument()); + std::vector after; + blob_db_->GetRootDB()->GetLiveFilesMetaData(&after); + ASSERT_EQ(before.size(), after.size()); + for (size_t i = 0; i < before.size(); ++i) { + ASSERT_EQ(before[i].file_number, after[i].file_number); + } + VerifyDB({{"drop", "value"}}); + Destroy(); + } +} + TEST_F(BlobDBTest, UserCompactionFilter) { class CustomerFilter : public CompactionFilter { public: