Skip to content
This repository was archived by the owner on Dec 21, 2023. It is now read-only.

Commit c7cdb86

Browse files
author
Guihao Liang
authored
Customized s3 error handlings when encountering non-standard errors (#3116)
* better handling
1 parent ba4fa56 commit c7cdb86

File tree

1 file changed

+29
-6
lines changed

1 file changed

+29
-6
lines changed

src/core/storage/fileio/s3_api.cpp

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <chrono>
2222
#include <core/logging/assertions.hpp>
2323
#include <core/logging/logger.hpp>
24+
#include <core/random/random.hpp>
2425
#include <core/storage/fileio/fs_utils.hpp>
2526
#include <core/storage/fileio/general_fstream.hpp>
2627
#include <core/storage/fileio/get_s3_endpoint.hpp>
@@ -503,13 +504,33 @@ list_objects_response list_objects_impl(const s3url& parsed_url,
503504

504505
} else {
505506
auto error = outcome.GetError();
506-
// Unlike CoreErrors, S3Error Never retries. Use Http code instead.
507-
// check aws-cpp-sdk-s3/source/S3Error.cpp
508-
if (error.GetResponseCode() ==
509-
Aws::Http::HttpResponseCode::TOO_MANY_REQUESTS) {
510-
n_retry++;
507+
/*
508+
* Unlike CoreErrors, S3Error Never retries on S3 errors.
509+
* Retry can be based on HTTP code or HTTP body.
510+
*
511+
* 1. if SDK uses HTTP code, e.g., ShouldRetry return true on 429.
512+
* We don't need to retry on our own since SDK already made decision to
513+
* retry based on HTTP code.
514+
*
515+
* 2. if SDK doesn't use HTTP code but HTTP body, we check HTTP on our
516+
* own if SDK doesn't think it should retry based on messages in HTTP
517+
* body. Check https://guihao-liang.github.io/2020-04-12-aws-s3-retry/
518+
*
519+
* */
520+
if (!error.ShouldRetry()) {
521+
// SDK didn't retry for us, check retry on our own decisions
522+
// especially for non-standard AWS error reply
523+
if (error.GetErrorType() == Aws::S3::S3Errors::UNKNOWN &&
524+
error.GetResponseCode() ==
525+
Aws::Http::HttpResponseCode::TOO_MANY_REQUESTS) {
526+
n_retry++;
527+
} else {
528+
// it's standard AWS error, let's stop retry immediately
529+
// and report accordingly to user
530+
n_retry = 3;
531+
}
511532

512-
if (n_retry == 3) {
533+
if (n_retry >= 3) {
513534
// amend the error msg on the last retry failure
514535
std::stringstream ss;
515536
reportS3ErrorDetailed(ss, temp_url, S3Operation::List, outcome)
@@ -524,6 +545,8 @@ list_objects_response list_objects_impl(const s3url& parsed_url,
524545
}
525546

526547
} else {
548+
// error.ShouldRetry() == true
549+
// AWS SDK already retried 3 times
527550
std::stringstream ss;
528551
reportS3ErrorDetailed(ss, temp_url, S3Operation::List, outcome)
529552
<< std::endl;

0 commit comments

Comments
 (0)