From 694d3182eeaf7ef1131308d87d18bc3a626b270d Mon Sep 17 00:00:00 2001 From: Ypatia Tsavliri Date: Tue, 3 Sep 2024 12:16:49 +0300 Subject: [PATCH] Retry for most curl errors --- tiledb/sm/rest/curl.cc | 383 +++++++++++++++++++++++++++-------------- tiledb/sm/rest/curl.h | 57 ++++-- 2 files changed, 302 insertions(+), 138 deletions(-) diff --git a/tiledb/sm/rest/curl.cc b/tiledb/sm/rest/curl.cc index 6b3f138b5b3..b7163ff2952 100644 --- a/tiledb/sm/rest/curl.cc +++ b/tiledb/sm/rest/curl.cc @@ -44,33 +44,13 @@ #ifdef _WIN32 #include //provides needed visibility of two-argument tolower() prototype for win32 +#include "curl.h" #endif using namespace tiledb::common; namespace tiledb::sm { -/** - * Wraps opaque user data to be invoked with a write callback. - */ -struct WriteCbState { - /** Default constructor. */ - WriteCbState() - : reset(true) - , arg(NULL) - , skip_retries(false) { - } - - /** True if this is the first write callback invoked in a request retry. */ - bool reset; - - /** The opaque user data to pass to the write callback. */ - void* arg; - - /** True if the internal curl retries should be skipped. */ - bool skip_retries; -}; - /** * @brief Callback for saving unbuffered libcurl response data * @@ -482,6 +462,60 @@ CURLcode Curl::curl_easy_perform_instrumented( return curl_code; } +void Curl::set_curl_request_options( + const char* const url, + size_t (*write_cb)(void*, size_t, size_t, void*), + WriteCbState& write_cb_state) const { + CURL* curl = curl_.get(); + if (curl == nullptr) { + throw std::runtime_error( + "Cannot make curl request; curl instance is null."); + } + /* set url to fetch */ + curl_easy_setopt(curl, CURLOPT_URL, url); + + /* set callback function */ + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_cb); + + /* pass fetch buffer pointer */ + curl_easy_setopt( + curl, CURLOPT_WRITEDATA, static_cast(&write_cb_state)); + + /* Set curl verbose mode */ + curl_easy_setopt(curl, CURLOPT_VERBOSE, verbose_); + + /* set compression */ + const char* compressor = nullptr; + throw_if_not_ok(config_->get("rest.http_compressor", &compressor)); + + if (compressor != nullptr) { + // curl expects lowecase strings so let's convert + std::string comp(compressor); + std::locale loc; + for (std::string::size_type j = 0; j < comp.length(); ++j) + comp[j] = std::tolower(comp[j], loc); + + if (comp != "none") { + if (comp == "any") { + comp = ""; + } + curl_easy_setopt(curl, CURLOPT_ACCEPT_ENCODING, comp.c_str()); + } + } + + /* enable location redirects */ + curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1); + + /* set maximum allowed redirects */ + curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 1); + + /* enable forwarding auth to redirects */ + curl_easy_setopt(curl, CURLOPT_UNRESTRICTED_AUTH, 1L); + + /* Set max buffer size */ + curl_easy_setopt(curl, CURLOPT_BUFFERSIZE, curl_buffer_size_); +} + Status Curl::make_curl_request_common( stats::Stats* const stats, const char* const url, @@ -517,106 +551,55 @@ Status Curl::make_curl_request_common( data->set_offset(current_buffer_index, current_relative_offset); } - /* set url to fetch */ - curl_easy_setopt(curl, CURLOPT_URL, url); - - /* set callback function */ - curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_cb); - - /* pass fetch buffer pointer */ - curl_easy_setopt( - curl, CURLOPT_WRITEDATA, static_cast(&write_cb_state)); - - /* Set curl verbose mode */ - curl_easy_setopt(curl, CURLOPT_VERBOSE, verbose_); - - /* set default user agent */ - // curl_easy_setopt(curl, CURLOPT_USERAGENT, "libcurl-agent/1.0"); - - /* Fail on error */ - // curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1L); - - /* set timeout */ - // curl_easy_setopt(curl, CURLOPT_TIMEOUT, 5); - - /* set compression */ - const char* compressor = nullptr; - RETURN_NOT_OK(config_->get("rest.http_compressor", &compressor)); - - if (compressor != nullptr) { - // curl expects lowecase strings so let's convert - std::string comp(compressor); - std::locale loc; - for (std::string::size_type j = 0; j < comp.length(); ++j) - comp[j] = std::tolower(comp[j], loc); - - if (comp != "none") { - if (comp == "any") { - comp = ""; - } - curl_easy_setopt(curl, CURLOPT_ACCEPT_ENCODING, comp.c_str()); - } - } - - /* enable location redirects */ - curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1); + // set the necessary curl options on the request + set_curl_request_options(url, write_cb, write_cb_state); - /* set maximum allowed redirects */ - curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 1); - - /* enable forwarding auth to redirects */ - curl_easy_setopt(curl, CURLOPT_UNRESTRICTED_AUTH, 1L); - - /* Set max buffer size */ - curl_easy_setopt(curl, CURLOPT_BUFFERSIZE, curl_buffer_size_); - - /* fetch the url */ + /* perform the blocking network transfer */ CURLcode tmp_curl_code = curl_easy_perform_instrumented(url, i); - bool retry = false; - RETURN_NOT_OK(should_retry_based_on_http_status(&retry)); - - /* If Curl call was successful (not http status, but no socket error, etc) - * break */ - if (tmp_curl_code == CURLE_OK && !retry) { - break; - } - - /* If there is a write error we should not attempt a retry but instead - * return an error */ + long http_code = 0; if (tmp_curl_code != CURLE_OK) { - /* Only store the first non-OK curl code, because it will likely be more - * useful than the curl codes from the retries. */ - if (*curl_code == CURLE_OK) { - *curl_code = tmp_curl_code; + if (curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code) != + CURLE_OK) { + return LOG_STATUS(Status_RestError( + "Error checking curl error; could not get HTTP code.")); } - - return Status_RestError( - "Curl error reading response from server: " + - get_curl_errstr(tmp_curl_code) + "."); } - /* Retry on curl errors, unless the write callback has elected - * to skip it. */ - if (write_cb_state.skip_retries) { + // Exit if the request failed and we don't want to retry based on curl or + // HTTP code, or if the write callback has elected to skip retries + if (!should_retry_request(tmp_curl_code, http_code) || + write_cb_state.skip_retries) { break; } + /* Only store the first non-OK curl code, because it will likely be more + * useful than the curl codes from the retries. */ + if (*curl_code == CURLE_OK) { + *curl_code = tmp_curl_code; + } + + // Set up the actual retry logic // Only sleep if this isn't the last failed request allowed if (i < retry_count_ - 1) { - long http_code = 0; - if (curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code) != - CURLE_OK) - return LOG_STATUS(Status_RestError( - "Error checking curl error; could not get HTTP code.")); - - global_logger().debug( - "Request to {} failed with http response code {}, will sleep {}ms, " - "retry count {}", - url, - http_code, - retry_delay, - i); + if (tmp_curl_code != CURLE_OK) { + global_logger().debug( + "Request to {} failed with Curl error message \"{}\", will sleep " + "{}ms, " + "retry count {}", + url, + get_curl_errstr(tmp_curl_code), + retry_delay, + i); + } else { + global_logger().debug( + "Request to {} failed with http response code {}, will sleep {}ms, " + "retry count {}", + url, + http_code, + retry_delay, + i); + } // Increment counter for number of retries stats->add_counter("rest_http_retries", 1); stats->add_counter("rest_http_retry_time", retry_delay); @@ -630,29 +613,166 @@ Status Curl::make_curl_request_common( return Status::Ok(); } -Status Curl::should_retry_based_on_http_status(bool* retry) const { - // Set retry to false in case we get any errors from curl api calls - *retry = false; - - CURL* curl = curl_.get(); - if (curl == nullptr) - return LOG_STATUS( - Status_RestError("Error checking curl error; curl instance is null.")); - - long http_code = 0; - if (curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code) != CURLE_OK) - return LOG_STATUS(Status_RestError( - "Error checking curl error; could not get HTTP code.")); - +bool Curl::should_retry_based_on_http_status(long http_code) const { // Check http code for list of retries for (const auto& retry_code : retry_http_codes_) { if (http_code == (decltype(http_code))retry_code) { - *retry = true; - break; + return true; } } - return Status::Ok(); + return false; +} + +bool Curl::should_retry_based_on_curl_code(CURLcode curl_code) const { + switch (curl_code) { + // Curl status of okay or non transient errors shouldn't be retried + case CURLE_OK: + case CURLE_URL_MALFORMAT: /* 3 */ + case CURLE_SSL_SHUTDOWN_FAILED: /* 80 - Failed to shut down the SSL + connection */ + case CURLE_AUTH_ERROR: /* 94 - an authentication function returned an + error */ + return false; + case CURLE_UNSUPPORTED_PROTOCOL: /* 1 */ + case CURLE_FAILED_INIT: /* 2 */ + case CURLE_NOT_BUILT_IN: /* 4 - [was obsoleted in August 2007 for + 7.17.0, reused in April 2011 for 7.21.5] */ + case CURLE_COULDNT_RESOLVE_PROXY: /* 5 */ + case CURLE_COULDNT_RESOLVE_HOST: /* 6 */ + case CURLE_COULDNT_CONNECT: /* 7 */ + case CURLE_WEIRD_SERVER_REPLY: /* 8 */ + case CURLE_REMOTE_ACCESS_DENIED: /* 9 a service was denied by the server + due to lack of access - when login fails + this is not returned. */ + case CURLE_FTP_ACCEPT_FAILED: /* 10 - [was obsoleted in April 2006 for + 7.15.4, reused in Dec 2011 for 7.24.0]*/ + case CURLE_FTP_WEIRD_PASS_REPLY: /* 11 */ + case CURLE_FTP_ACCEPT_TIMEOUT: /* 12 - timeout occurred accepting server + [was obsoleted in August 2007 for 7.17.0, + reused in Dec 2011 for 7.24.0]*/ + case CURLE_FTP_WEIRD_PASV_REPLY: /* 13 */ + case CURLE_FTP_WEIRD_227_FORMAT: /* 14 */ + case CURLE_FTP_CANT_GET_HOST: /* 15 */ + case CURLE_HTTP2: /* 16 - A problem in the http2 framing layer. + [was obsoleted in August 2007 for 7.17.0, + reused in July 2014 for 7.38.0] */ + case CURLE_FTP_COULDNT_SET_TYPE: /* 17 */ + case CURLE_PARTIAL_FILE: /* 18 */ + case CURLE_FTP_COULDNT_RETR_FILE: /* 19 */ + case CURLE_OBSOLETE20: /* 20 - NOT USED */ + case CURLE_QUOTE_ERROR: /* 21 - quote command failure */ + case CURLE_HTTP_RETURNED_ERROR: /* 22 */ + case CURLE_WRITE_ERROR: /* 23 */ + case CURLE_OBSOLETE24: /* 24 - NOT USED */ + case CURLE_UPLOAD_FAILED: /* 25 - failed upload "command" */ + case CURLE_READ_ERROR: /* 26 - couldn't open/read from file */ + case CURLE_OUT_OF_MEMORY: /* 27 */ + case CURLE_OPERATION_TIMEDOUT: /* 28 - the timeout time was reached */ + case CURLE_OBSOLETE29: /* 29 - NOT USED */ + case CURLE_FTP_PORT_FAILED: /* 30 - FTP PORT operation failed */ + case CURLE_FTP_COULDNT_USE_REST: /* 31 - the REST command failed */ + case CURLE_OBSOLETE32: /* 32 - NOT USED */ + case CURLE_RANGE_ERROR: /* 33 - RANGE "command" didn't work */ + case CURLE_HTTP_POST_ERROR: /* 34 */ + case CURLE_SSL_CONNECT_ERROR: /* 35 - wrong when connecting with SSL */ + case CURLE_BAD_DOWNLOAD_RESUME: /* 36 - couldn't resume download */ + case CURLE_FILE_COULDNT_READ_FILE: /* 37 */ + case CURLE_LDAP_CANNOT_BIND: /* 38 */ + case CURLE_LDAP_SEARCH_FAILED: /* 39 */ + case CURLE_OBSOLETE40: /* 40 - NOT USED */ + case CURLE_FUNCTION_NOT_FOUND: /* 41 - NOT USED starting with 7.53.0 */ + case CURLE_ABORTED_BY_CALLBACK: /* 42 */ + case CURLE_BAD_FUNCTION_ARGUMENT: /* 43 */ + case CURLE_OBSOLETE44: /* 44 - NOT USED */ + case CURLE_INTERFACE_FAILED: /* 45 - CURLOPT_INTERFACE failed */ + case CURLE_OBSOLETE46: /* 46 - NOT USED */ + case CURLE_TOO_MANY_REDIRECTS: /* 47 - catch endless re-direct loops */ + case CURLE_UNKNOWN_OPTION: /* 48 - User specified an unknown option */ + case CURLE_SETOPT_OPTION_SYNTAX: /* 49 - Malformed setopt option */ + case CURLE_OBSOLETE50: /* 50 - NOT USED */ + case CURLE_OBSOLETE51: /* 51 - NOT USED */ + case CURLE_GOT_NOTHING: /* 52 - when this is a specific error */ + case CURLE_SSL_ENGINE_NOTFOUND: /* 53 - SSL crypto engine not found */ + case CURLE_SSL_ENGINE_SETFAILED: /* 54 - can not set SSL crypto engine as + default */ + case CURLE_SEND_ERROR: /* 55 - failed sending network data */ + case CURLE_RECV_ERROR: /* 56 - failure in receiving network data */ + case CURLE_OBSOLETE57: /* 57 - NOT IN USE */ + case CURLE_SSL_CERTPROBLEM: /* 58 - problem with the local certificate */ + case CURLE_SSL_CIPHER: /* 59 - couldn't use specified cipher */ + case CURLE_PEER_FAILED_VERIFICATION: /* 60 - peer's certificate or + fingerprint wasn't verified fine */ + case CURLE_BAD_CONTENT_ENCODING: /* 61 - Unrecognized/bad encoding */ + case CURLE_OBSOLETE62: /* 62 - NOT IN USE since 7.82.0 */ + case CURLE_FILESIZE_EXCEEDED: /* 63 - Maximum file size exceeded */ + case CURLE_USE_SSL_FAILED: /* 64 - Requested FTP SSL level failed */ + case CURLE_SEND_FAIL_REWIND: /* 65 - Sending the data requires a rewind + that failed */ + case CURLE_SSL_ENGINE_INITFAILED: /* 66 - failed to initialise ENGINE */ + case CURLE_LOGIN_DENIED: /* 67 - user, password or similar was not + accepted and we failed to login */ + case CURLE_TFTP_NOTFOUND: /* 68 - file not found on server */ + case CURLE_TFTP_PERM: /* 69 - permission problem on server */ + case CURLE_REMOTE_DISK_FULL: /* 70 - out of disk space on server */ + case CURLE_TFTP_ILLEGAL: /* 71 - Illegal TFTP operation */ + case CURLE_TFTP_UNKNOWNID: /* 72 - Unknown transfer ID */ + case CURLE_REMOTE_FILE_EXISTS: /* 73 - File already exists */ + case CURLE_TFTP_NOSUCHUSER: /* 74 - No such user */ + case CURLE_OBSOLETE75: /* 75 - NOT IN USE since 7.82.0 */ + case CURLE_OBSOLETE76: /* 76 - NOT IN USE since 7.82.0 */ + case CURLE_SSL_CACERT_BADFILE: /* 77 - could not load CACERT file, missing + or wrong format */ + case CURLE_REMOTE_FILE_NOT_FOUND: /* 78 - remote file not found */ + case CURLE_SSH: /* 79 - error from the SSH layer, somewhat + generic so the error message will be of + interest when this has happened */ + + case CURLE_AGAIN: /* 81 - socket is not ready for send/recv, + wait till it's ready and try again (Added + in 7.18.2) */ + case CURLE_SSL_CRL_BADFILE: /* 82 - could not load CRL file, missing or + wrong format (Added in 7.19.0) */ + case CURLE_SSL_ISSUER_ERROR: /* 83 - Issuer check failed. (Added in + 7.19.0) */ + case CURLE_FTP_PRET_FAILED: /* 84 - a PRET command failed */ + case CURLE_RTSP_CSEQ_ERROR: /* 85 - mismatch of RTSP CSeq numbers */ + case CURLE_RTSP_SESSION_ERROR: /* 86 - mismatch of RTSP Session Ids */ + case CURLE_FTP_BAD_FILE_LIST: /* 87 - unable to parse FTP file list */ + case CURLE_CHUNK_FAILED: /* 88 - chunk callback reported error */ + case CURLE_NO_CONNECTION_AVAILABLE: /* 89 - No connection available, the + session will be queued */ + case CURLE_SSL_PINNEDPUBKEYNOTMATCH: /* 90 - specified pinned public key did + not match */ + case CURLE_SSL_INVALIDCERTSTATUS: /* 91 - invalid certificate status */ + case CURLE_HTTP2_STREAM: /* 92 - stream error in HTTP/2 framing layer + */ + case CURLE_RECURSIVE_API_CALL: /* 93 - an api function was called from + inside a callback */ + case CURLE_HTTP3: /* 95 - An HTTP/3 layer problem */ + case CURLE_QUIC_CONNECT_ERROR: /* 96 - QUIC connection error */ + case CURLE_PROXY: /* 97 - proxy handshake error */ + case CURLE_SSL_CLIENTCERT: /* 98 - client-side certificate required */ + case CURLE_UNRECOVERABLE_POLL: /* 99 - poll/select returned fatal error */ + case CURLE_TOO_LARGE: /* 100 - a value/data met its maximum */ + case CURLE_ECH_REQUIRED: /* 101 - ECH tried but failed */ + return true; + default: + return false; + } +} + +bool Curl::should_retry_request(CURLcode curl_code, long http_code) const { + CURL* curl = curl_.get(); + if (curl == nullptr) + throw std::runtime_error( + "Cannot make curl request; curl instance is null."); + + if (curl_code != CURLE_OK) { + return should_retry_based_on_curl_code(curl_code); + } else { + return should_retry_based_on_http_status(http_code); + } } tuple> Curl::last_http_status_code() { @@ -680,12 +800,20 @@ Status Curl::check_curl_errors( return LOG_STATUS( Status_RestError("Error checking curl error; curl instance is null.")); + if (curl_code != CURLE_OK) { + std::stringstream msg; + msg << "Error in libcurl " << operation + << " operation: libcurl error message '" << get_curl_errstr(curl_code) + << "; "; + return Status_RestError(msg.str()); + } + long http_code = 0; if (curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code) != CURLE_OK) return LOG_STATUS(Status_RestError( "Error checking curl error; could not get HTTP code.")); - if (curl_code != CURLE_OK || http_code >= 400) { + if (http_code >= 400) { // TODO: Should see if message has error data object std::stringstream msg; msg << "Error in libcurl " << operation @@ -702,7 +830,6 @@ Status Curl::check_curl_errors( msg << "server response was empty."; } } - return Status_RestError(msg.str()); } diff --git a/tiledb/sm/rest/curl.h b/tiledb/sm/rest/curl.h index 47f8fc4b6db..a1f3c182e64 100644 --- a/tiledb/sm/rest/curl.h +++ b/tiledb/sm/rest/curl.h @@ -90,6 +90,27 @@ struct HeaderCbData { bool should_cache_redirect; }; +/** + * Wraps opaque user data to be invoked with a write callback. + */ +struct WriteCbState { + /** Default constructor. */ + WriteCbState() + : reset(true) + , arg(NULL) + , skip_retries(false) { + } + + /** True if this is the first write callback invoked in a request retry. */ + bool reset; + + /** The opaque user data to pass to the write callback. */ + void* arg; + + /** True if the internal curl retries should be skipped. */ + bool skip_retries; +}; + /** * This callback function gets called by libcurl as soon as a header has been * received. libcurl buffers headers and delivers only "full" headers, one by @@ -343,6 +364,15 @@ class Curl { */ tuple> last_http_status_code(); + /** + * Checks if the request should be retried + * + * @param curl_code the curl code of the attempted request + * @param http_code http code to check + * @return retry or not + */ + bool should_retry_request(CURLcode curl_code, long http_request) const; + private: /** * A libcurl initializer instance. This should remain @@ -482,17 +512,16 @@ class Curl { const char* const url, const uint8_t retry_number) const; /** - * Common code shared between variants of 'make_curl_options_request'. + * Set needed options on the curl request * - * @param stats The stats instance to record into * @param url URL to fetch - * @param curl_code Set to the return value of the curl call - * @return Status + * @param write_cb Callback to invoke as response data is received. + * @param write_cb_state A data pointer to pass to the write callback. */ - Status make_curl_request_options_common( - stats::Stats* const stats, + void set_curl_request_options( const char* const url, - CURLcode* const curl_code) const; + size_t (*write_cb)(void*, size_t, size_t, void*), + WriteCbState& write_cb_state) const; /** * Check the given curl code for errors, returning a TileDB error status if @@ -520,10 +549,18 @@ class Curl { /** * Checks the curl http status code to see if it matches a list of http * requests to retry - * @param retry true if the http code matches the retry list - * @return Status + * @param http_code http code to check + * @return true if should be retried, false otherwise + */ + bool should_retry_based_on_http_status(long http_code) const; + + /** + * Checks the curl return code to see if the request should be retried + * + * @param curl_code the curl code to check + * @return retry or not */ - Status should_retry_based_on_http_status(bool* retry) const; + bool should_retry_based_on_curl_code(CURLcode curl_code) const; }; } // namespace sm