Connection timeouts in JEP 321 (HTTP Client)

[Michael McMahon]

Simone,

Just to clarify a couple of points below:


On 31/07/2018, 22:07, Simone Bordet wrote:
> Hi,
>
> On Tue, Jul 31, 2018 at 9:58 PM Chris Hegarty<chris.hegarty at oracle.com>  wrote:
>> It is unfortunate that this has come up so late the JDK 11 lifecycle,
>> but since this has API impact, and is the right thing to do, it is
>> certainly worth addressing now ( rather than waiting to do something in
>> a future release).
>>
>> TL;DR provide API support for setting connection and response specific
>> timeouts, while continuing to support a single overall request timeout.
>>
>> The following issue has been file to track this:
>>    https://bugs.openjdk.java.net/browse/JDK-8208391
>>
>> Propose
>> -------
>>
>> In the `HttpRequest.Builder` class remove the single `timeout` method
>> and replace it with a pair of `connectTimeout` and `responseTimeout`.
>> The `connectTimeout` method can be used to set a timeout duration that
>> is specific to the connection phase. The `responseTimeout` method can be
>> used to set a timeout for the actual of the HTTP response. For example,
>>
>>      HttpRequest request = HttpRequest.newBuilder()
>>             .uri(URI.create("https://foo.com/"))
>>             .connectTimeout(Duration.ofSeconds(20))
>>             .responseTimeout(Duration.ofMinutes(1))
>>             .header("Content-Type", "application/json")
>>             .POST(BodyPublishers.ofFile(Paths.get("file.json")))
>>             .build();
>>
>> We consider the ability to be able to set a single timeout for the
>> overall response to be important, so the `responseTimeout` method can be
>> used to set an overall response timeout, in the absence of a more
>> specific `connectTimeout`. This means that the newly named
>> `responseTimeout` is semantically equivalent to the old `timeout` when
>> used without the new `connectTimeout`.
> I don't think this is precisely specified.
>
> AFAIU, connectTimeout is the time to perform DNS lookup plus the time
> to establish the TCP connection (SYN+SYN/ACK+ACK), so you may want to
> clarify that.

What about TLS? Should connectTimeout not include the handshake as well?
The definition above seems over specified and might preclude that.
> But responseTimeout also is not precisely defined, and assumes that a
> response starts after a request, but this is not always the case.
> Imagine the case of uploading a huge file to the server on a slow network.
> The server may reply with a 500 immediately so the response side is
> finished almost immediately, but the request content upload may
> continue for minutes after the response is arrived, depending on the
> server implementation.
> The good case, where the huge upload continues normally and completes
> before the response is sent by the server, is difficult to quantify
> beforehand and therefore to configure on the request object (as it
> depends on the network).
> Point being that "responseTimeout" does not convey the fact that in
> certain cases 99% of the time can be spent in the request, not in the
> response.
I can understand that a server might respond early with an error code
like 500 before the client has finished sending a very large request body,
but I don't see how a server would likely respond early with 200.
Surely, the server has to wait for the whole request body to be received
successfully, before sending a 200?

If that is the case, then I don't see how timing the entire request, 
separate
from the response, adds much value.

- Michael