Skip to content

Commit 38ed72e

Browse files
alexey-milovidovalexey-milovidov
committed
Merge branch 'master' of github.com:ClickHouse/ClickHouse into xray
2 parents c666e1d + 9732998 commit 38ed72e

18 files changed

Lines changed: 711 additions & 155 deletions

‎.github/workflows/master.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3643,7 +3643,7 @@ jobs:
36433643
cat >> "$GITHUB_ENV" << 'EOF'
36443644
TEMP_PATH=${{runner.temp}}/unit_tests_asan
36453645
REPORTS_PATH=${{runner.temp}}/reports_dir
3646-
CHECK_NAME=Unit tests (release-clang)
3646+
CHECK_NAME=Unit tests (release)
36473647
REPO_COPY=${{runner.temp}}/unit_tests_asan/ClickHouse
36483648
EOF
36493649
- name: Download json reports

‎.github/workflows/pull_request.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4541,7 +4541,7 @@ jobs:
45414541
cat >> "$GITHUB_ENV" << 'EOF'
45424542
TEMP_PATH=${{runner.temp}}/unit_tests_asan
45434543
REPORTS_PATH=${{runner.temp}}/reports_dir
4544-
CHECK_NAME=Unit tests (release-clang)
4544+
CHECK_NAME=Unit tests (release)
45454545
REPO_COPY=${{runner.temp}}/unit_tests_asan/ClickHouse
45464546
EOF
45474547
- name: Download json reports

‎src/Dictionaries/DictionaryFactory.cpp‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@ namespace ErrorCodes
1717
extern const int UNKNOWN_ELEMENT_IN_CONFIG;
1818
}
1919

20-
void DictionaryFactory::registerLayout(const std::string & layout_type, LayoutCreateFunction create_layout, bool is_layout_complex)
20+
void DictionaryFactory::registerLayout(const std::string & layout_type, LayoutCreateFunction create_layout, bool is_layout_complex, bool has_layout_complex)
2121
{
2222
auto it = registered_layouts.find(layout_type);
2323
if (it != registered_layouts.end())
2424
throw Exception(ErrorCodes::LOGICAL_ERROR, "DictionaryFactory: the layout name '{}' is not unique", layout_type);
2525

26-
RegisteredLayout layout { .layout_create_function = create_layout, .is_layout_complex = is_layout_complex };
26+
RegisteredLayout layout { .layout_create_function = create_layout, .is_layout_complex = is_layout_complex, .has_layout_complex = has_layout_complex };
2727
registered_layouts.emplace(layout_type, std::move(layout));
2828
}
2929

@@ -89,6 +89,25 @@ bool DictionaryFactory::isComplex(const std::string & layout_type) const
8989
return it->second.is_layout_complex;
9090
}
9191

92+
bool DictionaryFactory::convertToComplex(std::string & layout_type) const
93+
{
94+
auto it = registered_layouts.find(layout_type);
95+
96+
if (it == registered_layouts.end())
97+
{
98+
throw Exception(ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG,
99+
"Unknown dictionary layout type: {}",
100+
layout_type);
101+
}
102+
103+
if (!it->second.is_layout_complex && it->second.has_layout_complex)
104+
{
105+
layout_type = "complex_key_" + layout_type;
106+
return true;
107+
}
108+
return false;
109+
}
110+
92111

93112
DictionaryFactory & DictionaryFactory::instance()
94113
{

‎src/Dictionaries/DictionaryFactory.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,18 @@ class DictionaryFactory : private boost::noncopyable
5555

5656
bool isComplex(const std::string & layout_type) const;
5757

58-
void registerLayout(const std::string & layout_type, LayoutCreateFunction create_layout, bool is_layout_complex);
58+
/// If the argument `layout_type` is not complex layout and has corresponding complex layout,
59+
/// change `layout_type` to corresponding complex and return true; otherwise do nothing and return false.
60+
bool convertToComplex(std::string & layout_type) const;
61+
62+
void registerLayout(const std::string & layout_type, LayoutCreateFunction create_layout, bool is_layout_complex, bool has_layout_complex = true);
5963

6064
private:
6165
struct RegisteredLayout
6266
{
6367
LayoutCreateFunction layout_create_function;
6468
bool is_layout_complex;
69+
bool has_layout_complex;
6570
};
6671

6772
using LayoutRegistry = std::unordered_map<std::string, RegisteredLayout>;

‎src/Dictionaries/FlatDictionary.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -683,7 +683,7 @@ void registerDictionaryFlat(DictionaryFactory & factory)
683683
return std::make_unique<FlatDictionary>(dict_id, dict_struct, std::move(source_ptr), configuration);
684684
};
685685

686-
factory.registerLayout("flat", create_layout, false);
686+
factory.registerLayout("flat", create_layout, false, false);
687687
}
688688

689689

‎src/Dictionaries/getDictionaryConfigurationFromAST.cpp‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <Functions/FunctionFactory.h>
2020
#include <Common/isLocalAddress.h>
2121
#include <Interpreters/Context.h>
22+
#include <DataTypes/DataTypeFactory.h>
2223

2324

2425
namespace DB
@@ -614,6 +615,16 @@ getDictionaryConfigurationFromAST(const ASTCreateQuery & query, ContextPtr conte
614615

615616
checkPrimaryKey(all_attr_names_and_types, pk_attrs);
616617

618+
/// If the pk size is 1 and pk's DataType is not number, we should convert to complex.
619+
/// NOTE: the data type of Numeric key(simple layout) is UInt64, so if the type is not under UInt64, type casting will lead to precision loss.
620+
DataTypePtr first_key_type = DataTypeFactory::instance().get(all_attr_names_and_types.find(pk_attrs[0])->second.type);
621+
if ((pk_attrs.size() > 1 || (pk_attrs.size() == 1 && !isNumber(first_key_type)))
622+
&& !complex
623+
&& DictionaryFactory::instance().convertToComplex(dictionary_layout->layout_type))
624+
{
625+
complex = true;
626+
}
627+
617628
buildPrimaryKeyConfiguration(xml_document, structure_element, complex, pk_attrs, query.dictionary_attributes_list);
618629

619630
buildLayoutConfiguration(xml_document, current_dictionary, query.dictionary->dict_settings, dictionary_layout);

‎src/IO/S3/Client.cpp‎

Lines changed: 91 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#include <aws/core/utils/HashingUtils.h>
1414
#include <aws/core/utils/logging/ErrorMacros.h>
1515

16+
#include <Poco/Net/NetException.h>
17+
1618
#include <IO/S3Common.h>
1719
#include <IO/S3/Requests.h>
1820
#include <IO/S3/PocoHTTPClientFactory.h>
@@ -23,6 +25,15 @@
2325

2426
#include <Common/logger_useful.h>
2527

28+
namespace ProfileEvents
29+
{
30+
extern const Event S3WriteRequestsErrors;
31+
extern const Event S3ReadRequestsErrors;
32+
33+
extern const Event DiskS3WriteRequestsErrors;
34+
extern const Event DiskS3ReadRequestsErrors;
35+
}
36+
2637
namespace DB
2738
{
2839

@@ -346,12 +357,14 @@ Model::HeadObjectOutcome Client::HeadObject(const HeadObjectRequest & request) c
346357

347358
Model::ListObjectsV2Outcome Client::ListObjectsV2(const ListObjectsV2Request & request) const
348359
{
349-
return doRequest(request, [this](const Model::ListObjectsV2Request & req) { return ListObjectsV2(req); });
360+
return doRequestWithRetryNetworkErrors</*IsReadMethod*/ true>(
361+
request, [this](const Model::ListObjectsV2Request & req) { return ListObjectsV2(req); });
350362
}
351363

352364
Model::ListObjectsOutcome Client::ListObjects(const ListObjectsRequest & request) const
353365
{
354-
return doRequest(request, [this](const Model::ListObjectsRequest & req) { return ListObjects(req); });
366+
return doRequestWithRetryNetworkErrors</*IsReadMethod*/ true>(
367+
request, [this](const Model::ListObjectsRequest & req) { return ListObjects(req); });
355368
}
356369

357370
Model::GetObjectOutcome Client::GetObject(const GetObjectRequest & request) const
@@ -361,19 +374,19 @@ Model::GetObjectOutcome Client::GetObject(const GetObjectRequest & request) cons
361374

362375
Model::AbortMultipartUploadOutcome Client::AbortMultipartUpload(const AbortMultipartUploadRequest & request) const
363376
{
364-
return doRequest(
377+
return doRequestWithRetryNetworkErrors</*IsReadMethod*/ false>(
365378
request, [this](const Model::AbortMultipartUploadRequest & req) { return AbortMultipartUpload(req); });
366379
}
367380

368381
Model::CreateMultipartUploadOutcome Client::CreateMultipartUpload(const CreateMultipartUploadRequest & request) const
369382
{
370-
return doRequest(
383+
return doRequestWithRetryNetworkErrors</*IsReadMethod*/ false>(
371384
request, [this](const Model::CreateMultipartUploadRequest & req) { return CreateMultipartUpload(req); });
372385
}
373386

374387
Model::CompleteMultipartUploadOutcome Client::CompleteMultipartUpload(const CompleteMultipartUploadRequest & request) const
375388
{
376-
auto outcome = doRequest(
389+
auto outcome = doRequestWithRetryNetworkErrors</*IsReadMethod*/ false>(
377390
request, [this](const Model::CompleteMultipartUploadRequest & req) { return CompleteMultipartUpload(req); });
378391

379392
if (!outcome.IsSuccess() || provider_type != ProviderType::GCS)
@@ -403,32 +416,38 @@ Model::CompleteMultipartUploadOutcome Client::CompleteMultipartUpload(const Comp
403416

404417
Model::CopyObjectOutcome Client::CopyObject(const CopyObjectRequest & request) const
405418
{
406-
return doRequest(request, [this](const Model::CopyObjectRequest & req) { return CopyObject(req); });
419+
return doRequestWithRetryNetworkErrors</*IsReadMethod*/ false>(
420+
request, [this](const Model::CopyObjectRequest & req) { return CopyObject(req); });
407421
}
408422

409423
Model::PutObjectOutcome Client::PutObject(const PutObjectRequest & request) const
410424
{
411-
return doRequest(request, [this](const Model::PutObjectRequest & req) { return PutObject(req); });
425+
return doRequestWithRetryNetworkErrors</*IsReadMethod*/ false>(
426+
request, [this](const Model::PutObjectRequest & req) { return PutObject(req); });
412427
}
413428

414429
Model::UploadPartOutcome Client::UploadPart(const UploadPartRequest & request) const
415430
{
416-
return doRequest(request, [this](const Model::UploadPartRequest & req) { return UploadPart(req); });
431+
return doRequestWithRetryNetworkErrors</*IsReadMethod*/ false>(
432+
request, [this](const Model::UploadPartRequest & req) { return UploadPart(req); });
417433
}
418434

419435
Model::UploadPartCopyOutcome Client::UploadPartCopy(const UploadPartCopyRequest & request) const
420436
{
421-
return doRequest(request, [this](const Model::UploadPartCopyRequest & req) { return UploadPartCopy(req); });
437+
return doRequestWithRetryNetworkErrors</*IsReadMethod*/ false>(
438+
request, [this](const Model::UploadPartCopyRequest & req) { return UploadPartCopy(req); });
422439
}
423440

424441
Model::DeleteObjectOutcome Client::DeleteObject(const DeleteObjectRequest & request) const
425442
{
426-
return doRequest(request, [this](const Model::DeleteObjectRequest & req) { return DeleteObject(req); });
443+
return doRequestWithRetryNetworkErrors</*IsReadMethod*/ false>(
444+
request, [this](const Model::DeleteObjectRequest & req) { return DeleteObject(req); });
427445
}
428446

429447
Model::DeleteObjectsOutcome Client::DeleteObjects(const DeleteObjectsRequest & request) const
430448
{
431-
return doRequest(request, [this](const Model::DeleteObjectsRequest & req) { return DeleteObjects(req); });
449+
return doRequestWithRetryNetworkErrors</*IsReadMethod*/ false>(
450+
request, [this](const Model::DeleteObjectsRequest & req) { return DeleteObjects(req); });
432451
}
433452

434453
Client::ComposeObjectOutcome Client::ComposeObject(const ComposeObjectRequest & request) const
@@ -457,7 +476,8 @@ Client::ComposeObjectOutcome Client::ComposeObject(const ComposeObjectRequest &
457476
return ComposeObjectOutcome(MakeRequest(req, endpointResolutionOutcome.GetResult(), Aws::Http::HttpMethod::HTTP_PUT));
458477
};
459478

460-
return doRequest(request, request_fn);
479+
return doRequestWithRetryNetworkErrors</*IsReadMethod*/ false>(
480+
request, request_fn);
461481
}
462482

463483
template <typename RequestType, typename RequestFn>
@@ -538,6 +558,65 @@ Client::doRequest(const RequestType & request, RequestFn request_fn) const
538558
throw Exception(ErrorCodes::TOO_MANY_REDIRECTS, "Too many redirects");
539559
}
540560

561+
template <bool IsReadMethod, typename RequestType, typename RequestFn>
562+
std::invoke_result_t<RequestFn, RequestType>
563+
Client::doRequestWithRetryNetworkErrors(const RequestType & request, RequestFn request_fn) const
564+
{
565+
auto with_retries = [this, request_fn_ = std::move(request_fn)] (const RequestType & request_)
566+
{
567+
chassert(client_configuration.retryStrategy);
568+
const Int64 max_attempts = client_configuration.retryStrategy->GetMaxAttempts();
569+
std::exception_ptr last_exception = nullptr;
570+
for (Int64 attempt_no = 0; attempt_no < max_attempts; ++attempt_no)
571+
{
572+
try
573+
{
574+
/// S3 does retries network errors actually.
575+
/// But it is matter when errors occur.
576+
/// This code retries a specific case when
577+
/// network error happens when XML document is being read from the response body.
578+
/// Hence, the response body is a stream, network errors are possible at reading.
579+
/// S3 doesn't retry them.
580+
581+
/// Not all requests can be retried in that way.
582+
/// Requests that read out response body to build the result are possible to retry.
583+
/// Requests that expose the response stream as an answer are not retried with that code. E.g. GetObject.
584+
return request_fn_(request_);
585+
}
586+
catch (Poco::Net::ConnectionResetException &)
587+
{
588+
589+
if constexpr (IsReadMethod)
590+
{
591+
if (client_configuration.for_disk_s3)
592+
ProfileEvents::increment(ProfileEvents::DiskS3ReadRequestsErrors);
593+
else
594+
ProfileEvents::increment(ProfileEvents::S3ReadRequestsErrors);
595+
}
596+
else
597+
{
598+
if (client_configuration.for_disk_s3)
599+
ProfileEvents::increment(ProfileEvents::DiskS3WriteRequestsErrors);
600+
else
601+
ProfileEvents::increment(ProfileEvents::S3WriteRequestsErrors);
602+
}
603+
604+
tryLogCurrentException(log, "Will retry");
605+
last_exception = std::current_exception();
606+
607+
auto error = Aws::Client::AWSError<Aws::Client::CoreErrors>(Aws::Client::CoreErrors::NETWORK_CONNECTION, /*retry*/ true);
608+
client_configuration.retryStrategy->CalculateDelayBeforeNextRetry(error, attempt_no);
609+
continue;
610+
}
611+
}
612+
613+
chassert(last_exception);
614+
std::rethrow_exception(last_exception);
615+
};
616+
617+
return doRequest(request, with_retries);
618+
}
619+
541620
bool Client::supportsMultiPartCopy() const
542621
{
543622
return provider_type != ProviderType::GCS;

‎src/IO/S3/Client.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,10 @@ class Client : private Aws::S3::S3Client
250250
std::invoke_result_t<RequestFn, RequestType>
251251
doRequest(const RequestType & request, RequestFn request_fn) const;
252252

253+
template <bool IsReadMethod, typename RequestType, typename RequestFn>
254+
std::invoke_result_t<RequestFn, RequestType>
255+
doRequestWithRetryNetworkErrors(const RequestType & request, RequestFn request_fn) const;
256+
253257
void updateURIForBucket(const std::string & bucket, S3::URI new_uri) const;
254258
std::optional<S3::URI> getURIFromError(const Aws::S3::S3Error & error) const;
255259
std::optional<Aws::S3::S3Error> updateURIForBucketForHead(const std::string & bucket) const;

‎tests/ci/ci_config.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@
346346
"Compatibility check (aarch64)": {
347347
"required_build": "package_aarch64",
348348
},
349-
"Unit tests (release-clang)": {
349+
"Unit tests (release)": {
350350
"required_build": "binary_release",
351351
},
352352
"Unit tests (asan)": {
@@ -509,7 +509,7 @@
509509
"Style Check",
510510
"Unit tests (asan)",
511511
"Unit tests (msan)",
512-
"Unit tests (release-clang)",
512+
"Unit tests (release)",
513513
"Unit tests (tsan)",
514514
"Unit tests (ubsan)",
515515
]

0 commit comments

Comments
 (0)