gavinchou commented on code in PR #48624: URL: https://github.com/apache/doris/pull/48624#discussion_r1979756623
########## cloud/src/recycler/s3_accessor.cpp: ########## @@ -263,8 +354,30 @@ int S3Accessor::init() { // Therefore, you only need to add a policy to check if the response code is 429 and if the retry count meets the condition, it can record the retry count. options.PerRetryPolicies.emplace_back( std::make_unique<AzureRetryRecordPolicy>(config::max_s3_client_retry)); + auto container_client = std::make_shared<Azure::Storage::Blobs::BlobContainerClient>( uri_, cred, std::move(options)); + + try { + // Test Azure connection + auto connection_result = testAzureConnection(container_client, conf_.bucket); + switch (connection_result) { + case ConnectionTestResult::SUCCESS: + break; + case ConnectionTestResult::AUTH_ERROR: + LOG(WARNING) << ("Azure S3 authentication failed"); + return -1; + case ConnectionTestResult::TIMEOUT_ERROR: + LOG(WARNING) << ("Azure S3 connection timed out"); + return -1; + default: + LOG(WARNING) << ("Azure S3 connection failed"); + return -1; + } + } catch (const std::exception& e) { Review Comment: `testAzureConnection()` already catch all exceptions why do we need this? and the switch case can be hidden in `testAzureConnection()` too. ########## cloud/src/recycler/s3_accessor.cpp: ########## @@ -292,12 +406,35 @@ int S3Accessor::init() { if (config::s3_client_http_scheme == "http") { aws_config.scheme = Aws::Http::Scheme::HTTP; } + aws_config.retryStrategy = std::make_shared<S3CustomRetryStrategy>( config::max_s3_client_retry /*scaleFactor = 25*/); auto s3_client = std::make_shared<Aws::S3::S3Client>( std::move(aws_cred), std::move(aws_config), Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never, - conf_.use_virtual_addressing /* useVirtualAddressing */); + conf_.use_virtual_addressing); + + try { + // Test AWS S3 connection + auto connection_result = testAwsS3Connection(s3_client, conf_.bucket); + switch (connection_result) { + case ConnectionTestResult::SUCCESS: + break; + case ConnectionTestResult::AUTH_ERROR: + LOG(WARNING) << "AWS S3 authentication failed"; + return -1; + case ConnectionTestResult::TIMEOUT_ERROR: + LOG(WARNING) << "AWS S3 connection timed out"; + return -1; + default: + LOG(WARNING) << "AWS S3 connection failed"; + return -1; + } + } catch (const std::exception& e) { Review Comment: ditto -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org