[jdk8u-dev] RFR: 8278067: Make HttpURLConnection default keep alive timeout configurable [v4]
Andrew John Hughes
andrew at openjdk.org
Tue Feb 27 18:32:55 UTC 2024
On Wed, 21 Feb 2024 00:09:10 GMT, Dhamoder Nalla <dhanalla at openjdk.org> wrote:
>> The JDK11 patch didn't apply cleanly 1/4 hunks needed to be port manually.
>>
>> Test performed and passed:
>>
>> 1. JTreg jdk_tier1, hotspot_tier1
>> 2. Testcase attached this PR
>
> Dhamoder Nalla has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>
> - Merge branch 'master' into Backport-JDK-8278067
> - Remove SystemProperty tag as it is not supported in JDK8
> - Fix indentation
> - Backport d8f44aa39e921594505864e6270f42b745265293
> - Backport of 2e31cc7ee1b875af7c7b3a5367ac8056fbc60650
The text in `jdk/src/share/classes/java/net/doc-files/net-properties.html` seems to be indented differently to the 11u version:
~~~
+--- a/jdk/src/share/classes/java/net/doc-files/net-properties.html
++++ b/jdk/src/share/classes/java/net/doc-files/net-properties.html
+@@ -172,6 +172,16 @@ <H2>Misc HTTP properties</H2>
If HTTP keepalive is enabled (see above) this value determines the
maximum number of idle connections that will be simultaneously kept
alive, per destination.</P>
-+ <LI><P><B>{@systemProperty http.keepAlive.time.server}</B> and
-+ <B>{@systemProperty http.keepAlive.time.proxy}</B> </P>
++ <LI><P><B>http.keepAlive.time.server</B> and
++ <B>http.keepAlive.time.proxy</B> </P>
+ <P>These properties modify the behavior of the HTTP keepalive cache in the case
-+ where the server (or proxy) has not specified a keepalive time. If the
-+ property is set in this case, then idle connections will be closed after the
-+ specified number of seconds. If the property is set, and the server does
-+ specify a keepalive time in a "Keep-Alive" response header, then the time specified
-+ by the server is used. If the property is not set and also the server
-+ does not specify a keepalive time, then connections are kept alive for an
-+ implementation defined time, assuming {@code http.keepAlive} is {@code true}.</P>
++ where the server (or proxy) has not specified a keepalive time. If the
++ property is set in this case, then idle connections will be closed after the
++ specified number of seconds. If the property is set, and the server does
++ specify a keepalive time in a "Keep-Alive" response header, then the time specified
++ by the server is used. If the property is not set and also the server
++ does not specify a keepalive time, then connections are kept alive for an
++ implementation defined time, assuming {@code http.keepAlive} is {@code true}.</P>
<LI><P><B>http.maxRedirects</B> (default: 20)<BR>
This integer value determines the maximum number, for a given request,
of HTTP redirects that will be automatically followed by the
~~~
Can we please align these? Removing the `@systemProperty` tag should not mean changing the indentation of the entire block. Comparing while ignoring whitespace changes shows that most of it is identical, bar the first two lines.
Otherwise seems mostly clean. The change to the test library looks fine and the absence of the empty line removal is due to [8279842: HTTPS Channel Binding support for Java GSS/Kerberos](https://bugs.openjdk.org/browse/JDK-8279842) being absent (which we should probably also backport at some point as it is in Oracle 8u341)
-------------
PR Review: https://git.openjdk.org/jdk8u-dev/pull/437#pullrequestreview-1904491483
More information about the jdk8u-dev
mailing list