On Tue, 1 Apr 2025 19:18:18 GMT, Volkan Yazici <vyaz...@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
