RFR: 8288717: Add a means to close idle connections in HTTP/2 connection pool [v5]

[Daniel Fuchs]

On Thu, 3 Nov 2022 16:28:08 GMT, Conor Cleary <ccleary at openjdk.org> 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