thr3ads.net - Libguestfs - [Libguestfs] [PATCH nbdkit v2 9/9] curl: Redefine connections=<N> parameter as number of HTTP connections [Jul 2023]

If this information is useful, please help other people find it:
Share via:

Richard W.M. Jones

2023-Jul-28 17:17 UTC

[Libguestfs] [PATCH nbdkit v2 9/9] curl: Redefine connections=<N> parameter as number of HTTP connections

Previously (nbdkit 1.34) this was the number of easy handles.  However
it turns out that easy handles can open multiple HTTP connections, and
in fact there's no good way to tell how many (and they are not
shared).

Now that we are using a curl multi, curl >= 7.30 provides a way to
limit the total number of actual HTTP connections, so we should just
use it.  This is closer to what I intended this parameter to mean.

Link: https://curl.se/mail/lib-2019-03/0102.html
Link: https://curl.se/mail/lib-2019-12/0044.html
---
 plugins/curl/nbdkit-curl-plugin.pod | 13 +++++++++----
 plugins/curl/curldefs.h             |  3 +++
 plugins/curl/config.c               |  2 +-
 plugins/curl/pool.c                 |  6 +++++-
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/plugins/curl/nbdkit-curl-plugin.pod
b/plugins/curl/nbdkit-curl-plugin.pod
index 0774adadc..8223d6da6 100644
--- a/plugins/curl/nbdkit-curl-plugin.pod
+++ b/plugins/curl/nbdkit-curl-plugin.pod
@@ -58,10 +58,10 @@ L<CURLOPT_CAPATH(3)> for more information.
 
 (nbdkit E<ge> 1.34)
 
-Open up to C<N> curl connections to the web server.  The default is 4.
-Curl connections are shared between all NBD clients, so you may wish
-to increase this if you expect many simultaneous NBD clients (or a
-single client using many multi-conn connections).
+Open up to C<N> connections to the web server.  The default is 16.
+Connections are shared between all NBD clients, so you may wish to
+increase this if you expect many simultaneous NBD clients (or a single
+client using many multi-conn connections).
 
 See L</NBD CONNECTIONS AND CURL HANDLES> below.
 
@@ -407,6 +407,11 @@ of curl handles in the pool with the C<connections>
parameter (default
 4).  Note that if there are more than 4 NBD connections, they will
 share the 4 web server connections, unless you adjust C<connections>.
 
+nbdkit E<ge> 1.36 changed this again to use a curl multi handle, which
+is more efficient especially with HTTP/2 and HTTP/3.  Now the
+C<connections> parameter controls the maximum number of HTTP
+connections made to the remote server, and the default is 16.
+
 =head1 HEADER AND COOKIE SCRIPTS
 
 While the C<header> and C<cookie> parameters can be used to specify
diff --git a/plugins/curl/curldefs.h b/plugins/curl/curldefs.h
index 02b28a133..5a2cfab8e 100644
--- a/plugins/curl/curldefs.h
+++ b/plugins/curl/curldefs.h
@@ -41,6 +41,9 @@
  * macro isn't present then Curl is very old.
  */
 #ifdef CURL_AT_LEAST_VERSION
+#if CURL_AT_LEAST_VERSION (7, 30, 0)
+#define HAVE_CURLMOPT_MAX_TOTAL_CONNECTIONS
+#endif
 #if CURL_AT_LEAST_VERSION (7, 55, 0)
 #define HAVE_CURLINFO_CONTENT_LENGTH_DOWNLOAD_T
 #endif
diff --git a/plugins/curl/config.c b/plugins/curl/config.c
index 46bec2bbb..3e55cf15e 100644
--- a/plugins/curl/config.c
+++ b/plugins/curl/config.c
@@ -491,7 +491,7 @@ curl_config_complete (void)
 const char *curl_config_help    "cainfo=<CAINFO>            Path to
Certificate Authority file.\n"
   "capath=<CAPATH>            Path to directory with CA
certificates.\n"
-  "connections=<N>            Number of libcurl connections to
use.\n"
+  "connections=<N>            Number of HTTP connections to
use.\n"
   "cookie=<COOKIE>            Set HTTP/HTTPS cookies.\n"
   "cookiefile=                Enable cookie processing.\n"
   "cookiefile=<FILENAME>      Read cookies from file.\n"
diff --git a/plugins/curl/pool.c b/plugins/curl/pool.c
index 2974cda3f..99bd160a9 100644
--- a/plugins/curl/pool.c
+++ b/plugins/curl/pool.c
@@ -88,7 +88,7 @@
 /* Use '-D curl.pool=1' to debug handle pool. */
 NBDKIT_DLL_PUBLIC int curl_debug_pool = 0;
 
-unsigned connections = 4;
+unsigned connections = 16;
 
 /* Pipe used to notify background thread that a command is pending in
  * the queue.  A pointer to the 'struct command' is sent over the
@@ -124,6 +124,10 @@ pool_get_ready (void)
     return -1;
   }
 
+#ifdef HAVE_CURLMOPT_MAX_TOTAL_CONNECTIONS
+  curl_multi_setopt(multi, CURLMOPT_MAX_TOTAL_CONNECTIONS, (long) connections);
+#endif
+
   return 0;
 }
 
-- 
2.41.0

Eric Blake

2023-Jul-31 21:36 UTC

head link

[Libguestfs] [PATCH nbdkit v2 9/9] curl: Redefine connections=<N> parameter as number of HTTP connections

On Fri, Jul 28, 2023 at 06:17:53PM +0100, Richard W.M. Jones
wrote:> Previously (nbdkit 1.34) this was the number of easy handles.  However
> it turns out that easy handles can open multiple HTTP connections, and
> in fact there's no good way to tell how many (and they are not
> shared).
> 
> Now that we are using a curl multi, curl >= 7.30 provides a way to
> limit the total number of actual HTTP connections, so we should just
> use it.  This is closer to what I intended this parameter to mean.
> 
> Link: https://curl.se/mail/lib-2019-03/0102.html
> Link: https://curl.se/mail/lib-2019-12/0044.html
> ---
>  plugins/curl/nbdkit-curl-plugin.pod | 13 +++++++++----
>  plugins/curl/curldefs.h             |  3 +++
>  plugins/curl/config.c               |  2 +-
>  plugins/curl/pool.c                 |  6 +++++-
>  4 files changed, 18 insertions(+), 6 deletions(-)
I still want to spend time reviewing 8/9 (even if you check it in
first, to get some soak time), but in the short term this one makes
obvious sense.

Reviewed-by: Eric Blake <eblake at redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Richard W.M. Jones

2023-Jul-31 21:44 UTC

head link

[Libguestfs] [PATCH nbdkit v2 9/9] curl: Redefine connections=<N> parameter as number of HTTP connections

On Mon, Jul 31, 2023 at 04:36:16PM -0500, Eric Blake
wrote:> On Fri, Jul 28, 2023 at 06:17:53PM +0100, Richard W.M. Jones wrote:
> > Previously (nbdkit 1.34) this was the number of easy handles.  However
> > it turns out that easy handles can open multiple HTTP connections, and
> > in fact there's no good way to tell how many (and they are not
> > shared).
> > 
> > Now that we are using a curl multi, curl >= 7.30 provides a way to
> > limit the total number of actual HTTP connections, so we should just
> > use it.  This is closer to what I intended this parameter to mean.
> > 
> > Link: https://curl.se/mail/lib-2019-03/0102.html
> > Link: https://curl.se/mail/lib-2019-12/0044.html
> > ---
> >  plugins/curl/nbdkit-curl-plugin.pod | 13 +++++++++----
> >  plugins/curl/curldefs.h             |  3 +++
> >  plugins/curl/config.c               |  2 +-
> >  plugins/curl/pool.c                 |  6 +++++-
> >  4 files changed, 18 insertions(+), 6 deletions(-)
> 
> I still want to spend time reviewing 8/9 (even if you check it in
> first, to get some soak time), but in the short term this one makes
> obvious sense.
> 
> Reviewed-by: Eric Blake <eblake at redhat.com>
Thanks - I want to do a bit more testing on the series before I push
it.  I have put the latest version here:

  https://gitlab.com/rwmjones/nbdkit/-/commits/2023-curl-multi/

Also for the record this series revealed an actual bug in curl:

  https://github.com/curl/curl/pull/11557

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Libguestfs - Jul 2023 - [PATCH nbdkit v2 9/9] curl: Redefine connections=<N> parameter as number of HTTP connections

[Libguestfs] [PATCH nbdkit v2 9/9] curl: Redefine connections=<N> parameter as number of HTTP connections

[Libguestfs] [PATCH nbdkit v2 9/9] curl: Redefine connections=<N> parameter as number of HTTP connections

[Libguestfs] [PATCH nbdkit v2 9/9] curl: Redefine connections=<N> parameter as number of HTTP connections