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

remove keepalive and rely on GOLANG default (since go 1.13 default is 15s) #1992

Merged
merged 1 commit into from
May 21, 2024

Conversation

h0nIg
Copy link
Contributor

@h0nIg h0nIg commented Apr 18, 2024

set keepalive to appropiate value (since go 1.13 default is 15s).

5 minute for initial keepalive checks are way to high, we should orient us here according to go defaults

https://www.reddit.com/r/golang/comments/d7v7dn/psa_go_113_introduces_15_sec_server_tcp/

@h0nIg h0nIg changed the title remove keepalive (since go 1.13 default is 15s) set keepalive to appopiate value (since go 1.13 default is 15s) Apr 18, 2024
@h0nIg
Copy link
Contributor Author

h0nIg commented Apr 23, 2024

@paudley "paudley reacted with thumbs down emoji" why?

@paudley
Copy link
Contributor

paudley commented Apr 23, 2024

@paudley "paudley reacted with thumbs down emoji" why?

Sorry, was a mistake! (undone) I'm doing some testing on the effects of shorter keepalives in our environments but I don't have results yet.

@jackc
Copy link
Owner

jackc commented May 9, 2024

That keep alive value goes all the way back to b46ee0a in 2014. At the time Go did not have TCP keepalive enabled by default.

If a change is to be made here, I believe the correct change would be to remove the 5 minute value and actually use the Go default rather than hard code it to the current Go default.

@h0nIg
Copy link
Contributor Author

h0nIg commented May 10, 2024

@jackc done

pgconn/config.go Outdated Show resolved Hide resolved
@h0nIg h0nIg requested a review from jackc May 15, 2024 10:57
pgconn/config.go Outdated Show resolved Hide resolved
@h0nIg h0nIg changed the title set keepalive to appopiate value (since go 1.13 default is 15s) remove keepalive and rely on GOLANG default (since go 1.13 default is 15s) May 21, 2024
@h0nIg h0nIg requested a review from jackc May 21, 2024 08:33
@jackc jackc merged commit 24c0a5e into jackc:master May 21, 2024
14 checks passed
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.

3 participants