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

Reply via email to