Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable TCP keepalive on listener sockets #2633

Closed
erwbgy opened this issue Jun 27, 2020 · 5 comments · Fixed by #2638
Closed

Enable TCP keepalive on listener sockets #2633

erwbgy opened this issue Jun 27, 2020 · 5 comments · Fixed by #2638
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/accepted Denotes an issue that has been triaged and determined to be valid. release-note Denotes a PR that will be considered when it comes time to generate release notes.

Comments

@erwbgy
Copy link
Contributor

erwbgy commented Jun 27, 2020

Contour should configure TCP keepalives for Envoy listener sockets so that half-open connections with clients (downstream in Envoy speak) are flushed. This helps Envoy work better with overlay networks which drop long running idle TCP connections. TCP keepalives are already enabled for server (upstream) and xDS connections so this would complete the set.

I have a pull request that implements this by adding SocketOptions if you're open to this change.
@youngnick youngnick assigned youngnick and unassigned youngnick Jun 28, 2020
@youngnick youngnick added kind/feature Categorizes issue or PR as related to a new feature. lifecycle/accepted Denotes an issue that has been triaged and determined to be valid. labels Jun 28, 2020
@youngnick
Copy link
Member

Hi @erwbgy, that sounds great. I'm a little disappointed that none of us noticed it sooner!

I guess the only question I would have is if any of us can think of a reason why keepalives would be disabled? If we could, then we should add a way to do it. However, I can't think of any reason to disable it off the top of my head, especially since we've got greater timeout configurability on the way.

Please send through your PR, can you make sure you check CONTRIBUTING.md for commit message style and other stuff? Thanks.

@erwbgy erwbgy mentioned this issue Jun 29, 2020
Merged
@jpeach jpeach mentioned this issue Jul 5, 2020
Open
@erwbgy
Copy link
Contributor Author

erwbgy commented Jul 8, 2020

@jpeach For documenting this change I was thinking that https://projectcontour.io/resources/envoy/ should be updated with a TCP Keepalive section at the end describing what they, noting that they're enabled by default for listener sockets and what the settings are. If that sounds okay I'll create a PR.

@jpeach
Copy link
Contributor

jpeach commented Jul 13, 2020

@jpeach For documenting this change I was thinking that https://projectcontour.io/resources/envoy/ should be updated with a TCP Keepalive section at the end describing what they, noting that they're enabled by default for listener sockets and what the settings are. If that sounds okay I'll create a PR.

I don't think that's the right place for this information. The current envoy page is supposed to document Envoy versioning, whereas this information is about configuration and deployment behavior, which is quite a different topic.

@erwbgy
Copy link
Contributor Author

erwbgy commented Jul 13, 2020

Do you have a suggestion for a different place to document this change then?

@jpeach
Copy link
Contributor

jpeach commented Jul 13, 2020

Do you have a suggestion for a different place to document this change then?

Unfortunately I don't see a good place for this in the current docs structure. Let's capture the test in an issue and we can revisit it in the future.

jpeach pushed a commit that referenced this issue Jul 17, 2020
@erwbgy
internal/envoy: configure Contour to enable TCP keepalive on Envoy li…
e7f3456 
…stener sockets (#2638)

With these changes Contour configures TCP keepalives for Envoy listener
sockets so that half-open connections with clients (downstream in Envoy
speak) are flushed which helps Envoy work better with networks that drop
long running idle TCP connections.  Once a TCP connection has been idle
for 45 seconds a keepalive packet is sent every 5 seconds and if 9 packets
receive no response then the connection will be terminated by Envoy.

Envoy is only supported on Linux so Contour always configures Linux TCP
keep-alive socket options regardless of the platform that Contour is
running on.

Fixes #2633

Signed-off-by: Keith Burdis <keith.burdis@gs.com>
@jpeach jpeach added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jul 17, 2020
@youngnick youngnick mentioned this issue Dec 17, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/accepted Denotes an issue that has been triaged and determined to be valid. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Enable TCP keepalive on listener sockets.
3 participants
@jpeach @erwbgy @youngnick