Allow utils::directories to provide paths to dirs

This change extends utils::directories class in the following way: - adds new member variables that correspond to fields from db::config that describe paths of directories - introduces a public interface to retrieve the values of the new members - allows construction of utils::directories object based on db::config to setup internal member variables related to paths to dirs The new members of utils::directories are overriden when the provided values are empty. The way of setting paths is taken from db::config. To ensure that the new logic works correctly `utils_directories_test` has been created. Refs: scylladb#5626 Signed-off-by: Patryk Wrobel <patryk.wrobel@scylladb.com>
2024-01-18 17:43:50 +01:00
parent 1b0ccaf4f2
commit 1cd676e438
6 changed files with 343 additions and 5 deletions
--- a/configure.py
+++ b/configure.py
@@ -596,6 +596,7 @@ scylla_tests = set([
    'test/boost/user_function_test',
    'test/boost/user_types_test',
    'test/boost/utf8_test',
+    'test/boost/utils_directories_test',
    'test/boost/view_build_test',
    'test/boost/view_complex_test',
    'test/boost/view_schema_test',
--- a/main.cc
+++ b/main.cc
@@ -724,6 +724,7 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl
            });

            cfg->setup_directories();
+            dirs.emplace(*cfg);

            // We're writing to a non-atomic variable here. But bool writes are atomic
            // in all supported architectures, and the broadcast_to_all_shards().get() below
@@ -972,7 +973,6 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl
            dir_set.add(cfg->data_file_directories());
            dir_set.add(cfg->commitlog_directory());
            dir_set.add(cfg->schema_commitlog_directory());
-            dirs.emplace(cfg->developer_mode());
            dirs->create_and_verify(std::move(dir_set)).get();

            auto hints_dir_initializer = db::hints::directory_initializer::make(*dirs, cfg->hints_directory()).get();
--- a/test/boost/CMakeLists.txt
+++ b/test/boost/CMakeLists.txt
@@ -331,6 +331,8 @@ add_scylla_test(user_function_test
  LIBRARIES idl)
 add_scylla_test(user_types_test
  KIND SEASTAR)
