No pipelining multiplex and connection persistency in HTTP/2 #736

Closed
fliser opened this Issue Mar 29, 2016 · 8 comments

Projects

None yet

2 participants

@fliser
fliser commented Mar 29, 2016

I did this

  1. Usage of libcurl (in C)
  2. Usage of multi interface
  3. Doing a lot (~1000) of http transfers, but sending to libcurl only 10 at a time
  4. Using HTTP/2 over TLS
  5. CURLMOPT_MAX_TOTAL_CONNECTIONS is set to 10

I expected the following

When using libcurl 7.47.1 I cheked that when I use multiplex it opens only one connection and all of the 10 parallel request are being sent in one TCP connection.
Using libcurl 7.48.0 I see that 10 connections are being opened and for each new http transfer a new connection is being opened.

curl/libcurl version

7.48.0
[curl -V output perhaps?]

operating system

iOS 9.3

@bagder bagder self-assigned this Mar 29, 2016
@bagder
Member
bagder commented Mar 29, 2016

Quoting the http2 docs:

To take advantage of multiplexing, you need to use the multi interface and set CURLMOPT_PIPELINING to CURLPIPE_MULTIPLEX. With that bit set, libcurl will attempt to re-use existing HTTP/2 connections and just add a new stream over that when doing subsequent parallel requests.

While libcurl sets up a connection to a HTTP server there is a period during which it doesn't know if it can pipeline or do multiplexing and if you add new transfers in that period, libcurl will default to start new connections for those transfers. With the new option CURLOPT_PIPEWAIT (added in 7.43.0), you can ask that a transfer should rather wait and see in case there's a connection for the same host in progress that might end up being possible to multiplex on. It favours keeping the number of connections low to the cost of slightly longer time to first byte transferred.

Additionally, these options are used by the http2 download example

@bagder bagder closed this Mar 29, 2016
@fliser
fliser commented Mar 29, 2016
  1. The CURLMOPT_PIPELINING was alrealy set to CURLPIPE_MULTIPLEX
  2. I now have added the CURLOPT_PIPEWAIT and received the same result, a lot of sockets are being created an no pipelining
  3. The same code works fine with libcurl 7.47.1

I have added verbose prints to understand the differences between the two versions and I get for

7.47.1

Info: on_frame_recv() header 0 stream 1
Info: on_stream_close(), error_code = 0, stream 1
Info: Removed stream 1 hash!
Info: nghttp2_session_mem_recv() returns 9
Info: All data in connection buffer processed
Info: http2_recv returns 0, http2_handle_stream_close
Info: nread <= 0, server closed connection, bailing
Info: STATE: PERFORM => DONE handle 0x104ad4008; line 1645 (connection #0)
Info: Curl_done
Info: free header_recvbuf!!
Info: Connection #0 to host ZZZZZZZ left intact
Info: Expire cleared
Info: STATE: INIT => CONNECT handle 0x104ad4008; line 1103 (connection #-5000)
Info: Found bundle for host ZZZZZZZ: 0x147fa4a88 [can multiplex]
Info: Re-using existing connection! (#0) with host ZZZZZZZ
Info: Connected to ZZZZZZZ (ZZZZZZZ) port 443 (#0)
Info: STATE: CONNECT => DO handle 0x104ad4008; line 1149 (connection #0)

7.48.0

Info: on_frame_recv() header 0 stream 1
Info: on_stream_close(), error_code = 0, stream 1
Info: Removed stream 1 hash!
Info: nghttp2_session_mem_recv() returns 9
Info: All data in connection buffer processed
Info: http2_recv returns 0, http2_handle_stream_close
Info: nread <= 0, server closed connection, bailing
Info: STATE: PERFORM => DONE handle 0x104a80008; line 1652 (connection #0)
Info: Curl_done
Info: free header_recvbuf!!
Info: Connection #0 to host ZZZZZZZ left intact
Info: Expire cleared
Info: STATE: INIT => CONNECT handle 0x104a80008; line 1108 (connection #-5000)
Info: Found bundle for host ZZZZZZZ: 0x14f7dc908 [can multiplex]
Info: Added connection 1. The cache now contains 2 members
Info: Hostname ZZZZZZZ was found in DNS cache
Info:   Trying ZZZZZZZ...
Info: STATE: CONNECT => WAITCONNECT handle 0x104a80008; line 1161 (connection #1)
Info: STATE: INIT => CONNECT handle 0x104b1c008; line 1108 (connection #-5000)
Info: Found bundle for host ZZZZZZZ: 0x14f7dc908 [can multiplex]
Info: Connection #1 has not started SSL connect, can't reuse
Info: Found pending candidate for reuse and CURLOPT_PIPEWAIT is set
Info: No connections available.
Info: STATE: CONNECT => CONNECT_PEND handle 0x104b1c008; line 1127 (connection #-5000)
Info: Connected to ZZZZZZZ (ZZZZZZZ) port 443 (#1)
@bagder bagder reopened this Mar 29, 2016
@bagder bagder added the regression label Mar 29, 2016
@bagder
Member
bagder commented Mar 29, 2016

On iOS you said, is that using the darwinssl/secure transport TLS backend?

@fliser
fliser commented Mar 29, 2016

No darwinssl/secure transport doesn't support ALPN so i cannot use it for HTTP/2
I'm using with openssl 1.0.2g

@bagder
Member
bagder commented Mar 29, 2016

Oh right, good point! 😃

@bagder
Member
bagder commented Mar 29, 2016

I haven't verified this, but I suspect commit a5aec58 caused the regression. Here's a fix that seems to correct the problem for me:

diff --git a/lib/http2.c b/lib/http2.c
index 91abbf0..13f8394 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -153,11 +153,11 @@ void Curl_http2_setup_conn(struct connectdata *conn)
  * HTTP2 handler interface. This isn't added to the general list of protocols
  * but will be used at run-time when the protocol is dynamically switched from
  * HTTP to HTTP2.
  */
 const struct Curl_handler Curl_handler_http2 = {
-  "HTTP2",                              /* scheme */
+  "HTTP",                               /* scheme */
   ZERO_NULL,                            /* setup_connection */
   Curl_http,                            /* do_it */
   Curl_http_done,                       /* done */
   ZERO_NULL,                            /* do_more */
   ZERO_NULL,                            /* connect_it */
@@ -173,11 +173,11 @@ const struct Curl_handler Curl_handler_http2 = {
   CURLPROTO_HTTP,                       /* protocol */
   PROTOPT_NONE                          /* flags */
 };

 const struct Curl_handler Curl_handler_http2_ssl = {
-  "HTTP2",                              /* scheme */
+  "HTTPS",                              /* scheme */
   ZERO_NULL,                            /* setup_connection */
   Curl_http,                            /* do_it */
   Curl_http_done,                       /* done */
   ZERO_NULL,                            /* do_more */
   ZERO_NULL,                            /* connect_it */
@fliser
fliser commented Mar 29, 2016

It corrects the problem.
Thanks!

@bagder bagder added a commit that closed this issue Mar 29, 2016
@bagder bagder http2: set correct scheme in handler structs [regression]
Since commit a5aec58 the handler schemes need to match for the
connections to be reused and for HTTP/2 multiplexing to work, reusing
connections is very important!

Closes #736
1fc7672
@bagder bagder closed this in 1fc7672 Mar 29, 2016
@bagder
Member
bagder commented Mar 29, 2016

Thanks for your report and verification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment