WIP pool changes
- 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
PoolConnectortrait superceding bothbefore_connect(requested but not yet implemented) andafter_connectcallbacks.- Implemented for closures returning
Future, albeit with a'staticrequirement for the returnedFuture(instead ofBoxFuture). - 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).
- Implemented for closures returning
- Use
usizefor all connection counts to get rid of weird inconsistencies.
Breaking Changes
Pool::set_connect_options()andget_connect_options()have been removed. Instead, implement the newPoolConnectortrait (or use a closure) using something likeArc<RwLock<impl ConnectOptions>>.PoolOptions::after_connect()has been removed. Instead, implementPoolConnector(or use a closure), open a connection and then apply any operations necessary.PoolOptions::min_connections(),PoolOptions::max_connections()andPool::size()now useusizeinstead ofu32.
Fixes #3513 Fixes #3315 Fixes #3132 Fixes #3117 Fixes #2848
Just tested this with #2805 and this PR seems to fix that issue 🎉
@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?
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.
The only reason I even checked is this item in your list above:
- acquire() should now be completely cancel-safe.
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.