Skip to content

Commit 23e2760

Browse files
Backport ClickHouse#88001 to 25.8: Add logs to S3 Client
1 parent 1a86422 commit 23e2760

File tree

15 files changed

+94
-30
lines changed

15 files changed

+94
-30
lines changed

src/Backups/BackupIO_S3.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ namespace
117117
local_settings[Setting::s3_slow_all_threads_after_retryable_error],
118118
local_settings[Setting::enable_s3_requests_logging],
119119
/* for_disk_s3 = */ false,
120+
/* opt_disk_name = */ {},
120121
request_settings.get_request_throttler,
121122
request_settings.put_request_throttler,
122123
s3_uri.uri.getScheme());

src/Coordination/KeeperSnapshotManagerS3.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ void KeeperSnapshotManagerS3::updateS3Configuration(const Poco::Util::AbstractCo
123123
s3_slow_all_threads_after_retryable_error,
124124
enable_s3_requests_logging,
125125
/* for_disk_s3 = */ false,
126+
/* opt_disk_name = */ {},
126127
/* get_request_throttler = */ {},
127128
/* put_request_throttler = */ {},
128129
new_uri.uri.getScheme());

src/Databases/DataLake/GlueCatalog.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,10 @@ GlueCatalog::GlueCatalog(
122122
s3_slow_all_threads_after_network_error,
123123
s3_slow_all_threads_after_retryable_error,
124124
enable_s3_requests_logging,
125-
false,
126-
nullptr,
127-
nullptr);
125+
/* for_disk_s3 = */ false,
126+
/* opt_disk_name = */ {},
127+
/* get_request_throttler = */ nullptr,
128+
/* put_request_throttler = */ nullptr);
128129

129130
Aws::Glue::GlueClientConfiguration client_configuration;
130131
client_configuration.maxConnections = static_cast<unsigned>(global_settings[DB::Setting::s3_max_connections]);

src/Disks/ObjectStorages/ObjectStorageFactory.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ void registerS3ObjectStorage(ObjectStorageFactory & factory)
189189
auto endpoint = getEndpoint(config, config_prefix, context);
190190
auto settings = std::make_unique<S3Settings>();
191191
settings->loadFromConfigForObjectStorage(config, config_prefix, context->getSettingsRef(), uri.uri.getScheme(), true);
192-
auto client = getClient(endpoint, *settings, context, /* for_disk_s3 */true);
192+
auto client = getClient(endpoint, *settings, context, /* for_disk_s3 */ true, name);
193193
auto key_generator = getKeyGenerator(uri, config, config_prefix);
194194

