On Thu, 2020-01-02 at 11:44 -0500, Frank Ch. Eigler wrote: > Hi - > > > > That suggests one timeout could be sufficient - the progress > > > timeout > > > you the one you found - just not too short and not too fast. > > > > How about the attached (untested) patch?
I finally tested it in various setups. Including your cool test server: https://sourceware.org/ml/systemtap/2020-q1/msg00002.html > That looks good, though I'd bump up the 60s -> 120s to give it a big > margin over already-observed latencies. Lets split the difference and make it 90s. That is more than twice the worst delay I ever observed. Pushed the attached. Cheers, Mark
From b8d85ed024a745cff05e56c6337d95d654d5294a Mon Sep 17 00:00:00 2001 From: Mark Wielaard <m...@klomp.org> Date: Thu, 2 Jan 2020 17:02:42 +0100 Subject: [PATCH] debuginfod: Use DEBUGINFOD_TIMEOUT as seconds to get at least 100K. Use just one timeout using CURLOPT_LOW_SPEED_TIME (default 90 seconds) and CURLOPT_LOW_SPEED_LIMIT (100K). Signed-off-by: Mark Wielaard <m...@klomp.org> --- debuginfod/ChangeLog | 8 ++++++++ debuginfod/debuginfod-client.c | 30 +++++++++++++----------------- doc/ChangeLog | 7 +++++++ doc/debuginfod-find.1 | 11 ++++------- doc/debuginfod.8 | 6 ++++-- doc/debuginfod_find_debuginfo.3 | 11 ++++------- tests/ChangeLog | 4 ++++ tests/run-debuginfod-find.sh | 2 +- 8 files changed, 45 insertions(+), 34 deletions(-) diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 6cfb4e24..18778521 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,11 @@ +2019-01-02 Mark Wielaard <m...@klomp.org> + + * debuginfod.cxx (default_connect_timeout): Removed. + (default_transfer_timeout): Removed. + (default_timeout): New. Default to 90 seconds. + (debuginfod_query_server): Parse server_timeout_envvar as one number. + Set as CURLOPT_LOW_SPEED_TIME, with CURL_OPT_LOW_SPEED_LIMITE as 100K. + 2020-01-09 Frank Ch. Eigler <f...@redhat.com> * debuginfod-client.c (add_extra_headers): New function, diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index 66ccb21a..e5a2e824 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -105,10 +105,9 @@ static const char *server_urls_envvar = DEBUGINFOD_URLS_ENV_VAR; static const char *url_delim = " "; static const char url_delim_char = ' '; -/* Timeout for debuginfods, in seconds. */ +/* Timeout for debuginfods, in seconds (to get at least 100K). */ static const char *server_timeout_envvar = DEBUGINFOD_TIMEOUT_ENV_VAR; -static const long default_connect_timeout = 5; -static const long default_transfer_timeout = -1; /* unlimited */ +static const long default_timeout = 90; /* Data associated with a particular CURL easy handle. Passed to @@ -483,18 +482,10 @@ debuginfod_query_server (debuginfod_client *c, return fd; } - long connect_timeout = default_connect_timeout; - long transfer_timeout = default_transfer_timeout; + long timeout = default_timeout; const char* timeout_envvar = getenv(server_timeout_envvar); if (timeout_envvar != NULL) - { - long ct, tt; - rc = sscanf(timeout_envvar, "%ld,%ld", &ct, &tt); - if (rc >= 1) - connect_timeout = ct; - if (rc >= 2) - transfer_timeout = tt; - } + timeout = atoi (timeout_envvar); /* make a copy of the envvar so it can be safely modified. */ server_urls = strdup(urls_envvar); @@ -586,10 +577,15 @@ debuginfod_query_server (debuginfod_client *c, CURLOPT_WRITEFUNCTION, debuginfod_write_callback); curl_easy_setopt(data[i].handle, CURLOPT_WRITEDATA, (void*)&data[i]); - if (connect_timeout >= 0) - curl_easy_setopt(data[i].handle, CURLOPT_CONNECTTIMEOUT, connect_timeout); - if (transfer_timeout >= 0) - curl_easy_setopt(data[i].handle, CURLOPT_TIMEOUT, transfer_timeout); + if (timeout > 0) + { + /* Make sure there is at least some progress, + try to get at least 100K per timeout seconds. */ + curl_easy_setopt (data[i].handle, CURLOPT_LOW_SPEED_TIME, + timeout); + curl_easy_setopt (data[i].handle, CURLOPT_LOW_SPEED_LIMIT, + 100 * 1024L); + } curl_easy_setopt(data[i].handle, CURLOPT_FILETIME, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_FOLLOWLOCATION, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_FAILONERROR, (long) 1); diff --git a/doc/ChangeLog b/doc/ChangeLog index 1422766d..b40a141b 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,3 +1,10 @@ +2020-01-02 Mark Wielaard <m...@klomp.org> + + * debuginfod.8 (DEBUGINFOD_TIMEOUT): Document as seconds to + provide 100K, default 90. + * debuginfod-find.1 (DEBUGINFOD_TIMEOUT): Likewise. + * debuginfod_find_debuginfo.3 (DEBUGINFOD_TIMEOUT): Likewise. + 2019-12-22 Frank Ch. Eigler <f...@redhat.com * debuginfod.8: Add -U (DEB) flag, generalize RPM to "archive". diff --git a/doc/debuginfod-find.1 b/doc/debuginfod-find.1 index 023acbb3..e71ca29b 100644 --- a/doc/debuginfod-find.1 +++ b/doc/debuginfod-find.1 @@ -119,13 +119,10 @@ debuginfod instances. Alternate URL prefixes are separated by space. .TP 21 .B DEBUGINFOD_TIMEOUT -This environment variable governs the timeouts for each debuginfod -HTTP connection. One or two comma-separated numbers may be given. -The first is the number of seconds for the connection establishment -(CURLOPT_CONNECTTIMEOUT), and the default is 5. The second is the -number of seconds for the transfer completion (CURLOPT_TIMEOUT), and -the default is no timeout. (Zero or negative also means "no -timeout".) +This environment variable governs the timeout for each debuginfod HTTP +connection. A server that fails to provide at least 100K of data +within this many seconds is skipped. The default is 90 seconds. (Zero +or negative means "no timeout".) .TP 21 .B DEBUGINFOD_CACHE_PATH diff --git a/doc/debuginfod.8 b/doc/debuginfod.8 index 342f524c..6184bcce 100644 --- a/doc/debuginfod.8 +++ b/doc/debuginfod.8 @@ -366,8 +366,10 @@ or indirectly - the results would be hilarious. .TP 21 .B DEBUGINFOD_TIMEOUT This environment variable governs the timeout for each debuginfod HTTP -connection. A server that fails to respond within this many seconds -is skipped. The default is 5. +connection. A server that fails to provide at least 100K of data +within this many seconds is skipped. The default is 90 seconds. (Zero +or negative means "no timeout".) + .TP 21 .B DEBUGINFOD_CACHE_PATH diff --git a/doc/debuginfod_find_debuginfo.3 b/doc/debuginfod_find_debuginfo.3 index ea8c6161..f6ea7a45 100644 --- a/doc/debuginfod_find_debuginfo.3 +++ b/doc/debuginfod_find_debuginfo.3 @@ -163,13 +163,10 @@ debuginfod instances. Alternate URL prefixes are separated by space. .TP 21 .B DEBUGINFOD_TIMEOUT -This environment variable governs the timeouts for each debuginfod -HTTP connection. One or two comma-separated numbers may be given. -The first is the number of seconds for the connection establishment -(CURLOPT_CONNECTTIMEOUT), and the default is 5. The second is the -number of seconds for the transfer completion (CURLOPT_TIMEOUT), and -the default is no timeout. (Zero or negative also means "no -timeout".) +This environment variable governs the timeout for each debuginfod HTTP +connection. A server that fails to provide at least 100K of data +within this many seconds is skipped. The default is 90 seconds. (Zero +or negative means "no timeout".) .TP 21 .B DEBUGINFOD_PROGRESS diff --git a/tests/ChangeLog b/tests/ChangeLog index 02a8f75f..2638f63c 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,7 @@ +2020-01-02 Mark Wielaard <m...@klomp.org> + + * run-debuginfod-find.sh: Set DEBUGINFOD_TIMEOUT to 10. + 2019-12-22 Frank Ch. Eigler <f...@redhat.com> * debuginfod-debs/*: New test files, based on diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh index 90dafe00..4ab47a31 100755 --- a/tests/run-debuginfod-find.sh +++ b/tests/run-debuginfod-find.sh @@ -94,7 +94,7 @@ wait_ready $PORT1 'ready' 1 export DEBUGINFOD_URLS=http://127.0.0.1:$PORT1/ # or without trailing / # Be patient when run on a busy machine things might take a bit. -export DEBUGINFOD_TIMEOUT=1,10 +export DEBUGINFOD_TIMEOUT=10 # We use -t0 and -g0 here to turn off time-based scanning & grooming. # For testing purposes, we just sic SIGUSR1 / SIGUSR2 at the process. -- 2.18.1