Add on-by-default option to use TCP keepalive in the REST client. (#5319

) [SC-56102](https://app.shortcut.com/tiledb-inc/story/56102/add-on-by-default-option-to-use-tcp-keepalive-in-the-rest-client) This PR enables by default TCP keepalive in the REST client connections, and adds an option to disable them. TCP keepalive is expected to reduce failure rates on large remote queries. --- TYPE: FEATURE DESC: Connections to TileDB Cloud use TCP keepalive by default. --- TYPE: CONFIG DESC: Add `rest.curl.tcp_keepalive` config option that controls using TCP keepalive for TileDB Cloud connections. It is enabled by default.
TileDB-Inc · Sep 30, 2024 · f67a4cb · f67a4cb
1 parent fa6a420
commit f67a4cb
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 2 deletions.
diff --git a/test/src/unit-capi-config.cc b/test/src/unit-capi-config.cc
@@ -229,6 +229,7 @@ void check_save_to_file() {
   ss << "rest.capnp_traversal_limit 2147483648\n";
   ss << "rest.curl.buffer_size 524288\n";
   ss << "rest.curl.retry_errors true\n";
+  ss << "rest.curl.tcp_keepalive true\n";
   ss << "rest.curl.verbose false\n";
   ss << "rest.http_compressor any\n";
   ss << "rest.load_enumerations_on_array_open false\n";
@@ -611,6 +612,7 @@ TEST_CASE("C API: Test config iter", "[capi][config]") {
   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.tcp_keepalive"] = "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/tiledb/api/c_api/config/config_api_external.h b/tiledb/api/c_api/config/config_api_external.h
@@ -742,6 +742,9 @@ TILEDB_EXPORT void tiledb_config_free(tiledb_config_t** config) TILEDB_NOEXCEPT;
  *    Set curl to run in verbose mode for REST requests <br>
  *    curl will print to stdout with this option
  *    **Default**: false
+ * -  `rest.curl.tcp_keepalive` <br>
+ *    Set curl to use TCP keepalive for REST requests <br>
+ *    **Default**: true
  * - `rest.load_metadata_on_array_open` <br>
  *    If true, array metadata will be loaded and sent to server together with
  *    the open array <br>

diff --git 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_TCP_KEEPALIVE = "true";
 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";
@@ -175,7 +176,7 @@ const std::string Config::VFS_FILE_POSIX_FILE_PERMISSIONS = "644";
 const std::string Config::VFS_FILE_POSIX_DIRECTORY_PERMISSIONS = "755";
 const std::string Config::VFS_READ_AHEAD_SIZE = "102400";          // 100KiB
 const std::string Config::VFS_READ_AHEAD_CACHE_SIZE = "10485760";  // 10MiB;
-const std::string VFS_LOG_OPERATIONS = "false";
+const std::string Config::VFS_LOG_OPERATIONS = "false";
 const std::string Config::VFS_READ_LOGGING_MODE = "";
 const std::string Config::VFS_AZURE_STORAGE_ACCOUNT_NAME = "";
 const std::string Config::VFS_AZURE_STORAGE_ACCOUNT_KEY = "";
@@ -257,6 +258,7 @@ const std::map<std::string, std::string> default_config_values = {
     std::make_pair("rest.curl.buffer_size", Config::REST_CURL_BUFFER_SIZE),
     std::make_pair(
         "rest.capnp_traversal_limit", Config::REST_CAPNP_TRAVERSAL_LIMIT),
+    std::make_pair("rest.curl.tcp_keepalive", Config::REST_CURL_TCP_KEEPALIVE),
     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(
@@ -405,7 +407,7 @@ const std::map<std::string, std::string> default_config_values = {
     std::make_pair("vfs.min_batch_gap", Config::VFS_MIN_BATCH_GAP),
     std::make_pair("vfs.min_batch_size", Config::VFS_MIN_BATCH_SIZE),
     std::make_pair("vfs.read_ahead_size", Config::VFS_READ_AHEAD_SIZE),
-    std::make_pair("vfs.log_operations", VFS_LOG_OPERATIONS),
+    std::make_pair("vfs.log_operations", Config::VFS_LOG_OPERATIONS),
     std::make_pair(
         "vfs.read_ahead_cache_size", Config::VFS_READ_AHEAD_CACHE_SIZE),
     std::make_pair(
@@ -803,6 +805,8 @@ Status Config::sanity_check(
     RETURN_NOT_OK(utils::parse::convert(value, &v));
   } else if (param == "ssl.verify") {
     RETURN_NOT_OK(utils::parse::convert(value, &v));
+  } else if (param == "rest.curl.tcp_keepalive") {
+    RETURN_NOT_OK(utils::parse::convert(value, &v));
   } else if (param == "vfs.min_parallel_size") {
     RETURN_NOT_OK(utils::parse::convert(value, &vuint64));
   } else if (param == "vfs.max_batch_size") {

diff --git 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;
 
+  /** Whether to use TCP keepalive when connecting to REST. */
+  static const std::string REST_CURL_TCP_KEEPALIVE;
+
   /** If we should retry Curl errors in requests to REST. */
   static const std::string REST_CURL_RETRY_ERRORS;
 
@@ -438,6 +441,9 @@ class Config {
   /** The maximum size (in bytes) of the VFS read-ahead cache . */
   static const std::string VFS_READ_AHEAD_CACHE_SIZE;
 
+  /** Whether to log individual VFS operations. */
+  static const std::string VFS_LOG_OPERATIONS;
+
   /** The type of read logging to perform in the VFS. */
   static const std::string VFS_READ_LOGGING_MODE;
 

diff --git a/tiledb/sm/cpp_api/config.h b/tiledb/sm/cpp_api/config.h
@@ -916,6 +916,9 @@ class Config {
    *    Set curl to run in verbose mode for REST requests <br>
    *    curl will print to stdout with this option
    *    **Default**: false
+   * -  `rest.curl.tcp_keepalive` <br>
+   *    Set curl to use TCP keepalive for REST requests <br>
+   *    **Default**: true
    * - `rest.load_metadata_on_array_open` <br>
    *    If true, array metadata will be loaded and sent to server together with
    *    the open array <br>

diff --git a/tiledb/sm/rest/curl.cc b/tiledb/sm/rest/curl.cc
@@ -292,6 +292,11 @@ Status Curl::init(
     curl_easy_setopt(curl_.get(), CURLOPT_CAPATH, ssl_cfg.ca_path().c_str());
   }
 
+  bool tcp_keepalive =
+      config_->get<bool>("rest.curl.tcp_keepalive", Config::must_find);
+
+  curl_easy_setopt(curl_.get(), CURLOPT_TCP_KEEPALIVE, tcp_keepalive ? 1L : 0L);
+
   bool found;
   RETURN_NOT_OK(
       config_->get<uint64_t>("rest.retry_count", &retry_count_, &found));