On Tue, 8 Apr 2025 19:27:17 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:
>> Adds `HttpResponse::connectionLabel` method that provides an identifier for
>> the connection.
>>
>> **Implementation note:** The feature is facilitated by
>> `HttpConnection::label`, which should not be confused with
>> `HttpConnection::id`. This distinction is explained in the JavaDoc of both
>> properties.
>
> Volkan Yazici has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Add `@implNote` to state that the built-in is always non-empty
Changes requested by dfuchs (Reviewer).
test/jdk/java/net/httpclient/HttpResponseConnectionLabelTest.java line 188:
> 186: private static HttpTestServer createServer(Version version,
> SSLContext sslContext) {
> 187: try {
> 188: return HttpTestServer.create(version, sslContext,
> ForkJoinPool.commonPool());
OK - we definitely MUST NOT use the common FJP here.
1. our HTTP/2 servers start their dispatching thread, which blocks on accept(),
in the executor.
2. the tests will block (CountDownLatch) in the exchange, which is a thread in
the executor.
3. the test creates at least two HTTP/2 servers (maybe more) upfront.
If you run this test on a machine that has a low number of processors - say 4 -
then the FJP will maybe have 3 threads at most. You can see where that goes...
-------------
PR Review: https://git.openjdk.org/jdk/pull/24154#pullrequestreview-2761151791
PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2040042478