+add_scylla_test(utils_directories_test
+  KIND BOOST)
 add_scylla_test(UUID_test
  KIND BOOST)
 add_scylla_test(view_build_test
--- a/test/boost/utils_directories_test.cc
+++ b/test/boost/utils_directories_test.cc
@@ -0,0 +1,230 @@
+/*
+ * Copyright (C) 2024-present ScyllaDB
+ */
+
+/*
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+#define BOOST_TEST_MODULE utils_directories_test
+
+#include <boost/test/unit_test.hpp>
+
+#include "db/config.hh"
+#include "utils/directories.hh"
+
+BOOST_AUTO_TEST_CASE(construction_from_config_with_default_values)
+{
+    // Given default constructed configuration.
+    db::config cfg{};
+
+    // When constructing directories object.
+    utils::directories dirs{cfg};
+
+    // Then paths point to a default workspace.
+    BOOST_CHECK_EQUAL("/var/lib/scylla", dirs.get_work_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/commitlog", dirs.get_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/commitlog/schema", dirs.get_schema_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/hints", dirs.get_hints_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/view_hints", dirs.get_view_hints_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/saved_caches", dirs.get_saved_caches_dir());
+
+    const auto data_file_dirs = dirs.get_data_file_dirs();
+    BOOST_REQUIRE_EQUAL(1u, data_file_dirs.size());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/data", data_file_dirs.at(0));
+}
+
+BOOST_AUTO_TEST_CASE(construction_from_config_with_custom_workdir)
+{
+    // Given a configuration with custom workspace directory.
+    db::config cfg{};
+    cfg.work_directory("/var/special/my_custom_work_dir");
+
+    // When constructing directories object.
+    utils::directories dirs{cfg};
+
+    // Then paths point to a custom workspace.
+    BOOST_CHECK_EQUAL("/var/special/my_custom_work_dir", dirs.get_work_dir());
+    BOOST_CHECK_EQUAL("/var/special/my_custom_work_dir/commitlog", dirs.get_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/special/my_custom_work_dir/commitlog/schema", dirs.get_schema_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/special/my_custom_work_dir/hints", dirs.get_hints_dir());
+    BOOST_CHECK_EQUAL("/var/special/my_custom_work_dir/view_hints", dirs.get_view_hints_dir());
+    BOOST_CHECK_EQUAL("/var/special/my_custom_work_dir/saved_caches", dirs.get_saved_caches_dir());
+
+    const auto data_file_dirs = dirs.get_data_file_dirs();
+    BOOST_REQUIRE_EQUAL(1u, data_file_dirs.size());
+    BOOST_CHECK_EQUAL("/var/special/my_custom_work_dir/data", data_file_dirs.at(0));
+}
+
+BOOST_AUTO_TEST_CASE(construction_from_config_with_custom_commitlog_dir)
+{
+    // Given a configuration with custom commitlog_directory.
+    db::config cfg{};
+    cfg.commitlog_directory("/var/specific/my_commitlog_directory");
+
+    // When constructing directories object.
+    utils::directories dirs{cfg};
+
+    // Then all paths except the ones related to commitlog point to a default workspace.
+    BOOST_CHECK_EQUAL("/var/lib/scylla", dirs.get_work_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/hints", dirs.get_hints_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/view_hints", dirs.get_view_hints_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/saved_caches", dirs.get_saved_caches_dir());
+
+    const auto data_file_dirs = dirs.get_data_file_dirs();
+    BOOST_REQUIRE_EQUAL(1u, data_file_dirs.size());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/data", data_file_dirs.at(0));
+
+    // And then the paths related to commitlog point to its directory.
+    BOOST_CHECK_EQUAL("/var/specific/my_commitlog_directory", dirs.get_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/specific/my_commitlog_directory/schema", dirs.get_schema_commitlog_dir());
+}
+
+BOOST_AUTO_TEST_CASE(construction_from_config_with_custom_schema_commitlog_dir)
+{
+    // Given a configuration with custom schema_commitlog_directory.
+    db::config cfg{};
+    cfg.schema_commitlog_directory("/var/specific/my_schema_commitlog_directory");
+
+    // When constructing directories object.
+    utils::directories dirs{cfg};
+
+    // Then all paths except schema_commitlog point to a default workspace.
+    BOOST_CHECK_EQUAL("/var/lib/scylla", dirs.get_work_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/commitlog", dirs.get_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/hints", dirs.get_hints_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/view_hints", dirs.get_view_hints_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/saved_caches", dirs.get_saved_caches_dir());
+
+    const auto data_file_dirs = dirs.get_data_file_dirs();
+    BOOST_REQUIRE_EQUAL(1u, data_file_dirs.size());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/data", data_file_dirs.at(0));
+
+    // And then schema_commitlog points to the custom dir.
+    BOOST_CHECK_EQUAL("/var/specific/my_schema_commitlog_directory", dirs.get_schema_commitlog_dir());
+}
+
+BOOST_AUTO_TEST_CASE(construction_from_config_with_custom_data_file_dirs)
+{
+    // Given a configuration with custom data_file_directories.
+    db::config cfg{};
+    cfg.data_file_directories({"/var/data_1", "/var/another_data", "/super_data/here"});
+
+    // When constructing directories object.
+    utils::directories dirs{cfg};
+
+    // Then all paths except data_file_directories point to a default workspace.
+    BOOST_CHECK_EQUAL("/var/lib/scylla", dirs.get_work_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/commitlog", dirs.get_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/commitlog/schema", dirs.get_schema_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/hints", dirs.get_hints_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/view_hints", dirs.get_view_hints_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/saved_caches", dirs.get_saved_caches_dir());
+
+    // And then data_file_directories point to the custom dirs.
+    const auto data_file_dirs = dirs.get_data_file_dirs();
+    BOOST_REQUIRE_EQUAL(3u, data_file_dirs.size());
+    BOOST_CHECK_EQUAL("/var/data_1", data_file_dirs.at(0));
+    BOOST_CHECK_EQUAL("/var/another_data", data_file_dirs.at(1));
+    BOOST_CHECK_EQUAL("/super_data/here", data_file_dirs.at(2));
+}
+
+BOOST_AUTO_TEST_CASE(construction_from_config_with_custom_hints_dir)
+{
+    // Given a configuration with custom hints_dir.
+    db::config cfg{};
+    cfg.hints_directory("/var/my_custom_hints_dir");
+
+    // When constructing directories object.
+    utils::directories dirs{cfg};
+
+    // Then all paths except hints_dir point to a default workspace.
+    BOOST_CHECK_EQUAL("/var/lib/scylla", dirs.get_work_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/commitlog", dirs.get_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/commitlog/schema", dirs.get_schema_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/view_hints", dirs.get_view_hints_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/saved_caches", dirs.get_saved_caches_dir());
+
+    const auto data_file_dirs = dirs.get_data_file_dirs();
+    BOOST_REQUIRE_EQUAL(1u, data_file_dirs.size());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/data", data_file_dirs.at(0));
+
+    // And then hints_dir point to the custom dir.
+    BOOST_CHECK_EQUAL("/var/my_custom_hints_dir", dirs.get_hints_dir());
+}
+
+BOOST_AUTO_TEST_CASE(construction_from_config_with_custom_view_hints_dir)
+{
+    // Given a configuration with custom view_hints_dir.
+    db::config cfg{};
+    cfg.view_hints_directory("/var/my_custom_view_hints_dir");
+
+    // When constructing directories object.
+    utils::directories dirs{cfg};
+
+    // Then all paths except view_hints_dir point to a default workspace.
+    BOOST_CHECK_EQUAL("/var/lib/scylla", dirs.get_work_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/commitlog", dirs.get_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/commitlog/schema", dirs.get_schema_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/hints", dirs.get_hints_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/saved_caches", dirs.get_saved_caches_dir());
+
+    const auto data_file_dirs = dirs.get_data_file_dirs();
+    BOOST_REQUIRE_EQUAL(1u, data_file_dirs.size());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/data", data_file_dirs.at(0));
+
+    // And then view_hints_dir point to the custom dir.
+    BOOST_CHECK_EQUAL("/var/my_custom_view_hints_dir", dirs.get_view_hints_dir());
+}
+
+BOOST_AUTO_TEST_CASE(construction_from_config_with_custom_saved_caches_dir)
+{
+    // Given a configuration with custom saved_caches_dir.
+    db::config cfg{};
+    cfg.saved_caches_directory("/var/caches/my_custom_cache");
+
+    // When constructing directories object.
+    utils::directories dirs{cfg};
+
+    // Then all paths except saved_caches_dir point to a default workspace.
+    BOOST_CHECK_EQUAL("/var/lib/scylla", dirs.get_work_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/commitlog", dirs.get_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/commitlog/schema", dirs.get_schema_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/hints", dirs.get_hints_dir());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/view_hints", dirs.get_view_hints_dir());
+
+    const auto data_file_dirs = dirs.get_data_file_dirs();
+    BOOST_REQUIRE_EQUAL(1u, data_file_dirs.size());
+    BOOST_CHECK_EQUAL("/var/lib/scylla/data", data_file_dirs.at(0));
+
+    // And then saved_caches_dir points to the custom path.
+    BOOST_CHECK_EQUAL("/var/caches/my_custom_cache", dirs.get_saved_caches_dir());
+}
+
+BOOST_AUTO_TEST_CASE(construction_from_config_with_multiple_custom_dirs)
+{
+    // Given a configuration with custom work_dir, commitlog_dir and saved_caches_dir.
+    db::config cfg{};
+    cfg.work_directory("/var/some/custom/workdir");
+    cfg.commitlog_directory("/var/custom/commitlog_dir");
+    cfg.saved_caches_directory("/var/caches/my_custom_cache");
+
+    // When constructing directories object.
+    utils::directories dirs{cfg};
+
+    // Then all paths except commitlog and saved_caches_dir point to a custom workspace.
+    BOOST_CHECK_EQUAL("/var/some/custom/workdir", dirs.get_work_dir());
+    BOOST_CHECK_EQUAL("/var/some/custom/workdir/hints", dirs.get_hints_dir());
+    BOOST_CHECK_EQUAL("/var/some/custom/workdir/view_hints", dirs.get_view_hints_dir());
+
+    const auto data_file_dirs = dirs.get_data_file_dirs();
+    BOOST_REQUIRE_EQUAL(1u, data_file_dirs.size());
+    BOOST_CHECK_EQUAL("/var/some/custom/workdir/data", data_file_dirs.at(0));
+
+    // And then paths related to commitlog points to the custom dir.
+    BOOST_CHECK_EQUAL("/var/custom/commitlog_dir", dirs.get_commitlog_dir());
+    BOOST_CHECK_EQUAL("/var/custom/commitlog_dir/schema", dirs.get_schema_commitlog_dir());
+
+    // And then saved_caches_dir points to the custom path.
+    BOOST_CHECK_EQUAL("/var/caches/my_custom_cache", dirs.get_saved_caches_dir());
+}
--- a/utils/directories.cc
+++ b/utils/directories.cc
@@ -10,6 +10,7 @@
 #include <seastar/core/seastar.hh>
 #include <seastar/core/smp.hh>

+#include "db/config.hh"
 #include "init.hh"
 #include "supervisor.hh"
 #include "directories.hh"
@@ -19,6 +20,25 @@

 using namespace seastar;

+namespace {
+
+std::vector<fs::path> as_paths(const std::vector<sstring>& dirs) {
+    std::vector<fs::path> paths;
+    paths.reserve(dirs.size());
+
+    for (const auto& dir_str : dirs) {
+        paths.emplace_back(dir_str);
+    }
+
+    return paths;
+}
+
+sstring to_sstring(const fs::path& p) {
+    return sstring{p.c_str()};
+}
+
+} // namespace
+
 namespace utils {

 static future<> disk_sanity(fs::path path, bool developer_mode) {
@@ -76,9 +96,40 @@ void directories::set::add_sharded(sstring p) {
    }
 }

-directories::directories(bool developer_mode)
-        : _developer_mode(developer_mode)
-{ }
+directories::directories(const ::db::config& cfg)
+        : _developer_mode{cfg.developer_mode()}
+        , _work_dir{cfg.work_directory()}
+        , _commitlog_dir{cfg.commitlog_directory()}
+        , _schema_commitlog_dir{cfg.schema_commitlog_directory()}
+        , _hints_dir{cfg.hints_directory()}
+        , _view_hints_dir{cfg.view_hints_directory()}
+        , _saved_caches_dir{cfg.saved_caches_directory()}
+        , _data_file_dirs{as_paths(cfg.data_file_directories())}
+{
+    override_empty_paths();
+}
+
+void directories::override_empty_paths() {
+    // if path is empty override it to be a subdirectory of the second argument
+    override_if_empty(_commitlog_dir, _work_dir, "commitlog");
+    override_if_empty(_schema_commitlog_dir, _commitlog_dir, "schema");
+    override_if_empty(_data_file_dirs, _work_dir, "data");
+    override_if_empty(_hints_dir, _work_dir, "hints");
+    override_if_empty(_view_hints_dir, _work_dir, "view_hints");
+    override_if_empty(_saved_caches_dir, _work_dir, "saved_caches");
+}
+
+void directories::override_if_empty(fs::path& p, const fs::path& dest_parent, std::string_view subdir) {
+    if (p.empty()) {
+        p = (dest_parent / fs::path{subdir});
+    }
+}
+
+void directories::override_if_empty(std::vector<fs::path>& v, const fs::path& dest_parent, std::string_view subdir) {
+    if (v.empty()) {
+        v.push_back(dest_parent / fs::path{subdir});
+    }
+}

 future<> directories::create_and_verify(directories::set dir_set) {
    return do_with(std::vector<file_lock>(), [this, dir_set = std::move(dir_set)] (std::vector<file_lock>& locks) {
@@ -148,4 +199,39 @@ future<> directories::verify_owner_and_mode(fs::path path, recursive recursive)
    return do_verify_owner_and_mode(std::move(path), recursive, 0);
 }

+sstring directories::get_work_dir() const {
+    return to_sstring(_work_dir);
+}
+
+sstring directories::get_commitlog_dir() const {
+    return to_sstring(_commitlog_dir);
+}
+
+sstring directories::get_schema_commitlog_dir() const {
+    return to_sstring(_schema_commitlog_dir);
+}
+
+sstring directories::get_hints_dir() const {
+    return to_sstring(_hints_dir);
+}
+
+sstring directories::get_view_hints_dir() const {
+    return to_sstring(_view_hints_dir);
+}
+
+sstring directories::get_saved_caches_dir() const {
+    return to_sstring(_saved_caches_dir);
+}
+
+std::vector<sstring> directories::get_data_file_dirs() const {
+    std::vector<sstring> dirs;
+    dirs.reserve(_data_file_dirs.size());
+
+    for (const auto & dir_path : _data_file_dirs) {
+        dirs.emplace_back(to_sstring(dir_path));
+    }
+
+    return dirs;
+}
+
 } // namespace utils
--- a/utils/directories.hh
+++ b/utils/directories.hh
@@ -32,15 +32,34 @@ public:
    class set;
    using recursive = seastar::bool_class<struct recursive_tag>;

-    directories(bool developer_mode);
+    directories(const ::db::config& cfg);
    seastar::future<> create_and_verify(set dir_set);
    static seastar::future<> verify_owner_and_mode(fs::path path, recursive r = recursive::yes);

+    seastar::sstring get_work_dir() const;
+    seastar::sstring get_commitlog_dir() const;
+    seastar::sstring get_schema_commitlog_dir() const;
+    seastar::sstring get_hints_dir() const;
+    seastar::sstring get_view_hints_dir() const;
+    seastar::sstring get_saved_caches_dir() const;
+    std::vector<seastar::sstring> get_data_file_dirs() const;
+
 private:
    static seastar::future<> do_verify_owner_and_mode(fs::path path, recursive, int level);

+    void override_empty_paths();
+    void override_if_empty(fs::path& p, const fs::path& dest_parent, std::string_view subdir);
+    void override_if_empty(std::vector<fs::path>& v, const fs::path& dest_parent, std::string_view subdir);
+
    bool _developer_mode;
    std::vector<file_lock> _locks;
+    fs::path _work_dir{};
+    fs::path _commitlog_dir{};
+    fs::path _schema_commitlog_dir{};
+    fs::path _hints_dir{};
+    fs::path _view_hints_dir{};
+    fs::path _saved_caches_dir{};
+    std::vector<fs::path> _data_file_dirs{};
 };

 class directories::set {