On Thu, 3 Nov 2022 16:28:08 GMT, Conor Cleary <[email protected]> wrote:
>> **Issue**
>> When using HTTP/2 with the HttpClient, it can often be necessary to close an
>> idle Http2 Connection before a server sends a GOAWAY frame. For example, a
>> server or cloud based tool could close a TCP connection silently when it is
>> idle for too long resulting in ConnectionResetException being thrown by the
>> HttpClient.
>>
>> **Proposed Solution**
>> A new system property, `jdk.httpclient.idleConnectionTimeout`, was added and
>> is used to specify in Milliseconds how long an idle connection (idle
>> connections are those which have no currently active streams) for the
>> HttpClient before the connection is closed.
>
> Conor Cleary has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8288717: IdleConnectionTimeout can use Keep Alive or Custom Value
New changes look good - some comments follow...
src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java line
1708:
> 1706: if (s != null) {
> 1707: long timeoutVal = Long.parseLong(s);
> 1708: System.err.println(timeoutVal);
Stray println? You could use `Log.logTrace` here to print the value.
src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java line
1711:
> 1709: if (timeoutVal >= 0) return timeoutVal;
> 1710: }
> 1711: } catch (NumberFormatException ignored) {}
Similarly - you could use `Log.logTrace` to print the error. It could be useful
for diagnostic.
test/jdk/java/net/httpclient/http2/IdleConnectionTimeoutTest.java line 41:
> 39: * @run testng/othervm -Djdk.httpclient.HttpClient.log=errors
> -Djdk.httpclient.keepalive.timeout.h2=-1 IdleConnectionTimeoutTest
> 40: * @run testng/othervm -Djdk.httpclient.HttpClient.log=errors
> -Djdk.httpclient.keepalive.timeout.h2=1 -Djdk.httpclient.keepalive.timeout=2
> IdleConnectionTimeoutTest
> 41: */
Maybe add a test that only sets `-Djdk.httpclient.keepalive.timeout=1` too?
-------------
PR: https://git.openjdk.org/jdk/pull/10183
