Skip to content
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion src/libstore/filetransfer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <curl/curl.h>

#include <algorithm>
#include <array>
#include <cstring>
#include <queue>
#include <random>
Expand Down Expand Up @@ -655,6 +656,14 @@ struct curlFileTransfer : public FileTransfer
#endif
curl_easy_setopt(req, CURLOPT_CONNECTTIMEOUT, fileTransfer.settings.connectTimeout.get());

/* Enable TCP keepalive to detect dead connections and server closures.
Probes every 30s to catch network failures and idle timeouts early. */
curl_easy_setopt(req, CURLOPT_TCP_KEEPALIVE, 1L);
curl_easy_setopt(req, CURLOPT_TCP_KEEPIDLE, 30L);
curl_easy_setopt(req, CURLOPT_TCP_KEEPINTVL, 30L);
/* Don't reuse idle connections older than 90s. */
curl_easy_setopt(req, CURLOPT_MAXAGE_CONN, 90L);

curl_easy_setopt(req, CURLOPT_LOW_SPEED_LIMIT, 1L);
curl_easy_setopt(req, CURLOPT_LOW_SPEED_TIME, fileTransfer.settings.stalledDownloadTimeout.get());

Expand Down Expand Up @@ -756,7 +765,38 @@ struct curlFileTransfer : public FileTransfer
// We treat most errors as transient, but won't retry when hopeless
Error err = Transient;

if (httpStatus == HttpStatus::NotFound || httpStatus == HttpStatus::Gone
// S3 returns certain retryable errors as HTTP 400/500/503 with XML error codes.
// These take precedence over the generic HTTP status handling below.
// Only parse the response body on status codes where S3 XML errors can appear.
static constexpr std::array<std::string_view, 12> s3RetryableErrors{{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomberek don't know how much of a difference it makes, but saves some heap allocation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also just de-compiled because I was interested, and it seems that it also generates less code for my variant.

"IncompleteBody", // HTTP 400 - network issue
"InternalError", // HTTP 500 - S3 internal failure
"InternalFailure", // HTTP 500 - alias for InternalError
"InternalServerError", // HTTP 500 - alias for InternalError
"RequestExpired", // HTTP 400 - clock skew / slow upload
"RequestTimeout", // HTTP 400 - stale connection reuse
"RequestTimeTooSkewed", // HTTP 403 - clock drift
"RequestThrottled", // HTTP 400 - throttling variant
"SlowDown", // HTTP 503 - throttling
"ServiceUnavailable", // HTTP 503 - temporary unavailability
"Throttling", // HTTP 400 - throttling variant
"ThrottledException", // HTTP 400 - throttling variant
}};
// S3 error responses have the form <Error><Code>...</Code>...</Error>.
// Require the <Error> root to avoid matching unrelated XML with a <Code> element.
static std::regex s3ErrorCodeRegex("<Error>[^]*<Code>([^<]+)</Code>");
std::smatch s3Match;
bool isS3XmlStatus = httpStatus == 400 || httpStatus == 403 || httpStatus == 500 || httpStatus == 503;
auto s3ErrorCode =
(isS3XmlStatus && errorSink && std::regex_search(errorSink->s, s3Match, s3ErrorCodeRegex))
? s3Match[1].str()
: "";

if (std::find(s3RetryableErrors.begin(), s3RetryableErrors.end(), s3ErrorCode)
!= s3RetryableErrors.end()) {
debug("S3 error '%s', will retry", s3ErrorCode);
} else if (
httpStatus == HttpStatus::NotFound || httpStatus == HttpStatus::Gone
|| code == CURLE_FILE_COULDNT_READ_FILE) {
// The file is definitely not there
err = NotFound;
Expand Down
Loading