Simone,
Thanks for you prompt and detailed reply. Comments inline.
> On 31 Jul 2018, at 22:07, Simone Bordet <simone.bor...@gmail.com> wrote:
>
> ...
> I would argue that you would put this method in HttpClient and so it
> is HttpClient that needs to be configured with the connectTimeout.
Good suggestion. I agree, and this leaves the `timeout` method name
space for the request, as you suggest below. Which is more preferable.
> I propose to keep "timeout" as a total request+response timeout (and
> possibly conversation timeout, your call) on the request builder, and
The current HttpRequest `timeout` has gotten us this far quite well. It’s
specification is, deliberately, a little vague so as to leave some of the
lower level details to an implementation. I think the HttpRequest
`timeout`, as specified in JDK 11 today, is pretty good, but could
benefit from some clarification.
From the point of view of an “average” developer the request "starts"
once send[Async] is invoked, and ends when either send[Async]
returns/completes. It’s not quite that simple for streaming subscribers,
but that’s close enough for general discussion. These timeouts need
to make sense from the perspective of the API, as well as adhering
to the preciseness of the terms relating to the underlying protocols.
Before trying to draft the javadoc, the semantic we want for this
overall timeout is that it should approximately related to the observed
time spent in send[Async]. Ignoring connect for now, this means that
the timer starts from the initial DNS lookup/TCP connection and ends
when the complete request has been sent and the complete response
has been received. This seems like the most useful overall timeout
semantic.
There is a question about whether or not the response body is part
of the timeout. It currently is not since the body handler API has the
ability to either stream the response body or return it as a higher-level
Java Object, e.g String, etc.
> to move "connectTimeout" to the HttpClient builder, specifying that it
> means DNS lookup plus TCP connection establishment.
I agree with adding `connectTimeout` to HttpClient. The low-level details
of what constitutes a new underlying connection is somewhat known. I
agree that it should take into account such things as DNS lookup, TCP
connect, and SSL handshake. These low-level details could be part of
javadoc, but not necessarily part of the normative specification. The
javadoc can mention such when describing the connect phase.
> In case a request needs to open a connection, "timeout" will still be
> the total time from when the request is sent to when both request and
> response are finished, including the connectTimeout.
Agreed. This seems like the most intuitive semantic of these timeouts.
Summary of modes:
1) HttpRequest `timeout` only - times both connect ( if any ) and
until both the request and the response are fully send and received,
respectively.
2) HttpClient `connectTimeout` only - times only the connect phase,
if any. Timer stops after connection has been established.
3) HttpClient `connectTimeout` and HttpRequest `timeout` - start
connect phase timer at DNS lookup, stop at TCP connect finished
or SSL handshake complete. Start request specific timer shortly
after connect timer stops, i.e. just before the HTTP request is sent.
Stop request specific timer when both the request and the response
are fully send and received, respectively.
> By having a typically shorter connectTimeout (say 5 seconds) and a
> longer total timeout (say 2 minutes), a request will fail in 5 seconds
> if HttpClient fails to open a connection to the origin server, and
> fail in 2 minutes if it takes too much.
Yes, this the exact scenario that the API should support.
> Finally, I would consider TLS handshake time as part of the request
> data: the cost will be paid by the first request and capped by "total"
> timeout (not connectTimeout).
> With TLS 1.3 optimizations, it may be just few TLS bytes before the
> encrypted request bytes.
I don’t have a strong opinion about which timeout bucket TLS handshake
falls into, so long as it falls into one or the other.
-Chris.
> Thanks!
>
> --
> Simone Bordet
> ---
> Finally, no matter how good the architecture and design are,
> to deliver bug-free software with optimal performance and reliability,
> the implementation technique must be flawless. Victoria Livschitz
