From 026f7e0a27f0a9c6a89314e5aff9e2a16f5dbf99 Mon Sep 17 00:00:00 2001 From: Anton Ivashkin Date: Fri, 13 Nov 2020 19:31:51 +0300 Subject: [PATCH] Add 's3_max_redirects' setting --- src/Core/Settings.h | 1 + src/Disks/S3/registerDiskS3.cpp | 3 ++- src/IO/S3/PocoHTTPClient.cpp | 10 +++++++--- src/IO/S3/PocoHTTPClient.h | 10 +++++++++- src/IO/S3Common.cpp | 15 +++++++++------ src/IO/S3Common.h | 10 +++++++--- src/Storages/StorageS3.cpp | 3 ++- 7 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/Core/Settings.h b/src/Core/Settings.h index 580756361b1..ede3e6dbe2e 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -65,6 +65,7 @@ class IColumn; M(UInt64, distributed_connections_pool_size, DBMS_DEFAULT_DISTRIBUTED_CONNECTIONS_POOL_SIZE, "Maximum number of connections with one remote server in the pool.", 0) \ M(UInt64, connections_with_failover_max_tries, DBMS_CONNECTION_POOL_WITH_FAILOVER_DEFAULT_MAX_TRIES, "The maximum number of attempts to connect to replicas.", 0) \ M(UInt64, s3_min_upload_part_size, 512*1024*1024, "The minimum size of part to upload during multipart upload to S3.", 0) \ + M(UInt64, s3_max_redirects, 10, "Max number of S3 redirects hops allowed.", 0) \ M(Bool, extremes, false, "Calculate minimums and maximums of the result columns. They can be output in JSON-formats.", IMPORTANT) \ M(Bool, use_uncompressed_cache, true, "Whether to use the cache of uncompressed blocks.", 0) \ M(Bool, replace_running_query, false, "Whether the running request should be canceled with the same id as the new one.", 0) \ diff --git a/src/Disks/S3/registerDiskS3.cpp b/src/Disks/S3/registerDiskS3.cpp index 862fd388476..1bdd86f9f57 100644 --- a/src/Disks/S3/registerDiskS3.cpp +++ b/src/Disks/S3/registerDiskS3.cpp @@ -132,7 +132,8 @@ void registerDiskS3(DiskFactory & factory) uri.is_virtual_hosted_style, config.getString(config_prefix + ".access_key_id", ""), config.getString(config_prefix + ".secret_access_key", ""), - context.getRemoteHostFilter()); + context.getRemoteHostFilter(), + context.getGlobalContext()); String metadata_path = config.getString(config_prefix + ".metadata_path", context.getPath() + "disks/" + name + "/"); diff --git a/src/IO/S3/PocoHTTPClient.cpp b/src/IO/S3/PocoHTTPClient.cpp index 49ccb6dc1b3..f55d95ae160 100644 --- a/src/IO/S3/PocoHTTPClient.cpp +++ b/src/IO/S3/PocoHTTPClient.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -48,9 +49,11 @@ namespace DB::S3 PocoHTTPClientConfiguration::PocoHTTPClientConfiguration( const Aws::Client::ClientConfiguration & cfg, - const RemoteHostFilter & remote_host_filter_) + const RemoteHostFilter & remote_host_filter_, + const Context & global_context_) : Aws::Client::ClientConfiguration(cfg) , remote_host_filter(remote_host_filter_) + , global_context(global_context_) { } @@ -81,6 +84,7 @@ PocoHTTPClient::PocoHTTPClient(const PocoHTTPClientConfiguration & clientConfigu Poco::Timespan(clientConfiguration.httpRequestTimeoutMs * 1000) /// receive timeout. )) , remote_host_filter(clientConfiguration.remote_host_filter) + , global_context(clientConfiguration.global_context) { } @@ -155,10 +159,10 @@ void PocoHTTPClient::makeRequestInternal( ProfileEvents::increment(select_metric(S3MetricType::Count)); - static constexpr int max_redirect_attempts = 10; + unsigned int max_redirect_attempts = global_context.getSettingsRef().s3_max_redirects; try { - for (int attempt = 0; attempt < max_redirect_attempts; ++attempt) + for (unsigned int attempt = 0; attempt < max_redirect_attempts; ++attempt) { Poco::URI poco_uri(uri); diff --git a/src/IO/S3/PocoHTTPClient.h b/src/IO/S3/PocoHTTPClient.h index 25055754519..385a5a22e48 100644 --- a/src/IO/S3/PocoHTTPClient.h +++ b/src/IO/S3/PocoHTTPClient.h @@ -11,14 +11,21 @@ namespace Aws::Http::Standard class StandardHttpResponse; } +namespace DB +{ +class Context; +} + namespace DB::S3 { struct PocoHTTPClientConfiguration : public Aws::Client::ClientConfiguration { const RemoteHostFilter & remote_host_filter; + const Context & global_context; - PocoHTTPClientConfiguration(const Aws::Client::ClientConfiguration & cfg, const RemoteHostFilter & remote_host_filter_); + PocoHTTPClientConfiguration(const Aws::Client::ClientConfiguration & cfg, const RemoteHostFilter & remote_host_filter_, + const Context & global_context_); void updateSchemeAndRegion(); }; @@ -48,6 +55,7 @@ private: std::function per_request_configuration; ConnectionTimeouts timeouts; const RemoteHostFilter & remote_host_filter; + const Context & global_context; }; } diff --git a/src/IO/S3Common.cpp b/src/IO/S3Common.cpp index 1304b6b5054..a69349a609b 100644 --- a/src/IO/S3Common.cpp +++ b/src/IO/S3Common.cpp @@ -164,14 +164,15 @@ namespace S3 bool is_virtual_hosted_style, const String & access_key_id, const String & secret_access_key, - const RemoteHostFilter & remote_host_filter) + const RemoteHostFilter & remote_host_filter, + const Context & global_context) { Aws::Client::ClientConfiguration cfg; if (!endpoint.empty()) cfg.endpointOverride = endpoint; - return create(cfg, is_virtual_hosted_style, access_key_id, secret_access_key, remote_host_filter); + return create(cfg, is_virtual_hosted_style, access_key_id, secret_access_key, remote_host_filter, global_context); } std::shared_ptr ClientFactory::create( // NOLINT @@ -179,11 +180,12 @@ namespace S3 bool is_virtual_hosted_style, const String & access_key_id, const String & secret_access_key, - const RemoteHostFilter & remote_host_filter) + const RemoteHostFilter & remote_host_filter, + const Context & global_context) { Aws::Auth::AWSCredentials credentials(access_key_id, secret_access_key); - PocoHTTPClientConfiguration client_configuration(cfg, remote_host_filter); + PocoHTTPClientConfiguration client_configuration(cfg, remote_host_filter, global_context); client_configuration.updateSchemeAndRegion(); @@ -201,9 +203,10 @@ namespace S3 const String & access_key_id, const String & secret_access_key, HeaderCollection headers, - const RemoteHostFilter & remote_host_filter) + const RemoteHostFilter & remote_host_filter, + const Context & global_context) { - PocoHTTPClientConfiguration client_configuration({}, remote_host_filter); + PocoHTTPClientConfiguration client_configuration({}, remote_host_filter, global_context); if (!endpoint.empty()) client_configuration.endpointOverride = endpoint; diff --git a/src/IO/S3Common.h b/src/IO/S3Common.h index d411c903676..ff422b5b511 100644 --- a/src/IO/S3Common.h +++ b/src/IO/S3Common.h @@ -19,6 +19,7 @@ namespace DB class RemoteHostFilter; struct HttpHeader; using HeaderCollection = std::vector; + class Context; } namespace DB::S3 @@ -36,14 +37,16 @@ public: bool is_virtual_hosted_style, const String & access_key_id, const String & secret_access_key, - const RemoteHostFilter & remote_host_filter); + const RemoteHostFilter & remote_host_filter, + const Context & global_context); std::shared_ptr create( Aws::Client::ClientConfiguration & cfg, bool is_virtual_hosted_style, const String & access_key_id, const String & secret_access_key, - const RemoteHostFilter & remote_host_filter); + const RemoteHostFilter & remote_host_filter, + const Context & global_context); std::shared_ptr create( const String & endpoint, @@ -51,7 +54,8 @@ public: const String & access_key_id, const String & secret_access_key, HeaderCollection headers, - const RemoteHostFilter & remote_host_filter); + const RemoteHostFilter & remote_host_filter, + const Context & global_context); private: ClientFactory(); diff --git a/src/Storages/StorageS3.cpp b/src/Storages/StorageS3.cpp index ce9ebbd53b3..caebdf7ccd0 100644 --- a/src/Storages/StorageS3.cpp +++ b/src/Storages/StorageS3.cpp @@ -218,7 +218,8 @@ StorageS3::StorageS3( credentials = Aws::Auth::AWSCredentials(std::move(settings.access_key_id), std::move(settings.secret_access_key)); client = S3::ClientFactory::instance().create( - uri_.endpoint, uri_.is_virtual_hosted_style, access_key_id_, secret_access_key_, std::move(settings.headers), context_.getRemoteHostFilter()); + uri_.endpoint, uri_.is_virtual_hosted_style, access_key_id_, secret_access_key_, std::move(settings.headers), + context_.getRemoteHostFilter(), context_.getGlobalContext()); }