Hi Frank,
On Sun, Dec 22, 2019 at 08:38:59PM -0500, Frank Ch. Eigler wrote:
> Yeah, a connection timeout per se is probably not really worth having.
> A URL having unreasolvable hosts will fail immediately. A reachable
> http server that is fairly busy will connect, just take time. The
> only common cases a connection timeout would catch is a running http
> server that is so overloaded that it can't even service its accept(4)
> backlog, or a nonexistent one that has been tarpit/firewalled. A
> minimal progress timeout can subsume cases too.
>
> OTOH, it's worth noting that these requests only take this kind of
> time if they are being seriously serviced, i.e., "they are worth it".
> Error cases fail relatively quickly. It's the success cases - and
> these huge vmlinux files - that take time. And once the data starts
> flowing - at all - the rest will follow as rapidly as the network
> allows.
>
> 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?
Cheers,
Mark
>From 9b90beee64299aca16df5b2322ae12f76d86e826 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 60 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 1582eba5..1f7a92b1 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 60 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.
+
2019-12-22 Frank Ch. Eigler <f...@redhat.com>
* debuginfod.cxx (*_rpm_*): Rename to *_archive_* throughout.
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 9a4a0e0f..72a385c9 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -104,10 +104,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 = 60;
/* Data associated with a particular CURL easy handle. Passed to
@@ -401,18 +400,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);
@@ -504,10 +495,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..57631c7d 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 60.
+ * 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..3563e173 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 60 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..47b9efd8 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 60 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..e4507752 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 60 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.20.1
