Skip to content

[netty-nio-client] Ensure initial channel used for protocol detection is released before re-acquiring #2882

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 7, 2021

Conversation

Bennett-Lynch
Copy link
Contributor

HttpOrHttp2ChannelPool establishes an initial connection to determine if H1 or H2 should be used for the ChannelPool. It then releases the Channel to the Pool to allow it to be re-acquired and reused. However, in some (perhaps rare) circumstances, it's possible for a subsequent acquire to race against the release, causing the second acquire to establish a new channel if the release had not yet completed. This may add unnecessary delay and incur unnecessary channel overhead. Delay because the release is likely to complete in sub-millisecond time while a new connection handshake may take many milliseconds to complete. Overhead because a second channel may linger in the pool even if the user only ever uses one channel at a time.

License

  • I confirm that this pull request can be released under the Apache 2 license

… is released before re-acquiring

HttpOrHttp2ChannelPool establishes an initial connection to determine if
H1 or H2 should be used for the ChannelPool. It then releases the Channel
to the Pool to allow it to be re-acquired and reused. However, in some
(perhaps rare) circumstances, it's possible for a subsequent acquire to race
against the release, causing the second acquire to establish a new channel
if the release had not yet completed. This may add unnecessary delay and
incur unnecessary channel overhead. Delay because the release is likely to
complete in sub-millisecond time while a new connection handshake may take
many milliseconds to complete. Overhead because a second channel may linger
in the pool even if the user only ever uses one channel at a time.
@Bennett-Lynch Bennett-Lynch requested a review from a team as a code owner December 2, 2021 22:54
@Bennett-Lynch Bennett-Lynch enabled auto-merge (squash) December 6, 2021 23:53
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 7, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@Bennett-Lynch Bennett-Lynch merged commit 6081647 into aws:master Dec 7, 2021
joviegas added a commit that referenced this pull request Dec 7, 2021
…etection is released before re-acquiring (#2882)"

This reverts commit 6081647.
joviegas added a commit that referenced this pull request Dec 7, 2021
…etection is released before re-acquiring (#2882)" (#2895)

This reverts commit 6081647.
joviegas added a commit that referenced this pull request Dec 7, 2021
…otocol detection is released before re-acquiring (#2882)" (#2895)"

This reverts commit d3cc7b8.
@joviegas joviegas mentioned this pull request Dec 8, 2021
joviegas added a commit that referenced this pull request Dec 8, 2021
…otocol detection is released before re-acquiring (#2882)" (#2895)" (#2899)

This reverts commit d3cc7b8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants