Skip to content
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

CURLMOPT_MAX_PIPELINE_LENGTH and SETTINGS_MAX_CONCURRENT_STREAMS #1059

Closed
jeroen opened this issue Oct 7, 2016 · 11 comments
Closed

CURLMOPT_MAX_PIPELINE_LENGTH and SETTINGS_MAX_CONCURRENT_STREAMS #1059

jeroen opened this issue Oct 7, 2016 · 11 comments

Comments

@jeroen
Copy link
Contributor

jeroen commented Oct 7, 2016

Currently the CURLMOPT_MAX_PIPELINE_LENGTH documentation states the default is 5. Is this also the case for HTTP/2 connections, i.e. whenCURLMOPT_PIPELINING is set to CURLPIPE_MULTIPLEX?

In this case, could we make an option that automatically sets CURLMOPT_MAX_PIPELINE_LENGTH to what the server specifies in SETTINGS_MAX_CONCURRENT_STREAMS? See also SO topic.

@bagder
Copy link
Member

bagder commented Oct 7, 2016

That option is only used for actual pipelining, not for HTTP/2. libcurl will add transfers to the same connection as long as the peer allows (according to SETTINGS_MAX_CONCURRENT_STREAMS) and if you add even more transfers it will create a new connection for those. Unless you limit the number of connections.

@bagder
Copy link
Member

bagder commented Oct 8, 2016

Only used if pipelining is enabled.

Is quite clearly mentioned in the docs. What else can we do to make that clearer?

@jeroen
Copy link
Contributor Author

jeroen commented Oct 10, 2016

So setting pipelining to multiplex does not mean pipelining is enabled?

curl_multi_setopt(CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX)

@jay
Copy link
Member

jay commented Oct 10, 2016

I think the confusion here is they're really two separate things. Pipelining is HTTP/1.1 and Multiplexing is HTTP/2. They are similar enough (but not really..) that we piggybacked on the option CURLMOPT_PIPELINING. bagder has a better explanation of HTTP/2 multiplexing somewhere, but I don't recall where

@jeroen
Copy link
Contributor Author

jeroen commented Oct 10, 2016

I get that but given that multiplexing is enabled via CURLMOPT_PIPELINING then one might think that libcurl conceptualizes it as a form of pipelining, hence CURLMOPT_MAX_PIPELINE_LENGTH applies...

If this is not the case then it should have been another CURLMOPT I think...

@jay
Copy link
Member

jay commented Oct 10, 2016

That seems like a fair point, so I think where we're at is either improve CURLMOPT_MAX_PIPELINE_LENGTH.3 doc or make it so that option overrides SETTINGS_MAX_CONCURRENT_STREAMS, if that's acceptable.

@jeroen
Copy link
Contributor Author

jeroen commented Oct 10, 2016

I think my confusion simply comes from this:

That option is only used for actual pipelining, not for HTTP/2.

How are users supposed to know that http/2 multiplex is not considered "actual pipelining" even though the user has to enable it via:

curl_multi_setopt(CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX)

This is simply confusing. You need to emphasize in the documentation for CURLMOPT_PIPELINING that the value for CURLPIPE_MULTIPLEX is not actual pipelining (whatever that may mean) and in the docs for CURLMOPT_MAX_PIPELINE_LENGTH that the option does not apply for CURLPIPE_MULTIPLEX pipes.

@bagder
Copy link
Member

bagder commented Oct 10, 2016

Maybe we should instead phrase it like:

"This option is only used for HTTP/1 Pipelining, not for HTTP/2 multiplexing"

@jay
Copy link
Member

jay commented Oct 10, 2016

well it is titled "CURLMOPT_PIPELINING - enable HTTP pipelining and multiplexing " and I think the description makes it clear there are two distinct features. for CURLMOPT_MAX_PIPELINE_LENGTH like I said I think you have a point. if we change the documentation it could be

diff --git a/docs/libcurl/opts/CURLMOPT_MAX_PIPELINE_LENGTH.3 b/docs/libcurl/opts/CURLMOPT_MAX_PIPELINE_LENGTH.3
index cac3c71..1bb10a2 100644
--- a/docs/libcurl/opts/CURLMOPT_MAX_PIPELINE_LENGTH.3
+++ b/docs/libcurl/opts/CURLMOPT_MAX_PIPELINE_LENGTH.3
@@ -29,8 +29,8 @@ CURLMOPT_MAX_PIPELINE_LENGTH \- maximum number of requests in a pipeline
 CURLMcode curl_multi_setopt(CURLM *handle, CURLMOPT_MAX_PIPELINE_LENGTH, long max);
 .SH DESCRIPTION
 Pass a long. The set \fBmax\fP number will be used as the maximum amount of
-outstanding requests in a pipelined connection. Only used if pipelining is
-enabled.
+outstanding requests in an HTTP/1.1 pipelined connection. Only used if
+HTTP/1.1 pipelining is enabled.

 When this limit is reached, libcurl will use another connection to the same
 host (see \fICURLMOPT_MAX_HOST_CONNECTIONS(3)\fP), or queue the request until

@bagder
Copy link
Member

bagder commented Oct 15, 2016

@jeroenooms so can you tell us what if anything you think can improve the docs to make this more apparent?

jay added a commit that referenced this issue Oct 28, 2016
- Clarify that this option is only for HTTP/1.1 pipelining.

Bug: #1059
Reported-by: Jeroen Ooms

Assisted-by: Daniel Stenberg
@jay
Copy link
Member

jay commented Oct 28, 2016

The documentation has been updated to make it clearer this is only for HTTP/1.1. Thanks

@jay jay closed this as completed Oct 28, 2016
@lock lock bot locked as resolved and limited conversation to collaborators May 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

3 participants