Change pool configuration at runtime

[bikeshedder]

Right now the timeouts, hooks, etc. are all set once at pool creation and can't be changed after the pool is constructed.

The only thing that can be changed is the max_size using the resize method. (#29)

For 1.0 I want to change the configuration so it allows all configuration to be modified at runtime including exchanging the manager.

[bikeshedder]

The PoolBuilder should probably be replaced by a ConfigBuilder that creates immutable Config structs. That way the Pool could just provide a set_config method that takes a Config object and changes the internals. Pools would then be generated using a Config and Manager which doesn't even need the PoolBuilder anymore.

[allan2]

This is how bb8 does it:

let pool = Pool::builder().build(PostgresConnectionManager::new(config, connector)).await.unwrap();

and r2d2_postgres:

let config = r2d2::Config::default();
let manager = PostgresConnectionManager::new("postgres://postgres@localhost", TlsMode::None).unwrap();
let pool = r2d2::Pool::new(config, manager).unwrap();

What do you think about those patterns with PoolBuilder?

[bikeshedder]

Neither bb8 nor r2d2 support changing the configuration at runtime.

That's why I want to replace the PoolBuilder by a ConfigBuilder and add a Pool::set_config method that can be used to change the configuration of a already built pool.

I really like the idea of a immutable config structure with a ConfigBuilder that is replaced as a whole. Both initial pool construction and later config updates could use the exact same API.

An alternative would be a Pool::replace method that replaces the internals of self by the pool that is being passed in. I'm not too fond of this solution though as Pool is made up of a Arc<PoolInner> right now and this would mean it actually must be a RefCell in order to take ownership of the InnerPool.

That's why I feel like a immutable Config that is created via a ConfigBuilder is a way cleaner approach to this problem.

The pool itself could store the configuration as a RwLock<Arc<Config>> so that running Pool::get calls can complete their operation using the cloned Arc<Config> and all future calls to Pool::get use the updated configuration. This would limit blocking to a bare minimum and add as little contestation as possible. Reconfigurations should be rare and that's what we need to optimize for.

Just throwing some ideas around...

[allan2]

Oh I see... it's to support changing the configuration of an already-built pool.

Would the pool reflect the changes? I'm not sure how that works. I've never encountered this need before but I'm guessing it's to change the timeout of a pool.

[bbigras]

That could also be useful when using dynamic secrets with HashiCorp vault.

[bikeshedder]

I want to use the arc-swap crate to implement that feature. It should be as simple as putting both the config and manager into an ArcSwap. This will be a breaking change because of the Pool::manager method:

/// Returns [`Manager`] of this [`Pool`].
#[must_use]
pub fn manager(&self) -> &M {
    &*self.inner.manager
}

Which is an interesting discovery by myself. For future APIs I probably don't want to expose borrowed things directly but rather wrap them so that implementations like that aren't leaked.

The next version of the API will return a ManagerRef<T> struct instead which implements Deref<T> and is implemented using the ArcSwap.

[bikeshedder]

After some consideration I feel like the manager should not be replaced but the managers themselves should provide a way to modify the internal configuration instead. e.g. the deadpool-postgres manager keeps track of all created objects in order to provide a way to clear the statement cache of all connections. Adding some kind of handover between managers would just complicate things and I don't think replaceable managers are really needed.

I haven't looked into actually implementing this. I just realized that swapping managers might not be the best idea.