On Fri, 4 Apr 2025 13:10:56 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:
>> @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? I don't have a strong opinion nor can I think of any issues this current JVM level counter might cause. If necessary, we can reconsider this if/when any issue shows up, without impacting the specification of `HttpResponse.connectionLabel()`. The current form looks good to me. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2030809033