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

Comments

Projects
None yet
3 participants
@jeroen
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

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Oct 7, 2016

Member

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.

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

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Oct 8, 2016

Member

Only used if pipelining is enabled.

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

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

This comment has been minimized.

Show comment
Hide comment
@jeroen

jeroen Oct 10, 2016

Contributor

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

curl_multi_setopt(CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX)
Contributor

jeroen commented Oct 10, 2016

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

curl_multi_setopt(CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX)
@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Oct 10, 2016

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@jeroen

jeroen Oct 10, 2016

Contributor

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...

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@jay

jay Oct 10, 2016

Member

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.

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

This comment has been minimized.

Show comment
Hide comment
@jeroen

jeroen Oct 10, 2016

Contributor

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.

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Oct 10, 2016

Member

Maybe we should instead phrase it like:

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

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

This comment has been minimized.

Show comment
Hide comment
@jay

jay Oct 10, 2016

Member

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
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

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Oct 15, 2016

Member

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

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

CURLMOPT_MAX_PIPELINE_LENGTH.3: Clarify it's not for HTTP/2
- Clarify that this option is only for HTTP/1.1 pipelining.

Bug: #1059
Reported-by: Jeroen Ooms

Assisted-by: Daniel Stenberg
@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Oct 28, 2016

Member

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

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 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.