h2: bootstrap max streams from multi handle if in use - #22418
h2: bootstrap max streams from multi handle if in use#22418CatboxParadox wants to merge 1 commit into
Conversation
|
I believe the original intent of CURLMOPT_MAX_CONCURRENT_STREAMS was to select fewer than the protocol defined 100 streams. Until the server side settings come in. To make curl use, let's say 10, instead of the max the server announces. Using it in the other direction only really works if you already know the server has more. Otherwise the application risks getting many streams refused. But in a known setup, I can see how that would be advantageous. Your change should be fine in both cases. |
|
Thanks. The way I understand this from the RFC, the existence of some "initial limit" itself is a bit unexpected, with values below 100 downright not recommended (emphasis mine):
|
There was a problem hiding this comment.
Pull request overview
This PR makes HTTP/2 connection concurrency bootstrap from the multi handle’s configured CURLMOPT_MAX_CONCURRENT_STREAMS (when available) instead of always using DEFAULT_MAX_CONCURRENT_STREAMS. This allows applications to tune the initial per-connection concurrency assumption before the server’s SETTINGS frame is received, helping avoid premature connection fan-out during sudden request spikes.
Changes:
- Initialize
ctx->max_concurrent_streamsfromCurl_multi_max_concurrent_streams(data->multi)whendata->multiis set. - Preserve existing default behavior (
DEFAULT_MAX_CONCURRENT_STREAMS) when no multi handle is present.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks! |
From the description of
CURLMOPT_MAX_CONCURRENT_STREAMSintroduced in #3806 I'd expect the initial no. of concurrent streams CURL can send over one connection before aSETTINGSframe from the server is received would be tunable, but it isn't. For HTTP/2, it just bootstraps the value sent in curl'sSETTINGSframe - but that applies to the serverPUSHdirection.For my application, the situation improved a bit with #6852 but when the application goes from idle state (no connections) to having to process a sudden request peak (say thousands within few milliseconds), curl quickly spawns as many connections as allowed with
CURLMOPT_MAX_TOTAL_CONNECTIONS(tens of connections;PIPEWAITis set on each handle so each fills up to currently hard-coded 100ctx->max_concurrent_streamsbefore the next one is created) even though the server would've been perfectly OK with those thousands of open streams over one connection, just didn't manage to get aSETTINGSframe to curl in time.Would this be an acceptable change?
If users don't set
CURLMOPT_MAX_CONCURRENT_STREAMS, the default frommulti->max_concurrent_streamsis exactly the same (100) as theDEFAULT_MAX_CONCURRENT_STREAMS, so no change in behavior. If they do set it - with a different aim than mine - the comment from #6852 still applies: