From 5808eebefdf83fc5a4b40074c74f888562766079 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Wed, 25 Sep 2024 18:06:32 +0300 Subject: [PATCH 1/2] Add on-by-default option to use TCP keepalive in the REST client. --- test/src/unit-capi-config.cc | 2 ++ tiledb/api/c_api/config/config_api_external.h | 3 +++ tiledb/sm/config/config.cc | 4 ++++ tiledb/sm/cpp_api/config.h | 3 +++ tiledb/sm/rest/curl.cc | 5 +++++ 5 files changed, 17 insertions(+) diff --git a/test/src/unit-capi-config.cc b/test/src/unit-capi-config.cc index 1f539e7a828..7fa18b7567b 100644 --- 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"; @@ -612,6 +613,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 index 4a4551bebbb..b37e1210337 100644 --- a/tiledb/api/c_api/config/config_api_external.h +++ b/tiledb/api/c_api/config/config_api_external.h @@ -739,6 +739,9 @@ TILEDB_EXPORT void tiledb_config_free(tiledb_config_t** config) TILEDB_NOEXCEPT; * Set curl to run in verbose mode for REST requests
* curl will print to stdout with this option * **Default**: false + * - `rest.curl.tcp_keepalive`
+ * Set curl to use TCP keepalive for REST requests
+ * **Default**: true * - `rest.load_metadata_on_array_open`
* If true, array metadata will be loaded and sent to server together with * the open array
diff --git a/tiledb/sm/config/config.cc b/tiledb/sm/config/config.cc index e4cf8bb667c..19a1eb5726d 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 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"; @@ -257,6 +258,7 @@ const std::map 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", 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( @@ -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/cpp_api/config.h b/tiledb/sm/cpp_api/config.h index 08c410658ea..cbcb8911e04 100644 --- a/tiledb/sm/cpp_api/config.h +++ b/tiledb/sm/cpp_api/config.h @@ -913,6 +913,9 @@ class Config { * Set curl to run in verbose mode for REST requests
* curl will print to stdout with this option * **Default**: false + * - `rest.curl.tcp_keepalive`
+ * Set curl to use TCP keepalive for REST requests
+ * **Default**: true * - `rest.load_metadata_on_array_open`
* If true, array metadata will be loaded and sent to server together with * the open array
diff --git a/tiledb/sm/rest/curl.cc b/tiledb/sm/rest/curl.cc index 921fce0db19..c50be2ad8e9 100644 --- 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("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("rest.retry_count", &retry_count_, &found)); From 237632f74e531c18b5412813da1694565dc47ee9 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Fri, 27 Sep 2024 13:17:29 +0300 Subject: [PATCH 2/2] Declare config option value constants in the header. --- tiledb/sm/config/config.cc | 8 ++++---- tiledb/sm/config/config.h | 6 ++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/tiledb/sm/config/config.cc b/tiledb/sm/config/config.cc index 19a1eb5726d..39acaf41894 100644 --- a/tiledb/sm/config/config.cc +++ b/tiledb/sm/config/config.cc @@ -91,7 +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 REST_CURL_TCP_KEEPALIVE = "true"; +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"; @@ -176,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 = ""; @@ -258,7 +258,7 @@ const std::map 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", REST_CURL_TCP_KEEPALIVE), + 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( @@ -407,7 +407,7 @@ const std::map 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( diff --git a/tiledb/sm/config/config.h b/tiledb/sm/config/config.h index dc6142b5728..1f75e6f97a0 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; + /** 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;