bikeshedder / deadpool

Dead simple pool implementation for rust with async-await
Apache License 2.0
1.08k stars 137 forks source link

Redis Sentinel connection pool #338

Closed smf8 closed 2 months ago

smf8 commented 5 months ago

Hello, Is there a plan to add the sentinel client connection pool support to the deadpool-redis crate?

Thanks in advance

smf8 commented 4 months ago

I'm fairly new to Rust, so I apologize beforehand if I ask obvious questions.

Based on current implementations of Client and ClusterClient. I think by combining changes from cluster and normal clients we can create a Sentinel client.

The only issue I've faced is that Manager.get_async_connection() requires &mut self. I didn't find any workaround except using a Send + Sync mutex (e.g. tokio::sync::Mutex) Which introduces a new dependency and additional locking/unlocking for connection creation.

So it will look like this:

pub struct Manager {
    client:  Mutex<SentinelClient>,
    ping_number: AtomicUsize,
}

async fn create(&self) -> Result<MultiplexedConnection, RedisError> {
    let mut client = self.client.lock().await;
    let conn = client.get_async_connection().await?;
    Ok(conn)
}

Is there any better way of achieving this?

bikeshedder commented 4 months ago

You can add a tokio::sync::Mutex without introducing any additional dependencies. deadpool itself already uses the tokio::sync::Semaphore internally (which is runtime agnostic). No other parts of tokio is a requirement though as is explained in the README:

https://github.com/bikeshedder/deadpool/blob/7b933c585d39f2ef57304353a35e139174b28f8e/README.md?plain=1#L181-L190

If you wanted to avoid the use of the tokio::sync::Mutex you could use a tokio::sync::Semaphore with a single permit and store the client inside a std::cell::Cell. Though I'd just go with the Mutex as it's just a bit cleaner and easier to reason about.

As far as I know the Rust compiler still compiles the entire tokio::sync module even if only the Semaphore is used. So using the Mutex from tokio shouldn't add any additional compile time.

bikeshedder commented 4 months ago

I forgot to answer your question...

Is there any better way of achieving this?

No. I think that's the way to go.

Feel free to create a PR adding this feature to deadpool-redis.

bikeshedder commented 2 months ago

I just published deadpool-redis 0.17.0 on crates.io including your changes:

Thanks a lot!