diff --git a/compaction/compaction.cc b/compaction/compaction.cc index 4ad38b2e03..bac75f9441 100644 --- a/compaction/compaction.cc +++ b/compaction/compaction.cc @@ -1799,7 +1799,7 @@ static future scrub_sstables_validate_mode(sstables::compacti if (validation_errors != 0) { for (auto& sst : *sstables->all()) { - co_await sst->move_to_quarantine(); + co_await sst->change_state(sstables::quarantine_dir); } } diff --git a/replica/table.cc b/replica/table.cc index d111d739c7..f127cd796c 100644 --- a/replica/table.cc +++ b/replica/table.cc @@ -2543,7 +2543,7 @@ future<> table::move_sstables_from_staging(std::vector // completed first. // The _sstable_deletion_sem prevents list update on off-strategy completion and move_sstables_from_staging() // from stepping on each other's toe. - co_await sst->move_to_new_dir(dir(), sst->generation(), &delay_commit); + co_await sst->change_state(sstables::normal_dir, &delay_commit); // If view building finished faster, SSTable with repair origin still exists. // It can also happen the SSTable is not going through reshape, so it doesn't have a repair origin. // That being said, we'll only add this SSTable to tracker if its origin is other than repair. diff --git a/sstables/sstables.cc b/sstables/sstables.cc index cb61eadb13..f5bca4650d 100644 --- a/sstables/sstables.cc +++ b/sstables/sstables.cc @@ -2241,6 +2241,35 @@ future<> sstable::filesystem_storage::quarantine(const sstable& sst, delayed_com co_await move(sst, std::move(new_dir), sst.generation(), delay_commit); } +future<> sstable::filesystem_storage::change_state(const sstable& sst, sstring to, generation_type new_generation, delayed_commit_changes* delay_commit) { + auto path = fs::path(dir); + auto current = path.filename().native(); + + // Moving between states means moving between basedir/state subdirectories. + // However, normal state maps to the basedir itself and thus there's no way + // to check if current is normal_dir. The best that can be done here is to + // check that it's not anything else + if (current == staging_dir || current == upload_dir || current == quarantine_dir) { + if (to == quarantine_dir && current != staging_dir) { + // Legacy exception -- quarantine from anything but staging + // moves to the current directory quarantine subdir + path = path / to; + } else { + path = path.parent_path() / to; + } + } else { + current = normal_dir; + path = path / to; + } + + if (current == to) { + co_return; // Already there + } + + sstlog.info("Moving sstable {} to {} in {}", sst.get_filename(), to, path); + co_await move(sst, path.native(), std::move(new_generation), delay_commit); +} + future<> sstable::move_to_quarantine(delayed_commit_changes* delay_commit) { if (is_quarantined()) { return make_ready_future<>(); @@ -2249,6 +2278,10 @@ future<> sstable::move_to_quarantine(delayed_commit_changes* delay_commit) { return _storage.quarantine(*this, delay_commit); } +future<> sstable::change_state(sstring to, delayed_commit_changes* delay_commit) { + co_await _storage.change_state(*this, to, _generation, delay_commit); +} + future<> sstable::delayed_commit_changes::commit() { return parallel_for_each(_dirs, [] (sstring dir) { return sync_directory(dir); diff --git a/sstables/sstables.hh b/sstables/sstables.hh index 50e76fbed2..ac0593c821 100644 --- a/sstables/sstables.hh +++ b/sstables/sstables.hh @@ -113,6 +113,7 @@ private: friend class sstables_manager; }; +constexpr const char* normal_dir = ""; constexpr const char* staging_dir = "staging"; constexpr const char* upload_dir = "upload"; constexpr const char* snapshots_dir = "snapshots"; @@ -220,6 +221,12 @@ public: // will move it into a quarantine_dir subdirectory of its current directory. future<> move_to_quarantine(delayed_commit_changes* delay = nullptr); + // Move the sstable between states + // + // Known states are normal, staging, upload and quarantine. + // It's up to the storage driver how to implement this. + future<> change_state(sstring to, delayed_commit_changes* delay = nullptr); + generation_type generation() const { return _generation; } @@ -491,6 +498,13 @@ public: future<> seal(const sstable& sst); future<> snapshot(const sstable& sst, sstring dir, absolute_path abs) const; future<> quarantine(const sstable& sst, delayed_commit_changes* delay); + + // Moves the files around with .move() method. States are basedir subdirectories + // with the exception that normal state maps to the basedir itself. If the sstable + // is already in the target state, this is a noop. + // Moving in a snapshot or upload will move to a subdirectory of the current directory. + future<> change_state(const sstable& sst, sstring to, generation_type generation, delayed_commit_changes* delay); + future<> move(const sstable& sst, sstring new_dir, generation_type generation, delayed_commit_changes* delay); // runs in async context void open(sstable& sst, const io_priority_class& pc); diff --git a/test/boost/database_test.cc b/test/boost/database_test.cc index a8b9e82190..1f7fc69b09 100644 --- a/test/boost/database_test.cc +++ b/test/boost/database_test.cc @@ -1137,7 +1137,7 @@ SEASTAR_TEST_CASE(populate_from_quarantine_works) { auto idx = tests::random::get_int(0, sstables.size() - 1); testlog.debug("Moving sstable #{} out of {} to quarantine", idx, sstables.size()); auto sst = sstables[idx]; - co_await sst->move_to_quarantine(); + co_await sst->change_state(sstables::quarantine_dir); found |= true; }); co_return found; @@ -1191,7 +1191,7 @@ SEASTAR_TEST_CASE(snapshot_with_quarantine_works) { } auto idx = tests::random::get_int(0, sstables.size() - 1); auto sst = sstables[idx]; - co_await sst->move_to_quarantine(); + co_await sst->change_state(sstables::quarantine_dir); }); }); }