HttpClient may not send queued requests #5855
Assignees
Labels
Comments
sbordet
added
Sponsored
|
This issue has been introduced by the changes for #5217 as well as the work done in #4904. The remarks done in #4904 continue to apply: in case of concurrent requests, there is no guarantee that the pool will open the minimum amount of connections. For example, sending 20 requests concurrently may open 21 connections. |
sbordet
added a commit
that referenced
this issue
Jan 5, 2021
Changed the AbstractConnectionPool.acquire() logic to call tryCreate() even when create=false. This is necessary when e.g. a sender thread T2 with create=true steals a connection whose creation was triggered by another sender thread T1. In the old code, T2 did not trigger the creation of a connection, possibly leaving a request queued. In the new code, T2 would call tryCreate(queuedRequests), possibly triggering the creation of a connection. This change re-introduces the fact that when sending e.g. 20 requests concurrently, 20+ connections may be created. However, it is better to err on creating more than creating less and leaving requests queued. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
gregw
added a commit
that referenced
this issue
Jan 6, 2021
Avoid the race on request queue size by enhancing Pool to track demand.
gregw
added a commit
that referenced
this issue
Jan 6, 2021
increment demand even if pool is at max size.
gregw
added a commit
that referenced
this issue
Jan 6, 2021
removed duplicate code to FutureConnection.
gregw
added a commit
that referenced
this issue
Jan 6, 2021
moved pending from Pool to AbstractConnectionPool
gregw
added a commit
that referenced
this issue
Jan 6, 2021
updates from review: + always call tryCreate, now passing demanded to indicate if demand should be incremented + simplify demand > supply calculation + if not multiplexing reduce demand on connection failure. If multiplexing, failure will not decrease demand, so some additional connections may be created until supply catches up.
gregw
added a commit
that referenced
this issue
Jan 6, 2021
revert to using destination queue size for demand
sbordet
pushed a commit
that referenced
this issue
Jan 7, 2021
Issue #5855 - HttpClient may not send queued requests Moved field pending from Pool to AbstractConnectionPool. As a consequence, AbstractConnectionPool.tryCreate() now performs a demand/supply calculation to decide whether to create a new connection. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This was referenced Mar 10, 2021
This was referenced Mar 10, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Jetty version
9.4.x
Description
Concurrent usage of
HttpClientmay cause requests to remain queued for longer then necessary.For example, when sending N requests and having the server hold them until the Nth is received by the server, it may happen that the client only opens N-1 connections, therefore leaving one request queued.
Typically this request would be sent as soon as one of the other requests is responded, but if the server holds them all, it will remain queued, so in normal operation this bug is hard to trigger.
The text was updated successfully, but these errors were encountered: