RFR: 8350279: HttpClient: Add a new HttpResponse method to identify connections [v4]
Volkan Yazici
vyazici at openjdk.org
Fri Apr 4 13:13:58 UTC 2025
On Tue, 1 Apr 2025 19:18:18 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:
>> src/java.net.http/share/classes/jdk/internal/net/http/HttpConnection.java line 81:
>>
>>> 79: = Comparator.comparing(HttpConnection::id);
>>> 80:
>>> 81: private static final AtomicLong LABEL_COUNTER = new AtomicLong();
>>
>> In the API documentation on `HttpResponse.connectionLabel()` we talk about the connection label being unique within a `HttpClient` scope. i.e. two different `HttpClient` instances could have the same connectionLabel for a connection. I think that's the right scoping.
>>
>> So given that, having a `static` field on a `HttpConnection` which keeps track of a connection label, may not be the right place to keep track of that state. In this proposed form, no two connections in two different HttpClient instances will ever have the same connectionLabel. I think this counter should probably be present on the `HttpClientImpl` as an instance field.
>
> @jaikiran, what you're suggesting makes sense. I'm still exploring the interplay between the connection label and HTTP/3 server pushes. I will return back to your suggestion soon.
> In this proposed form, no two connections in two different HttpClient instances will ever have the same `connectionLabel`.
Yes, but this is neither required by the connection label contract. That is, the contract doesn't say that they must have overlapping connection labels. It only states connection labels are unique-per-client. Our implementation is more stricter than that: unique-per-VM, but that is just an implementation detail.
> I think this counter should probably be present on the `HttpClientImpl` as an instance field.
I've given this approach an attempt. Though I find it difficult to reason about in the code that a counter _only_ `HttpConnection` uses is situated in `HttpClientImpl`, just to make two `HttpConnection`s from different `HttpClientImpl`s share the connection label.
I also agree with @dfuch that having a unique-per-VM connection label helps with troubleshooting too.
@jaikiran, unless you're strongly opinionated about this, I prefer to leave the connection label counter in `HttpConnection`. How shall we proceed?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2028788343
More information about the net-dev
mailing list