From 5d98e34c167c94005c7a86854a5ae43ac3e7aef0 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 13 Feb 2023 18:55:27 +0300 Subject: [PATCH 1/8] sstable_directory: Keep scan_state on components_lister The scan_state keeps the state of listing directory with sstables. It now lives on the .process_sstable_dir() stack, but it can as well live on the lister itself. Signed-off-by: Pavel Emelyanov --- sstables/sstable_directory.cc | 6 +++--- sstables/sstable_directory.hh | 24 ++++++++++++++---------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/sstables/sstable_directory.cc b/sstables/sstable_directory.cc index 58ddc16bba..15d129bb86 100644 --- a/sstables/sstable_directory.cc +++ b/sstables/sstable_directory.cc @@ -35,6 +35,7 @@ bool manifest_json_filter(const fs::path&, const directory_entry& entry) { sstable_directory::components_lister::components_lister(std::filesystem::path dir) : _lister(dir, lister::dir_entry_types::of(), &manifest_json_filter) + , _state(std::make_unique()) { } @@ -61,7 +62,7 @@ sstable_directory::sstable_directory(sstables_manager& manager, {} void -sstable_directory::handle_component(scan_state& state, sstables::entry_descriptor desc, fs::path filename) { +sstable_directory::handle_component(components_lister::scan_state& state, sstables::entry_descriptor desc, fs::path filename) { if ((generation_value(desc.generation) % smp::count) != this_shard_id()) { return; } @@ -180,9 +181,8 @@ sstable_directory::process_sstable_dir(process_flags flags) { // - If all shards scan in parallel, they can start loading sooner. That is faster than having // a separate step to fetch all files, followed by another step to distribute and process. - scan_state state; - auto sstable_dir_lister = _manager.get_components_lister(_sstable_dir); + auto& state = *sstable_dir_lister._state; std::exception_ptr ex; try { while (true) { diff --git a/sstables/sstable_directory.hh b/sstables/sstable_directory.hh index 4447cf063a..8b7009943f 100644 --- a/sstables/sstable_directory.hh +++ b/sstables/sstable_directory.hh @@ -65,7 +65,20 @@ public: }; class components_lister { + public: + struct scan_state { + using scan_multimap = std::unordered_multimap; + using scan_descriptors = utils::chunked_vector; + using scan_descriptors_map = std::unordered_map; + + scan_multimap generations_found; + scan_descriptors temp_toc_found; + scan_descriptors_map descriptors; + }; + directory_lister _lister; + std::unique_ptr _state; + public: future get(); components_lister(std::filesystem::path dir); @@ -73,15 +86,6 @@ public: }; private: - using scan_multimap = std::unordered_multimap; - using scan_descriptors = utils::chunked_vector; - using scan_descriptors_map = std::unordered_map; - - struct scan_state { - scan_multimap generations_found; - scan_descriptors temp_toc_found; - scan_descriptors_map descriptors; - }; // SSTable files to be deleted: things with a Temporary TOC, missing TOC files, // TemporaryStatistics, etc. Not part of the scan state, because we want to do a 2-phase @@ -121,7 +125,7 @@ private: future<> process_descriptor(sstables::entry_descriptor desc, process_flags flags); void validate(sstables::shared_sstable sst, process_flags flags) const; - void handle_component(scan_state& state, sstables::entry_descriptor desc, std::filesystem::path filename); + void handle_component(components_lister::scan_state& state, sstables::entry_descriptor desc, std::filesystem::path filename); template future<> parallel_for_each_restricted(Container&& C, Func&& func); From df5384cb1ebaafec911fd37bcae411bed277eb0f Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 13 Feb 2023 19:00:31 +0300 Subject: [PATCH 2/8] sstable_directory: Keep components_lister aboard The lister is supposed to be alive throughout .process_sstable_dir() and can die after .commit_directory_changes(). Signed-off-by: Pavel Emelyanov --- sstables/sstable_directory.cc | 4 +++- sstables/sstable_directory.hh | 1 + sstables/sstables_manager.cc | 4 ++-- sstables/sstables_manager.hh | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/sstables/sstable_directory.cc b/sstables/sstable_directory.cc index 15d129bb86..ec50f6b9c4 100644 --- a/sstables/sstable_directory.cc +++ b/sstables/sstable_directory.cc @@ -58,6 +58,7 @@ sstable_directory::sstable_directory(sstables_manager& manager, , _sstable_dir(std::move(sstable_dir)) , _io_priority(std::move(io_prio)) , _error_handler_gen(error_handler_gen) + , _lister(_manager.get_components_lister(_sstable_dir)) , _unshared_remote_sstables(smp::count) {} @@ -181,7 +182,7 @@ sstable_directory::process_sstable_dir(process_flags flags) { // - If all shards scan in parallel, they can start loading sooner. That is faster than having // a separate step to fetch all files, followed by another step to distribute and process. - auto sstable_dir_lister = _manager.get_components_lister(_sstable_dir); + auto& sstable_dir_lister = *_lister; auto& state = *sstable_dir_lister._state; std::exception_ptr ex; try { @@ -250,6 +251,7 @@ sstable_directory::process_sstable_dir(process_flags flags) { future<> sstable_directory::commit_directory_changes() { + _lister.reset(); // Remove all files scheduled for removal return parallel_for_each(std::exchange(_files_for_removal, {}), [] (sstring path) { dirlog.info("Removing file {}", path); diff --git a/sstables/sstable_directory.hh b/sstables/sstable_directory.hh index 8b7009943f..a108118815 100644 --- a/sstables/sstable_directory.hh +++ b/sstables/sstable_directory.hh @@ -102,6 +102,7 @@ private: std::filesystem::path _sstable_dir; ::io_priority_class _io_priority; io_error_handler_gen _error_handler_gen; + std::unique_ptr _lister; generation_type _max_generation_seen = generation_from_value(0); sstables::sstable_version_types _max_version_seen = sstables::sstable_version_types::ka; diff --git a/sstables/sstables_manager.cc b/sstables/sstables_manager.cc index 985906db0d..b0246efc12 100644 --- a/sstables/sstables_manager.cc +++ b/sstables/sstables_manager.cc @@ -107,8 +107,8 @@ future<> sstables_manager::close() { co_await _sstable_metadata_concurrency_sem.stop(); } -sstable_directory::components_lister sstables_manager::get_components_lister(std::filesystem::path dir) { - return sstable_directory::components_lister(std::move(dir)); +std::unique_ptr sstables_manager::get_components_lister(std::filesystem::path dir) { + return std::make_unique(std::move(dir)); } } // namespace sstables diff --git a/sstables/sstables_manager.hh b/sstables/sstables_manager.hh index 53c141b299..755fd39803 100644 --- a/sstables/sstables_manager.hh +++ b/sstables/sstables_manager.hh @@ -85,7 +85,7 @@ public: io_error_handler_gen error_handler_gen = default_io_error_handler_gen(), size_t buffer_size = default_sstable_buffer_size); - sstable_directory::components_lister get_components_lister(std::filesystem::path dir); + std::unique_ptr get_components_lister(std::filesystem::path dir); virtual sstable_writer_config configure_writer(sstring origin) const; const db::config& config() const { return _db_config; } From 58f4076117fe615e6a0d05f2e9d4feae72275615 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 13 Feb 2023 19:02:56 +0300 Subject: [PATCH 3/8] sstable_directory: Keep files_for_removal on scan_state This list is the list of on-disk files, which is the property of filesystem scan state. When committing directory changes (read: removing those files) the list can be moved-from the state. Signed-off-by: Pavel Emelyanov --- sstables/sstable_directory.cc | 9 +++++---- sstables/sstable_directory.hh | 12 ++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/sstables/sstable_directory.cc b/sstables/sstable_directory.cc index ec50f6b9c4..212c157f1e 100644 --- a/sstables/sstable_directory.cc +++ b/sstables/sstable_directory.cc @@ -77,7 +77,7 @@ sstable_directory::handle_component(components_lister::scan_state& state, sstabl // for instance on mutate_level. We should delete it - so we mark it for deletion // here, but just the component. The old statistics file should still be there // and we'll go with it. - _files_for_removal.insert(filename.native()); + state.files_for_removal.insert(filename.native()); break; case component_type::TOC: state.descriptors.emplace(desc.generation, std::move(desc)); @@ -213,7 +213,7 @@ sstable_directory::process_sstable_dir(process_flags flags) { for (auto it = range.first; it != range.second; ++it) { auto& path = it->second; dirlog.trace("Scheduling to remove file {}, from an SSTable with a Temporary TOC", path.native()); - _files_for_removal.insert(path.native()); + state.files_for_removal.insert(path.native()); } state.generations_found.erase(range.first, range.second); state.descriptors.erase(desc.generation); @@ -244,16 +244,17 @@ sstable_directory::process_sstable_dir(process_flags flags) { throw sstables::malformed_sstable_exception(format("At directory: {}: no TOC found for SSTable {}!. Refusing to boot", _sstable_dir.native(), path.native())); } else { dirlog.info("Found incomplete SSTable {} at directory {}. Removing", path.native(), _sstable_dir.native()); - _files_for_removal.insert(path.native()); + state.files_for_removal.insert(path.native()); } } } future<> sstable_directory::commit_directory_changes() { + auto files_for_removal = std::exchange(_lister->_state->files_for_removal, {}); _lister.reset(); // Remove all files scheduled for removal - return parallel_for_each(std::exchange(_files_for_removal, {}), [] (sstring path) { + return parallel_for_each(std::move(files_for_removal), [] (sstring path) { dirlog.info("Removing file {}", path); return remove_file(std::move(path)); }); diff --git a/sstables/sstable_directory.hh b/sstables/sstable_directory.hh index a108118815..d7ee2f70dd 100644 --- a/sstables/sstable_directory.hh +++ b/sstables/sstable_directory.hh @@ -74,6 +74,12 @@ public: scan_multimap generations_found; scan_descriptors temp_toc_found; scan_descriptors_map descriptors; + + // SSTable files to be deleted: things with a Temporary TOC, missing TOC files, + // TemporaryStatistics, etc. Not part of the scan state, because we want to do a 2-phase + // delete: maybe one of the shards will have signaled an error. And in the case of an error + // we don't want to delete anything. + std::unordered_set files_for_removal; }; directory_lister _lister; @@ -87,12 +93,6 @@ public: private: - // SSTable files to be deleted: things with a Temporary TOC, missing TOC files, - // TemporaryStatistics, etc. Not part of the scan state, because we want to do a 2-phase - // delete: maybe one of the shards will have signaled an error. And in the case of an error - // we don't want to delete anything. - std::unordered_set _files_for_removal; - // prevents an object that respects a phaser (usually a table) from disappearing in the middle of the operation. // Will be destroyed when this object is destroyed. std::optional _operation_barrier; From 4c4aeba9b620c7294b8001ab403155e6d34dd002 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 13 Feb 2023 19:04:35 +0300 Subject: [PATCH 4/8] sstable_directory: Move .handle_component() to components_lister This method is in charge of collecting a found file on scan_state, it logically belogs to the components_lister and its internals. Signed-off-by: Pavel Emelyanov --- sstables/sstable_directory.cc | 13 ++++++------- sstables/sstable_directory.hh | 3 ++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sstables/sstable_directory.cc b/sstables/sstable_directory.cc index 212c157f1e..650c443eb9 100644 --- a/sstables/sstable_directory.cc +++ b/sstables/sstable_directory.cc @@ -62,14 +62,13 @@ sstable_directory::sstable_directory(sstables_manager& manager, , _unshared_remote_sstables(smp::count) {} -void -sstable_directory::handle_component(components_lister::scan_state& state, sstables::entry_descriptor desc, fs::path filename) { +void sstable_directory::components_lister::handle(sstables::entry_descriptor desc, fs::path filename) { if ((generation_value(desc.generation) % smp::count) != this_shard_id()) { return; } dirlog.trace("for SSTable directory, scanning {}", filename); - state.generations_found.emplace(desc.generation, filename); + _state->generations_found.emplace(desc.generation, filename); switch (desc.component) { case component_type::TemporaryStatistics: @@ -77,13 +76,13 @@ sstable_directory::handle_component(components_lister::scan_state& state, sstabl // for instance on mutate_level. We should delete it - so we mark it for deletion // here, but just the component. The old statistics file should still be there // and we'll go with it. - state.files_for_removal.insert(filename.native()); + _state->files_for_removal.insert(filename.native()); break; case component_type::TOC: - state.descriptors.emplace(desc.generation, std::move(desc)); + _state->descriptors.emplace(desc.generation, std::move(desc)); break; case component_type::TemporaryTOC: - state.temp_toc_found.push_back(std::move(desc)); + _state->temp_toc_found.push_back(std::move(desc)); break; default: // Do nothing, and will validate when trying to load the file. @@ -192,7 +191,7 @@ sstable_directory::process_sstable_dir(process_flags flags) { break; } auto comps = sstables::entry_descriptor::make_descriptor(_sstable_dir.native(), name); - handle_component(state, std::move(comps), _sstable_dir / name); + sstable_dir_lister.handle(std::move(comps), _sstable_dir / name); } } catch (...) { ex = std::current_exception(); diff --git a/sstables/sstable_directory.hh b/sstables/sstable_directory.hh index d7ee2f70dd..a29fbdf5ff 100644 --- a/sstables/sstable_directory.hh +++ b/sstables/sstable_directory.hh @@ -82,6 +82,8 @@ public: std::unordered_set files_for_removal; }; + void handle(sstables::entry_descriptor desc, std::filesystem::path filename); + directory_lister _lister; std::unique_ptr _state; @@ -126,7 +128,6 @@ private: future<> process_descriptor(sstables::entry_descriptor desc, process_flags flags); void validate(sstables::shared_sstable sst, process_flags flags) const; - void handle_component(components_lister::scan_state& state, sstables::entry_descriptor desc, std::filesystem::path filename); template future<> parallel_for_each_restricted(Container&& C, Func&& func); From c4037270a32d8f78c7b396d2e4db9cbb6c60efc7 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 13 Feb 2023 19:07:35 +0300 Subject: [PATCH 5/8] sstable_directory: Move most of .process_sstable_dir() on lister Processing storage with sstable files/objects is storage-specific. The components_lister is the right components to handle it. Signed-off-by: Pavel Emelyanov --- sstables/sstable_directory.cc | 17 ++++++++++------- sstables/sstable_directory.hh | 2 ++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/sstables/sstable_directory.cc b/sstables/sstable_directory.cc index 650c443eb9..6a12209187 100644 --- a/sstables/sstable_directory.cc +++ b/sstables/sstable_directory.cc @@ -165,10 +165,12 @@ sstable_directory::highest_version_seen() const { return _max_version_seen; } -future<> -sstable_directory::process_sstable_dir(process_flags flags) { +future<> sstable_directory::process_sstable_dir(process_flags flags) { dirlog.debug("Start processing directory {} for SSTables", _sstable_dir); + return _lister->process(*this, _sstable_dir, flags); +} +future<> sstable_directory::components_lister::process(sstable_directory& directory, fs::path location, process_flags flags) { // It seems wasteful that each shard is repeating this scan, and to some extent it is. // However, we still want to open the files and especially call process_dir() in a distributed // fashion not to overload any shard. Also in the common case the SSTables will all be @@ -181,8 +183,9 @@ sstable_directory::process_sstable_dir(process_flags flags) { // - If all shards scan in parallel, they can start loading sooner. That is faster than having // a separate step to fetch all files, followed by another step to distribute and process. - auto& sstable_dir_lister = *_lister; + auto& sstable_dir_lister = *this; auto& state = *sstable_dir_lister._state; + auto& _sstable_dir = location; std::exception_ptr ex; try { while (true) { @@ -218,20 +221,20 @@ sstable_directory::process_sstable_dir(process_flags flags) { state.descriptors.erase(desc.generation); } - _max_generation_seen = boost::accumulate(state.generations_found | boost::adaptors::map_keys, generation_from_value(0), [] (generation_type a, generation_type b) { + directory._max_generation_seen = boost::accumulate(state.generations_found | boost::adaptors::map_keys, generation_from_value(0), [] (generation_type a, generation_type b) { return std::max(a, b); }); dirlog.debug("After {} scanned, seen generation {}. {} descriptors found, {} different files found ", - _sstable_dir, _max_generation_seen, state.descriptors.size(), state.generations_found.size()); + _sstable_dir, directory._max_generation_seen, state.descriptors.size(), state.generations_found.size()); // _descriptors is everything with a TOC. So after we remove this, what's left is // SSTables for which a TOC was not found. - co_await parallel_for_each_restricted(state.descriptors, [this, flags, &state] (std::tuple&& t) { + co_await directory.parallel_for_each_restricted(state.descriptors, [this, flags, &state, &directory] (std::tuple&& t) { auto& desc = std::get<1>(t); state.generations_found.erase(desc.generation); // This will try to pre-load this file and throw an exception if it is invalid - return process_descriptor(std::move(desc), flags); + return directory.process_descriptor(std::move(desc), flags); }); // For files missing TOC, it depends on where this is coming from. diff --git a/sstables/sstable_directory.hh b/sstables/sstable_directory.hh index a29fbdf5ff..7094d2044d 100644 --- a/sstables/sstable_directory.hh +++ b/sstables/sstable_directory.hh @@ -91,6 +91,8 @@ public: future get(); components_lister(std::filesystem::path dir); future<> close(); + + future<> process(sstable_directory& directory, fs::path location, process_flags flags); }; private: From 70d6bfc1097fae3b8a1c82d0792e67cb3018747e Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 13 Feb 2023 19:09:28 +0300 Subject: [PATCH 6/8] sstable_directory: Remove temporary aliases Previous patches created a bunch of local aliases-references in components_lister::process(). This patch just removes those aliases, no functional changes are made here. Signed-off-by: Pavel Emelyanov --- sstables/sstable_directory.cc | 39 ++++++++++++++++------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/sstables/sstable_directory.cc b/sstables/sstable_directory.cc index 6a12209187..430bc02447 100644 --- a/sstables/sstable_directory.cc +++ b/sstables/sstable_directory.cc @@ -183,25 +183,22 @@ future<> sstable_directory::components_lister::process(sstable_directory& direct // - If all shards scan in parallel, they can start loading sooner. That is faster than having // a separate step to fetch all files, followed by another step to distribute and process. - auto& sstable_dir_lister = *this; - auto& state = *sstable_dir_lister._state; - auto& _sstable_dir = location; std::exception_ptr ex; try { while (true) { - sstring name = co_await sstable_dir_lister.get(); + sstring name = co_await get(); if (name == "") { break; } - auto comps = sstables::entry_descriptor::make_descriptor(_sstable_dir.native(), name); - sstable_dir_lister.handle(std::move(comps), _sstable_dir / name); + auto comps = sstables::entry_descriptor::make_descriptor(location.native(), name); + handle(std::move(comps), location / name); } } catch (...) { ex = std::current_exception(); } - co_await sstable_dir_lister.close(); + co_await close(); if (ex) { - dirlog.debug("Could not process sstable directory {}: {}", _sstable_dir, ex); + dirlog.debug("Could not process sstable directory {}: {}", location, ex); // FIXME: waiting for https://github.com/scylladb/seastar/pull/1090 // co_await coroutine::return_exception(std::move(ex)); std::rethrow_exception(std::move(ex)); @@ -210,29 +207,29 @@ future<> sstable_directory::components_lister::process(sstable_directory& direct // Always okay to delete files with a temporary TOC. We want to do it before we process // the generations seen: it's okay to reuse those generations since the files will have // been deleted anyway. - for (auto& desc: state.temp_toc_found) { - auto range = state.generations_found.equal_range(desc.generation); + for (auto& desc: _state->temp_toc_found) { + auto range = _state->generations_found.equal_range(desc.generation); for (auto it = range.first; it != range.second; ++it) { auto& path = it->second; dirlog.trace("Scheduling to remove file {}, from an SSTable with a Temporary TOC", path.native()); - state.files_for_removal.insert(path.native()); + _state->files_for_removal.insert(path.native()); } - state.generations_found.erase(range.first, range.second); - state.descriptors.erase(desc.generation); + _state->generations_found.erase(range.first, range.second); + _state->descriptors.erase(desc.generation); } - directory._max_generation_seen = boost::accumulate(state.generations_found | boost::adaptors::map_keys, generation_from_value(0), [] (generation_type a, generation_type b) { + directory._max_generation_seen = boost::accumulate(_state->generations_found | boost::adaptors::map_keys, generation_from_value(0), [] (generation_type a, generation_type b) { return std::max(a, b); }); dirlog.debug("After {} scanned, seen generation {}. {} descriptors found, {} different files found ", - _sstable_dir, directory._max_generation_seen, state.descriptors.size(), state.generations_found.size()); + location, directory._max_generation_seen, _state->descriptors.size(), _state->generations_found.size()); // _descriptors is everything with a TOC. So after we remove this, what's left is // SSTables for which a TOC was not found. - co_await directory.parallel_for_each_restricted(state.descriptors, [this, flags, &state, &directory] (std::tuple&& t) { + co_await directory.parallel_for_each_restricted(_state->descriptors, [this, flags, &directory] (std::tuple&& t) { auto& desc = std::get<1>(t); - state.generations_found.erase(desc.generation); + _state->generations_found.erase(desc.generation); // This will try to pre-load this file and throw an exception if it is invalid return directory.process_descriptor(std::move(desc), flags); }); @@ -241,12 +238,12 @@ future<> sstable_directory::components_lister::process(sstable_directory& direct // If scylla was supposed to have generated this SSTable, this is not okay and // we refuse to proceed. If this coming from, say, an import, then we just delete, // log and proceed. - for (auto& path : state.generations_found | boost::adaptors::map_values) { + for (auto& path : _state->generations_found | boost::adaptors::map_values) { if (flags.throw_on_missing_toc) { - throw sstables::malformed_sstable_exception(format("At directory: {}: no TOC found for SSTable {}!. Refusing to boot", _sstable_dir.native(), path.native())); + throw sstables::malformed_sstable_exception(format("At directory: {}: no TOC found for SSTable {}!. Refusing to boot", location.native(), path.native())); } else { - dirlog.info("Found incomplete SSTable {} at directory {}. Removing", path.native(), _sstable_dir.native()); - state.files_for_removal.insert(path.native()); + dirlog.info("Found incomplete SSTable {} at directory {}. Removing", path.native(), location.native()); + _state->files_for_removal.insert(path.native()); } } } From e6941d0baae5d635fef20ef6ffd20a72509946ce Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 13 Feb 2023 19:11:14 +0300 Subject: [PATCH 7/8] sstable_directory: Move most of .commit_directory_changes() on lister Committing any changes made while scanning the storage is storage-specific. Just like .process() was moved on lister, the .commit() now does the same. Signed-off-by: Pavel Emelyanov --- sstables/sstable_directory.cc | 11 ++++++----- sstables/sstable_directory.hh | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/sstables/sstable_directory.cc b/sstables/sstable_directory.cc index 430bc02447..916752f84f 100644 --- a/sstables/sstable_directory.cc +++ b/sstables/sstable_directory.cc @@ -248,12 +248,13 @@ future<> sstable_directory::components_lister::process(sstable_directory& direct } } -future<> -sstable_directory::commit_directory_changes() { - auto files_for_removal = std::exchange(_lister->_state->files_for_removal, {}); - _lister.reset(); +future<> sstable_directory::commit_directory_changes() { + return _lister->commit().finally([x = std::move(_lister)] {}); +} + +future<> sstable_directory::components_lister::commit() { // Remove all files scheduled for removal - return parallel_for_each(std::move(files_for_removal), [] (sstring path) { + return parallel_for_each(std::exchange(_state->files_for_removal, {}), [] (sstring path) { dirlog.info("Removing file {}", path); return remove_file(std::move(path)); }); diff --git a/sstables/sstable_directory.hh b/sstables/sstable_directory.hh index 7094d2044d..263671b4e9 100644 --- a/sstables/sstable_directory.hh +++ b/sstables/sstable_directory.hh @@ -93,6 +93,7 @@ public: future<> close(); future<> process(sstable_directory& directory, fs::path location, process_flags flags); + future<> commit(); }; private: From 398f7704dc44d788c9a83c031dd5dcaf6a28e55e Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 13 Feb 2023 19:12:18 +0300 Subject: [PATCH 8/8] sstable_directory: Keep lister internals private Now the lister procides two-calls API to the user -- process and commit. The rest can and should be marked as private. Signed-off-by: Pavel Emelyanov --- sstables/sstable_directory.hh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sstables/sstable_directory.hh b/sstables/sstable_directory.hh index 263671b4e9..eed7af55bf 100644 --- a/sstables/sstable_directory.hh +++ b/sstables/sstable_directory.hh @@ -65,7 +65,6 @@ public: }; class components_lister { - public: struct scan_state { using scan_multimap = std::unordered_multimap; using scan_descriptors = utils::chunked_vector; @@ -87,11 +86,12 @@ public: directory_lister _lister; std::unique_ptr _state; - public: future get(); - components_lister(std::filesystem::path dir); future<> close(); + public: + components_lister(std::filesystem::path dir); + future<> process(sstable_directory& directory, fs::path location, process_flags flags); future<> commit(); };