RFR: 8350451: HttpClient should not wait until the request body has been fully sent before reading the response headers
Daniel Fuchs
dfuchs at openjdk.org
Fri Feb 21 13:41:52 UTC 2025
On Fri, 21 Feb 2025 13:18:29 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:
>> Hi - Please find here a PR that improves streaming strategy in the HttpClient.
>>
>> The HttpClient currently waits until the full request body has been sent before starting to listen for a response. This is not optimal, in particular in cases where the server sends back e.g. 500 without even reading the body. It also prevents starting to stream the response body from the server before having sent the full request body, which prevents the server to stream back the request body to the client without reading it fully first.
>>
>> While writing a test to verify the fix, I also noticed a few places where additional tasks needed to be performed asynchronously (= delegated to the executor) to support this.
>
> test/jdk/java/net/httpclient/PostFromGetTest.java line 530:
>
>> 528: * the get response will pause before the connection window is filled.
>> 529: */
>> 530: static abstract class HTTPGetPostHandler implements HttpTestHandler {
>
> This PR aims to enable reading response in parallel with sending the request body. Maybe naive of me, but... can we _only_ test this? That is,
>
> 1. Client uses a request body publisher that blocks waiting for a `responseBodyReceived` signal, and upon wake up, raises the `requestBodySent` flag
> 2. Server responds (payload can be a constant)
> 3. Client response body reader consumes the response and raises the `responseBodyReceived` flag (blocked request body publisher gets released)
> 4. Test waits on the `requestBodySent` flag
> 5. Test verifies the received response
We could add a new test to test that specifically. It is true that this PR does more than this - but that was the main road block I stumbled on when adding the new test. The HttpResponse will only be available after the responses are received, and the InputStream to read the response body from, will only be available at that point. If you stream the response from GET, feed it in POST, and stream back the POST response, then that InputStream will only get available after you have finished sending the POST body; With HTTP/2 flow control - this means that eveything will get wedged if the body bytes exceeds the connection window, because the post response will get blocked if data isn't pulled, which means that the POST request will get blocked before sending the whole data, which means that the HttpResponse that would make the InputStream to pull from available to pull from it will never complete.
So the new test is also an effective test for that specific issue.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23716#discussion_r1965491958
More information about the net-dev
mailing list