From d02459ea1ba65de9bd9412259c992d8c5417a5b8 Mon Sep 17 00:00:00 2001 From: Xingbo Wang Date: Sat, 31 Oct 2026 12:20:08 -0700 Subject: [PATCH] Support merge-before-filter compaction (#15517) Summary: Pull Request resolved: https://github.com/meta/rocksdb/pull/15517 ## Why Some useful compaction filters only make sense on merged results. For example, a filter that removes counters below a threshold must see the counter's total, not each `+N` operand. With the current order, the compaction filter runs before the merge operator, so it sees (and can remove) individual operands and never sees the merge result. Important limit: compaction can only merge what is in its own inputs. If a key's base value or older operands are in files outside the compaction, the filter sees a partial merge result (or single operands), not the logical value of the key. ## Behavior - **Default (unchanged):** `CompactionFilter::MergeOrder::kFilterBeforeMerge`. Operands are filtered one by one before merging. - **New opt-in:** a filter or factory returns `kMergeBeforeFilter` from the new `GetMergeOrder()`. During compaction the merge operator runs first, then the filter sees: - the full merge result as `kValue` / `kWideColumnEntity`, if the inputs include the base (Put or tombstone), or the compaction is bottommost with no older entries; - otherwise the operands as `kMergeOperand`: one partial result from `PartialMergeMulti`, or each operand by itself if partial merge declines. - A filter result is not merged again in the same compaction. Filters that only implement FilterV2/V3 also work. - A merge result is offered only if no snapshot can see any of its operands. Plain values are filtered as before, regardless of snapshots. - `kPurge` on a value is treated as `kRemove` (ordinary Delete), because SingleDelete does not mix with Merge history. `kPurge` and `kChangeWideColumnEntity` on an operand fail the compaction with NotSupported. - `kRemoveAndSkipUntil` on any operand of an uncombined batch drops the whole batch, the same result as the default order. - `COMPACTION_KEY_DROP_USER` keeps the same meaning in both orders (skipped ranges are not counted). - The new order applies only to compaction. Flush, recovery, MemPurge, and repair keep the default order. ### Example Counter merge operator (add) with a filter that removes counters below 10. The compaction input for one key is `Merge(+3)`, `Merge(+5)`, `Put(4)`. Before: the filter sees the raw operands `+3` and `+5`, removes both, and the output is `Put(4)`; it never sees the merged value. After: the merge runs first (4 + 5 + 3 = 12), the filter sees `kValue` "12", and keeps it. If the base is not in the compaction, partial merge gives `+8` and the filter sees it as `kMergeOperand`. ## Implementation - `MergeHelper` carries the merge order and records each merge outcome (full, partial, uncombined) and whether it can be filtered under snapshots. - `CompactionIterator` reads the order from `MergeHelper` and filters after the merge. A full result changed to Delete goes through the normal `ProcessDeletion()` path. Uncombined operand batches are filtered as a whole before any of them is output, so a skip decision can drop the full batch. - Order checks (`GetCompactionFilterMergeOrder()`) run outside the DB mutex. A factory's non-null filters must declare the same order; otherwise the compaction fails with InvalidArgument. A factory that returns a null filter means no filtering, which is valid with either order. - **Remote compaction:** `CompactionService::SupportsMergeBeforeFilter()` (default false) lets a service receive new-order jobs; otherwise they run locally. The request carries `merge_before_filter`, and the worker acknowledges the order it actually used. A missing or wrong acknowledgment (for example, from an older worker) falls back to local compaction before any output is imported. Real compaction errors stay errors. A worker with no filter accepts either order. - **Early rejection:** DB open and CF creation reject a new-order declaration with user-defined timestamps, WritePrepared/WriteUnprepared transactions, stacked BlobDB, or a factory that selects non-compaction table file creation. This avoids a background error that would stop writes at the first compaction. - `ShouldFilterTableFileCreation()` checks use an exhaustive switch over `TableFileCreationReason`, so a new reason must be classified. - **Bug fix included:** `MemPurge()` returned with the DB mutex unlocked when the flush filter failed a check (for example, `IgnoreSnapshots() == false`). The new order check adds another early return on the same path, so the re-lock is fixed here, with a `bug_fixes` release note. - **Stress test:** with `merge_before_compaction_filter`, the db_stress filter now makes real decisions that never change what a reader sees: `kChangeValue` with the same bytes for about 1 in N records (`merge_before_filter_change_one_in`); with `merge_before_filter_remove_deleted`, `kRemove` for keys deleted in the expected state, and `kRemoveAndSkipUntil` over just that key in full compactions (where every version of the key is an input). It also aborts if a value it sees is not a well-formed stress value for its key, or if the new-order factory is asked to filter a non-compaction file. It prints per-type decision counts at the end of a run. The crash test picks these modes randomly and uses merges in about 3/4 of new-order runs; the remove/skip mode uses the same no-snapshot settings as the default-order filter. - `Repairer` only marks `BuildTable()`'s `IOStatus` as checked, because `BuildTable()` can now return early before reading it. Repair behavior does not change. ## Safety and limitations - The default order and existing filters behave as before. There is no SST format change, no new CF option, and no C/Java binding. - Not supported in the new order: user-defined timestamps, WritePrepared/WriteUnprepared transactions (SnapshotChecker), and stacked BlobDB. Integrated BlobDB is supported. - Do not use SingleDelete on keys handled by a new-order filter, even after switching back to the default order. - How operands are grouped into partial results can differ between compactions, and SST files keep no record of earlier groupings. Filter decisions must be correct for any grouping. - **Stress test limits:** the stress merge operators keep only the newest operand, and every stress filter decision is chosen so that reads do not change. So both orders give the same visible result, and the oracle checks robustness of the new paths (wrong output key or sequence, lost or returning data, buffer and pinning errors, crash recovery, remote fallback), not whether the filter saw the right input. A same-bytes rewrite also cannot detect a change that RocksDB ignores. Skips only run in full compactions, which are rare in leveled runs. The decision counters print only when db_stress exits normally (not in blackbox kills). - **Unit tests cover the rest:** which input the filter sees (full, partial, uncombined), the snapshot rule, changed values in the output (`MergeBeforeFilterFullReplacementAndFailure`, `MergeBeforeFilterPartialAndDecline`, remote full/partial tests), skip and removal results, kPurge handling, unsupported decisions, early rejection, remote routing and fallback, and BlobDB. - Validation: the full suites of the touched test binaries pass, and local db_stress and crash-test runs with the new modes pass. See the Test Plan for details and the benchmark results. Differential Revision: D128313536 --- db/builder.cc | 10 +- db/compaction/compaction_iterator.cc | 422 +++++++++---- db/compaction/compaction_iterator.h | 19 +- db/compaction/compaction_iterator_test.cc | 577 +++++++++++++++++- db/compaction/compaction_job.cc | 63 +- db/compaction/compaction_job.h | 27 +- db/compaction/compaction_service_job.cc | 76 ++- db/compaction/compaction_service_test.cc | 532 ++++++++++++++++ db/db_compaction_filter_test.cc | 571 +++++++++++++++++ db/db_impl/db_impl.cc | 19 +- db/db_impl/db_impl_open.cc | 12 +- db/flush_job.cc | 13 +- db/flush_job_test.cc | 97 +++ db/merge_helper.cc | 11 +- db/merge_helper.h | 18 +- db/merge_helper_test.cc | 59 +- db/repair.cc | 4 + db_stress_tool/db_stress_common.h | 3 + db_stress_tool/db_stress_compaction_filter.h | 212 ++++++- db_stress_tool/db_stress_compaction_service.h | 3 + db_stress_tool/db_stress_gflags.cc | 15 + db_stress_tool/db_stress_test_base.cc | 3 + db_stress_tool/db_stress_tool.cc | 20 +- include/rocksdb/compaction_filter.h | 72 ++- include/rocksdb/options.h | 14 + options/options_helper.cc | 85 +++ options/options_helper.h | 11 + test_util/merge_filter_test_util.h | 200 ++++++ tools/db_bench_tool.cc | 100 +++ tools/db_crashtest.py | 35 +- tools/db_crashtest_test.py | 68 +++ .../bug_fixes/mempurge_filter_error_relock.md | 1 + .../merge_before_compaction_filter.md | 1 + utilities/blob_db/blob_compaction_filter.h | 5 + utilities/blob_db/blob_db_impl.cc | 23 +- utilities/blob_db/blob_db_test.cc | 151 +++++ 36 files changed, 3377 insertions(+), 175 deletions(-) create mode 100644 test_util/merge_filter_test_util.h create mode 100644 unreleased_history/bug_fixes/mempurge_filter_error_relock.md create mode 100644 unreleased_history/new_features/merge_before_compaction_filter.md diff --git a/db/builder.cc b/db/builder.cc index b7d3b6259b1c..e9336a68d6c6 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 7df7f46eded5..5d6d21b4c2c5 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 25ab3a8f6309..efe3841a923e 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 01f1aadf32db..578a94cbcf28 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 3483fe18dc40..89087381062f 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 92b1078d7ff7..e690723949cc 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 dbba0b965c2b..5c4a04d6d7e1 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 5a320cc0d0e1..bc1df581446c 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 f7060f814e9f..41db7e4ddcd1 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 32abb7fe8ecf..ef2e5070ff64 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 1eb3586e03f2..55c039e138e3 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 fd8425ff4c27..53e7aef24a78 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 0384807b1b71..f6a3074d3609 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 7affb365c9fd..fd19d6925fa8 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 621facb9b955..9c9588faf230 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 e4fd40ba5af4..602034672930 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 37c01bc2a7b7..25af90fbf0e2 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 325f68879f12..cc521db59bea 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 bcda3e72ba50..7ec9bce58bd1 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 fef085f4041c..bace95830da2 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 6b8edd3e80ea..e15be59be3f4 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 df4687884cfc..6133ac5e2630 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 482be883d5aa..38fae20bb56e 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 e3b6fca185ff..1bf151ec0b55 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 3fc057a645ec..81accaaa9586 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 466705a0afa4..7ab17285e962 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 23cf30ec1071..bc8406484920 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 000000000000..b96f9226ddc9 --- /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 b932c2b63a76..a97527e4fa19 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 b6eb7d218508..0816f470906f 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 017e0965b351..781ce182b9f7 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 000000000000..8b3357815113 --- /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 000000000000..497aaba710a6 --- /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 1c55a53c2460..e3a10aafc4c2 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 9bb30871fe89..f1848766bec3 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 20f208ca3224..ffd25f5255dc 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: