Skip to content

breaking: new sqlx::Pool architecture#3582

Open
abonander wants to merge 37 commits intomainfrom
ab/pool-changes
Open

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

Conversation

@abonander
Copy link
Copy Markdown
Collaborator

@abonander abonander commented Oct 29, 2024

  • 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
Copy link
Copy Markdown
Contributor

FSMaxB commented Oct 29, 2025

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

@abonander
Copy link
Copy Markdown
Collaborator Author

@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
Copy link
Copy Markdown
Contributor

FSMaxB commented Oct 29, 2025

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
Copy link
Copy Markdown
Contributor

FSMaxB commented Oct 29, 2025

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

  • acquire() should now be completely cancel-safe.

@FSMaxB
Copy link
Copy Markdown
Contributor

FSMaxB commented Oct 29, 2025

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.

@abonander 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
@abonander abonander changed the title WIP pool changes breaking: new sqlx::Pool architecture Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

4 participants