bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Merge 'Untangle sstable-directory vs sstable in pending log creation code' from Pavel Emelyanov

Browse Source 
There's a sstable_directory::create_pending_deletion_log() helper method that's called by sstable's filesystem_storage atomic-delete methods and that prepares the deletion log for a bunch of sstables. For that method to do its job it needs to get private sstable->_storage field (which is always the filesystem_storage one), also the atomic-delete transparent context object is leaked into the sstable_directory code and low-level sstable storage code needs to include higher-level sstable_directory header.

This patch unties these knots. As the result:
- friendship between sstable and sstable_directory is removed
- transparent atomic_delete_context is encapsulated in storage.(cc|hh) code
- less code for create_pending_deletion_log() to dump TOC filename into log

Closes scylladb/scylladb#22823

* github.com:scylladb/scylladb:
  sstable: Unfriend sstable_directory class
  sstable_directory: Move sstable_directory::pending_delete_result
  sstable_directory: Calculate prefixes outside of create_pending_deletion_log()
  sstable_directory: Introduce local pending_delete_log variable
  sstable_directory: Relax toc file dumping to deletion log
This commit is contained in:
Botond Dénes
2025-02-24 14:58:37 +02:00
parent 854d2917a1 d79eec2e76
commit 6ab16006a2
5 changed files with 26 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
sstables/sstable_directory.cc
View File

@@ -626,20 +626,18 @@ bool sstable_directory::compare_sstable_storage_prefix(const sstring& prefix_a,
return size_a == size_b && sstring::traits_type::compare(prefix_a.begin(), prefix_b.begin(), size_a) == 0;
}
future<sstable_directory::pending_delete_result> sstable_directory::create_pending_deletion_log(opened_directory& base_dir, const std::vector<shared_sstable>& ssts) {
future<sstring> sstable_directory::create_pending_deletion_log(opened_directory& base_dir, const std::vector<shared_sstable>& ssts) {
return seastar::async([&] {
min_max_tracker<generation_type> gen_tracker;
pending_delete_result res;
sstring pending_delete_log;
for (const auto& sst : ssts) {
auto prefix = sst->_storage->prefix();
res.prefixes.insert(prefix);
gen_tracker.update(sst->generation());
}
sstring pending_delete_dir = (base_dir.path() / sstables::pending_delete_dir).native();
res.pending_delete_log = format("{}/sstables-{}-{}.log", pending_delete_dir, gen_tracker.min(), gen_tracker.max());
sstring tmp_pending_delete_log = res.pending_delete_log + ".tmp";
pending_delete_log = format("{}/sstables-{}-{}.log", pending_delete_dir, gen_tracker.min(), gen_tracker.max());
sstring tmp_pending_delete_log = pending_delete_log + ".tmp";
dirlog.trace("Writing {}", tmp_pending_delete_log);
touch_directory(pending_delete_dir).get();
@@ -653,17 +651,13 @@ future<sstable_directory::pending_delete_result> sstable_directory::create_pendi
try {
auto trim_size = base_dir.native().size() + 1; // Account for the '/' delimiter
for (const auto& sst : ssts) {
auto prefix = sst->_storage->prefix();
if (prefix.size() > trim_size) {
out.write(prefix.begin() + trim_size, prefix.size() - trim_size).get();
out.write("/").get();
auto toc = sst->toc_filename();
if (toc.size() <= trim_size) {
on_internal_error(dirlog, fmt::format("Sstable {} outside of basedir {} is scheduled for deletion", toc, base_dir.native()));
}
auto toc = sst->component_basename(component_type::TOC);
out.write(toc).get();
out.write(toc.begin() + trim_size, toc.size() - trim_size).get();
out.write("\n").get();
dirlog.trace("Wrote '{}{}' to {}",
prefix.size() > trim_size ? sstring(prefix.begin() + trim_size, prefix.size() - trim_size) + "/" : "",
sst->component_basename(component_type::TOC), tmp_pending_delete_log);
dirlog.trace("Wrote '{}' to {}", sstring(toc.begin() + trim_size, toc.size() - trim_size), tmp_pending_delete_log);
}
out.flush().get();
@@ -673,13 +667,13 @@ future<sstable_directory::pending_delete_result> sstable_directory::create_pendi
}
// Once flushed and closed, the temporary log file can be renamed.
io_check(rename_file, tmp_pending_delete_log, res.pending_delete_log).get();
io_check(rename_file, tmp_pending_delete_log, pending_delete_log).get();
// Guarantee that the changes above reached the disk.
base_dir.sync(general_disk_error_handler).get();
dirlog.debug("{} written successfully.", res.pending_delete_log);
dirlog.debug("{} written successfully.", pending_delete_log);
return res;
return pending_delete_log;
});
}

7
sstables/sstable_directory.hh
View File

@@ -291,11 +291,6 @@ public:
using can_be_remote = bool_class<struct can_be_remote_tag>;
future<> collect_output_unshared_sstables(std::vector<sstables::shared_sstable> resharded_sstables, can_be_remote);
struct pending_delete_result {
sstring pending_delete_log;
std::unordered_set<sstring> prefixes;
};
// When we compact sstables, we have to atomically instantiate the new
// sstable and delete the old ones. Otherwise, if we compact A+B into C,
// and if A contained some data that was tombstoned by B, and if B was
@@ -312,7 +307,7 @@ public:
// Creates the deletion log for atomic deletion of sstables at `base_dir` (helper for the
// above function that's also used by tests)
// Returns the name of the pending_delete_log and an unordered_set of sstable prefixes.
static future<pending_delete_result> create_pending_deletion_log(opened_directory& base_dir, const std::vector<shared_sstable>& ssts);
static future<sstring> create_pending_deletion_log(opened_directory& base_dir, const std::vector<shared_sstable>& ssts);
static bool compare_sstable_storage_prefix(const sstring& a, const sstring& b) noexcept;
sstable_state state() const noexcept { return _state; }

1
sstables/sstables.hh
View File

@@ -526,7 +526,6 @@ private:
}
friend class sstable_stream_sink_impl;
friend class sstable_directory;
friend class filesystem_storage;
friend class s3_storage;
friend class tiered_storage;

10
sstables/storage.cc
View File

@@ -480,7 +480,15 @@ future<> filesystem_storage::wipe(const sstable& sst, sync_dir sync) noexcept {
}
future<atomic_delete_context> filesystem_storage::atomic_delete_prepare(const std::vector<shared_sstable>& ssts) const {
co_return co_await sstable_directory::create_pending_deletion_log(_base_dir, ssts);
atomic_delete_context res;
for (const auto& sst : ssts) {
auto prefix = sst->_storage->prefix();
res.prefixes.insert(prefix);
}
res.pending_delete_log = co_await sstable_directory::create_pending_deletion_log(_base_dir, ssts);
co_return std::move(res);
}
future<> filesystem_storage::atomic_delete_complete(atomic_delete_context ctx) const {

6
sstables/storage.hh
View File

@@ -23,7 +23,6 @@
#include "sstables/component_type.hh"
#include "sstables/generation_type.hh"
#include "utils/disk-error-handler.hh"
#include "sstables/sstable_directory.hh"
class schema;
@@ -41,7 +40,10 @@ class sstable;
class sstables_manager;
class entry_descriptor;
using atomic_delete_context = sstable_directory::pending_delete_result;
struct atomic_delete_context {
sstring pending_delete_log;
std::unordered_set<sstring> prefixes;
};
class opened_directory final {
std::filesystem::path _pathname;