feat: add TCP keepalive for MySQL and PostgresSQL. by xuehaonan27 · Pull Request #3559 · launchbadge/sqlx

xuehaonan27 · 2024-10-12T06:09:41Z

BREAK CHANGE: [sqlx_core::net::socket::connect_tcp]. New parameter added.

Add TCP keepalive configuration which could be enabled by [PgConnectOptions::tcp_keep_alive] and [MySqlConnectOptions::tcp_keep_alive].

Does your PR solve an issue?

fixes #3540

BREAK CHANGE: [`sqlx_core::net::socket::connect_tcp`]. New parameter added. Add TCP keepalive configuration which could be enabled by [`PgConnectOptions::tcp_keep_alive`] and [`MySqlConnectOptions::tcp_keep_alive`].

CommanderStorm

(not a maintainer)

I thought you might still appreciate an external review ^^

CommanderStorm · 2024-10-12T06:25:56Z

sqlx-core/src/net/socket/mod.rs

+#[derive(Debug, Clone)]
+pub struct TcpKeepalive {
+    pub time: Duration,
+    pub interval: Duration,
+    pub retries: u32,
+}


https://docs.rs/socket2/latest/socket2/struct.TcpKeepalive.html

I think we should use the builder pattern too and copy the platform support of socket2 given that this is explicitly done in their case.

Also adding documentation via docstrings is really helpful => copying their docs is likely fine.

~~Likely we should make this Copy too.~~

Thank you for giving me a review!
That's reasonable, or we might have to maintain the consistency between TcpKeepalive defined in socket2 and in sqlx_core.
Should I re-export TcpKeepalive definition in socket2?

Current code is a build failour on the following platforms (assuming one tries to use the code):

any( target_os = "openbsd", target_os = "redox", target_os = "solaris", target_os = "nto", target_os = "espidf", target_os = "vita", target_os = "haiku", )

Please use the same platform cfg scoping as the new dependency.

CommanderStorm · 2024-10-12T06:52:06Z

sqlx-core/src/net/socket/mod.rs

    host: &str,
    port: u16,
    with_socket: Ws,
+    keepalive: &Option<TcpKeepalive>,


Suggested change

keepalive: &Option<TcpKeepalive>,

keepalive: Option<&TcpKeepalive>,

Would this not be a better API?
https://users.rust-lang.org/t/api-design-option-t-vs-option-t/34139/2

~~Given that socket2::TcpKeepalive is Copy, I think copying that derive and dropping the reference might be more ergonomic. What do you think?~~

I didn't notice that TcpKeepalive in socket2 is Copy.
I will adjust it. Thx.

I didn't notice the API design preference in Rust. Thank you for pointing this out for me!

xuehaonan27 · 2024-10-12T09:11:34Z

Should I re-export TcpKeepalive in socket2 or give a definition on our own?

`TcpKeepalive`. fix: `connect_tcp` API design.

CommanderStorm · 2024-10-13T19:52:43Z

sqlx-core/src/net/socket/mod.rs

+pub struct TcpKeepalive {
+    pub time: Option<Duration>,
+    pub interval: Option<Duration>,
+    pub retries: Option<u32>,
+}


If we want to encapsulate instead of using socket2::TcpKeepalive in the public api (that is something that @abonander should decide), we should use a newtype wrapper instead of just copying the other code.

Suggested change

pub struct TcpKeepalive {

pub time: Option<Duration>,

pub interval: Option<Duration>,

pub retries: Option<u32>,

}

pub struct TcpKeepalive(socket2::TcpKeepalive)

That's reasonable, thx

That's a good idea. But socket2::TcpKeepalive is not Copy (Sorry I made a mistake before).
So doing so we won't have Copy trait.

Interesting. I don't know how I came up with the idea that it does. Must have misread a part of the code.

Yeah, I have mixed feelings about using a bespoke struct here.

These builder methods could just be directly on the ConnectOptions structs.

I suppose being able to define TcpKeepalive separately in a const could be nice for reusability, but at the same time, having to import and build a separate type would also be a little annoying if you only use it once.

I think maybe because interval and retries aren't supported on all platforms, we should only expose the tcp_keepalive_time() on the ConnectOptions builders. We can always add the others later.

These fields should also be #[doc(hidden)] or just private.

abonander · 2024-10-15T17:39:46Z

After some thought, I'm not entirely sure how much value this actually has.

As I worked out in estuary/flow#1676 (comment), keepalive likely won't solve the original reporter's problem.

It could be useful for other reasons, such as keeping connections from timing out, or catching when a server disconnected abruptly without sending a FIN packet.

However, because the connection state is managed directly, we won't see a keepalive timeout until the next time we try to read from or write to the socket, at which point we're already trying to use it anyway.

I suppose that's still preferable to it hanging forever on a read that will never complete, though.

sqlx-core/src/net/socket/mod.rs

abonander · 2024-10-15T21:42:28Z

sqlx-core/src/net/socket/mod.rs

+pub struct TcpKeepalive {
+    pub time: Option<Duration>,
+    pub interval: Option<Duration>,
+    pub retries: Option<u32>,
+}


Yeah, I have mixed feelings about using a bespoke struct here.

These builder methods could just be directly on the ConnectOptions structs.

I suppose being able to define TcpKeepalive separately in a const could be nice for reusability, but at the same time, having to import and build a separate type would also be a little annoying if you only use it once.

I think maybe because interval and retries aren't supported on all platforms, we should only expose the tcp_keepalive_time() on the ConnectOptions builders. We can always add the others later.

tcp_keepalive.rs and update related code - Moved the TCP keepalive type definition from `mod.rs` to new source file `tcp_keepalive.rs` - Modified method `tcp_keep_alive` to `tcp_keepalive_time` in `MySqlConnectOptions` and `PgConnectOptions` - Updated `socket2` dependency in `sqlx-core` to enable feature `all`.

svanharmelen · 2025-08-26T06:41:34Z

I think this logic is required for pgnotify connections to recover after the connection is somehow broken. Could you take another look at this one @abonander to see what is needed to get this merged? Thanks!

acheung-harmonicinc · 2025-12-02T23:53:53Z

Catching situations where a server disconnects abruptly without sending a FIN packet is a real-world use case for us.

Our application subscribes to PostgreSQL notifications via database triggers. After an infrastructure issue, we observed that the application silently stopped receiving notifications while the process itself remained running and unaware of any connection problem.

I was able to reproduce this by using iptables rules to drop all packets from the database server. In this scenario, the PgListener continues to wait indefinitely and never detects that the connection is broken. I believe a similar situation would occur if the database host is physically disconnected from the network: no FIN or RST is sent, so the client never gets notified that the connection is actually gone.

When I applied the LD_PRELOAD-based keepalive workaround described here:

https://www.redpill-linpro.com/techblog/2024/12/17/failovers-and-keepalive.html

the PgListener was able to detect the dead connection and recover correctly.

However, relying on the LD_PRELOAD trick is hacky and has undesirable side effects: it affects all TCP connections from the process, not just the database connections.

It would be very helpful if sqlx exposed dedicated TCP keepalive configuration for its database connections, so we can:

Enable keepalive explicitly for DB connections only.
Tune keepalive parameters (e.g., idle time, interval, probe count) to match our failover and detection requirements.

This would allow PgListener (and other long-lived connections) to reliably detect broken connections in scenarios where the server disappears without a clean shutdown.

c-solo · 2026-02-10T06:19:19Z

We're hitting the same issue in production — long-lived PostgreSQL connections silently go stale after network changes, and without TCP keepalive there's no way to detect it. Would love to see this merged, thank you for the work on this!

feat: add TCP keepalive for MySQL and PostgresSQL.

59a7157

BREAK CHANGE: [`sqlx_core::net::socket::connect_tcp`]. New parameter added. Add TCP keepalive configuration which could be enabled by [`PgConnectOptions::tcp_keep_alive`] and [`MySqlConnectOptions::tcp_keep_alive`].

xuehaonan27 mentioned this pull request Oct 12, 2024

Add the ability to enable TCP keepalive on Postgres client connections #3540

Open

CommanderStorm reviewed Oct 12, 2024

View reviewed changes

xuehaonan27 requested a review from CommanderStorm October 12, 2024 13:25

update: add build pattern and Copy trait to

2bb710d

`TcpKeepalive`. fix: `connect_tcp` API design.

CommanderStorm reviewed Oct 13, 2024

View reviewed changes

abonander requested changes Oct 15, 2024

View reviewed changes

xuehaonan27 requested a review from abonander October 16, 2024 03:24

sargarass mentioned this pull request Feb 10, 2026

feat(postgres): keepalives for tcp #4159

Open

	keepalive: &Option<TcpKeepalive>,
	keepalive: Option<&TcpKeepalive>,

Comments

Conversation

xuehaonan27 commented Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Does your PR solve an issue?

Uh oh!

CommanderStorm left a comment

Choose a reason for hiding this comment

Uh oh!

CommanderStorm Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CommanderStorm Oct 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CommanderStorm Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xuehaonan27 commented Oct 12, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abonander commented Oct 15, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

svanharmelen commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acheung-harmonicinc commented Dec 2, 2025

Uh oh!

c-solo commented Feb 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

xuehaonan27 commented Oct 12, 2024 •

edited

Loading

CommanderStorm Oct 12, 2024 •

edited

Loading

CommanderStorm Oct 13, 2024 •

edited

Loading

CommanderStorm Oct 12, 2024 •

edited

Loading

svanharmelen commented Aug 26, 2025 •

edited

Loading