-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set timeouts for HTTP requests #613
Comments
A quick test on top of #603 diff --git a/cpp/src/shim/libcurl.cpp b/cpp/src/shim/libcurl.cpp
index 825c38a..d2912d1 100644
--- a/cpp/src/shim/libcurl.cpp
+++ b/cpp/src/shim/libcurl.cpp
@@ -110,6 +110,7 @@ CurlHandle::CurlHandle(LibCurl::UniqueHandlePtr handle,
// Make curl_easy_perform() fail when receiving HTTP code errors.
setopt(CURLOPT_FAILONERROR, 1L);
+ setopt(CURLOPT_TIMEOUT, 5L);
}
CurlHandle::~CurlHandle() noexcept { LibCurl::instance().retain_handle(std::move(_handle)); }
diff --git a/python/kvikio/tests/test_http_io.py b/python/kvikio/tests/test_http_io.py
index b084f81..2998baf 100644
--- a/python/kvikio/tests/test_http_io.py
+++ b/python/kvikio/tests/test_http_io.py
@@ -3,6 +3,7 @@
import http
+import time
from http.server import SimpleHTTPRequestHandler
from typing import Literal
@@ -68,6 +69,7 @@ class HTTP500Handler(SimpleHTTPRequestHandler):
return super().do_HEAD()
def do_GET(self) -> None:
+ time.sleep(10)
return self._do_with_error_count("GET")
def do_HEAD(self) -> None: The HTTP server sleeps for 10 seconds before responding to GET requests. kvikio is configured to time out after 5s. The first GET request time out, and isn't retried:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
xref #603 (comment)
Investigate whether it's possible for the current remote IO implementation to hang indefinitely. https://curl.se/libcurl/c/CURLOPT_TIMEOUT.html describes the option we probably want. We can add that as an option in
libcurl.cpp
next to the other CURLOPTs.We'd need to figure out a good default value for this parameter and a way to configure it.
There's an interaction with #603: timeout errors raised by libcurl is one of the non-completed HTTP request / response errors that should be retried automatically (along with things like closed sockets).
The text was updated successfully, but these errors were encountered: