breaking: new `sqlx::Pool` architecture by abonander · Pull Request #3582 · transact-rs/sqlx

abonander · 2024-10-29T22:01:55Z

Use a separate waiting queue for new connections.
Pool inheritance (used for testing) only steals connect permits, not acquire permits.
Spawn connection attempts as their own task so they may complete even if the acquire() call is cancelled.
Race opening a new connection with acquiring one from the idle queue.
acquire() should now be completely cancel-safe.
Separate timeout for connecting.
New PoolConnector trait superceding both before_connect (requested but not yet implemented) and after_connect callbacks.
- Implemented for closures returning Future, albeit with a 'static requirement for the returned Future (instead of BoxFuture).
- May be updated to use async closures in a future release (hopefully backwards compatible but will require an MSRV bump): https://blog.rust-lang.org/inside-rust/2024/08/09/async-closures-call-for-testing.html
- Can be used to support high availability, or implement custom backoff or connection throttling schemes (e.g. token bucket).
Use usize for all connection counts to get rid of weird inconsistencies.

Breaking Changes

Pool::set_connect_options() and get_connect_options() have been removed. Instead, implement the new PoolConnector trait (or use a closure) using something like Arc<RwLock<impl ConnectOptions>>.
PoolOptions::after_connect() has been removed. Instead, implement PoolConnector (or use a closure), open a connection and then apply any operations necessary.
PoolOptions::min_connections(), PoolOptions::max_connections() and Pool::size() now use usize instead of u32.

Fixes #3513
Fixes #3315
Fixes #3132
Fixes #3117
Fixes #2848

FSMaxB · 2025-10-29T05:27:39Z

Just tested this with #2805 and this PR seems to fix that issue 🎉

abonander · 2025-10-29T09:24:09Z

@FSMaxB there aren't any changes in this PR that I would expect to fix #2805. Can you make sure it's not just a flaky test?

FSMaxB · 2025-10-29T09:46:43Z

Yup, pretty sure it's not flaky. If I test with 0.9.0-alpha.1, I repeatably get between 50 and 80 warning logs about transaction already in progress or there not being any transactions.

If I run it with the ab/pool-changes branch, I get none. You can test it yourself with the reproducer in that issue.

FSMaxB · 2025-10-29T09:48:57Z

The only reason I even checked is this item in your list above:

acquire() should now be completely cancel-safe.

FSMaxB · 2025-10-29T09:50:10Z

It's perfectly possible that this PR doesn't fix all races that can lead to the issue in #2805, but it seems to have at least fixed the race that the reproducer is triggering.

…ersions`" This reverts commit 53ad46f.

rbtcollins · 2026-04-13T18:49:50Z

Should this be added to https://github.com/launchbadge/sqlx/milestone/10 ? I would love to get the security toil reduction this would enable, and hate it if this was missed due being forgotten at some critical time :)

abonander · 2026-04-14T00:46:14Z

This won't be forgotten, but it still needs more work and I don't want to hold the 0.9.0 release much longer.

Fixes transact-rs#4349. `Floating::return_to_pool` holds the pool's `DecrementSizeGuard` while it does I/O on the connection: an `after_release` hook, a `ping()` to check the connection is still usable, or a graceful `close()`. None of those were bounded. That matters because the I/O can be on a socket that is dead in a way the socket cannot report. If the server disappeared without closing the connection, and the client has nothing left to retransmit, no RST is ever provoked and reads never complete. The returning task then holds its permit forever and the pool shrinks by one connection. After `max_connections` of those, every `acquire()` fails with `PoolTimedOut` and the pool never recovers, even though the server is back. Each step now gets `RETURN_TO_POOL_TIMEOUT` (5s, matching the existing `CLOSE_ON_DROP_TIMEOUT`), and on expiry the connection is dropped via `close_hard()`, which does no I/O. Cancelling `close()` likewise drops the connection and releases the permit. The regression test uses an `after_release` hook that never completes, which is a deterministic stand-in for a socket that never answers: before this change it exhausts `acquire_timeout`, after it the pool is usable again once the bound expires. I realise transact-rs#3582 reworks this area and includes a timeout of its own; this is meant for main until that lands.

Review pointed out that the `ErrorResponse` closing a failed OAUTHBEARER exchange says only that authentication failed. Everything an application needs in order to go and get a token is in the server's status document, which the driver was throwing away, and a token provider cannot be the place where that happens because the document never reaches it. So the document is what comes out now. `Error::OAuth` carries an `OAuthChallenge`, and a connection with no token asks the server for its parameters instead of failing locally: an empty `auth` value, which is how RFC 7628 §4.3 and libpq ask. A missing token and a rejected one therefore end the same way, with the caller holding the issuer and the scope it needs to mint a token and dial again. Nothing re-dials inside `establish`, which has no retries and may be handed a socket by the caller. `oauth_token_provider` goes with it: it could refresh a token but never discover one, and keeping a pool supplied with fresh tokens is the job of the connection callback in transact-rs#3582 rather than of a second mechanism here. `oauth_token` documents the sequence, including that a token set there is not refreshed. `tests/postgres/oauth.rs` covers an accepted token, a rejected one, a connection with no token, a token that would forge a message, and the dial-again sequence as documented, against a real PostgreSQL 18. The `postgres_18_oauth` service compiles a validator module for that, because the server ships none and refuses to run an OAuth exchange without one. Those tests are `#[ignore]`d, since the mechanism is the server's choice and no other service asks for a token; the `postgres-oauth` CI job and `x.py` run them with `--include-ignored`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

abonander added the breaking label Oct 29, 2024

This was referenced Oct 29, 2024

