Add TcpKeepAlive

[TreyLawrence]

I'm using this package with amazon's redshift, and am using it to do some long running queries (max around 15 minutes). The problem is that sometimes the connection is lost during these queries and the client is never notified, so gets blocked waiting for a response. It'd be nice if there were support for TcpKeepAlive to keep these connections from timing out.

[johto]

If you were using an actual postgres, you could configure keepalive from server side.

That said, client-side keepalives don't sound like a bad idea.

[cbandy]

http://felixge.de/2014/08/26/tcp-keepalive-with-golang.html

Looking at the options available to libpq and the above,

• we can do keepalives_idle and keepalives_interval, but not separately;
• we cannot do keepalives_count.

[johto]

Looking at the options available to libpq and the above,

we can do keepalives_idle and keepalives_interval, but not separately;
we cannot do keepalives_count.

Yeah, I think you're right. I don't see any nice way of accessing the underlying file descriptor of a TCPConn, which is a great shame. I wonder if this is a case of nanny city or whether we could do something about this for Go 1.5.

[TreyLawrence]

Is there a reason the net.Conn interface is used instead of the net.TCPConn type? It looks like the connection is always hardcoded to tcp anyways:
https://github.com/lib/pq/blob/master/conn.go#L259

If you guys are cool with switching to a net.TCPConn type, we could expose these:
https://golang.org/pkg/net/#TCPConn.SetKeepAlive
https://golang.org/pkg/net/#TCPConn.SetKeepAlivePeriod

[johto]

It looks like the connection is always hardcoded to tcp anyways:
https://github.com/lib/pq/blob/master/conn.go#L259

Huh? What about this path?

[TreyLawrence]

Yeah, that's a derp. Sorry. What about getting rid of the Dialer interface and defaultDialer, and using net.Dialer instead?

That way the dial function can instantiate a net.Dialer and set the timeout, deadline, and keep alive on it, and then call Dial(ntw, addr) on that.

[johto]

Yeah, that's a derp. Sorry. What about getting rid of the Dialer interface and defaultDialer, and using net.Dialer instead?

That would break applications for no real reason.

That way the dial function can instantiate a net.Dialer and set the timeout, deadline, and keep alive on it, and then call Dial(ntw, addr) on that.

I don't see why you couldn't just special-case the TCP path and set the keepalive parameters there. If people are using custom dialers, they should set said parameters themselves.

[johto]

I don't see why you couldn't just special-case the TCP path and set the keepalive parameters there. If people are using custom dialers, they should set said parameters themselves.

Or we could do it for custom dialers as well if a type assertion to a *net.TCPConn holds.

[TreyLawrence]

That would break applications for no real reason.

I don't think I understand how you can use custom dialers. I see that drv implements the driver.Driver interface. But I'm not sure how you can use a dialer other than defaultDialer, since it's hardcoded here.

But I think you're right, a special-case for TCP will work best.

[johto]

I don't think I understand how you can use custom dialers.

By registering your own driver and calling pq.DialOpen instead of implementing your own.

[tg]

+1 on this enhancement. When DB failover is occurring on Amazon RDS service, I'm seeing all my DB connection hanging indefinitely. I'll go with johto advice and implement my own driver, wrapping pq.

[johto]

When DB failover is occurring on Amazon RDS service, I'm seeing all my DB connection hanging indefinitely.

I'm not sure this will actually help to be honest. IIRC once there's un-ACKed data in the pipe (i.e. you're actually doing a query and not just sitting idle), the keepalive intervals are ignored and much higher timeouts kick in. But I might as well be wrong.

[tg]

Thanks for pointing this out, will need to experiment for sure, good thing this is reasonably easy to reproduce.

Other solution I'm thinking of would be to simply maintain max idle period for every connection by wrapping net.Conn and applying Set(Read)Deadline on every read/send. I've been doing something similar for http.Server already, so this should be easy to test. Let's fork!

BTW I assume there's no heartbeating protocol in postgres, so idle connections effectively stay silent, is this correct?

[johto]

BTW I assume there's no heartbeating protocol in postgres, so idle connections effectively stay silent, is this correct?

You probably figured this out already, but yes, idle connections are idle.

[vincepri]

Any update for this?

[himanshugpt]

Did anyone try cbandy's solution?

[LasTshaMAN]

I believe for most users lib/pq the absence of TcpKeepAlive parameter shouldn't be an issue if they are using a recent Go version, as since golang/go@5bd7e9c
if supported by the underlying operating system TcpKeepAlive will be set to 15 seconds on TCP socket (before that commit by default Go didn't set TcpKeepAlive leaving this as user's responsibility).

Thus in the future we shouldn't see problems similar to the one originally discussed in this issue.

[kingcr7]

Thanks for pointing this out, will need to experiment for sure, good thing this is reasonably easy to reproduce.

Other solution I'm thinking of would be to simply maintain max idle period for every connection by wrapping net.Conn and applying Set(Read)Deadline on every read/send. I've been doing something similar for http.Server already, so this should be easy to test. Let's fork!

BTW I assume there's no heartbeating protocol in postgres, so idle connections effectively stay silent, is this correct?

+1 on this enhancement. When DB failover is occurring on Amazon RDS service, I'm seeing all my DB connection hanging indefinitely. I'll go with johto advice and implement my own driver, wrapping pq.

Did you implement the driver and pg wrap? i want to learn how you implement and use it ?thank you very much.

[tg]

@kingcr7 since golang 1.6 my problem has been solved by setting max connection lifetime via sql package (https://golang.org/pkg/database/sql/#DB.SetConnMaxLifetime). Works like a charm, haven't had any issues with zombie connection for a few years now.

[alok87]

Why cannot the keep alive change like this Clever#1 be merged in this library?

[marselester]

It would be great if you could try this #999 TCP keepalive solution on real workloads. Here is an example https://github.com/marselester/pg-keepalive.

[LasTshaMAN]

I think it is also worth pointing out that TCP keepalive alone won't help with the original issue, because:

A socket can only be in the keepalive state if it is idle. If there is outstanding data, the socket will be in the on state as it retransmits the data over and over because of the network failure.

It seems a common misconception that a connection with keep-alives enabled is going to return an error within TCP_KEEPIDLE + TCP_KEEPINTV * TCP_KEEPCNT seconds from a network failure. This is often not the case.

see currently open Golang issue for details - golang/go#31490