[Backport release-2.26] Enable curl error retries. (#5273) (#5275)

Backport of #5273 to release-2.26

---
TYPE: IMPROVEMENT
DESC: Enable curl error retries.

---------

Co-authored-by: Ypatia Tsavliri <[email protected]>
Co-authored-by: Shaun M Reed <[email protected]>

Author: 3 people

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";

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<std::string, std::string> extra_headers;
+  std::unordered_map<std::string, std::string> 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);
+}

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 <br>
  *    **Default**: 1.25
+ * - `rest.curl.retry_errors` <br>
+ *    If true any curl requests that returned an error will be retried <br>
+ *    **Default**: true
  * - `rest.curl.verbose` <br>
  *    Set curl to run in verbose mode for REST requests <br>
  *    curl will print to stdout with this option

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<std::string, std::string> 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),

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;
 

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 <br>
    *    **Default**: 1.25
+   * - `rest.curl.retry_errors` <br>
+   *    If true any curl requests that returned an error will be retried <br>
+   *    **Default**: true
    * - `rest.curl.verbose` <br>
    *    Set curl to run in verbose mode for REST requests <br>
    *    curl will print to stdout with this option