Skip to content

Enable TCP Keepalives on TCP connections to MySQL #194

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 2 commits into from
Dec 13, 2013

Conversation

aaron42net
Copy link
Contributor

We recently took had a production outage when we had an Amazon RDS MySQL instance lock up with many requests pending. We rebooted it, and it came up on a new IP, but we had to restart our Go apps to bring the site back up, which had hit their (*DB).SetMaxOpenConns() limit with outstanding requests. The go-mysql-driver doesn't enable TCP SO_KEEPALIVE and doesn't have any concept of client-side timeouts, so it would've waited forever for a response from a server that doesn't exist any more.

This patch enables the TCP SO_KEEPALIVE option after connecting to MySQL via TCP, which allows the kernel to notice when the server has dropped off of the network for an extended period (the Linux default is 2 hours), and then throw a TCP read error. Without this option set, a TCP connection will never timeout on its own.

For comparison, the mysql-connector in C (libmysql) unconditionally enables TCP keepalives at sql-common/client.c line 3834.

This also adds a DSN option "keepalivePeriod", which is passed to (*TCPConn).SetKeepAlivePeriod(), to change how often the kernel sends a keepalive packet to the server, soliciting a response.

I tried to use the existing style as much as possible, ran gofmt, updated the tests, and updated the documentation. Let me know I missed anything. Thanks!

-- Aaron

@aaron42net
Copy link
Contributor Author

Ah, fun. Apparently (*TCPConn).SetKeepAlivePeriod() is a go1.2-ism.

@julienschmidt
Copy link
Member

Thanks for the PR!
You forgot the entry in the AUTHORS file 😉

We haven't dropped Go 1.1 yet, Go 1.2 is still very new. I'd suggest to put the keepalivePeriod param in a follow-up PR, which we merge, when the Go 1.1 support is dropped for the master branch.

@aaron42net
Copy link
Contributor Author

Makes sense. I've removed the keepalivePeriod DSN option for now, and added an AUTHORS entry for myself, making what's left a few line change to the connection startup.

Take another look?

-- Aaron

@arnehormann
Copy link
Member

LGTM

julienschmidt added a commit that referenced this pull request Dec 13, 2013
Enable TCP Keepalives on TCP connections to MySQL
@julienschmidt julienschmidt merged commit 45271d0 into go-sql-driver:master Dec 13, 2013
zenazn added a commit to zenazn/mysql that referenced this pull request Apr 12, 2016
This patch adds a DSN-controllable way to call net.TCPConn's
SetKeepAlivePeriod, which controls the kernel's keepalive behavior.

This is a resubmission of the unmerged portion of go-sql-driver#194. Now that Go 1.2
has had a little bit of time to settle, hopefully this change will be a
bit more palatable :)
zenazn added a commit to zenazn/mysql that referenced this pull request Apr 12, 2016
This patch adds a DSN-controllable way to call net.TCPConn's
SetKeepAlivePeriod, which controls the kernel's keepalive behavior.

This is a resubmission of the unmerged portion of go-sql-driver#194. Now that Go 1.2
has had a little bit of time to settle, hopefully this change will be a
bit more palatable :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants