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.

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

net: use the same default TCPKeepAlive interval in net.Listen and net.Dial #31510

Closed
CAFxX opened this issue Apr 17, 2019 · 7 comments
Closed

net: use the same default TCPKeepAlive interval in net.Listen and net.Dial #31510

CAFxX opened this issue Apr 17, 2019 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.13

Comments

@CAFxX
Copy link
Contributor

CAFxX commented Apr 17, 2019 •
edited

In #23378 (comment) we noted that there is a mismatch between the default TCP keepalive used by net.Listen (3m, previously in http.ListenAndServe(TLS)) and the one used by net.Dial (15s).

For uniformity, it would be advisable to ensure these two defaults match.

If there are no counter-proposals, my suggestion would be to use 15s as the interval (see #23459 for prior discussion).

An explicit non-goal of this issue is that of making the default value part of the API. For the reasons discussed in https://go-review.googlesource.com/c/go/+/107196 this should remain undefined, and users that have specific requirements in terms of exact keepalive durations should specify a custom configuration.
@FiloSottile
Copy link
Contributor

Does http.ListenAndServe still actually set a TCP Keep-Alive? When I looked yesterday, I couldn't find it anymore, and assumed it was now relying on the new net.Listen default.

@FiloSottile
Copy link
Contributor

Yeah, this is not in net/http as of 1abf3aa.

The Accept() default is 3 minutes though, not 15s.

@CAFxX
Copy link
Contributor Author

CAFxX commented Apr 17, 2019

Yeah, but the net.Listen default is 3m because 1abf3aa lifted the default out of ListenAndServe and into net.Listen. This is orthogonal though to the point that these two defaults (in net.Listen and net.Dial) should be the same for consistency.

@FiloSottile FiloSottile changed the title net/http: use the same default TCPKeepAlive interval as net.Dialer net: use the same default TCPKeepAlive interval in net.Listen and net.Dial Apr 17, 2019
@FiloSottile FiloSottile added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Apr 17, 2019
@FiloSottile FiloSottile added this to the Go1.13 milestone Apr 17, 2019
@FiloSottile
Copy link
Contributor

Changed the title accordingly. release-blocker because the net.Listen keep-alive is new, so we should decide if we are changing it before shipping it, ideally.

@CAFxX
Copy link
Contributor Author

CAFxX commented Apr 30, 2019 •
edited

Are there specific concerns against the approach outlined above?

If there are no counter-proposals, my suggestion would be to use 15s as the interval (see #23459 for prior discussion).

If there are none, I'll go ahead and send a CL.

@rsc
Copy link
Contributor

rsc commented May 1, 2019

I can't see why these should be different. Go ahead and send a CL.

@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label May 1, 2019
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 1, 2019
@gopherbot
Copy link

Change https://golang.org/cl/175259 mentions this issue: net: use the same default TCPKeepAlive interval for accept

@golang golang locked and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
4 participants
@rsc @CAFxX @FiloSottile @gopherbot