From e53450c9487e6bf1b1ae2309767b856cb25a952d Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Thu, 12 Mar 2026 09:11:42 +0100 Subject: [PATCH 1/5] Replace AsyncConnectionConfig with Manager builder pattern Removes `Manager::new_with_config` which exposed internal redis settings. Adds `Manager::builder()` with `connection_timeout()` and `response_timeout()` setters that only surface user-facing options. Co-Authored-By: Claude Opus 4.6 --- crates/deadpool-redis/src/lib.rs | 113 +++++++++++++----- .../tests/redis_connection_config.rs | 23 ++-- 2 files changed, 97 insertions(+), 39 deletions(-) diff --git a/crates/deadpool-redis/src/lib.rs b/crates/deadpool-redis/src/lib.rs index ab035442..c055b638 100644 --- a/crates/deadpool-redis/src/lib.rs +++ b/crates/deadpool-redis/src/lib.rs @@ -31,6 +31,7 @@ pub mod sentinel; use std::{ ops::{Deref, DerefMut}, sync::atomic::{AtomicUsize, Ordering}, + time::Duration, }; use deadpool::managed; @@ -127,21 +128,14 @@ impl ConnectionLike for Connection { /// [`Manager`] for creating and recycling [`redis`] connections. /// /// [`Manager`]: managed::Manager +#[derive(Debug)] pub struct Manager { client: Client, - connection_config: Option, + connection_timeout: Option>, + response_timeout: Option>, ping_number: AtomicUsize, } -impl std::fmt::Debug for Manager { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("Manager") - .field("client", &self.client) - .field("ping_number", &self.ping_number) - .finish() - } -} - impl Manager { /// Creates a new [`Manager`] from the given `params`. /// @@ -151,27 +145,81 @@ impl Manager { pub fn new(params: T) -> RedisResult { Ok(Self { client: Client::open(params)?, - connection_config: None, + connection_timeout: None, + response_timeout: None, ping_number: AtomicUsize::new(0), }) } - /// Creates a new [`Manager`] from the given `params` and - /// [`AsyncConnectionConfig`]. + /// Returns a [`ManagerBuilder`] for the given `params`. + /// + /// # Errors + /// + /// [`ManagerBuilder::build`] will fail if establishing a new [`Client`] + /// fails. + pub fn builder(params: T) -> ManagerBuilder { + ManagerBuilder { + params, + connection_timeout: None, + response_timeout: None, + } + } +} + +/// Builder for [`Manager`]. +/// +/// Use [`Manager::builder`] to create one. +/// +/// # Example +/// +/// ```rust +/// use std::time::Duration; +/// use deadpool_redis::Manager; +/// +/// let manager = Manager::builder("redis://127.0.0.1") +/// .connection_timeout(Some(Duration::from_secs(5))) +/// .response_timeout(None) +/// .build() +/// .unwrap(); +/// ``` +#[derive(Debug)] +pub struct ManagerBuilder { + params: T, + connection_timeout: Option>, + response_timeout: Option>, +} + +impl ManagerBuilder { + /// Sets the connection timeout. + /// + /// Pass `Some(duration)` to set a specific timeout, or `None` to explicitly + /// disable it. If not called, the redis crate's default timeout is used. + #[must_use] + pub fn connection_timeout(mut self, timeout: Option) -> Self { + self.connection_timeout = Some(timeout); + self + } + + /// Sets the response timeout. /// - /// This allows configuring connection-level settings such as response timeouts and connection - /// timeouts. + /// Pass `Some(duration)` to set a specific timeout, or `None` to explicitly + /// disable it. If not called, the redis crate's default timeout is used. + #[must_use] + pub fn response_timeout(mut self, timeout: Option) -> Self { + self.response_timeout = Some(timeout); + self + } + + /// Builds the [`Manager`]. /// /// # Errors /// /// If establishing a new [`Client`] fails. - pub fn new_with_config( - params: T, - connection_config: AsyncConnectionConfig, - ) -> RedisResult { - Ok(Self { - client: Client::open(params)?, - connection_config: Some(connection_config), + pub fn build(self) -> RedisResult { + Ok(Manager { + client: Client::open(self.params)?, + connection_timeout: self.connection_timeout, + response_timeout: self.response_timeout, ping_number: AtomicUsize::new(0), }) } @@ -182,13 +230,22 @@ impl managed::Manager for Manager { type Error = RedisError; async fn create(&self) -> Result { - let conn = match &self.connection_config { - Some(config) => { - self.client - .get_multiplexed_async_connection_with_config(config) - .await? + let conn = if self.connection_timeout.is_some() || self.response_timeout.is_some() { + let mut config = AsyncConnectionConfig::new(); + + if let Some(timeout) = self.connection_timeout { + config = config.set_connection_timeout(timeout); } - None => self.client.get_multiplexed_async_connection().await?, + + if let Some(timeout) = self.response_timeout { + config = config.set_response_timeout(timeout); + } + + self.client + .get_multiplexed_async_connection_with_config(&config) + .await? + } else { + self.client.get_multiplexed_async_connection().await? }; Ok(conn) diff --git a/crates/deadpool-redis/tests/redis_connection_config.rs b/crates/deadpool-redis/tests/redis_connection_config.rs index 2bc5dad3..259205a6 100644 --- a/crates/deadpool-redis/tests/redis_connection_config.rs +++ b/crates/deadpool-redis/tests/redis_connection_config.rs @@ -3,7 +3,7 @@ use std::time::Duration; use deadpool_redis::{Manager, Pool, Runtime}; -use redis::{AsyncCommands, AsyncConnectionConfig}; +use redis::AsyncCommands; use serde::{Deserialize, Serialize}; #[derive(Debug, Default, Deserialize, Serialize)] @@ -27,8 +27,11 @@ fn redis_url() -> String { Config::from_env().redis.url.unwrap_or_default() } -fn create_pool_with_config(connection_config: AsyncConnectionConfig) -> Pool { - let manager = Manager::new_with_config(redis_url(), connection_config).unwrap(); +fn create_pool_with_no_response_timeout() -> Pool { + let manager = Manager::builder(redis_url()) + .response_timeout(None) + .build() + .unwrap(); Pool::builder(manager) .max_size(1) .runtime(Runtime::Tokio1) @@ -45,16 +48,14 @@ fn create_pool_default() -> Pool { .unwrap() } -/// Verifies that `new_with_config` with `set_response_timeout(None)` allows commands that take -/// longer than the default 500ms timeout. +/// Verifies that `Manager::builder` with `response_timeout(None)` allows commands that take +/// longer than the default response timeout. /// -/// Uses `BLPOP` on an empty list with a 1-second timeout. With the default `AsyncConnectionConfig` -/// (500ms response timeout), this would fail. With `set_response_timeout(None)`, it waits the full -/// second and returns nil. +/// Uses `BLPOP` on an empty list with a 1-second timeout. With the default response timeout this +/// would fail. With `response_timeout(None)`, it waits the full second and returns nil. #[tokio::test] async fn test_response_timeout_can_be_disabled() { - let config = AsyncConnectionConfig::new().set_response_timeout(None); - let pool = create_pool_with_config(config); + let pool = create_pool_with_no_response_timeout(); let mut conn = pool.get().await.unwrap(); let result: Option<(String, String)> = conn @@ -65,7 +66,7 @@ async fn test_response_timeout_can_be_disabled() { } /// Verifies that the default `Manager::new` (without config) uses the redis crate's default -/// timeouts, which causes blocking commands exceeding 500ms to fail. +/// response timeout, which causes blocking commands exceeding it to fail. #[tokio::test] async fn test_default_manager_times_out_on_slow_commands() { let pool = create_pool_default(); From fbc1a7b1b55d2ba8524a6628d7d977213cc339d2 Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Wed, 25 Mar 2026 12:15:17 +0100 Subject: [PATCH 2/5] Add timeout fields to Config, revert ManagerBuilder Add connection_timeout and response_timeout to the serde-deserializable Config struct so timeouts can be configured via environment variables or config files, matching the pattern used by other deadpool crates. This reverts the ManagerBuilder added in the previous commits in favor of the Config-based approach suggested. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/deadpool-redis/src/config.rs | 77 +++++++++++- crates/deadpool-redis/src/lib.rs | 114 ++++-------------- .../tests/redis_connection_config.rs | 64 ++++------ 3 files changed, 119 insertions(+), 136 deletions(-) diff --git a/crates/deadpool-redis/src/config.rs b/crates/deadpool-redis/src/config.rs index f6fdf7be..57c44cc7 100644 --- a/crates/deadpool-redis/src/config.rs +++ b/crates/deadpool-redis/src/config.rs @@ -1,4 +1,4 @@ -use std::{fmt, path::PathBuf}; +use std::{fmt, path::PathBuf, time::Duration}; use redis::RedisError; #[cfg(feature = "serde")] @@ -17,6 +17,10 @@ use crate::{CreatePoolError, Pool, PoolBuilder, PoolConfig, RedisResult, Runtime /// REDIS__POOL__MAX_SIZE=16 /// REDIS__POOL__TIMEOUTS__WAIT__SECS=2 /// REDIS__POOL__TIMEOUTS__WAIT__NANOS=0 +/// REDIS__CONNECTION_TIMEOUT__SECS=5 +/// REDIS__CONNECTION_TIMEOUT__NANOS=0 +/// REDIS__RESPONSE_TIMEOUT__SECS=2 +/// REDIS__RESPONSE_TIMEOUT__NANOS=0 /// ``` /// ```rust /// #[derive(serde::Deserialize)] @@ -47,6 +51,22 @@ pub struct Config { /// Pool configuration. pub pool: Option, + + /// Connection timeout applied when creating new Redis connections. + /// + /// - `None`: use the redis crate's default. + /// - `Some(None)`: disable the timeout. + /// - `Some(Some(duration))`: use this specific timeout. + #[cfg_attr(feature = "serde", serde(default))] + pub connection_timeout: Option>, + + /// Response timeout applied when waiting for Redis responses. + /// + /// - `None`: use the redis crate's default. + /// - `Some(None)`: disable the timeout. + /// - `Some(Some(duration))`: use this specific timeout. + #[cfg_attr(feature = "serde", serde(default))] + pub response_timeout: Option>, } impl Config { @@ -70,15 +90,38 @@ impl Config { /// See [`ConfigError`] for details. pub fn builder(&self) -> Result { let manager = match (&self.url, &self.connection) { - (Some(url), None) => crate::Manager::new(url.as_str())?, - (None, Some(connection)) => crate::Manager::new(connection.clone())?, - (None, None) => crate::Manager::new(ConnectionInfo::default())?, + (Some(url), None) => self.build_manager(url.as_str())?, + (None, Some(connection)) => self.build_manager(connection.clone())?, + (None, None) => self.build_manager(ConnectionInfo::default())?, (Some(_), Some(_)) => return Err(ConfigError::UrlAndConnectionSpecified), }; + let pool_config = self.get_pool_config(); + Ok(Pool::builder(manager).config(pool_config)) } + fn build_manager( + &self, + params: T, + ) -> Result { + if self.connection_timeout.is_some() || self.response_timeout.is_some() { + let mut config = redis::AsyncConnectionConfig::new(); + + if let Some(timeout) = self.connection_timeout { + config = config.set_connection_timeout(timeout); + } + + if let Some(timeout) = self.response_timeout { + config = config.set_response_timeout(timeout); + } + + Ok(crate::Manager::new_with_config(params, config)?) + } else { + Ok(crate::Manager::new(params)?) + } + } + /// Returns [`deadpool::managed::PoolConfig`] which can be used to construct /// a [`deadpool::managed::Pool`] instance. #[must_use] @@ -94,6 +137,8 @@ impl Config { url: Some(url.into()), connection: None, pool: None, + connection_timeout: None, + response_timeout: None, } } @@ -105,8 +150,30 @@ impl Config { url: None, connection: Some(connection_info.into()), pool: None, + connection_timeout: None, + response_timeout: None, } } + + /// Sets the connection timeout. + /// + /// Pass `Some(duration)` to set a specific timeout, or `None` to + /// explicitly disable it. + #[must_use] + pub fn with_connection_timeout(mut self, timeout: Option) -> Self { + self.connection_timeout = Some(timeout); + self + } + + /// Sets the response timeout. + /// + /// Pass `Some(duration)` to set a specific timeout, or `None` to + /// explicitly disable it. + #[must_use] + pub fn with_response_timeout(mut self, timeout: Option) -> Self { + self.response_timeout = Some(timeout); + self + } } impl Default for Config { @@ -115,6 +182,8 @@ impl Default for Config { url: None, connection: Some(ConnectionInfo::default()), pool: None, + connection_timeout: None, + response_timeout: None, } } } diff --git a/crates/deadpool-redis/src/lib.rs b/crates/deadpool-redis/src/lib.rs index c055b638..d5fb6f0f 100644 --- a/crates/deadpool-redis/src/lib.rs +++ b/crates/deadpool-redis/src/lib.rs @@ -31,7 +31,6 @@ pub mod sentinel; use std::{ ops::{Deref, DerefMut}, sync::atomic::{AtomicUsize, Ordering}, - time::Duration, }; use deadpool::managed; @@ -128,14 +127,21 @@ impl ConnectionLike for Connection { /// [`Manager`] for creating and recycling [`redis`] connections. /// /// [`Manager`]: managed::Manager -#[derive(Debug)] pub struct Manager { client: Client, - connection_timeout: Option>, - response_timeout: Option>, + connection_config: Option, ping_number: AtomicUsize, } +impl std::fmt::Debug for Manager { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Manager") + .field("client", &self.client) + .field("ping_number", &self.ping_number) + .finish() + } +} + impl Manager { /// Creates a new [`Manager`] from the given `params`. /// @@ -145,81 +151,18 @@ impl Manager { pub fn new(params: T) -> RedisResult { Ok(Self { client: Client::open(params)?, - connection_timeout: None, - response_timeout: None, + connection_config: None, ping_number: AtomicUsize::new(0), }) } - /// Returns a [`ManagerBuilder`] for the given `params`. - /// - /// # Errors - /// - /// [`ManagerBuilder::build`] will fail if establishing a new [`Client`] - /// fails. - pub fn builder(params: T) -> ManagerBuilder { - ManagerBuilder { - params, - connection_timeout: None, - response_timeout: None, - } - } -} - -/// Builder for [`Manager`]. -/// -/// Use [`Manager::builder`] to create one. -/// -/// # Example -/// -/// ```rust -/// use std::time::Duration; -/// use deadpool_redis::Manager; -/// -/// let manager = Manager::builder("redis://127.0.0.1") -/// .connection_timeout(Some(Duration::from_secs(5))) -/// .response_timeout(None) -/// .build() -/// .unwrap(); -/// ``` -#[derive(Debug)] -pub struct ManagerBuilder { - params: T, - connection_timeout: Option>, - response_timeout: Option>, -} - -impl ManagerBuilder { - /// Sets the connection timeout. - /// - /// Pass `Some(duration)` to set a specific timeout, or `None` to explicitly - /// disable it. If not called, the redis crate's default timeout is used. - #[must_use] - pub fn connection_timeout(mut self, timeout: Option) -> Self { - self.connection_timeout = Some(timeout); - self - } - - /// Sets the response timeout. - /// - /// Pass `Some(duration)` to set a specific timeout, or `None` to explicitly - /// disable it. If not called, the redis crate's default timeout is used. - #[must_use] - pub fn response_timeout(mut self, timeout: Option) -> Self { - self.response_timeout = Some(timeout); - self - } - - /// Builds the [`Manager`]. - /// - /// # Errors - /// - /// If establishing a new [`Client`] fails. - pub fn build(self) -> RedisResult { - Ok(Manager { - client: Client::open(self.params)?, - connection_timeout: self.connection_timeout, - response_timeout: self.response_timeout, + fn new_with_config( + params: T, + connection_config: AsyncConnectionConfig, + ) -> RedisResult { + Ok(Self { + client: Client::open(params)?, + connection_config: Some(connection_config), ping_number: AtomicUsize::new(0), }) } @@ -230,22 +173,13 @@ impl managed::Manager for Manager { type Error = RedisError; async fn create(&self) -> Result { - let conn = if self.connection_timeout.is_some() || self.response_timeout.is_some() { - let mut config = AsyncConnectionConfig::new(); - - if let Some(timeout) = self.connection_timeout { - config = config.set_connection_timeout(timeout); + let conn = match &self.connection_config { + Some(config) => { + self.client + .get_multiplexed_async_connection_with_config(config) + .await? } - - if let Some(timeout) = self.response_timeout { - config = config.set_response_timeout(timeout); - } - - self.client - .get_multiplexed_async_connection_with_config(&config) - .await? - } else { - self.client.get_multiplexed_async_connection().await? + None => self.client.get_multiplexed_async_connection().await?, }; Ok(conn) diff --git a/crates/deadpool-redis/tests/redis_connection_config.rs b/crates/deadpool-redis/tests/redis_connection_config.rs index 259205a6..86e43c57 100644 --- a/crates/deadpool-redis/tests/redis_connection_config.rs +++ b/crates/deadpool-redis/tests/redis_connection_config.rs @@ -2,7 +2,7 @@ use std::time::Duration; -use deadpool_redis::{Manager, Pool, Runtime}; +use deadpool_redis::Runtime; use redis::AsyncCommands; use serde::{Deserialize, Serialize}; @@ -27,64 +27,44 @@ fn redis_url() -> String { Config::from_env().redis.url.unwrap_or_default() } -fn create_pool_with_no_response_timeout() -> Pool { - let manager = Manager::builder(redis_url()) - .response_timeout(None) - .build() +/// Verifies that disabling the response timeout allows commands that take longer than the +/// default timeout. Uses `BLPOP` on an empty list with a 1-second server-side timeout. +#[tokio::test] +async fn test_response_timeout_disabled() { + let pool = deadpool_redis::Config::from_url(redis_url()) + .with_response_timeout(None) + .create_pool(Some(Runtime::Tokio1)) .unwrap(); - Pool::builder(manager) - .max_size(1) - .runtime(Runtime::Tokio1) - .build() - .unwrap() -} -fn create_pool_default() -> Pool { - let manager = Manager::new(redis_url()).unwrap(); - Pool::builder(manager) - .max_size(1) - .runtime(Runtime::Tokio1) - .build() - .unwrap() -} - -/// Verifies that `Manager::builder` with `response_timeout(None)` allows commands that take -/// longer than the default response timeout. -/// -/// Uses `BLPOP` on an empty list with a 1-second timeout. With the default response timeout this -/// would fail. With `response_timeout(None)`, it waits the full second and returns nil. -#[tokio::test] -async fn test_response_timeout_can_be_disabled() { - let pool = create_pool_with_no_response_timeout(); let mut conn = pool.get().await.unwrap(); let result: Option<(String, String)> = conn - .blpop("deadpool/nonexistent_timeout_test_key", 1.0) + .blpop("deadpool/test_timeout_disabled", 1.0) .await .unwrap(); + assert_eq!(result, None); } -/// Verifies that the default `Manager::new` (without config) uses the redis crate's default -/// response timeout, which causes blocking commands exceeding it to fail. +/// Verifies that setting an explicit response timeout causes commands exceeding it to fail. #[tokio::test] -async fn test_default_manager_times_out_on_slow_commands() { - let pool = create_pool_default(); +async fn test_response_timeout_causes_timeout() { + let pool = deadpool_redis::Config::from_url(redis_url()) + .with_response_timeout(Some(Duration::from_millis(100))) + .create_pool(Some(Runtime::Tokio1)) + .unwrap(); + let mut conn = pool.get().await.unwrap(); let start = std::time::Instant::now(); - let result: Result, _> = conn - .blpop("deadpool/nonexistent_default_timeout_key", 1.0) - .await; + let result: Result, _> = + conn.blpop("deadpool/test_timeout_short", 1.0).await; let elapsed = start.elapsed(); + assert!(result.is_err(), "expected timeout error"); assert!( - result.is_err(), - "expected timeout error with default config" - ); - assert!( - elapsed < Duration::from_millis(900), - "should have timed out before the 1s BLPOP completed, took {:?}", + elapsed < Duration::from_millis(500), + "should have timed out well before the 1s BLPOP, took {:?}", elapsed ); } From f5cf5d91fec6e710990334e99bf6091bf20a4920 Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Wed, 25 Mar 2026 20:52:51 +0100 Subject: [PATCH 3/5] Add timeout config to Config, restore Manager builder Add connection_timeout and response_timeout as Option fields to Config with serde support and hardcoded defaults matching the redis crate (1s connection, 500ms response). Restore the Manager builder pattern for programmatic use, keeping AsyncConnectionConfig as an internal detail. --- crates/deadpool-redis/src/config.rs | 74 +++++++++++++---------------- crates/deadpool-redis/src/lib.rs | 73 +++++++++++++++++++++++++--- 2 files changed, 101 insertions(+), 46 deletions(-) diff --git a/crates/deadpool-redis/src/config.rs b/crates/deadpool-redis/src/config.rs index 57c44cc7..4f8a70e2 100644 --- a/crates/deadpool-redis/src/config.rs +++ b/crates/deadpool-redis/src/config.rs @@ -6,6 +6,9 @@ use serde::{Deserialize, Serialize}; use crate::{CreatePoolError, Pool, PoolBuilder, PoolConfig, RedisResult, Runtime}; +const DEFAULT_CONNECTION_TIMEOUT: Option = Some(Duration::from_secs(1)); +const DEFAULT_RESPONSE_TIMEOUT: Option = Some(Duration::from_millis(500)); + /// Configuration object. /// /// # Example (from environment) @@ -17,10 +20,10 @@ use crate::{CreatePoolError, Pool, PoolBuilder, PoolConfig, RedisResult, Runtime /// REDIS__POOL__MAX_SIZE=16 /// REDIS__POOL__TIMEOUTS__WAIT__SECS=2 /// REDIS__POOL__TIMEOUTS__WAIT__NANOS=0 -/// REDIS__CONNECTION_TIMEOUT__SECS=5 +/// REDIS__CONNECTION_TIMEOUT__SECS=1 /// REDIS__CONNECTION_TIMEOUT__NANOS=0 -/// REDIS__RESPONSE_TIMEOUT__SECS=2 -/// REDIS__RESPONSE_TIMEOUT__NANOS=0 +/// REDIS__RESPONSE_TIMEOUT__SECS=0 +/// REDIS__RESPONSE_TIMEOUT__NANOS=500000000 /// ``` /// ```rust /// #[derive(serde::Deserialize)] @@ -54,19 +57,15 @@ pub struct Config { /// Connection timeout applied when creating new Redis connections. /// - /// - `None`: use the redis crate's default. - /// - `Some(None)`: disable the timeout. - /// - `Some(Some(duration))`: use this specific timeout. - #[cfg_attr(feature = "serde", serde(default))] - pub connection_timeout: Option>, + /// Set to `None` to disable the timeout. Defaults to 1 second. + #[cfg_attr(feature = "serde", serde(default = "default_connection_timeout"))] + pub connection_timeout: Option, /// Response timeout applied when waiting for Redis responses. /// - /// - `None`: use the redis crate's default. - /// - `Some(None)`: disable the timeout. - /// - `Some(Some(duration))`: use this specific timeout. - #[cfg_attr(feature = "serde", serde(default))] - pub response_timeout: Option>, + /// Set to `None` to disable the timeout. Defaults to 500 milliseconds. + #[cfg_attr(feature = "serde", serde(default = "default_response_timeout"))] + pub response_timeout: Option, } impl Config { @@ -105,21 +104,10 @@ impl Config { &self, params: T, ) -> Result { - if self.connection_timeout.is_some() || self.response_timeout.is_some() { - let mut config = redis::AsyncConnectionConfig::new(); - - if let Some(timeout) = self.connection_timeout { - config = config.set_connection_timeout(timeout); - } - - if let Some(timeout) = self.response_timeout { - config = config.set_response_timeout(timeout); - } - - Ok(crate::Manager::new_with_config(params, config)?) - } else { - Ok(crate::Manager::new(params)?) - } + Ok(crate::Manager::builder(params) + .connection_timeout(self.connection_timeout) + .response_timeout(self.response_timeout) + .build()?) } /// Returns [`deadpool::managed::PoolConfig`] which can be used to construct @@ -136,9 +124,7 @@ impl Config { Config { url: Some(url.into()), connection: None, - pool: None, - connection_timeout: None, - response_timeout: None, + ..Default::default() } } @@ -149,29 +135,27 @@ impl Config { Config { url: None, connection: Some(connection_info.into()), - pool: None, - connection_timeout: None, - response_timeout: None, + ..Default::default() } } /// Sets the connection timeout. /// /// Pass `Some(duration)` to set a specific timeout, or `None` to - /// explicitly disable it. + /// disable it. #[must_use] pub fn with_connection_timeout(mut self, timeout: Option) -> Self { - self.connection_timeout = Some(timeout); + self.connection_timeout = timeout; self } /// Sets the response timeout. /// /// Pass `Some(duration)` to set a specific timeout, or `None` to - /// explicitly disable it. + /// disable it. #[must_use] pub fn with_response_timeout(mut self, timeout: Option) -> Self { - self.response_timeout = Some(timeout); + self.response_timeout = timeout; self } } @@ -182,12 +166,22 @@ impl Default for Config { url: None, connection: Some(ConnectionInfo::default()), pool: None, - connection_timeout: None, - response_timeout: None, + connection_timeout: DEFAULT_CONNECTION_TIMEOUT, + response_timeout: DEFAULT_RESPONSE_TIMEOUT, } } } +#[cfg(feature = "serde")] +fn default_connection_timeout() -> Option { + DEFAULT_CONNECTION_TIMEOUT +} + +#[cfg(feature = "serde")] +fn default_response_timeout() -> Option { + DEFAULT_RESPONSE_TIMEOUT +} + /// This is a 1:1 copy of the [`redis::ConnectionAddr`] enumeration (excluding `tls_params` since it is entirely opaque to consumers). /// /// This is duplicated here in order to add support for the diff --git a/crates/deadpool-redis/src/lib.rs b/crates/deadpool-redis/src/lib.rs index d5fb6f0f..fe594dbf 100644 --- a/crates/deadpool-redis/src/lib.rs +++ b/crates/deadpool-redis/src/lib.rs @@ -31,6 +31,7 @@ pub mod sentinel; use std::{ ops::{Deref, DerefMut}, sync::atomic::{AtomicUsize, Ordering}, + time::Duration, }; use deadpool::managed; @@ -156,12 +157,72 @@ impl Manager { }) } - fn new_with_config( - params: T, - connection_config: AsyncConnectionConfig, - ) -> RedisResult { - Ok(Self { - client: Client::open(params)?, + /// Returns a [`ManagerBuilder`] for the given `params`. + pub fn builder(params: T) -> ManagerBuilder { + ManagerBuilder { + params, + connection_timeout: None, + response_timeout: None, + } + } +} + +/// Builder for [`Manager`]. +/// +/// Use [`Manager::builder`] to create one. +/// +/// # Example +/// +/// ```rust +/// use std::time::Duration; +/// use deadpool_redis::Manager; +/// +/// let manager = Manager::builder("redis://127.0.0.1") +/// .connection_timeout(Some(Duration::from_secs(5))) +/// .response_timeout(None) +/// .build() +/// .unwrap(); +/// ``` +#[derive(Debug)] +pub struct ManagerBuilder { + params: T, + connection_timeout: Option, + response_timeout: Option, +} + +impl ManagerBuilder { + /// Sets the connection timeout. + /// + /// Pass `Some(duration)` to set a specific timeout, or `None` to + /// disable it. + #[must_use] + pub fn connection_timeout(mut self, timeout: Option) -> Self { + self.connection_timeout = timeout; + self + } + + /// Sets the response timeout. + /// + /// Pass `Some(duration)` to set a specific timeout, or `None` to + /// disable it. + #[must_use] + pub fn response_timeout(mut self, timeout: Option) -> Self { + self.response_timeout = timeout; + self + } + + /// Builds the [`Manager`]. + /// + /// # Errors + /// + /// If establishing a new [`Client`] fails. + pub fn build(self) -> RedisResult { + let connection_config = AsyncConnectionConfig::new() + .set_connection_timeout(self.connection_timeout) + .set_response_timeout(self.response_timeout); + + Ok(Manager { + client: Client::open(self.params)?, connection_config: Some(connection_config), ping_number: AtomicUsize::new(0), }) From 9ba0ff158bf90a56afcd2515cc43bc5285b5eea6 Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Wed, 25 Mar 2026 21:53:40 +0100 Subject: [PATCH 4/5] Replace builder pattern with `ManagerConfig` --- crates/deadpool-redis/src/config.rs | 19 +++-- crates/deadpool-redis/src/lib.rs | 113 +++++++++++++--------------- 2 files changed, 64 insertions(+), 68 deletions(-) diff --git a/crates/deadpool-redis/src/config.rs b/crates/deadpool-redis/src/config.rs index 4f8a70e2..05977aea 100644 --- a/crates/deadpool-redis/src/config.rs +++ b/crates/deadpool-redis/src/config.rs @@ -4,10 +4,10 @@ use redis::RedisError; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -use crate::{CreatePoolError, Pool, PoolBuilder, PoolConfig, RedisResult, Runtime}; - -const DEFAULT_CONNECTION_TIMEOUT: Option = Some(Duration::from_secs(1)); -const DEFAULT_RESPONSE_TIMEOUT: Option = Some(Duration::from_millis(500)); +use crate::{ + CreatePoolError, DEFAULT_CONNECTION_TIMEOUT, DEFAULT_RESPONSE_TIMEOUT, ManagerConfig, Pool, + PoolBuilder, PoolConfig, RedisResult, Runtime, +}; /// Configuration object. /// @@ -104,10 +104,13 @@ impl Config { &self, params: T, ) -> Result { - Ok(crate::Manager::builder(params) - .connection_timeout(self.connection_timeout) - .response_timeout(self.response_timeout) - .build()?) + Ok(crate::Manager::new_with_config( + params, + ManagerConfig { + connection_timeout: self.connection_timeout, + response_timeout: self.response_timeout, + }, + )?) } /// Returns [`deadpool::managed::PoolConfig`] which can be used to construct diff --git a/crates/deadpool-redis/src/lib.rs b/crates/deadpool-redis/src/lib.rs index fe594dbf..da9dc502 100644 --- a/crates/deadpool-redis/src/lib.rs +++ b/crates/deadpool-redis/src/lib.rs @@ -34,6 +34,9 @@ use std::{ time::Duration, }; +const DEFAULT_CONNECTION_TIMEOUT: Option = Some(Duration::from_secs(1)); +const DEFAULT_RESPONSE_TIMEOUT: Option = Some(Duration::from_millis(500)); + use deadpool::managed; use redis::{ AsyncConnectionConfig, Client, IntoConnectionInfo, RedisError, RedisResult, @@ -143,86 +146,76 @@ impl std::fmt::Debug for Manager { } } -impl Manager { - /// Creates a new [`Manager`] from the given `params`. - /// - /// # Errors - /// - /// If establishing a new [`Client`] fails. - pub fn new(params: T) -> RedisResult { - Ok(Self { - client: Client::open(params)?, - connection_config: None, - ping_number: AtomicUsize::new(0), - }) - } - - /// Returns a [`ManagerBuilder`] for the given `params`. - pub fn builder(params: T) -> ManagerBuilder { - ManagerBuilder { - params, - connection_timeout: None, - response_timeout: None, - } - } -} - -/// Builder for [`Manager`]. -/// -/// Use [`Manager::builder`] to create one. +/// Connection configuration for the [`Manager`]. /// /// # Example /// /// ```rust /// use std::time::Duration; -/// use deadpool_redis::Manager; +/// use deadpool_redis::{ManagerConfig, Manager}; /// -/// let manager = Manager::builder("redis://127.0.0.1") -/// .connection_timeout(Some(Duration::from_secs(5))) -/// .response_timeout(None) -/// .build() -/// .unwrap(); +/// let manager = Manager::new_with_config( +/// "redis://127.0.0.1", +/// ManagerConfig { +/// connection_timeout: Some(Duration::from_secs(5)), +/// response_timeout: None, +/// }, +/// ) +/// .unwrap(); /// ``` -#[derive(Debug)] -pub struct ManagerBuilder { - params: T, - connection_timeout: Option, - response_timeout: Option, -} +#[derive(Clone, Debug)] +#[non_exhaustive] +pub struct ManagerConfig { + /// Timeout for establishing a connection. + /// + /// Set to `None` to disable. Defaults to 1 second. + pub connection_timeout: Option, -impl ManagerBuilder { - /// Sets the connection timeout. + /// Timeout for waiting for a response. /// - /// Pass `Some(duration)` to set a specific timeout, or `None` to - /// disable it. - #[must_use] - pub fn connection_timeout(mut self, timeout: Option) -> Self { - self.connection_timeout = timeout; - self + /// Set to `None` to disable. Defaults to 500 milliseconds. + pub response_timeout: Option, +} + +impl Default for ManagerConfig { + fn default() -> Self { + Self { + connection_timeout: DEFAULT_CONNECTION_TIMEOUT, + response_timeout: DEFAULT_RESPONSE_TIMEOUT, + } } +} - /// Sets the response timeout. +impl Manager { + /// Creates a new [`Manager`] from the given `params`. /// - /// Pass `Some(duration)` to set a specific timeout, or `None` to - /// disable it. - #[must_use] - pub fn response_timeout(mut self, timeout: Option) -> Self { - self.response_timeout = timeout; - self + /// # Errors + /// + /// If establishing a new [`Client`] fails. + pub fn new(params: T) -> RedisResult { + Ok(Self { + client: Client::open(params)?, + connection_config: None, + ping_number: AtomicUsize::new(0), + }) } - /// Builds the [`Manager`]. + /// Creates a new [`Manager`] from the given `params` and + /// [`ManagerConfig`]. /// /// # Errors /// /// If establishing a new [`Client`] fails. - pub fn build(self) -> RedisResult { + pub fn new_with_config( + params: T, + config: ManagerConfig, + ) -> RedisResult { let connection_config = AsyncConnectionConfig::new() - .set_connection_timeout(self.connection_timeout) - .set_response_timeout(self.response_timeout); + .set_connection_timeout(config.connection_timeout) + .set_response_timeout(config.response_timeout); - Ok(Manager { - client: Client::open(self.params)?, + Ok(Self { + client: Client::open(params)?, connection_config: Some(connection_config), ping_number: AtomicUsize::new(0), }) From 6475b019fe731668c1c7229ec35d0fa3e83cb362 Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Wed, 25 Mar 2026 23:14:46 +0100 Subject: [PATCH 5/5] Drop `non_exhaustive` --- crates/deadpool-redis/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/deadpool-redis/src/lib.rs b/crates/deadpool-redis/src/lib.rs index da9dc502..3e836c20 100644 --- a/crates/deadpool-redis/src/lib.rs +++ b/crates/deadpool-redis/src/lib.rs @@ -163,8 +163,7 @@ impl std::fmt::Debug for Manager { /// ) /// .unwrap(); /// ``` -#[derive(Clone, Debug)] -#[non_exhaustive] +#[derive(Clone, Copy, Debug)] pub struct ManagerConfig { /// Timeout for establishing a connection. ///