Merge pull request ClickHouse#58357 from ClickHouse/better-disks-conf…

…iguration Flexible configuration for disks
myrrc · Jan 15, 2024 · ae88476 · ae88476
2 parents be37140 + abd800d
commit ae88476
Show file tree

Hide file tree

Showing 35 changed files with 868 additions and 641 deletions.
diff --git a/docker/test/upgrade/run.sh b/docker/test/upgrade/run.sh
@@ -78,6 +78,7 @@ remove_keeper_config "create_if_not_exists" "[01]"
 rm /etc/clickhouse-server/config.d/merge_tree.xml
 rm /etc/clickhouse-server/config.d/enable_wait_for_shutdown_replicated_tables.xml
 rm /etc/clickhouse-server/config.d/zero_copy_destructive_operations.xml
+rm /etc/clickhouse-server/config.d/storage_conf_02963.xml
 rm /etc/clickhouse-server/users.d/nonconst_timezone.xml
 rm /etc/clickhouse-server/users.d/s3_cache_new.xml
 rm /etc/clickhouse-server/users.d/replicated_ddl_entry.xml
@@ -117,6 +118,7 @@ sudo chgrp clickhouse /etc/clickhouse-server/config.d/s3_storage_policy_by_defau
 rm /etc/clickhouse-server/config.d/merge_tree.xml
 rm /etc/clickhouse-server/config.d/enable_wait_for_shutdown_replicated_tables.xml
 rm /etc/clickhouse-server/config.d/zero_copy_destructive_operations.xml
+rm /etc/clickhouse-server/config.d/storage_conf_02963.xml
 rm /etc/clickhouse-server/users.d/nonconst_timezone.xml
 rm /etc/clickhouse-server/users.d/s3_cache_new.xml
 rm /etc/clickhouse-server/users.d/replicated_ddl_entry.xml

diff --git a/programs/keeper/CMakeLists.txt b/programs/keeper/CMakeLists.txt
@@ -127,15 +127,17 @@ if (BUILD_STANDALONE_KEEPER)
         ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/DiskObjectStorage.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/DiskObjectStorageTransaction.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/DiskObjectStorageRemoteMetadataRestoreHelper.cpp
-        ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/DiskObjectStorageCommon.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/ObjectStorageIteratorAsync.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/ObjectStorageIterator.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/StoredObject.cpp
 
-        ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/S3/registerDiskS3.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/S3/S3ObjectStorage.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/S3/S3Capabilities.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/S3/diskSettings.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/S3/DiskS3Utils.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/ObjectStorageFactory.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/MetadataStorageFactory.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/ObjectStorages/RegisterDiskObjectStorage.cpp
 
         ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/IO/createReadBufferFromFileBase.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/../../src/Disks/IO/ReadBufferFromRemoteFSGather.cpp

diff --git a/src/Disks/DiskFactory.cpp b/src/Disks/DiskFactory.cpp
@@ -14,7 +14,7 @@ DiskFactory & DiskFactory::instance()
     return factory;
 }
 
