From 82ef2a77307f7346f190825222460c7def359cec Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Tue, 10 Dec 2019 16:03:35 +0300 Subject: [PATCH 1/8] file_lock: Work with fs::path, not sstring The main.cc code that converts sstring to fs::path will be patched soon, the file_desc::open belongs to seastar and works on sstrings. Signed-off-by: Pavel Emelyanov --- main.cc | 2 +- utils/file_lock.cc | 19 +++++++++---------- utils/file_lock.hh | 11 +++++++---- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/main.cc b/main.cc index 063ccf08b4..6ceca35ee7 100644 --- a/main.cc +++ b/main.cc @@ -229,7 +229,7 @@ public: return io_check([path] { return recursive_touch_directory(path); }).then_wrapped([this, path] (future<> f) { try { f.get(); - return utils::file_lock::acquire(path + "/.lock").then([this](utils::file_lock lock) { + return utils::file_lock::acquire(fs::path(path) / ".lock").then([this](utils::file_lock lock) { _locks.emplace_back(std::move(lock)); }).handle_exception([path](auto ep) { // only do this because "normal" unhandled exception exit in seastar diff --git a/utils/file_lock.cc b/utils/file_lock.cc index b8df3654a0..b8102e3071 100644 --- a/utils/file_lock.cc +++ b/utils/file_lock.cc @@ -29,12 +29,13 @@ class utils::file_lock::impl { public: - impl(sstring path) + impl(fs::path path) : _path(std::move(path)), _fd( - file_desc::open(_path, O_RDWR | O_CREAT | O_CLOEXEC, + file_desc::open(_path.native(), O_RDWR | O_CREAT | O_CLOEXEC, S_IRWXU)) { if (::lockf(_fd.get(), F_TLOCK, 0) != 0) { - throw std::system_error(errno, std::system_category(), "Could not acquire lock: " + _path); + throw std::system_error(errno, std::system_category(), + "Could not acquire lock: " + _path.native()); } } impl(impl&&) = default; @@ -46,13 +47,11 @@ public: auto r = ::lockf(_fd.get(), F_ULOCK, 0); assert(r == 0); } - sstring - _path; - file_desc - _fd; + fs::path _path; + file_desc _fd; }; -utils::file_lock::file_lock(sstring path) +utils::file_lock::file_lock(fs::path path) : _impl(std::make_unique(std::move(path))) {} @@ -63,11 +62,11 @@ utils::file_lock::file_lock(file_lock&& f) noexcept utils::file_lock::~file_lock() {} -sstring utils::file_lock::path() const { +fs::path utils::file_lock::path() const { return _impl ? _impl->_path : ""; } -future utils::file_lock::acquire(sstring path) { +future utils::file_lock::acquire(fs::path path) { // meh. not really any future stuff here. but pretend, for the // day when a future version of lock etc is added. try { diff --git a/utils/file_lock.hh b/utils/file_lock.hh index 73b612796a..ec885d7da8 100644 --- a/utils/file_lock.hh +++ b/utils/file_lock.hh @@ -23,11 +23,14 @@ #include #include +#include #include #include #include "seastarx.hh" +namespace fs = std::filesystem; + namespace utils { class file_lock { public: @@ -38,15 +41,15 @@ namespace utils { file_lock& operator=(file_lock&&) = default; - static future acquire(sstring); + static future acquire(fs::path); - sstring path() const; + fs::path path() const; sstring to_string() const { - return path(); + return path().native(); } private: class impl; - file_lock(sstring); + file_lock(fs::path); std::unique_ptr _impl; }; From f2b3c17e664d997263dff8e47b2ae8328f6a1f28 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Tue, 10 Dec 2019 15:54:31 +0300 Subject: [PATCH 2/8] directories: Move all the dirs code into .init method The seastar::async usage is tempoarary, added for bisect-safety, soon it will go away. For this reason the indentation in the .init method is not "canonical", but is prepared for one-patch drop of the seastar::async. The hinted_handoff_enabled arg is there, as it's not just a parameter on config, it had been parsed in main.cc. Signed-off-by: Pavel Emelyanov --- main.cc | 86 ++++++++++++++++++++++++++++++++------------------------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/main.cc b/main.cc index 6ceca35ee7..55ed4cd1fb 100644 --- a/main.cc +++ b/main.cc @@ -260,11 +260,58 @@ public: future<> touch_and_lock(_Range&& r) { return touch_and_lock(std::begin(r), std::end(r)); } + + future<> init(db::config& cfg, bool hinted_handoff_enabled); private: std::vector _locks; }; +future<> directories::init(db::config& cfg, bool hinted_handoff_enabled) { + // XXX -- this indentation is temporary, wil go away with next patches + return seastar::async([&] { + supervisor::notify("creating data directories"); + touch_and_lock(cfg.data_file_directories()).get(); + supervisor::notify("creating commitlog directory"); + touch_and_lock(cfg.commitlog_directory()).get(); + std::unordered_set directories; + directories.insert(cfg.data_file_directories().cbegin(), + cfg.data_file_directories().cend()); + directories.insert(cfg.commitlog_directory()); + + supervisor::notify("creating hints directories"); + if (hinted_handoff_enabled) { + fs::path hints_base_dir(cfg.hints_directory()); + touch_and_lock(cfg.hints_directory()).get(); + directories.insert(cfg.hints_directory()); + for (unsigned i = 0; i < smp::count; ++i) { + sstring shard_dir((hints_base_dir / seastar::to_sstring(i).c_str()).native()); + touch_and_lock(shard_dir).get(); + directories.insert(std::move(shard_dir)); + } + } + fs::path view_pending_updates_base_dir = fs::path(cfg.view_hints_directory()); + sstring view_pending_updates_base_dir_str = view_pending_updates_base_dir.native(); + touch_and_lock(view_pending_updates_base_dir_str).get(); + directories.insert(view_pending_updates_base_dir_str); + for (unsigned i = 0; i < smp::count; ++i) { + sstring shard_dir((view_pending_updates_base_dir / seastar::to_sstring(i).c_str()).native()); + touch_and_lock(shard_dir).get(); + directories.insert(std::move(shard_dir)); + } + + supervisor::notify("verifying directories"); + parallel_for_each(directories, [&cfg] (sstring pathname) { + return disk_sanity(pathname, cfg.developer_mode()).then([dir = std::move(pathname)] { + return distributed_loader::verify_owner_and_mode(fs::path(dir)).handle_exception([](auto ep) { + startlog.error("Failed owner and mode verification: {}", ep); + return make_exception_future<>(ep); + }); + }); + }).get(); + }); +} + static void verify_rlimit(bool developer_mode) { @@ -750,45 +797,8 @@ int main(int ac, char** av) { }); verify_seastar_io_scheduler(opts.count("max-io-requests"), opts.count("io-properties") || opts.count("io-properties-file"), cfg->developer_mode()).get(); - supervisor::notify("creating data directories"); - dirs.touch_and_lock(db.local().get_config().data_file_directories()).get(); - supervisor::notify("creating commitlog directory"); - dirs.touch_and_lock(db.local().get_config().commitlog_directory()).get(); - std::unordered_set directories; - directories.insert(db.local().get_config().data_file_directories().cbegin(), - db.local().get_config().data_file_directories().cend()); - directories.insert(db.local().get_config().commitlog_directory()); - supervisor::notify("creating hints directories"); - if (hinted_handoff_enabled) { - fs::path hints_base_dir(db.local().get_config().hints_directory()); - dirs.touch_and_lock(db.local().get_config().hints_directory()).get(); - directories.insert(db.local().get_config().hints_directory()); - for (unsigned i = 0; i < smp::count; ++i) { - sstring shard_dir((hints_base_dir / seastar::to_sstring(i).c_str()).native()); - dirs.touch_and_lock(shard_dir).get(); - directories.insert(std::move(shard_dir)); - } - } - fs::path view_pending_updates_base_dir = fs::path(db.local().get_config().view_hints_directory()); - sstring view_pending_updates_base_dir_str = view_pending_updates_base_dir.native(); - dirs.touch_and_lock(view_pending_updates_base_dir_str).get(); - directories.insert(view_pending_updates_base_dir_str); - for (unsigned i = 0; i < smp::count; ++i) { - sstring shard_dir((view_pending_updates_base_dir / seastar::to_sstring(i).c_str()).native()); - dirs.touch_and_lock(shard_dir).get(); - directories.insert(std::move(shard_dir)); - } - - supervisor::notify("verifying directories"); - parallel_for_each(directories, [&db] (sstring pathname) { - return disk_sanity(pathname, db.local().get_config().developer_mode()).then([dir = std::move(pathname)] { - return distributed_loader::verify_owner_and_mode(fs::path(dir)).handle_exception([](auto ep) { - startlog.error("Failed owner and mode verification: {}", ep); - return make_exception_future<>(ep); - }); - }); - }).get(); + dirs.init(*cfg, bool(hinted_handoff_enabled)).get(); // Initialization of a keyspace is done by shard 0 only. For system // keyspace, the procedure will go through the hardcoded column From 71a528d404a0682d79c3213e5d7c4ebfc810c5a7 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Tue, 10 Dec 2019 15:57:10 +0300 Subject: [PATCH 3/8] directories: Move the whole stuff into own .cc file In order not to pollute the root dir place the code in utils/ directory, "utils" namespace. While doing this -- move the touch_and_lock from the class declaration. Signed-off-by: Pavel Emelyanov --- configure.py | 1 + main.cc | 102 +-------------------------------------- utils/directories.cc | 110 +++++++++++++++++++++++++++++++++++++++++++ utils/directories.hh | 53 +++++++++++++++++++++ 4 files changed, 166 insertions(+), 100 deletions(-) create mode 100644 utils/directories.cc create mode 100644 utils/directories.hh diff --git a/configure.py b/configure.py index 1f907c419e..16148e9d8a 100755 --- a/configure.py +++ b/configure.py @@ -495,6 +495,7 @@ scylla_core = (['database.cc', 'utils/buffer_input_stream.cc', 'utils/limiting_data_source.cc', 'utils/updateable_value.cc', + 'utils/directories.cc', 'mutation_partition.cc', 'mutation_partition_view.cc', 'mutation_partition_serializer.cc', diff --git a/main.cc b/main.cc index 55ed4cd1fb..82fc5e041e 100644 --- a/main.cc +++ b/main.cc @@ -42,8 +42,8 @@ #include "db/commitlog/commitlog_replayer.hh" #include "db/view/view_builder.hh" #include "utils/runtime.hh" -#include "utils/file_lock.hh" #include "log.hh" +#include "utils/directories.hh" #include "debug.hh" #include "init.hh" #include "release.hh" @@ -214,104 +214,6 @@ public: } }; -static future<> disk_sanity(sstring path, bool developer_mode) { - return check_direct_io_support(path).then([] { - return make_ready_future<>(); - }).handle_exception([path](auto ep) { - startlog.error("Could not access {}: {}", path, ep); - return make_exception_future<>(ep); - }); -}; - -class directories { -public: - future<> touch_and_lock(sstring path) { - return io_check([path] { return recursive_touch_directory(path); }).then_wrapped([this, path] (future<> f) { - try { - f.get(); - return utils::file_lock::acquire(fs::path(path) / ".lock").then([this](utils::file_lock lock) { - _locks.emplace_back(std::move(lock)); - }).handle_exception([path](auto ep) { - // only do this because "normal" unhandled exception exit in seastar - // _drops_ system_error message ("what()") and thus does not quite deliver - // the relevant info to the user - try { - std::rethrow_exception(ep); - } catch (std::exception& e) { - startlog.error("Could not initialize {}: {}", path, e.what()); - throw; - } catch (...) { - throw; - } - }); - } catch (...) { - startlog.error("Directory '{}' cannot be initialized. Tried to do it but failed with: {}", path, std::current_exception()); - throw; - } - }); - } - template - future<> touch_and_lock(_Iter i, _Iter e) { - return parallel_for_each(i, e, [this](sstring path) { - return touch_and_lock(std::move(path)); - }); - } - template - future<> touch_and_lock(_Range&& r) { - return touch_and_lock(std::begin(r), std::end(r)); - } - - future<> init(db::config& cfg, bool hinted_handoff_enabled); -private: - std::vector - _locks; -}; - -future<> directories::init(db::config& cfg, bool hinted_handoff_enabled) { - // XXX -- this indentation is temporary, wil go away with next patches - return seastar::async([&] { - supervisor::notify("creating data directories"); - touch_and_lock(cfg.data_file_directories()).get(); - supervisor::notify("creating commitlog directory"); - touch_and_lock(cfg.commitlog_directory()).get(); - std::unordered_set directories; - directories.insert(cfg.data_file_directories().cbegin(), - cfg.data_file_directories().cend()); - directories.insert(cfg.commitlog_directory()); - - supervisor::notify("creating hints directories"); - if (hinted_handoff_enabled) { - fs::path hints_base_dir(cfg.hints_directory()); - touch_and_lock(cfg.hints_directory()).get(); - directories.insert(cfg.hints_directory()); - for (unsigned i = 0; i < smp::count; ++i) { - sstring shard_dir((hints_base_dir / seastar::to_sstring(i).c_str()).native()); - touch_and_lock(shard_dir).get(); - directories.insert(std::move(shard_dir)); - } - } - fs::path view_pending_updates_base_dir = fs::path(cfg.view_hints_directory()); - sstring view_pending_updates_base_dir_str = view_pending_updates_base_dir.native(); - touch_and_lock(view_pending_updates_base_dir_str).get(); - directories.insert(view_pending_updates_base_dir_str); - for (unsigned i = 0; i < smp::count; ++i) { - sstring shard_dir((view_pending_updates_base_dir / seastar::to_sstring(i).c_str()).native()); - touch_and_lock(shard_dir).get(); - directories.insert(std::move(shard_dir)); - } - - supervisor::notify("verifying directories"); - parallel_for_each(directories, [&cfg] (sstring pathname) { - return disk_sanity(pathname, cfg.developer_mode()).then([dir = std::move(pathname)] { - return distributed_loader::verify_owner_and_mode(fs::path(dir)).handle_exception([](auto ep) { - startlog.error("Failed owner and mode verification: {}", ep); - return make_exception_future<>(ep); - }); - }); - }).get(); - }); -} - static void verify_rlimit(bool developer_mode) { @@ -551,7 +453,7 @@ int main(int ac, char** av) { auto& mm = service::get_migration_manager(); api::http_context ctx(db, proxy); httpd::http_server_control prometheus_server; - directories dirs; + utils::directories dirs; sharded feature_service; return app.run(ac, av, [&] () -> future { diff --git a/utils/directories.cc b/utils/directories.cc new file mode 100644 index 0000000000..bba9bcc143 --- /dev/null +++ b/utils/directories.cc @@ -0,0 +1,110 @@ +/* + * Copyright (C) 2019 ScyllaDB + */ + +/* + * This file is part of Scylla. + * + * Scylla is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Scylla is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Scylla. If not, see . + */ + +#include "init.hh" +#include "supervisor.hh" +#include "directories.hh" +#include "distributed_loader.hh" +#include "disk-error-handler.hh" + +namespace utils { + +static future<> disk_sanity(sstring path, bool developer_mode) { + return check_direct_io_support(path).then([] { + return make_ready_future<>(); + }).handle_exception([path](auto ep) { + startlog.error("Could not access {}: {}", path, ep); + return make_exception_future<>(ep); + }); +}; + +future<> directories::touch_and_lock(sstring path) { + return io_check([path] { return recursive_touch_directory(path); }).then_wrapped([this, path] (future<> f) { + try { + f.get(); + return file_lock::acquire(fs::path(path) / ".lock").then([this](file_lock lock) { + _locks.emplace_back(std::move(lock)); + }).handle_exception([path](auto ep) { + // only do this because "normal" unhandled exception exit in seastar + // _drops_ system_error message ("what()") and thus does not quite deliver + // the relevant info to the user + try { + std::rethrow_exception(ep); + } catch (std::exception& e) { + startlog.error("Could not initialize {}: {}", path, e.what()); + throw; + } catch (...) { + throw; + } + }); + } catch (...) { + startlog.error("Directory '{}' cannot be initialized. Tried to do it but failed with: {}", path, std::current_exception()); + throw; + } + }); +} + +future<> directories::init(db::config& cfg, bool hinted_handoff_enabled) { + // XXX -- this indentation is temporary, wil go away with next patches + return seastar::async([&] { + supervisor::notify("creating data directories"); + touch_and_lock(cfg.data_file_directories()).get(); + supervisor::notify("creating commitlog directory"); + touch_and_lock(cfg.commitlog_directory()).get(); + std::unordered_set directories; + directories.insert(cfg.data_file_directories().cbegin(), + cfg.data_file_directories().cend()); + directories.insert(cfg.commitlog_directory()); + + supervisor::notify("creating hints directories"); + if (hinted_handoff_enabled) { + fs::path hints_base_dir(cfg.hints_directory()); + touch_and_lock(cfg.hints_directory()).get(); + directories.insert(cfg.hints_directory()); + for (unsigned i = 0; i < smp::count; ++i) { + sstring shard_dir((hints_base_dir / seastar::to_sstring(i).c_str()).native()); + touch_and_lock(shard_dir).get(); + directories.insert(std::move(shard_dir)); + } + } + fs::path view_pending_updates_base_dir = fs::path(cfg.view_hints_directory()); + sstring view_pending_updates_base_dir_str = view_pending_updates_base_dir.native(); + touch_and_lock(view_pending_updates_base_dir_str).get(); + directories.insert(view_pending_updates_base_dir_str); + for (unsigned i = 0; i < smp::count; ++i) { + sstring shard_dir((view_pending_updates_base_dir / seastar::to_sstring(i).c_str()).native()); + touch_and_lock(shard_dir).get(); + directories.insert(std::move(shard_dir)); + } + + supervisor::notify("verifying directories"); + parallel_for_each(directories, [&cfg] (sstring pathname) { + return disk_sanity(pathname, cfg.developer_mode()).then([dir = std::move(pathname)] { + return distributed_loader::verify_owner_and_mode(fs::path(dir)).handle_exception([](auto ep) { + startlog.error("Failed owner and mode verification: {}", ep); + return make_exception_future<>(ep); + }); + }); + }).get(); + }); +} + +} // namespace utils diff --git a/utils/directories.hh b/utils/directories.hh new file mode 100644 index 0000000000..b74fc55133 --- /dev/null +++ b/utils/directories.hh @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2019 ScyllaDB + */ + +/* + * This file is part of Scylla. + * + * Scylla is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Scylla is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Scylla. If not, see . + */ + +#pragma once + +#include +#include +#include +#include "utils/file_lock.hh" + +using namespace seastar; + +namespace utils { + +class directories { +public: + future<> touch_and_lock(sstring path); + + template + future<> touch_and_lock(_Iter i, _Iter e) { + return parallel_for_each(i, e, [this](sstring path) { + return touch_and_lock(std::move(path)); + }); + } + template + future<> touch_and_lock(_Range&& r) { + return touch_and_lock(std::begin(r), std::end(r)); + } + + future<> init(db::config& cfg, bool hinted_handoff_enabled); +private: + std::vector _locks; +}; + +} // namespace utils From 8d0c820aa19c6bcb5a7f05c656e0f43c1cb44fa8 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Tue, 10 Dec 2019 16:11:22 +0300 Subject: [PATCH 4/8] directories: Do touch_and_lock in parallel The list of paths that should be touch-and-locked is already at hands, this shortens the code and makes it slightly faster (in theory). Signed-off-by: Pavel Emelyanov --- utils/directories.cc | 14 +++++--------- utils/directories.hh | 14 +------------- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/utils/directories.cc b/utils/directories.cc index bba9bcc143..37ebbd4c8c 100644 --- a/utils/directories.cc +++ b/utils/directories.cc @@ -65,36 +65,32 @@ future<> directories::touch_and_lock(sstring path) { future<> directories::init(db::config& cfg, bool hinted_handoff_enabled) { // XXX -- this indentation is temporary, wil go away with next patches return seastar::async([&] { - supervisor::notify("creating data directories"); - touch_and_lock(cfg.data_file_directories()).get(); - supervisor::notify("creating commitlog directory"); - touch_and_lock(cfg.commitlog_directory()).get(); std::unordered_set directories; directories.insert(cfg.data_file_directories().cbegin(), cfg.data_file_directories().cend()); directories.insert(cfg.commitlog_directory()); - supervisor::notify("creating hints directories"); if (hinted_handoff_enabled) { fs::path hints_base_dir(cfg.hints_directory()); - touch_and_lock(cfg.hints_directory()).get(); directories.insert(cfg.hints_directory()); for (unsigned i = 0; i < smp::count; ++i) { sstring shard_dir((hints_base_dir / seastar::to_sstring(i).c_str()).native()); - touch_and_lock(shard_dir).get(); directories.insert(std::move(shard_dir)); } } fs::path view_pending_updates_base_dir = fs::path(cfg.view_hints_directory()); sstring view_pending_updates_base_dir_str = view_pending_updates_base_dir.native(); - touch_and_lock(view_pending_updates_base_dir_str).get(); directories.insert(view_pending_updates_base_dir_str); for (unsigned i = 0; i < smp::count; ++i) { sstring shard_dir((view_pending_updates_base_dir / seastar::to_sstring(i).c_str()).native()); - touch_and_lock(shard_dir).get(); directories.insert(std::move(shard_dir)); } + supervisor::notify("creating directories"); + parallel_for_each(directories, [this] (sstring path) { + return touch_and_lock(path); + }).get(); + supervisor::notify("verifying directories"); parallel_for_each(directories, [&cfg] (sstring pathname) { return disk_sanity(pathname, cfg.developer_mode()).then([dir = std::move(pathname)] { diff --git a/utils/directories.hh b/utils/directories.hh index b74fc55133..2b84a60201 100644 --- a/utils/directories.hh +++ b/utils/directories.hh @@ -32,21 +32,9 @@ namespace utils { class directories { public: - future<> touch_and_lock(sstring path); - - template - future<> touch_and_lock(_Iter i, _Iter e) { - return parallel_for_each(i, e, [this](sstring path) { - return touch_and_lock(std::move(path)); - }); - } - template - future<> touch_and_lock(_Range&& r) { - return touch_and_lock(std::begin(r), std::end(r)); - } - future<> init(db::config& cfg, bool hinted_handoff_enabled); private: + future<> touch_and_lock(sstring path); std::vector _locks; }; From 06f4f3e6d8e17a1feb78481933433b44c92f68f5 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Tue, 10 Dec 2019 16:18:39 +0300 Subject: [PATCH 5/8] directories: Do touch_and_lock and verify sequentially The goal is to drop the seastar::async() usage. Currently we have two places that return futures -- calls to parallel_for_each-s. We can either chain them together or, since both are working on the same set of directories, chain actions inside them. For code simplicity I propose to chain actions. Signed-off-by: Pavel Emelyanov --- utils/directories.cc | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/utils/directories.cc b/utils/directories.cc index 37ebbd4c8c..3a3281a278 100644 --- a/utils/directories.cc +++ b/utils/directories.cc @@ -86,17 +86,14 @@ future<> directories::init(db::config& cfg, bool hinted_handoff_enabled) { directories.insert(std::move(shard_dir)); } - supervisor::notify("creating directories"); - parallel_for_each(directories, [this] (sstring path) { - return touch_and_lock(path); - }).get(); - - supervisor::notify("verifying directories"); - parallel_for_each(directories, [&cfg] (sstring pathname) { - return disk_sanity(pathname, cfg.developer_mode()).then([dir = std::move(pathname)] { - return distributed_loader::verify_owner_and_mode(fs::path(dir)).handle_exception([](auto ep) { - startlog.error("Failed owner and mode verification: {}", ep); - return make_exception_future<>(ep); + supervisor::notify("creating and verifying directories"); + parallel_for_each(directories, [this, &cfg] (sstring path) { + return touch_and_lock(path).then([path = std::move(path), &cfg] { + return disk_sanity(path, cfg.developer_mode()).then([path = std::move(path)] { + return distributed_loader::verify_owner_and_mode(fs::path(path)).handle_exception([](auto ep) { + startlog.error("Failed owner and mode verification: {}", ep); + return make_exception_future<>(ep); + }); }); }); }).get(); From 14437da76985555c9e4a11e504abefab7db94269 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Tue, 10 Dec 2019 16:19:43 +0300 Subject: [PATCH 6/8] directories: Drop seastar::async usage Now the only future-able operation remained is the call to parallel_for_each(), all the rest is non-blocking preparation, so we can drop the seastar::async and just return the future from parallel_for_each. The indendation is now good, as in previous patch is was prepared just for that. Signed-off-by: Pavel Emelyanov --- utils/directories.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/utils/directories.cc b/utils/directories.cc index 3a3281a278..48331e1a0e 100644 --- a/utils/directories.cc +++ b/utils/directories.cc @@ -63,8 +63,6 @@ future<> directories::touch_and_lock(sstring path) { } future<> directories::init(db::config& cfg, bool hinted_handoff_enabled) { - // XXX -- this indentation is temporary, wil go away with next patches - return seastar::async([&] { std::unordered_set directories; directories.insert(cfg.data_file_directories().cbegin(), cfg.data_file_directories().cend()); @@ -87,7 +85,7 @@ future<> directories::init(db::config& cfg, bool hinted_handoff_enabled) { } supervisor::notify("creating and verifying directories"); - parallel_for_each(directories, [this, &cfg] (sstring path) { + return parallel_for_each(directories, [this, &cfg] (sstring path) { return touch_and_lock(path).then([path = std::move(path), &cfg] { return disk_sanity(path, cfg.developer_mode()).then([path = std::move(path)] { return distributed_loader::verify_owner_and_mode(fs::path(path)).handle_exception([](auto ep) { @@ -96,8 +94,7 @@ future<> directories::init(db::config& cfg, bool hinted_handoff_enabled) { }); }); }); - }).get(); - }); + }); } } // namespace utils From 373fcfdb3e519263902f9e355d514bd834da1e1b Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Tue, 10 Dec 2019 16:38:24 +0300 Subject: [PATCH 7/8] directories: Cleanup adding dirs to the vector to work on The unordered_set is turned into vector since for fs::path there's no hash() method that's needed for set. Signed-off-by: Pavel Emelyanov --- utils/directories.cc | 50 +++++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/utils/directories.cc b/utils/directories.cc index 48331e1a0e..eb2664cd80 100644 --- a/utils/directories.cc +++ b/utils/directories.cc @@ -62,30 +62,42 @@ future<> directories::touch_and_lock(sstring path) { }); } -future<> directories::init(db::config& cfg, bool hinted_handoff_enabled) { - std::unordered_set directories; - directories.insert(cfg.data_file_directories().cbegin(), - cfg.data_file_directories().cend()); - directories.insert(cfg.commitlog_directory()); +static void add(fs::path path, std::vector& to) { + to.push_back(path); +} +static void add(sstring path, std::vector& to) { + add(fs::path(path), to); +} + +static void add(std::vector paths, std::vector& to) { + for (auto& path : paths) { + add(path, to); + } +} + +static void add_sharded(sstring p, std::vector& to) { + fs::path path(p); + + add(path, to); + for (unsigned i = 0; i < smp::count; i++) { + add(path / seastar::to_sstring(i).c_str(), to); + } +} + +future<> directories::init(db::config& cfg, bool hinted_handoff_enabled) { + std::vector paths; + + add(cfg.data_file_directories(), paths); + add(cfg.commitlog_directory(), paths); if (hinted_handoff_enabled) { - fs::path hints_base_dir(cfg.hints_directory()); - directories.insert(cfg.hints_directory()); - for (unsigned i = 0; i < smp::count; ++i) { - sstring shard_dir((hints_base_dir / seastar::to_sstring(i).c_str()).native()); - directories.insert(std::move(shard_dir)); - } - } - fs::path view_pending_updates_base_dir = fs::path(cfg.view_hints_directory()); - sstring view_pending_updates_base_dir_str = view_pending_updates_base_dir.native(); - directories.insert(view_pending_updates_base_dir_str); - for (unsigned i = 0; i < smp::count; ++i) { - sstring shard_dir((view_pending_updates_base_dir / seastar::to_sstring(i).c_str()).native()); - directories.insert(std::move(shard_dir)); + add_sharded(cfg.hints_directory(), paths); } + add_sharded(cfg.view_hints_directory(), paths); supervisor::notify("creating and verifying directories"); - return parallel_for_each(directories, [this, &cfg] (sstring path) { + return parallel_for_each(paths, [this, &cfg] (fs::path p) { + sstring path = p.native(); return touch_and_lock(path).then([path = std::move(path), &cfg] { return disk_sanity(path, cfg.developer_mode()).then([path = std::move(path)] { return distributed_loader::verify_owner_and_mode(fs::path(path)).handle_exception([](auto ep) { From 23a8d32920d42588d4532b3a53b95546f0bd7d47 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Tue, 10 Dec 2019 16:51:04 +0300 Subject: [PATCH 8/8] directories: Make internals work on fs::path Signed-off-by: Pavel Emelyanov --- utils/directories.cc | 15 +++++++-------- utils/directories.hh | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/utils/directories.cc b/utils/directories.cc index eb2664cd80..925a66b5b5 100644 --- a/utils/directories.cc +++ b/utils/directories.cc @@ -27,8 +27,8 @@ namespace utils { -static future<> disk_sanity(sstring path, bool developer_mode) { - return check_direct_io_support(path).then([] { +static future<> disk_sanity(fs::path path, bool developer_mode) { + return check_direct_io_support(path.native()).then([] { return make_ready_future<>(); }).handle_exception([path](auto ep) { startlog.error("Could not access {}: {}", path, ep); @@ -36,11 +36,11 @@ static future<> disk_sanity(sstring path, bool developer_mode) { }); }; -future<> directories::touch_and_lock(sstring path) { - return io_check([path] { return recursive_touch_directory(path); }).then_wrapped([this, path] (future<> f) { +future<> directories::touch_and_lock(fs::path path) { + return io_check([path] { return recursive_touch_directory(path.native()); }).then_wrapped([this, path] (future<> f) { try { f.get(); - return file_lock::acquire(fs::path(path) / ".lock").then([this](file_lock lock) { + return file_lock::acquire(path / ".lock").then([this](file_lock lock) { _locks.emplace_back(std::move(lock)); }).handle_exception([path](auto ep) { // only do this because "normal" unhandled exception exit in seastar @@ -96,11 +96,10 @@ future<> directories::init(db::config& cfg, bool hinted_handoff_enabled) { add_sharded(cfg.view_hints_directory(), paths); supervisor::notify("creating and verifying directories"); - return parallel_for_each(paths, [this, &cfg] (fs::path p) { - sstring path = p.native(); + return parallel_for_each(paths, [this, &cfg] (fs::path path) { return touch_and_lock(path).then([path = std::move(path), &cfg] { return disk_sanity(path, cfg.developer_mode()).then([path = std::move(path)] { - return distributed_loader::verify_owner_and_mode(fs::path(path)).handle_exception([](auto ep) { + return distributed_loader::verify_owner_and_mode(path).handle_exception([](auto ep) { startlog.error("Failed owner and mode verification: {}", ep); return make_exception_future<>(ep); }); diff --git a/utils/directories.hh b/utils/directories.hh index 2b84a60201..dc4b690e2d 100644 --- a/utils/directories.hh +++ b/utils/directories.hh @@ -34,7 +34,7 @@ class directories { public: future<> init(db::config& cfg, bool hinted_handoff_enabled); private: - future<> touch_and_lock(sstring path); + future<> touch_and_lock(fs::path path); std::vector _locks; };