Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
HTTP/2 connections live in
ChannelManager's registry, not the channel pool, soconnectionTtlnever applied to them. Nothing else bounds their age: the registry has no reaper and PING keeps an idle connection alive. A client that setsconnectionTtlto follow DNS or load-balancer changes keeps every HTTP/2 connection for good.Modification:
Arm one task per HTTP/2 connection on its event loop when the pipeline is upgraded, delayed by 90-100% of
connectionTtlso connections opened together do not expire together. On expiry the connection is retired: it leaves the registry, gives back itsROUND_ROBINpermit, finishes the streams it holds and the openers queued for a slot, and closes once it has neither. Idle check and stream admission are one CAS on the stream count, so a late request either keeps the connection open or is refused before anything is written. A refused request, or one that finds the retired connection already closed, is sent once more throughsendNextRequestwithout spendingmaxRequestRetry. No GOAWAY is sent early; Netty'sclose()sends GOAWAY(NO_ERROR) when the idle connection closes.Result:
connectionTtlbounds HTTP/2 connections as it does HTTP/1.1 ones. With the default of -1 nothing is added to the pipeline.Fixes #2370