Fix: spawn task when opening new connection in acquire. #3516

Closed

feat: Implement before_connect callback to modify connect options. #3562

Closed

abonander commented Oct 29, 2024

View reviewed changes

Comment thread sqlx-core/src/pool/connect.rs Outdated

nitnelave mentioned this pull request Oct 30, 2024

[BUG] Postgresql reports "could not receive data from client: Connection reset by peer" lldap/lldap#980

Closed

svix-jplatte reviewed Nov 8, 2024

View reviewed changes

Comment thread sqlx-core/src/rt/mod.rs Outdated

abonander force-pushed the ab/pool-changes branch 8 times, most recently from 8420b43 to d671b98 Compare November 11, 2024 22:01

abonander mentioned this pull request May 3, 2025

sqlx-postgres, sqlx-aws: impl dynamic passwords #3851

Closed

abonander force-pushed the ab/pool-changes branch from d671b98 to 656e751 Compare August 22, 2025 06:38

abonander force-pushed the ab/pool-changes branch 2 times, most recently from d500c21 to 2be94a9 Compare September 6, 2025 00:36

abonander force-pushed the ab/pool-changes branch from 2be94a9 to c58291b Compare September 23, 2025 15:00

abonander mentioned this pull request Oct 15, 2025

sqlx-core pools optionally immediately return ConnectionRefused errors instead of hanging and returning PoolTimedOut #3929

Closed

abonander force-pushed the ab/pool-changes branch from b6d4757 to 7bb8521 Compare October 29, 2025 18:37

abonander force-pushed the ab/pool-changes branch 2 times, most recently from 97c3af6 to 0dd92b4 Compare December 25, 2025 07:07

This was referenced Jan 17, 2026

Add optional ping timeout #4146

Closed

Bump whoami to v2 #4143

Merged

abonander added 4 commits February 27, 2026 14:05

WIP refactor: replace sharding with single connection set (5)

77d076f

WIP refactor: replace sharding with single connection set (6)

4485e09

fix: run cargo fmt

d6d4e59

fix: change minimal-versions check to use direct-minimal-versions

53ad46f

abonander force-pushed the ab/pool-changes branch from 0baa295 to 53ad46f Compare February 27, 2026 22:06

abonander added 4 commits February 27, 2026 16:13

chore: fix warnings

f40a3fd

refactor: delete pool parenting

b09d956

fix warnings

f35ebbd

fix more warnings

e0e4346

abonander changed the title ~~WIP pool changes~~ breaking: new sqlx::Pool architecture Mar 6, 2026

abonander mentioned this pull request Mar 26, 2026

fix(pool): replace yield_now with exponential backoff in acquire retry loop #4185

Closed

abonander added 4 commits March 27, 2026 18:16

Revert "fix: change minimal-versions check to use `direct-minimal-v…

4161e89

…ersions`" This reverts commit 53ad46f.

fix: don't poll future after yield in SpawnOnDrop

bb5a2b0

fix: Pool example

c68ddf5

fix: tests/any/pool.rs

fd61b99

coderabbitai Bot mentioned this pull request Apr 17, 2026

Integrate new conductor db holochain/holochain#5501

Merged

2 tasks

abonander mentioned this pull request Jun 19, 2026

docs: document sqlx test connection limits #4278

Merged

yuhao-su mentioned this pull request Jul 1, 2026

Flaky e2e: system hung causing timeout risingwavelabs/risingwave#24991

Closed

abonander mentioned this pull request Jul 13, 2026

WIP refactor: new #[sqlx::test] architecture #4338

Draft

adriangb mentioned this pull request Jul 24, 2026

fix(pool): bound each step of returning a connection to the pool #4354

Closed

coderabbitai Bot mentioned this pull request Aug 21, 2026

refactor(tests): move resource pool parallel test to api-db dsx-ai-factory/infra-controller#5245

Merged

2 of 10 tasks

abonander mentioned this pull request Sep 9, 2026

feat(postgres): support SASL OAUTHBEARER authentication #4400

Open

abonander mentioned this pull request Sep 10, 2026

fix(pool): bound return_to_pool so a dead connection can't leak its permit #4350

Open

KimSoungRyoul mentioned this pull request Sep 14, 2026

fix(pool): bound the entire connection return operation #4407

Open

wtsnz mentioned this pull request Oct 3, 2026

sqlx pings every pooled connection as it's returned: a Postgres round trip after every statement wtsnz/ash-rust#65

Closed

Navigation Menu

Repository navigation

breaking: new `sqlx::Pool` architecture - #3582

breaking: new `sqlx::Pool` architecture#3582
abonander wants to merge 37 commits into
mainfrom
ab/pool-changes

abonander commented Oct 29, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

FSMaxB commented Oct 29, 2025

Uh oh!

abonander commented Oct 29, 2025

Uh oh!

FSMaxB commented Oct 29, 2025

Uh oh!

FSMaxB commented Oct 29, 2025

Uh oh!

FSMaxB commented Oct 29, 2025

Uh oh!

rbtcollins commented Apr 13, 2026

Uh oh!

abonander commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

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

Uh oh!

Breaking Changes

Uh oh!

Uh oh!

Uh oh!

FSMaxB commented Oct 29, 2025

Uh oh!

abonander commented Oct 29, 2025

Uh oh!

FSMaxB commented Oct 29, 2025

Uh oh!

FSMaxB commented Oct 29, 2025

Uh oh!

FSMaxB commented Oct 29, 2025

Uh oh!

rbtcollins commented Apr 13, 2026

Uh oh!

abonander commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

abonander commented Oct 29, 2024 •

edited

Loading