From 881f67de89ec2bf2bdf47c0c2dd641847f14c40b Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Mon, 23 Feb 2026 19:49:05 +0200 Subject: [PATCH] db: snapshot-ctl: add cancel_expiration Cancel outstanding snapshot expiration when a snapshot is backed up and when clear_snapshot is called from the api. Fixes SCYLLADB-787 Fixes SCYLLADB-789 Signed-off-by: Benny Halevy --- db/snapshot-ctl.cc | 28 ++++++++++++++++++++++++++++ db/snapshot-ctl.hh | 6 ++++++ 2 files changed, 34 insertions(+) diff --git a/db/snapshot-ctl.cc b/db/snapshot-ctl.cc index f5907f730a..c0bbbdde85 100644 --- a/db/snapshot-ctl.cc +++ b/db/snapshot-ctl.cc @@ -12,6 +12,8 @@ #include #include +#include + #include #include #include @@ -201,6 +203,9 @@ future<> snapshot_ctl::do_take_column_family_snapshot(sstring ks_name, std::vect future<> snapshot_ctl::clear_snapshot(sstring tag, std::vector keyspace_names, sstring cf_name) { snap_log.debug("clear_snapshot: tag={} keyspaces={} table={}", tag, fmt::join(keyspace_names, ","), cf_name); + co_await container().invoke_on(0, [&] (auto& sc) { + return sc.cancel_expiration(tag, keyspace_names, cf_name); + }); co_return co_await run_snapshot_modify_operation([this, tag = std::move(tag), keyspace_names = std::move(keyspace_names), cf_name = std::move(cf_name)] (this auto) -> future<> { // clear_snapshot enumerates keyspace_names and uses cf_name as a // filter in each. When cf_name needs resolution (e.g. logical index @@ -276,6 +281,7 @@ future snapshot_ctl::start_backup(sstring endpoint, sstring buck if (!storage_options.is_local_type()) { throw std::invalid_argument("not able to backup a non-local table"); } + cancel_expiration(snapshot_name, {keyspace}, table); auto& local_storage_options = std::get(storage_options.value); // // The keyspace data directories and their snapshots are arranged as follows: @@ -366,4 +372,26 @@ void snapshot_ctl::schedule_expiration(gc_clock::time_point when, sstring ks_nam } } +void snapshot_ctl::cancel_expiration(sstring tag, std::vector ks_names, sstring table_name) { + if (this_shard_id() != 0) { + on_internal_error(snap_log, "cancel_expiration must be called on shard 0"); + } + snap_log.debug("Cancel expiration of snapshots with tag='{}' in keyspaces={} table={}", tag, fmt::join(ks_names, ","), table_name); + std::unordered_set keyspaces; + std::ranges::move(ks_names, std::inserter(keyspaces, keyspaces.end())); + _expiration_queue.erase(std::remove_if(_expiration_queue.begin(), _expiration_queue.end(), [&] (const expiration_info& info) { + if (!tag.empty() && info.tag != tag) { + return false; + } + if (!keyspaces.empty() && !keyspaces.contains(info.ks_name)) { + return false; + } + if (!table_name.empty() && info.table_name != table_name) { + return false; + } + return true; + }), _expiration_queue.end()); + std::ranges::make_heap(_expiration_queue, std::greater{}, &expiration_info::expires_at); +} + } // namespace db diff --git a/db/snapshot-ctl.hh b/db/snapshot-ctl.hh index b0030bfc06..95e9c5b8d3 100644 --- a/db/snapshot-ctl.hh +++ b/db/snapshot-ctl.hh @@ -126,6 +126,12 @@ public: // Must be called on shard 0 void schedule_expiration(gc_clock::time_point when, sstring ks_name, sstring table_name, sstring tag); + + // For canceling expiration, ks_name or table_name can be empty + // And then all snapshots with the given tag (or all, if `tag` is empty) are erased from the expiration queue + // within the given scope. + // Must be called on shard 0 + void cancel_expiration(sstring tag, std::vector ks_names = {}, sstring table_name = ""); private: config _config; sharded& _db;