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

Reply via email to