feat: Add set_connect_options method to Pool

[moatra]

This allows external updates of the ConnectionOption used when a new connection needs to be opened for the pool. The primary use case is to support dynamically updated (read: rotated) credentials used by systems like AWS RDS.

Closes #445

Normally I shy away from using a .expect() invocation in code, but to my understanding the RwLock poisoning concept in the stdlib is generally regarded as a mistake.

I looked at using Tokio's RwLock instead, but I didn't want to make the dynamic config update async-friendly. Doing so might encourage anyone using the dynamic config to hold the write lock while fetching the updated credentials which in turn would block any new connections from being spawned in the pool. Using a sync RwLock forces minimal holds on the lock, but Tokio's sync RwLock panics if used in an async context.

ParkingLot's RwLock does not have the poisoned concept (and therefore wouldn't require the .expect()) but it's not currently a dependency of the project.

[abonander]

Please see the discussion in #1941, that's the direction we wanted to go with this.

[moatra]

Please see the discussion in #1941, that's the direction we wanted to go with this.

Thanks for the link!

My understanding of the design direction from that thread (and please correct me if I'm mistaken) is to have an async-enabled ConnectOptionsProvider that can be invoked whenever a new connection needs to be opened.

@jamiebrynes7 mentioned that the connect_options() method on the pool would not support waiting for the async provider. I want to expand that argument: I think waiting every time the pool wants to open a new connection for the ConnectOptionsProvider is a design smell.

1. Typical implementations (fetch the credential when requested) will result in extra delays when opening new connections. The workaround would be for the provider implementation to implement its own caching. The design in this PR essentially provides that caching behavior out of the box, leaving it up to the would-be Provider implementer to decide when to update the credentials.
   Theoretically the Pool itself could add caching behavior to debounce or pre-cache calls to the provider, but that's basically recreating the work done in this PR with extra async and timeout config overhead.
2. Documenting the panic behavior of connect_options() if the provider is present is a band-aid; I'd rather update the types to make that case impossible. That's a much larger shift in the Pool interface, though. The design in this PR forces a ConnectOption to be provided up front, so the connect_options() method can synchronously return the reference to the cached value.

If y'all still want to go with the Provider approach, feel free to close out this PR. I can take a stab at that implementation.

[abonander]

I think it would be very easy to get confused by dynamic_connect_with(). At a glance, I would guess that it just opens a connection with the given ConnectOptions but protected by the pool's semaphore. Changing the pool's ConnectOptions at the same time violates the principle of least surprise.

At the same time, I'm starting to think the ConnectOptionsProvider interface I proposed is a bit overkill. I'd be in favor of a simple .set_connect_options() call that just overwrites the value the pool is currently using.

[abonander]

To avoid deep-cloning the ConnectOptions on every iteration of PoolInner::connect(), it should be stored in an Arc. That should also reduce contention on the lock.

[abonander]

Since this PR wants to make breaking changes anyways, I've changed the target to the 0.7 development branch.

[moatra]

@abonander - sanity checking: The error: failed to select a version for 'libsqlite3-sys' failures in the checks for this PR are unrelated, correct?

[abonander]

Yeah, if you want to fix that real quick you can increase the version here to 0.25:

sqlx/Cargo.toml

Line 151 in 1379eb6

libsqlite3-sys = { version = "0.24", features = ["bundled-sqlcipher"] }

[moatra]

Rebased on the 0.7-dev branch, copied over the sqlite fix from #2098