Skip to content

Commit

Permalink
Add config var to toggle curl error retries
Browse files Browse the repository at this point in the history
  • Loading branch information
ypatia committed Sep 3, 2024
1 parent d9a38d7 commit 42a2335
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 1 deletion.
2 changes: 2 additions & 0 deletions test/src/unit-capi-config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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";
Expand Down
5 changes: 5 additions & 0 deletions test/src/unit-curl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,4 +212,9 @@ TEST_CASE("Test retry logic", "[rest-client][retries]") {
// 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());
curl_code = CURLE_RECV_ERROR;
REQUIRE(curl.should_retry_request(curl_code, http_code) == false);
}
3 changes: 3 additions & 0 deletions tiledb/api/c_api/config/config_api_external.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions tiledb/sm/config/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 3 additions & 0 deletions tiledb/sm/config/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 3 additions & 0 deletions tiledb/sm/cpp_api/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion tiledb/sm/rest/curl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,8 @@ Curl::Curl(const std::shared_ptr<Logger>& 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(
Expand Down Expand Up @@ -323,6 +324,10 @@ Status Curl::init(
"rest.curl.buffer_size", &curl_buffer_size_, &found));
assert(found);

RETURN_NOT_OK(config_->get<bool>(
"rest.curl.retry_errors", &retry_curl_errors_, &found));
assert(found);

return Status::Ok();
}

Expand Down Expand Up @@ -625,6 +630,10 @@ bool Curl::should_retry_based_on_http_status(long http_code) const {
}

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:
Expand Down
3 changes: 3 additions & 0 deletions tiledb/sm/rest/curl.h
Original file line number Diff line number Diff line change
Expand Up @@ -420,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.
Expand Down

0 comments on commit 42a2335

Please sign in to comment.