feat: TCP keepalive for Postgres and MySQL connections - #4355
Open
adriangb wants to merge 1 commit into
Open
Conversation
adriangb
force-pushed
the
feat/tcp-keepalive
branch
6 times, most recently
from
July 24, 2026 21:10
1e8c1d9 to
be545ed
Compare
adriangb
force-pushed
the
feat/tcp-keepalive
branch
3 times, most recently
from
August 21, 2026 12:47
c64c770 to
ad13dc6
Compare
Author
|
@abonander could I ask you to review this change? We've been running this in production for a couple months now, it's essential for reliable operation in a large production system where the sorts of failures listed in the PR description are all but guaranteed to happen and thus sqlx is all but guaranteed to lock up production applications. Thanks for taking the time to review. |
adriangb
force-pushed
the
feat/tcp-keepalive
branch
from
August 21, 2026 15:28
52eaf41 to
fe0c7a7
Compare
Adds `TcpKeepalive` and `net::connect_tcp_with`, plus `tcp_keepalive()` on `PgConnectOptions` and `MySqlConnectOptions`. Keepalive is off by default, so nothing changes for existing users. Motivation: without it, a connection whose server disappeared *without* closing the socket never finds out. A failover, a killed container, or a dropped NAT mapping leaves the client blocked reading a response that will never arrive; there is nothing left to retransmit, so no RST is ever provoked and the read waits forever. `TCP_NODELAY`, which is all SQLx sets today, does not help, and a server-side `statement_timeout` cannot fire on a server that is gone. This is the case @abonander allowed for in transact-rs#3559 (comment): "I suppose that's still preferable to it hanging forever on a read that will never complete." Worth adding on the objection raised there, that a keepalive timeout is only noticed the next time the socket is used: that is not true of a blocked reader. When the probes are exhausted the kernel sets `sk_err` to `ETIMEDOUT` and wakes anyone parked on the socket, so the pending read fails rather than waiting for someone to poke it. We hit this in production: a maintenance loop that had a query in flight when its Postgres instance went away stopped doing work permanently, while other loops in the same process recovered in seconds. Reproduced by holding an `ACCESS EXCLUSIVE` lock so the query blocks server-side, then removing the server from the network and restarting it. Implementation follows transact-rs#3559 by @xuehaonan27, rebased onto the current `connect_tcp` and reduced to an additive API: `connect_tcp` keeps its signature and delegates, so external drivers are unaffected. Parameters are normalized before they reach `setsockopt`: zero means "use the system default" as it does in libpq, and durations round up to whole seconds, since `socket2` truncates and Linux rejects a literal 0 with `EINVAL`. A value the kernel still rejects fails the connection with an `Error::Configuration` that names the parameters. No platform loses the ability to build. Where a socket option does not exist, that parameter is skipped rather than failing to compile: `idle` is ignored on OpenBSD, Haiku, NTO and Vita, `interval`/`retries` wherever `TCP_KEEPINTVL`/`TCP_KEEPCNT` are absent, and the whole call is a no-op on targets without `setsockopt`. Windows writes idle and interval together through `SIO_KEEPALIVE_VALS` and cannot leave one of them alone, so an unset one gets libpq's own substitute there (2h idle, 1s interval). The `tokio` floor moves to 1.27, the first release with `AsFd`/`AsSocket` on `TcpStream`, which `socket2::SockRef::from()` requires. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
adriangb
force-pushed
the
feat/tcp-keepalive
branch
from
August 21, 2026 21:03
fe0c7a7 to
ff61226
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds TCP keepalive support, closing #3540. This is @xuehaonan27's #3559 rebased onto the current
connect_tcpand reduced to an additive API; the design credit is theirs.Why
Without keepalive, a connection whose server disappeared without closing the socket never finds out. A failover, a killed container, or a dropped NAT mapping leaves the client blocked reading a response that will never arrive. There is nothing left to retransmit, so no RST is ever provoked, and the read waits forever.
TCP_NODELAY, which is all SQLx sets today, does not help, and a server-sidestatement_timeoutcannot fire on a server that is gone.We hit this in production: we have a ~ hot loop that is constantly querying so we are almost guaranteed to hit this every server restart (pgbouncer scale down in our case). It reproduces reliably by holding an
ACCESS EXCLUSIVElock so the query blocks server-side, then removing the server from the network and restarting it. Notably a restart w/o an in-flight query won't reproduce: the query has to be already acknowledged and executing, which is exactly the common case during a real failover.@abonander, in #3559 (comment) you allowed for this case: "I suppose that's still preferable to it hanging forever on a read that will never complete." One correction on the objection raised there, that a keepalive timeout is only noticed the next time we use the socket: that is not true for a blocked reader. When the probes are exhausted the kernel sets
sk_errtoETIMEDOUTand wakes anyone parked on the socket, so a pending read fails rather than waiting for someone to poke it. That is precisely the case that hangs today.Since #3559 was opened, three other users have reported the same silent-stale-connection failure in it (Aug 2025, Dec 2025, Feb 2026), and the author of the follow-up #4159 moved to
tokio-postgresover it.What this adds
sqlx_core::net::TcpKeepalive, withidle/interval/retries.net::connect_tcp_with, taking aTcpConnectOptions.connect_tcpkeeps its signature and delegates with the defaults, so external drivers (Compile-time support for external drivers聽#3889) are unaffected.PgConnectOptions::tcp_keepalive()andMySqlConnectOptions::tcp_keepalive().Keepalive stays off by default, so behaviour is unchanged for existing users. (libpq defaults it on with a 2h idle; I did not want to change a default in this PR, but happy to if you would prefer parity.)
Parameters are normalised before they reach
setsockopt: zero means "use the system default" as in libpq, and durations round up to whole seconds, sincesocket2truncates and Linux rejects a literal0withEINVAL. A value the kernel still rejects fails the connection with anError::Configurationnaming the parameters, rather than a bareIo(EINVAL).Platforms
No platform loses the ability to build. Where a socket option does not exist the parameter is skipped rather than failing to compile:
idleis ignored on OpenBSD, Haiku, NTO and Vita, wheresocket2has noTCP_KEEPIDLEequivalent.interval/retriesare ignored whereverTCP_KEEPINTVL/TCP_KEEPCNTare absent.setsockoptat all the call is a no-op.SIO_KEEPALIVE_VALSand cannot leave one alone, so an unset one gets libpq's own substitute there (2h idle, 1s interval), matchingpqSetKeepalivesWin32.Verified by cross-compiling
sqlx-coreforx86_64-pc-solaris(a unix outsidesocket2's interval/retries list) andx86_64-unknown-illumos(inside it).The
tokiofloor moves to 1.27, the first release withAsFd/AsSocketonTcpStream, whichsocket2::SockRef::from()requires. Previously the workspace declared 1.25, which no longer compiles against this code.Tests
Unit tests for the builder, plus tests that drive a real loopback socket through
setsockoptand read the values back withsocket2's getters, covering the zero / sub-second cases and a value the kernel rejects. These run under both the tokio and async-io runtimes.Follow-up
Deliberately left out to keep this reviewable: libpq-compatible URL parameters for Postgres (
keepalives,keepalives_idle,keepalives_interval,keepalives_count). Today these hit theignoring unrecognized connect parameterbranch, so anyone copying a libpq DSN silently gets no keepalive. I have that written already and will file it as an issue and open it as a follow-up PR once this lands, if you would like it.I am carrying this as a patched fork in the meantime, so I am happy to iterate on the API shape or split MySQL out if that makes review easier.