From bdecb0ccdefcc0ed0971e0874fc6b2d7ba22a761 Mon Sep 17 00:00:00 2001 From: "Raphael S. Carvalho" Date: Fri, 5 Dec 2025 13:46:52 -0300 Subject: [PATCH] compaction: Preserve state of input sstable in maybe_split_new_sstable() This is crucial with MVs, since the splitting must preserve the state of the original sstable. We want the sstable to be in staging dir, so it's excluded when calculating the diff for performing pushes to view replicas. Signed-off-by: Raphael S. Carvalho (cherry picked from commit 2dae0a73809270a3e4661729efb65c02b1470f56) --- compaction/compaction_group_view.hh | 3 ++- compaction/compaction_manager.cc | 11 ++++++++--- replica/table.cc | 4 ++-- test/boost/compaction_group_test.cc | 2 +- test/lib/test_services.cc | 2 +- tools/scylla-sstable.cc | 4 ++-- 6 files changed, 16 insertions(+), 10 deletions(-) diff --git a/compaction/compaction_group_view.hh b/compaction/compaction_group_view.hh index bf9b344e47..3157608c93 100644 --- a/compaction/compaction_group_view.hh +++ b/compaction/compaction_group_view.hh @@ -12,6 +12,7 @@ #include #include "schema/schema_fwd.hh" +#include "sstables/open_info.hh" #include "compaction_descriptor.hh" class reader_permit; @@ -44,7 +45,7 @@ public: virtual compaction_strategy_state& get_compaction_strategy_state() noexcept = 0; virtual reader_permit make_compaction_reader_permit() const = 0; virtual sstables::sstables_manager& get_sstables_manager() noexcept = 0; - virtual sstables::shared_sstable make_sstable() const = 0; + virtual sstables::shared_sstable make_sstable(sstables::sstable_state) const = 0; virtual sstables::sstable_writer_config configure_writer(sstring origin) const = 0; virtual api::timestamp_type min_memtable_timestamp() const = 0; virtual api::timestamp_type min_memtable_live_timestamp() const = 0; diff --git a/compaction/compaction_manager.cc b/compaction/compaction_manager.cc index 9d9d0143c6..a25b40938b 100644 --- a/compaction/compaction_manager.cc +++ b/compaction/compaction_manager.cc @@ -416,7 +416,9 @@ future compaction_task_executor::compact_sstables(compaction_ descriptor.enable_garbage_collection(co_await sstable_set_for_tombstone_gc(t)); } descriptor.creator = [&t] (shard_id) { - return t.make_sstable(); + // All compaction types going through this path will work on normal input sstables only. + // Off-strategy, for example, waits until the sstables move out of staging state. + return t.make_sstable(sstables::sstable_state::normal); }; descriptor.replacer = [this, &t, &on_replace, offstrategy] (compaction_completion_desc desc) { t.get_compaction_strategy().notify_completion(t, desc.old_sstables, desc.new_sstables); @@ -2303,8 +2305,11 @@ compaction_manager::maybe_split_new_sstable(sstables::shared_sstable sst, compac compaction_progress_monitor monitor; compaction_data info = create_compaction_data(); compaction_descriptor desc = split_compaction_task_executor::make_descriptor(sst, opt); - desc.creator = [&t] (shard_id _) { - return t.make_sstable(); + desc.creator = [&t, sst] (shard_id _) { + // NOTE: preserves the sstable state, since we want the output to be on the same state as the original. + // For example, if base table has views, it's important that sstable produced by repair will be + // in the staging state. + return t.make_sstable(sst->state()); }; desc.replacer = [&] (compaction_completion_desc d) { std::move(d.new_sstables.begin(), d.new_sstables.end(), std::back_inserter(ret)); diff --git a/replica/table.cc b/replica/table.cc index 756752db57..f513560b36 100644 --- a/replica/table.cc +++ b/replica/table.cc @@ -2604,8 +2604,8 @@ public: sstables::sstables_manager& get_sstables_manager() noexcept override { return _t.get_sstables_manager(); } - sstables::shared_sstable make_sstable() const override { - return _t.make_sstable(); + sstables::shared_sstable make_sstable(sstables::sstable_state state) const override { + return _t.make_sstable(state); } sstables::sstable_writer_config configure_writer(sstring origin) const override { auto cfg = _t.get_sstables_manager().configure_writer(std::move(origin)); diff --git a/test/boost/compaction_group_test.cc b/test/boost/compaction_group_test.cc index 8aae36a8eb..fcff5a7c23 100644 --- a/test/boost/compaction_group_test.cc +++ b/test/boost/compaction_group_test.cc @@ -110,7 +110,7 @@ public: virtual compaction::compaction_strategy_state& get_compaction_strategy_state() noexcept override { return _compaction_strategy_state; } virtual reader_permit make_compaction_reader_permit() const override { return _semaphore.make_permit(); } virtual sstables::sstables_manager& get_sstables_manager() noexcept override { return _sst_man; } - virtual sstables::shared_sstable make_sstable() const override { return _sstable_factory(); } + virtual sstables::shared_sstable make_sstable(sstables::sstable_state) const override { return _sstable_factory(); } virtual sstables::sstable_writer_config configure_writer(sstring origin) const override { return _sst_man.configure_writer(std::move(origin)); } virtual api::timestamp_type min_memtable_timestamp() const override { return api::min_timestamp; } virtual api::timestamp_type min_memtable_live_timestamp() const override { return api::min_timestamp; } diff --git a/test/lib/test_services.cc b/test/lib/test_services.cc index 98aaf15808..d2ec419328 100644 --- a/test/lib/test_services.cc +++ b/test/lib/test_services.cc @@ -90,7 +90,7 @@ public: sstables::sstables_manager& get_sstables_manager() noexcept override { return _sstables_manager; } - sstables::shared_sstable make_sstable() const override { + sstables::shared_sstable make_sstable(sstables::sstable_state) const override { return table().make_sstable(); } sstables::sstable_writer_config configure_writer(sstring origin) const override { diff --git a/tools/scylla-sstable.cc b/tools/scylla-sstable.cc index 42e66977ca..bf99cda45e 100644 --- a/tools/scylla-sstable.cc +++ b/tools/scylla-sstable.cc @@ -790,7 +790,7 @@ public: virtual compaction::compaction_strategy_state& get_compaction_strategy_state() noexcept override { return _compaction_strategy_state; } virtual reader_permit make_compaction_reader_permit() const override { return _permit; } virtual sstables::sstables_manager& get_sstables_manager() noexcept override { return _sst_man; } - virtual sstables::shared_sstable make_sstable() const override { return do_make_sstable(); } + virtual sstables::shared_sstable make_sstable(sstables::sstable_state) const override { return do_make_sstable(); } virtual sstables::sstable_writer_config configure_writer(sstring origin) const override { return do_configure_writer(std::move(origin)); } virtual api::timestamp_type min_memtable_timestamp() const override { return api::min_timestamp; } virtual api::timestamp_type min_memtable_live_timestamp() const override { return api::min_timestamp; } @@ -880,7 +880,7 @@ void scrub_operation(schema_ptr schema, reader_permit permit, const std::vector< auto compaction_descriptor = compaction::compaction_descriptor(std::move(sstables)); compaction_descriptor.options = compaction::compaction_type_options::make_scrub(scrub_mode, compaction::compaction_type_options::scrub::quarantine_invalid_sstables::no); - compaction_descriptor.creator = [&compaction_group_view] (shard_id) { return compaction_group_view.make_sstable(); }; + compaction_descriptor.creator = [&compaction_group_view] (shard_id) { return compaction_group_view.make_sstable(sstables::sstable_state::normal); }; compaction_descriptor.replacer = [] (compaction::compaction_completion_desc) { }; auto compaction_data = compaction::compaction_data{};