195195
auto object_storage = createObjectStorage<S3ObjectStorage>(
@@ -217,7 +217,7 @@ void registerS3PlainObjectStorage(ObjectStorageFactory & factory)
217217
auto endpoint = getEndpoint(config, config_prefix, context);
218218
auto settings = std::make_unique<S3Settings>();
219219
settings->loadFromConfigForObjectStorage(config, config_prefix, context->getSettingsRef(), uri.uri.getScheme(), true);
220-
auto client = getClient(endpoint, *settings, context, /* for_disk_s3 */true);
220+
auto client = getClient(endpoint, *settings, context, /* for_disk_s3 */ true, name);
221221
auto key_generator = getKeyGenerator(uri, config, config_prefix);
222222

223223
auto object_storage = std::make_shared<PlainObjectStorage<S3ObjectStorage>>(
@@ -244,7 +244,7 @@ void registerS3PlainRewritableObjectStorage(ObjectStorageFactory & factory)
244244
auto endpoint = getEndpoint(config, config_prefix, context);
245245
auto settings = std::make_unique<S3Settings>();
246246
settings->loadFromConfigForObjectStorage(config, config_prefix, context->getSettingsRef(), uri.uri.getScheme(), true);
247-
auto client = getClient(endpoint, *settings, context, /* for_disk_s3 */true);
247+
auto client = getClient(endpoint, *settings, context, /* for_disk_s3 */ true, name);
248248
auto key_generator = getKeyGenerator(uri, config, config_prefix);
249249

250250
auto metadata_storage_metrics = DB::MetadataStorageMetrics::create<S3ObjectStorage, MetadataStorageType::PlainRewritable>();

src/Disks/ObjectStorages/S3/S3ObjectStorage.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ void S3ObjectStorage::applyNewSettings(
508508
if (options.allow_client_change
509509
&& (current_settings->auth_settings.hasUpdates(modified_settings->auth_settings) || for_disk_s3))
510510
{
511-
auto new_client = getClient(uri, *modified_settings, context, for_disk_s3);
511+
auto new_client = getClient(uri, *modified_settings, context, for_disk_s3, disk_name);
512512
client.set(std::move(new_client));
513513
}
514514
s3_settings.set(std::move(modified_settings));

src/Disks/ObjectStorages/S3/diskSettings.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,19 +72,18 @@ std::unique_ptr<S3::Client> getClient(
7272
const std::string & endpoint,
7373
const S3Settings & settings,
7474
ContextPtr context,
75-
bool for_disk_s3)
75+
bool for_disk_s3,
76+
std::optional<std::string> opt_disk_name)
77+
7678
{
7779
auto url = S3::URI(endpoint);
7880
if (!url.key.ends_with('/'))
7981
url.key.push_back('/');
80-
return getClient(url, settings, context, for_disk_s3);
82+
return getClient(url, settings, context, for_disk_s3, opt_disk_name);
8183
}
8284

83-
std::unique_ptr<S3::Client> getClient(
84-
const S3::URI & url,
85-
const S3Settings & settings,
86-
ContextPtr context,
87-
bool for_disk_s3)
85+
std::unique_ptr<S3::Client>
86+
getClient(const S3::URI & url, const S3Settings & settings, ContextPtr context, bool for_disk_s3, std::optional<std::string> opt_disk_name)
8887
{
8988
const Settings & global_settings = context->getGlobalContext()->getSettingsRef();
9089
const auto & auth_settings = settings.auth_settings;
@@ -129,6 +128,7 @@ std::unique_ptr<S3::Client> getClient(
129128
s3_slow_all_threads_after_retryable_error,
130129
enable_s3_requests_logging,
131130
for_disk_s3,
131+
opt_disk_name,
132132
request_settings.get_request_throttler,
133133
request_settings.put_request_throttler,
134134
url.uri.getScheme());

src/Disks/ObjectStorages/S3/diskSettings.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,11 @@ std::unique_ptr<S3::Client> getClient(
1717
const std::string & endpoint,
1818
const S3Settings & settings,
1919
ContextPtr context,
20-
bool for_disk_s3);
20+
bool for_disk_s3,
21+
std::optional<std::string> opt_disk_name = {});
2122

2223
std::unique_ptr<S3::Client> getClient(
23-
const S3::URI & url_,
24-
const S3Settings & settings,
25-
ContextPtr context,
26-
bool for_disk_s3);
27-
24+
const S3::URI & url_, const S3Settings & settings, ContextPtr context, bool for_disk_s3, std::optional<std::string> opt_disk_name = {});
2825
}
2926

3027
#endif

src/IO/S3/Client.cpp

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ namespace S3
8484

8585
Client::RetryStrategy::RetryStrategy(const PocoHTTPClientConfiguration::RetryStrategy & config_)
8686
: config(config_)
87+
, log(getLogger("S3ClientRetryStrategy"))
8788
{
8889
chassert(config.max_delay_ms <= (1.0 + config.jitter_factor) * config.initial_delay_ms * (1ul << 31l));
8990
chassert(config.jitter_factor >= 0 && config.jitter_factor <= 1);
@@ -133,7 +134,7 @@ long Client::RetryStrategy::CalculateDelayBeforeNextRetry(const Aws::Client::AWS
133134
else
134135
res = std::min<uint64_t>(config.initial_delay_ms * backoffLimitedPow, config.max_delay_ms);
135136

136-
LOG_TEST(getLogger("RetryStrategy"), "Next retry in {} ms", res);
137+
LOG_TEST(log, "Next retry in {} ms", res);
137138
return res;
138139
}
139140

@@ -143,6 +144,37 @@ long Client::RetryStrategy::GetMaxAttempts() const
143144
return config.max_retries + 1;
144145
}
145146

147+
void Client::RetryStrategy::RequestBookkeeping(const Aws::Client::HttpResponseOutcome & httpResponseOutcome)
148+
{
149+
if (!httpResponseOutcome.IsSuccess())
150+
{
151+
const auto & error = httpResponseOutcome.GetError();
152+
if (error.ShouldRetry())
153+
LOG_TRACE(
154+
log,
155+
"Attempt {}/{} failed with retryable error: {}, {}",
156+
httpResponseOutcome.GetRetryCount() + 1,
157+
GetMaxAttempts(),
158+
static_cast<size_t>(error.GetResponseCode()),
159+
error.GetMessage());
160+
}
161+
}
162+
163+
void Client::RetryStrategy::RequestBookkeeping(
164+
const Aws::Client::HttpResponseOutcome & httpResponseOutcome, const Aws::Client::AWSError<Aws::Client::CoreErrors> & lastError)
165+
{
166+
if (httpResponseOutcome.IsSuccess())
167+
LOG_TRACE(
168+
log,
169+
"Attempt {}/{} succeeded with response code {}, last error: {}, {}",
170+
httpResponseOutcome.GetRetryCount() + 1,
171+
GetMaxAttempts(),
172+
static_cast<size_t>(httpResponseOutcome.GetResult()->GetResponseCode()),
173+
static_cast<size_t>(lastError.GetResponseCode()),
174+
lastError.GetMessage());
175+
RequestBookkeeping(httpResponseOutcome);
176+
}
177+
146178
namespace
147179
{
148180

@@ -254,15 +286,27 @@ Client::Client(
254286

255287
LOG_TRACE(log, "API mode of the S3 client: {}", api_mode);
256288

257-
LOG_TRACE(
258-
log,
259-
"Slowing down threads on retryable errors is {}",
260-
client_configuration.s3_slow_all_threads_after_retryable_error ? "enabled" : "disabled");
261-
262-
LOG_TRACE(
263-
log,
264-
"Slowing down threads on network errors is {}",
265-
client_configuration.s3_slow_all_threads_after_network_error ? "enabled" : "disabled");
289+
if (client_configuration.for_disk_s3)
290+
{
291+
LOG_TRACE(
292+
log,
293+
"S3 client for disk '{}' initialized with s3_retry_attempts: {}",
294+
client_configuration.opt_disk_name.value_or(""),
295+
client_configuration.retry_strategy.max_retries);
296+
LOG_TRACE(
297+
log,
298+
"S3 client for disk '{}': slowing down threads on retryable errors is {}",
299+
client_configuration.opt_disk_name.value_or(""),
300+
client_configuration.s3_slow_all_threads_after_retryable_error ? "enabled" : "disabled");
301+
}
302+
else
303+
{
304+
LOG_TRACE(log, "S3 client initialized with s3_retry_attempts: {}", client_configuration.retry_strategy.max_retries);
305+
LOG_TRACE(
306+
log,
307+
"S3 client: slowing down threads on retryable errors is {}",
308+
client_configuration.s3_slow_all_threads_after_retryable_error ? "enabled" : "disabled");
309+
}
266310

267311
detect_region = provider_type == ProviderType::AWS && explicit_region == Aws::Region::AWS_GLOBAL;
268312

@@ -1143,6 +1187,7 @@ PocoHTTPClientConfiguration ClientFactory::createClientConfiguration( // NOLINT
11431187
bool s3_slow_all_threads_after_retryable_error,
11441188
bool enable_s3_requests_logging,
11451189
bool for_disk_s3,
1190+
std::optional<std::string> opt_disk_name,
11461191
const ThrottlerPtr & get_request_throttler,
11471192
const ThrottlerPtr & put_request_throttler,
11481193
const String & protocol)
@@ -1164,6 +1209,7 @@ PocoHTTPClientConfiguration ClientFactory::createClientConfiguration( // NOLINT
11641209
s3_slow_all_threads_after_retryable_error,
11651210
enable_s3_requests_logging,
11661211
for_disk_s3,
1212+
opt_disk_name,
11671213
context->getGlobalContext()->getSettingsRef()[Setting::s3_use_adaptive_timeouts],
11681214
get_request_throttler,
11691215
put_request_throttler,

src/IO/S3/Client.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,11 @@ class Client : private Aws::S3::S3Client
165165
/// NOLINTNEXTLINE(google-runtime-int)
166166
long GetMaxAttempts() const override;
167167

168+
void RequestBookkeeping(const Aws::Client::HttpResponseOutcome & httpResponseOutcome) override;
169+
void RequestBookkeeping(
170+
const Aws::Client::HttpResponseOutcome & httpResponseOutcome,
171+
const Aws::Client::AWSError<Aws::Client::CoreErrors> & lastError) override;
172+
168173
/// Sometimes [1] GCS may suggest to use Rewrite over CopyObject, i.e.:
169174
///
170175
/// AWSError 'InternalError': Copy spanning locations and/or storage classes could not complete within 30 seconds. Please use the Rewrite method in the JSON API (https://cloud.google.com/storage/docs/json_api/v1/objects/rewrite) instead.
@@ -177,6 +182,7 @@ class Client : private Aws::S3::S3Client
177182

178183
private:
179184
PocoHTTPClientConfiguration::RetryStrategy config;
185+
LoggerPtr log;
180186
};
181187

182188
/// SSE-KMS headers MUST be signed, so they need to be added before the SDK signs the message
@@ -350,6 +356,7 @@ class ClientFactory
350356
bool s3_slow_all_threads_after_retryable_error,
351357
bool enable_s3_requests_logging,
352358
bool for_disk_s3,
359+
std::optional<std::string> opt_disk_name,
353360
const ThrottlerPtr & get_request_throttler,
354361
const ThrottlerPtr & put_request_throttler,
355362
const String & protocol = "https");

src/IO/S3/Credentials.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,7 @@ S3CredentialsProviderChain::S3CredentialsProviderChain(
742742
configuration.s3_slow_all_threads_after_retryable_error,
743743
configuration.enable_s3_requests_logging,
744744
configuration.for_disk_s3,
745+
configuration.opt_disk_name,
745746
configuration.get_request_throttler,
746747
configuration.put_request_throttler);
747748
AddProvider(std::make_shared<AwsAuthSTSAssumeRoleWebIdentityCredentialsProvider>(aws_client_configuration, credentials_configuration.expiration_window_seconds, credentials_configuration.kms_role_arn));
@@ -759,6 +760,7 @@ S3CredentialsProviderChain::S3CredentialsProviderChain(
759760
configuration.s3_slow_all_threads_after_retryable_error,
760761
configuration.enable_s3_requests_logging,
761762
configuration.for_disk_s3,
763+
configuration.opt_disk_name,
762764
configuration.get_request_throttler,
763765
configuration.put_request_throttler);
764766
AddProvider(std::make_shared<SSOCredentialsProvider>(
@@ -811,6 +813,7 @@ S3CredentialsProviderChain::S3CredentialsProviderChain(
811813
configuration.s3_slow_all_threads_after_retryable_error,
812814
configuration.enable_s3_requests_logging,
813815
configuration.for_disk_s3,
816+
configuration.opt_disk_name,
814817
configuration.get_request_throttler,
815818
configuration.put_request_throttler,
816819
Aws::Http::SchemeMapper::ToString(Aws::Http::Scheme::HTTP));

0 commit comments

Comments
 (0)