-void DiskFactory::registerDiskType(const String & disk_type, DB::DiskFactory::Creator creator)
+void DiskFactory::registerDiskType(const String & disk_type, Creator creator)
 {
     if (!registry.emplace(disk_type, creator).second)
         throw Exception(ErrorCodes::LOGICAL_ERROR, "DiskFactory: the disk type '{}' is not unique", disk_type);
@@ -31,7 +31,10 @@ DiskPtr DiskFactory::create(
 
     const auto found = registry.find(disk_type);
     if (found == registry.end())
-        throw Exception(ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG, "DiskFactory: the disk '{}' has unknown disk type: {}", name, disk_type);
+    {
+        throw Exception(ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG,
+                        "DiskFactory: the disk '{}' has unknown disk type: {}", name, disk_type);
+    }
 
     const auto & disk_creator = found->second;
     return disk_creator(name, config, config_prefix, context, map);

diff --git a/src/Disks/ObjectStorages/AzureBlobStorage/AzureBlobStorageAuth.cpp b/src/Disks/ObjectStorages/AzureBlobStorage/AzureBlobStorageAuth.cpp
@@ -65,8 +65,14 @@ AzureBlobStorageEndpoint processAzureBlobStorageEndpoint(const Poco::Util::Abstr
     }
     else
     {
-        storage_url = config.getString(config_prefix + ".connection_string");
+        if (config.has(config_prefix + ".connection_string"))
+            storage_url = config.getString(config_prefix + ".connection_string");
+        else if (config.has(config_prefix + ".endpoint"))
+            storage_url = config.getString(config_prefix + ".endpoint");
+        else
+            throw Exception(ErrorCodes::BAD_ARGUMENTS, "Expected either `connection_string` or `endpoint` in config");
     }
+
     String container_name = config.getString(config_prefix + ".container_name", "default-container");
     validateContainerName(container_name);
     std::optional<bool> container_already_exists {};
@@ -100,11 +106,14 @@ template <class T>
 std::unique_ptr<T> getAzureBlobStorageClientWithAuth(
     const String & url, const String & container_name, const Poco::Util::AbstractConfiguration & config, const String & config_prefix)
 {
+    std::string connection_str;
     if (config.has(config_prefix + ".connection_string"))
-    {
-        String connection_str = config.getString(config_prefix + ".connection_string");
+        connection_str = config.getString(config_prefix + ".connection_string");
+    else if (config.has(config_prefix + ".endpoint"))
+        connection_str = config.getString(config_prefix + ".endpoint");
+
+    if (!connection_str.empty())
         return getClientWithConnectionString<T>(connection_str, container_name);
-    }
 
     if (config.has(config_prefix + ".account_key") && config.has(config_prefix + ".account_name"))
     {

diff --git a/src/Disks/ObjectStorages/AzureBlobStorage/AzureObjectStorage.h b/src/Disks/ObjectStorages/AzureBlobStorage/AzureObjectStorage.h
@@ -3,7 +3,6 @@
 
 #if USE_AZURE_BLOB_STORAGE
 
-#include <Disks/ObjectStorages/DiskObjectStorageCommon.h>
 #include <Disks/IO/ReadBufferFromRemoteFSGather.h>
 #include <Disks/ObjectStorages/IObjectStorage.h>
 #include <Common/MultiVersion.h>
@@ -64,6 +63,8 @@ class AzureObjectStorage : public IObjectStorage
 
     std::string getName() const override { return "AzureObjectStorage"; }
 
+    std::string getCommonKeyPrefix() const override { return ""; } /// No namespaces in azure.
+
     bool exists(const StoredObject & object) const override;
 
     std::unique_ptr<ReadBufferFromFileBase> readObject( /// NOLINT

diff --git a/src/Disks/ObjectStorages/AzureBlobStorage/registerDiskAzureBlobStorage.cpp b/src/Disks/ObjectStorages/AzureBlobStorage/registerDiskAzureBlobStorage.cpp
diff --git a/src/Disks/ObjectStorages/Cached/CachedObjectStorage.cpp b/src/Disks/ObjectStorages/Cached/CachedObjectStorage.cpp
@@ -1,6 +1,5 @@
 #include "CachedObjectStorage.h"
 
-#include <Disks/ObjectStorages/DiskObjectStorageCommon.h>
 #include <IO/BoundedReadBuffer.h>
 #include <Disks/IO/CachedOnDiskWriteBufferFromFile.h>
 #include <Disks/IO/CachedOnDiskReadBufferFromFile.h>

diff --git a/src/Disks/ObjectStorages/Cached/CachedObjectStorage.h b/src/Disks/ObjectStorages/Cached/CachedObjectStorage.h
@@ -24,6 +24,8 @@ class CachedObjectStorage final : public IObjectStorage
 
     std::string getName() const override { return fmt::format("CachedObjectStorage-{}({})", cache_config_name, object_storage->getName()); }
 
+    std::string getCommonKeyPrefix() const override { return object_storage->getCommonKeyPrefix(); }
+
     bool exists(const StoredObject & object) const override;
 
     std::unique_ptr<ReadBufferFromFileBase> readObject( /// NOLINT

diff --git a/src/Disks/ObjectStorages/Cached/registerDiskCache.cpp b/src/Disks/ObjectStorages/Cached/registerDiskCache.cpp
@@ -39,7 +39,10 @@ void registerDiskCache(DiskFactory & factory, bool /* global_skip_access_check *
         }
 
         FileCacheSettings file_cache_settings;
-        auto predefined_configuration = config.has("cache_name") ? NamedCollectionFactory::instance().tryGet(config.getString("cache_name")) : nullptr;
+        auto predefined_configuration = config.has("cache_name")
+            ? NamedCollectionFactory::instance().tryGet(config.getString("cache_name"))
+            : nullptr;
+
         if (predefined_configuration)
             file_cache_settings.loadFromCollection(*predefined_configuration);
         else

diff --git a/src/Disks/ObjectStorages/DiskObjectStorageCommon.cpp b/src/Disks/ObjectStorages/DiskObjectStorageCommon.cpp
diff --git a/src/Disks/ObjectStorages/DiskObjectStorageCommon.h b/src/Disks/ObjectStorages/DiskObjectStorageCommon.h
diff --git a/src/Disks/ObjectStorages/HDFS/HDFSObjectStorage.h b/src/Disks/ObjectStorages/HDFS/HDFSObjectStorage.h
@@ -48,6 +48,7 @@ class HDFSObjectStorage : public IObjectStorage
         , hdfs_builder(createHDFSBuilder(hdfs_root_path_, config))
         , hdfs_fs(createHDFSFS(hdfs_builder.get()))
         , settings(std::move(settings_))
+        , hdfs_root_path(hdfs_root_path_)
     {
         data_source_description.type = DataSourceType::HDFS;
         data_source_description.description = hdfs_root_path_;
@@ -57,6 +58,8 @@ class HDFSObjectStorage : public IObjectStorage
 
     std::string getName() const override { return "HDFSObjectStorage"; }
 
+    std::string getCommonKeyPrefix() const override { return hdfs_root_path; }
+
     DataSourceDescription getDataSourceDescription() const override
     {
         return data_source_description;
@@ -123,8 +126,8 @@ class HDFSObjectStorage : public IObjectStorage
 
     HDFSBuilderWrapper hdfs_builder;
     HDFSFSPtr hdfs_fs;
-
     SettingsPtr settings;
+    const std::string hdfs_root_path;
 
     DataSourceDescription data_source_description;
 };

diff --git a/src/Disks/ObjectStorages/HDFS/registerDiskHDFS.cpp b/src/Disks/ObjectStorages/HDFS/registerDiskHDFS.cpp
diff --git a/src/Disks/ObjectStorages/IObjectStorage.h b/src/Disks/ObjectStorages/IObjectStorage.h
@@ -84,6 +84,8 @@ class IObjectStorage
 
     virtual std::string getName() const = 0;
 
+    virtual std::string getCommonKeyPrefix() const = 0;
+
     /// Object exists or not
     virtual bool exists(const StoredObject & object) const = 0;
 

diff --git a/src/Disks/ObjectStorages/Local/LocalObjectStorage.cpp b/src/Disks/ObjectStorages/Local/LocalObjectStorage.cpp
@@ -1,6 +1,5 @@
 #include <Disks/ObjectStorages/Local/LocalObjectStorage.h>
 
-#include <Disks/ObjectStorages/DiskObjectStorageCommon.h>
 #include <Interpreters/Context.h>
 #include <Common/filesystemHelpers.h>
 #include <Common/logger_useful.h>
@@ -28,7 +27,7 @@ LocalObjectStorage::LocalObjectStorage(String key_prefix_)
     : key_prefix(std::move(key_prefix_))
     , log(&Poco::Logger::get("LocalObjectStorage"))
 {
-    data_source_description.type = DataSourceType::Local;
+    data_source_description.type = DataSourceType::LocalBlobStorage;
     if (auto block_device_id = tryGetBlockDeviceId("/"); block_device_id.has_value())
         data_source_description.description = *block_device_id;
     else