From 0ac064b3297acae608a59a8fafa33028a9923e49 Mon Sep 17 00:00:00 2001 From: Ypatia Tsavliri Date: Wed, 4 Sep 2024 11:49:32 +0300 Subject: [PATCH] Enable curl error retries. (#5273) Core-side retries on curl errors have been explicitly disallowed in the past in https://github.com/TileDB-Inc/TileDB/pull/3712 to protect from double writes in case of errors in a connection teardown phase, so after a fragment has been written, where a retry would just re-write the same fragment. However, we have experienced spurious "Connection reset by peer" type of errors especially during ingestion (Write queries), which we think could be mitigating by allowing retries for most of curl errors. (exclude those that typically happen during connection teardown). [[sc-54377](https://app.shortcut.com/tiledb-inc/story/54277/enable-retries-for-most-curl-error-codes-in-core)] --- TYPE: IMPROVEMENT DESC: Enable curl error retries. --------- Co-authored-by: Shaun M Reed --- test/src/unit-capi-config.cc | 2 + test/src/unit-curl.cc | 37 ++ tiledb/api/c_api/config/config_api_external.h | 3 + tiledb/sm/config/config.cc | 2 + tiledb/sm/config/config.h | 3 + tiledb/sm/cpp_api/config.h | 3 + tiledb/sm/rest/curl.cc | 388 ++++++++++++------ tiledb/sm/rest/curl.h | 60 ++- 8 files changed, 359 insertions(+), 139 deletions(-) diff --git a/test/src/unit-capi-config.cc b/test/src/unit-capi-config.cc index 8875898ce62..c3e4160b0e9 100644 --- a/test/src/unit-capi-config.cc +++ b/test/src/unit-capi-config.cc @@ -228,6 +228,7 @@ void check_save_to_file() { ss << "filestore.buffer_size 104857600\n"; ss << "rest.capnp_traversal_limit 2147483648\n"; ss << "rest.curl.buffer_size 524288\n"; + ss << "rest.curl.retry_errors true\n"; ss << "rest.curl.verbose false\n"; ss << "rest.http_compressor any\n"; ss << "rest.load_enumerations_on_array_open false\n"; @@ -609,6 +610,7 @@ TEST_CASE("C API: Test config iter", "[capi][config]") { all_param_values["rest.retry_http_codes"] = "503"; all_param_values["rest.capnp_traversal_limit"] = "2147483648"; all_param_values["rest.curl.buffer_size"] = "524288"; + all_param_values["rest.curl.retry_errors"] = "true"; all_param_values["rest.curl.verbose"] = "false"; all_param_values["rest.load_metadata_on_array_open"] = "false"; all_param_values["rest.load_non_empty_domain_on_array_open"] = "false"; diff --git a/test/src/unit-curl.cc b/test/src/unit-curl.cc index ee178c52082..e24deb32ab8 100644 --- a/test/src/unit-curl.cc +++ b/test/src/unit-curl.cc @@ -182,3 +182,40 @@ TEST_CASE( const auto& extra_headers = resources.rest_client()->extra_headers(); CHECK(extra_headers.at("X-Payer") == "foo"); } + +TEST_CASE("Test retry logic", "[rest-client][retries]") { + // set the HTTP codes to retry + tiledb::sm::Config cfg; + REQUIRE(cfg.set("rest.retry_http_codes", "503").ok()); + + std::unordered_map extra_headers; + std::unordered_map res_headers; + std::mutex res_mtx; + Curl curl(tiledb::test::g_helper_logger()); + auto rc = curl.init(&cfg, extra_headers, &res_headers, &res_mtx); + + // http error not in retry list + CURLcode curl_code = CURLE_OK; + long http_code = 504; + REQUIRE(curl.should_retry_request(curl_code, http_code) == false); + + // http error in retry list + http_code = 503; + REQUIRE(curl.should_retry_request(curl_code, http_code) == true); + + // Curl error not in retry list + curl_code = CURLE_SSL_SHUTDOWN_FAILED; + REQUIRE(curl.should_retry_request(curl_code, http_code) == false); + + // Curl error in retry list + curl_code = CURLE_RECV_ERROR; + // and http error not in retry list + http_code = 504; + REQUIRE(curl.should_retry_request(curl_code, http_code) == true); + + // Curl error in retry list but retries disabled in config + REQUIRE(cfg.set("rest.curl.retry_errors", "false").ok()); + rc = curl.init(&cfg, extra_headers, &res_headers, &res_mtx); + curl_code = CURLE_RECV_ERROR; + REQUIRE(curl.should_retry_request(curl_code, http_code) == false); +} diff --git a/tiledb/api/c_api/config/config_api_external.h b/tiledb/api/c_api/config/config_api_external.h index 73893c7c86b..a071dbe554b 100644 --- a/tiledb/api/c_api/config/config_api_external.h +++ b/tiledb/api/c_api/config/config_api_external.h @@ -728,6 +728,9 @@ TILEDB_EXPORT void tiledb_config_free(tiledb_config_t** config) TILEDB_NOEXCEPT; * The delay factor to exponentially wait until further retries of a failed * REST request
* **Default**: 1.25 + * - `rest.curl.retry_errors`
+ * If true any curl requests that returned an error will be retried
+ * **Default**: true * - `rest.curl.verbose`
* Set curl to run in verbose mode for REST requests
* curl will print to stdout with this option diff --git a/tiledb/sm/config/config.cc b/tiledb/sm/config/config.cc index 107770cc239..660f14dd5e4 100644 --- a/tiledb/sm/config/config.cc +++ b/tiledb/sm/config/config.cc @@ -91,6 +91,7 @@ const std::string Config::REST_RETRY_DELAY_FACTOR = "1.25"; const std::string Config::REST_CURL_BUFFER_SIZE = "524288"; const std::string Config::REST_CAPNP_TRAVERSAL_LIMIT = "2147483648"; const std::string Config::REST_CURL_VERBOSE = "false"; +const std::string Config::REST_CURL_RETRY_ERRORS = "true"; const std::string Config::REST_LOAD_ENUMERATIONS_ON_ARRAY_OPEN = "false"; const std::string Config::REST_LOAD_METADATA_ON_ARRAY_OPEN = "true"; const std::string Config::REST_LOAD_NON_EMPTY_DOMAIN_ON_ARRAY_OPEN = "true"; @@ -256,6 +257,7 @@ const std::map default_config_values = { std::make_pair( "rest.capnp_traversal_limit", Config::REST_CAPNP_TRAVERSAL_LIMIT), std::make_pair("rest.curl.verbose", Config::REST_CURL_VERBOSE), + std::make_pair("rest.curl.retry_errors", Config::REST_CURL_RETRY_ERRORS), std::make_pair( "rest.load_enumerations_on_array_open", Config::REST_LOAD_ENUMERATIONS_ON_ARRAY_OPEN), diff --git a/tiledb/sm/config/config.h b/tiledb/sm/config/config.h index 0ed2e5e050d..dc6142b5728 100644 --- a/tiledb/sm/config/config.h +++ b/tiledb/sm/config/config.h @@ -110,6 +110,9 @@ class Config { /** The default for Curl's verbose mode used by REST. */ static const std::string REST_CURL_VERBOSE; + /** If we should retry Curl errors in requests to REST. */ + static const std::string REST_CURL_RETRY_ERRORS; + /** If the array enumerations should be loaded on array open */ static const std::string REST_LOAD_ENUMERATIONS_ON_ARRAY_OPEN; diff --git a/tiledb/sm/cpp_api/config.h b/tiledb/sm/cpp_api/config.h index dd50be155d5..6e0105ed999 100644 --- a/tiledb/sm/cpp_api/config.h +++ b/tiledb/sm/cpp_api/config.h @@ -903,6 +903,9 @@ class Config { * The delay factor to exponentially wait until further retries of a failed * REST request
* **Default**: 1.25 + * - `rest.curl.retry_errors`
+ * If true any curl requests that returned an error will be retried
+ * **Default**: true * - `rest.curl.verbose`
* Set curl to run in verbose mode for REST requests
* curl will print to stdout with this option diff --git a/tiledb/sm/rest/curl.cc b/tiledb/sm/rest/curl.cc index 6b3f138b5b3..ca16bfa3118 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 * @@ -257,7 +237,8 @@ Curl::Curl(const std::shared_ptr& logger) , retry_delay_factor_(0) , retry_initial_delay_ms_(0) , logger_(logger->clone("curl ", ++logger_id_)) - , verbose_(false) { + , verbose_(false) + , retry_curl_errors_(true) { } Status Curl::init( @@ -343,6 +324,9 @@ Status Curl::init( "rest.curl.buffer_size", &curl_buffer_size_, &found)); assert(found); + retry_curl_errors_ = + config_->get("rest.curl.retry_errors", Config::must_find); + return Status::Ok(); } @@ -482,6 +466,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 +555,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"); + // set the necessary curl options on the request + set_curl_request_options(url, write_cb, write_cb_state); - /* 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); + /* perform the blocking network transfer */ + CURLcode tmp_curl_code = curl_easy_perform_instrumented(url, i); - if (comp != "none") { - if (comp == "any") { - comp = ""; - } - curl_easy_setopt(curl, CURLOPT_ACCEPT_ENCODING, comp.c_str()); + long http_code = 0; + if (tmp_curl_code == CURLE_OK) { + 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.")); } } - /* 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_); - - /* fetch the url */ - 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) { + // 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; } - /* If there is a write error we should not attempt a retry but instead - * return an error */ - 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; - } - - 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) { - 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 +617,165 @@ 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 { + if (!retry_curl_errors_) { + return false; + } + + 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 { + 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 +803,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 +833,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..43ceeab8022 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 @@ -390,6 +420,9 @@ class Curl { /** Max curl buffer size for received data. */ uint64_t curl_buffer_size_; + /* Retry requests with curl errors */ + bool retry_curl_errors_; + /** * Populates the curl slist with authorization (token or username+password), * and any extra headers. @@ -482,17 +515,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 +552,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