From e8203f8a768e52c4e6aeefcec7ce23a513ec63aa Mon Sep 17 00:00:00 2001 From: Nikolay Degterinsky Date: Thu, 30 Nov 2023 03:09:55 +0000 Subject: [PATCH 1/2] Initialize only required disks --- programs/disks/CommandCopy.cpp | 6 +++--- programs/disks/CommandLink.cpp | 4 ++-- programs/disks/CommandList.cpp | 4 ++-- programs/disks/CommandListDisks.cpp | 21 +++++++++++++++++--- programs/disks/CommandMkDir.cpp | 4 ++-- programs/disks/CommandMove.cpp | 4 ++-- programs/disks/CommandRead.cpp | 4 ++-- programs/disks/CommandRemove.cpp | 4 ++-- programs/disks/CommandWrite.cpp | 4 ++-- programs/disks/DisksApp.cpp | 30 ++++++++++++++++++++++++++++- programs/disks/ICommand.h | 3 ++- src/Coordination/KeeperContext.cpp | 2 +- src/Disks/DiskSelector.cpp | 4 ++-- src/Disks/DiskSelector.h | 2 +- 14 files changed, 70 insertions(+), 26 deletions(-) diff --git a/programs/disks/CommandCopy.cpp b/programs/disks/CommandCopy.cpp index 296fc708411..0929ee46afc 100644 --- a/programs/disks/CommandCopy.cpp +++ b/programs/disks/CommandCopy.cpp @@ -36,7 +36,7 @@ public: void execute( const std::vector & command_arguments, - DB::ContextMutablePtr & global_context, + std::shared_ptr & disk_selector, Poco::Util::LayeredConfiguration & config) override { if (command_arguments.size() != 2) @@ -51,8 +51,8 @@ public: const String & path_from = command_arguments[0]; const String & path_to = command_arguments[1]; - DiskPtr disk_from = global_context->getDisk(disk_name_from); - DiskPtr disk_to = global_context->getDisk(disk_name_to); + DiskPtr disk_from = disk_selector->get(disk_name_from); + DiskPtr disk_to = disk_selector->get(disk_name_to); String relative_path_from = validatePathAndGetAsRelative(path_from); String relative_path_to = validatePathAndGetAsRelative(path_to); diff --git a/programs/disks/CommandLink.cpp b/programs/disks/CommandLink.cpp index 357832865fb..dbaa3162f82 100644 --- a/programs/disks/CommandLink.cpp +++ b/programs/disks/CommandLink.cpp @@ -27,7 +27,7 @@ public: void execute( const std::vector & command_arguments, - DB::ContextMutablePtr & global_context, + std::shared_ptr & disk_selector, Poco::Util::LayeredConfiguration & config) override { if (command_arguments.size() != 2) @@ -41,7 +41,7 @@ public: const String & path_from = command_arguments[0]; const String & path_to = command_arguments[1]; - DiskPtr disk = global_context->getDisk(disk_name); + DiskPtr disk = disk_selector->get(disk_name); String relative_path_from = validatePathAndGetAsRelative(path_from); String relative_path_to = validatePathAndGetAsRelative(path_to); diff --git a/programs/disks/CommandList.cpp b/programs/disks/CommandList.cpp index 48b54b70014..ea84cd0682d 100644 --- a/programs/disks/CommandList.cpp +++ b/programs/disks/CommandList.cpp @@ -33,7 +33,7 @@ public: void execute( const std::vector & command_arguments, - DB::ContextMutablePtr & global_context, + std::shared_ptr & disk_selector, Poco::Util::LayeredConfiguration & config) override { if (command_arguments.size() != 1) @@ -46,7 +46,7 @@ public: const String & path = command_arguments[0]; - DiskPtr disk = global_context->getDisk(disk_name); + DiskPtr disk = disk_selector->get(disk_name); String relative_path = validatePathAndGetAsRelative(path); diff --git a/programs/disks/CommandListDisks.cpp b/programs/disks/CommandListDisks.cpp index 7b2fcd16107..fd362810a29 100644 --- a/programs/disks/CommandListDisks.cpp +++ b/programs/disks/CommandListDisks.cpp @@ -26,8 +26,8 @@ public: void execute( const std::vector & command_arguments, - DB::ContextMutablePtr & global_context, - Poco::Util::LayeredConfiguration &) override + std::shared_ptr &, + Poco::Util::LayeredConfiguration & config) override { if (!command_arguments.empty()) { @@ -35,8 +35,23 @@ public: throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "Bad Arguments"); } - for (const auto & [disk_name, _] : global_context->getDisksMap()) + constexpr auto config_prefix = "storage_configuration.disks"; + constexpr auto default_disk_name = "default"; + + Poco::Util::AbstractConfiguration::Keys keys; + config.keys(config_prefix, keys); + + bool has_default_disk = false; + + for (const auto & disk_name : keys) + { + if (disk_name == default_disk_name) + has_default_disk = true; std::cout << disk_name << '\n'; + } + + if (!has_default_disk) + std::cout << default_disk_name << '\n'; } }; } diff --git a/programs/disks/CommandMkDir.cpp b/programs/disks/CommandMkDir.cpp index e5df982d896..6d33bdec498 100644 --- a/programs/disks/CommandMkDir.cpp +++ b/programs/disks/CommandMkDir.cpp @@ -34,7 +34,7 @@ public: void execute( const std::vector & command_arguments, - DB::ContextMutablePtr & global_context, + std::shared_ptr & disk_selector, Poco::Util::LayeredConfiguration & config) override { if (command_arguments.size() != 1) @@ -47,7 +47,7 @@ public: const String & path = command_arguments[0]; - DiskPtr disk = global_context->getDisk(disk_name); + DiskPtr disk = disk_selector->get(disk_name); String relative_path = validatePathAndGetAsRelative(path); bool recursive = config.getBool("recursive", false); diff --git a/programs/disks/CommandMove.cpp b/programs/disks/CommandMove.cpp index 654090b2138..75cf96252ed 100644 --- a/programs/disks/CommandMove.cpp +++ b/programs/disks/CommandMove.cpp @@ -26,7 +26,7 @@ public: void execute( const std::vector & command_arguments, - DB::ContextMutablePtr & global_context, + std::shared_ptr & disk_selector, Poco::Util::LayeredConfiguration & config) override { if (command_arguments.size() != 2) @@ -40,7 +40,7 @@ public: const String & path_from = command_arguments[0]; const String & path_to = command_arguments[1]; - DiskPtr disk = global_context->getDisk(disk_name); + DiskPtr disk = disk_selector->get(disk_name); String relative_path_from = validatePathAndGetAsRelative(path_from); String relative_path_to = validatePathAndGetAsRelative(path_to); diff --git a/programs/disks/CommandRead.cpp b/programs/disks/CommandRead.cpp index b6cacdd2c61..85041faf22c 100644 --- a/programs/disks/CommandRead.cpp +++ b/programs/disks/CommandRead.cpp @@ -36,7 +36,7 @@ public: void execute( const std::vector & command_arguments, - DB::ContextMutablePtr & global_context, + std::shared_ptr & disk_selector, Poco::Util::LayeredConfiguration & config) override { if (command_arguments.size() != 1) @@ -47,7 +47,7 @@ public: String disk_name = config.getString("disk", "default"); - DiskPtr disk = global_context->getDisk(disk_name); + DiskPtr disk = disk_selector->get(disk_name); String relative_path = validatePathAndGetAsRelative(command_arguments[0]); diff --git a/programs/disks/CommandRemove.cpp b/programs/disks/CommandRemove.cpp index ff8d4a1c6bb..0c631eacff3 100644 --- a/programs/disks/CommandRemove.cpp +++ b/programs/disks/CommandRemove.cpp @@ -26,7 +26,7 @@ public: void execute( const std::vector & command_arguments, - DB::ContextMutablePtr & global_context, + std::shared_ptr & disk_selector, Poco::Util::LayeredConfiguration & config) override { if (command_arguments.size() != 1) @@ -39,7 +39,7 @@ public: const String & path = command_arguments[0]; - DiskPtr disk = global_context->getDisk(disk_name); + DiskPtr disk = disk_selector->get(disk_name); String relative_path = validatePathAndGetAsRelative(path); diff --git a/programs/disks/CommandWrite.cpp b/programs/disks/CommandWrite.cpp index d075daf3215..7ded37e067a 100644 --- a/programs/disks/CommandWrite.cpp +++ b/programs/disks/CommandWrite.cpp @@ -37,7 +37,7 @@ public: void execute( const std::vector & command_arguments, - DB::ContextMutablePtr & global_context, + std::shared_ptr & disk_selector, Poco::Util::LayeredConfiguration & config) override { if (command_arguments.size() != 1) @@ -50,7 +50,7 @@ public: const String & path = command_arguments[0]; - DiskPtr disk = global_context->getDisk(disk_name); + DiskPtr disk = disk_selector->get(disk_name); String relative_path = validatePathAndGetAsRelative(path); diff --git a/programs/disks/DisksApp.cpp b/programs/disks/DisksApp.cpp index b81cd52f8c8..ded324fd0da 100644 --- a/programs/disks/DisksApp.cpp +++ b/programs/disks/DisksApp.cpp @@ -209,7 +209,35 @@ int DisksApp::main(const std::vector & /*args*/) po::parsed_options parsed = parser.run(); args = po::collect_unrecognized(parsed.options, po::collect_unrecognized_mode::include_positional); } - command->execute(args, global_context, config()); + + std::unordered_set disks + { + config().getString("disk", "default"), + config().getString("disk-from", config().getString("disk", "default")), + config().getString("disk-to", config().getString("disk", "default")), + }; + + auto validator = [&disks]( + const Poco::Util::AbstractConfiguration & config, + const std::string & disk_config_prefix, + const std::string & disk_name) + { + if (!disks.contains(disk_name)) + return false; + + const auto disk_type = config.getString(disk_config_prefix + ".type", "local"); + + if (disk_type == "cache") + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Disk type 'cache' of disk {} is not supported by clickhouse-disks", disk_name); + + return true; + }; + + constexpr auto config_prefix = "storage_configuration.disks"; + auto disk_selector = std::make_shared(); + disk_selector->initialize(config(), config_prefix, global_context, validator); + + command->execute(args, disk_selector, config()); return Application::EXIT_OK; } diff --git a/programs/disks/ICommand.h b/programs/disks/ICommand.h index de41eedec35..da106e1084e 100644 --- a/programs/disks/ICommand.h +++ b/programs/disks/ICommand.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include @@ -25,7 +26,7 @@ public: virtual void execute( const std::vector & command_arguments, - DB::ContextMutablePtr & global_context, + std::shared_ptr & disk_selector, Poco::Util::LayeredConfiguration & config) = 0; const std::optional & getCommandOptions() const { return command_option_description; } diff --git a/src/Coordination/KeeperContext.cpp b/src/Coordination/KeeperContext.cpp index c3cb166abee..7e0b75a6353 100644 --- a/src/Coordination/KeeperContext.cpp +++ b/src/Coordination/KeeperContext.cpp @@ -69,7 +69,7 @@ void KeeperContext::initialize(const Poco::Util::AbstractConfiguration & config, namespace { -bool diskValidator(const Poco::Util::AbstractConfiguration & config, const std::string & disk_config_prefix) +bool diskValidator(const Poco::Util::AbstractConfiguration & config, const std::string & disk_config_prefix, const std::string &) { const auto disk_type = config.getString(disk_config_prefix + ".type", "local"); diff --git a/src/Disks/DiskSelector.cpp b/src/Disks/DiskSelector.cpp index 415e10a55fc..dad1c728560 100644 --- a/src/Disks/DiskSelector.cpp +++ b/src/Disks/DiskSelector.cpp @@ -44,9 +44,9 @@ void DiskSelector::initialize(const Poco::Util::AbstractConfiguration & config, if (disk_name == default_disk_name) has_default_disk = true; - auto disk_config_prefix = config_prefix + "." + disk_name; + const auto disk_config_prefix = config_prefix + "." + disk_name; - if (disk_validator && !disk_validator(config, disk_config_prefix)) + if (disk_validator && !disk_validator(config, disk_config_prefix, disk_name)) continue; disks.emplace(disk_name, factory.create(disk_name, config, disk_config_prefix, context, disks)); diff --git a/src/Disks/DiskSelector.h b/src/Disks/DiskSelector.h index c91c3acb3bd..6669b428158 100644 --- a/src/Disks/DiskSelector.h +++ b/src/Disks/DiskSelector.h @@ -23,7 +23,7 @@ public: DiskSelector() = default; DiskSelector(const DiskSelector & from) = default; - using DiskValidator = std::function; + using DiskValidator = std::function; void initialize(const Poco::Util::AbstractConfiguration & config, const String & config_prefix, ContextPtr context, DiskValidator disk_validator = {}); DiskSelectorPtr updateFromConfig( From 93995f6c06d5695d19449c6d62fad12ba3620d0a Mon Sep 17 00:00:00 2001 From: Nikolay Degterinsky Date: Sat, 2 Dec 2023 23:59:46 +0000 Subject: [PATCH 2/2] Fix test --- programs/disks/CommandListDisks.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/programs/disks/CommandListDisks.cpp b/programs/disks/CommandListDisks.cpp index fd362810a29..79da021fd00 100644 --- a/programs/disks/CommandListDisks.cpp +++ b/programs/disks/CommandListDisks.cpp @@ -43,15 +43,21 @@ public: bool has_default_disk = false; + /// For the output to be ordered + std::set disks; + for (const auto & disk_name : keys) { if (disk_name == default_disk_name) has_default_disk = true; - std::cout << disk_name << '\n'; + disks.insert(disk_name); } if (!has_default_disk) - std::cout << default_disk_name << '\n'; + disks.insert(default_disk_name); + + for (const auto & disk : disks) + std::cout << disk << '\n'; } }; }