From a82fd6c94aa4ce11fe685f9ccfb85c596d596c6e Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Thu, 27 Feb 2020 14:25:06 -0800 Subject: [PATCH] feat(client): rename `client::Builder` pool options (#2142) - Renamed `keep_alive_timeout` to `pool_idle_timeout`. - Renamed `max_idle_per_host` to `pool_max_idle_per_host`. - Deprecated `keep_alive(bool)` due to confusing name. To disable the connection pool, call `pool_max_idle_per_host(0)`. --- src/client/mod.rs | 76 ++++++++++++++++++++++++++++---------------- src/client/pool.rs | 19 ++++++----- tests/client.rs | 10 ++---- tests/support/mod.rs | 3 -- 4 files changed, 63 insertions(+), 45 deletions(-) diff --git a/src/client/mod.rs b/src/client/mod.rs index a9f7ab0666..f1eaa1643b 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -129,10 +129,11 @@ impl Client<(), Body> { /// ``` /// # #[cfg(feature = "runtime")] /// # fn run () { + /// use std::time::Duration; /// use hyper::Client; /// /// let client = Client::builder() - /// .keep_alive(true) + /// .pool_idle_timeout(Duration::from_secs(30)) /// .http2_only(true) /// .build_http(); /// # let infer: Client<_, hyper::Body> = client; @@ -842,10 +843,11 @@ fn set_scheme(uri: &mut Uri, scheme: Scheme) { /// ``` /// # #[cfg(feature = "runtime")] /// # fn run () { +/// use std::time::Duration; /// use hyper::Client; /// /// let client = Client::builder() -/// .keep_alive(true) +/// .pool_idle_timeout(Duration::from_secs(30)) /// .http2_only(true) /// .build_http(); /// # let infer: Client<_, hyper::Body> = client; @@ -870,22 +872,38 @@ impl Default for Builder { }, conn_builder: conn::Builder::new(), pool_config: pool::Config { - enabled: true, - keep_alive_timeout: Some(Duration::from_secs(90)), - max_idle_per_host: ::std::usize::MAX, + idle_timeout: Some(Duration::from_secs(90)), + max_idle_per_host: std::usize::MAX, }, } } } impl Builder { - /// Enable or disable keep-alive mechanics. - /// - /// Default is enabled. - #[inline] + #[doc(hidden)] + #[deprecated( + note = "name is confusing, to disable the connection pool, call pool_max_idle_per_host(0)" + )] pub fn keep_alive(&mut self, val: bool) -> &mut Self { - self.pool_config.enabled = val; - self + if !val { + // disable + self.pool_max_idle_per_host(0) + } else if self.pool_config.max_idle_per_host == 0 { + // enable + self.pool_max_idle_per_host(std::usize::MAX) + } else { + // already enabled + self + } + } + + #[doc(hidden)] + #[deprecated(note = "renamed to `pool_idle_timeout`")] + pub fn keep_alive_timeout(&mut self, val: D) -> &mut Self + where + D: Into>, + { + self.pool_idle_timeout(val) } /// Set an optional timeout for idle sockets being kept-alive. @@ -893,15 +911,30 @@ impl Builder { /// Pass `None` to disable timeout. /// /// Default is 90 seconds. - #[inline] - pub fn keep_alive_timeout(&mut self, val: D) -> &mut Self + pub fn pool_idle_timeout(&mut self, val: D) -> &mut Self where D: Into>, { - self.pool_config.keep_alive_timeout = val.into(); + self.pool_config.idle_timeout = val.into(); self } + #[doc(hidden)] + #[deprecated(note = "renamed to `pool_max_idle_per_host`")] + pub fn max_idle_per_host(&mut self, max_idle: usize) -> &mut Self { + self.pool_config.max_idle_per_host = max_idle; + self + } + + /// Sets the maximum idle connection per host allowed in the pool. + /// + /// Default is `usize::MAX` (no limit). + pub fn pool_max_idle_per_host(&mut self, max_idle: usize) -> &mut Self { + self.pool_config.max_idle_per_host = max_idle; + self + } + // HTTP/1 options + /// Set whether HTTP/1 connections should try to use vectored writes, /// or always flatten into a single buffer. /// @@ -910,7 +943,6 @@ impl Builder { /// support vectored writes well, such as most TLS implementations. /// /// Default is `true`. - #[inline] pub fn http1_writev(&mut self, val: bool) -> &mut Self { self.conn_builder.h1_writev(val); self @@ -921,7 +953,6 @@ impl Builder { /// Note that setting this option unsets the `http1_max_buf_size` option. /// /// Default is an adaptive read buffer. - #[inline] pub fn http1_read_buf_exact_size(&mut self, sz: usize) -> &mut Self { self.conn_builder.h1_read_buf_exact_size(Some(sz)); self @@ -936,7 +967,6 @@ impl Builder { /// # Panics /// /// The minimum value allowed is 8192. This method panics if the passed `max` is less than the minimum. - #[inline] pub fn http1_max_buf_size(&mut self, max: usize) -> &mut Self { self.conn_builder.h1_max_buf_size(max); self @@ -1006,14 +1036,6 @@ impl Builder { self } - /// Sets the maximum idle connection per host allowed in the pool. - /// - /// Default is `usize::MAX` (no limit). - pub fn max_idle_per_host(&mut self, max_idle: usize) -> &mut Self { - self.pool_config.max_idle_per_host = max_idle; - self - } - /// Set whether to retry requests that get disrupted before ever starting /// to write. /// @@ -1060,8 +1082,8 @@ impl Builder { B::Data: Send, { let mut connector = HttpConnector::new(); - if self.pool_config.enabled { - connector.set_keepalive(self.pool_config.keep_alive_timeout); + if self.pool_config.is_enabled() { + connector.set_keepalive(self.pool_config.idle_timeout); } self.build(connector) } diff --git a/src/client/pool.rs b/src/client/pool.rs index 951a840872..8c1ee24c0d 100644 --- a/src/client/pool.rs +++ b/src/client/pool.rs @@ -88,14 +88,19 @@ struct WeakOpt(Option>); #[derive(Clone, Copy, Debug)] pub(super) struct Config { - pub(super) enabled: bool, - pub(super) keep_alive_timeout: Option, + pub(super) idle_timeout: Option, pub(super) max_idle_per_host: usize, } +impl Config { + pub(super) fn is_enabled(&self) -> bool { + self.max_idle_per_host > 0 + } +} + impl Pool { pub fn new(config: Config, __exec: &Exec) -> Pool { - let inner = if config.enabled { + let inner = if config.is_enabled() { Some(Arc::new(Mutex::new(PoolInner { connecting: HashSet::new(), idle: HashMap::new(), @@ -105,7 +110,7 @@ impl Pool { waiters: HashMap::new(), #[cfg(feature = "runtime")] exec: __exec.clone(), - timeout: config.keep_alive_timeout, + timeout: config.idle_timeout, }))) } else { None @@ -797,8 +802,7 @@ mod tests { fn pool_max_idle_no_timer(max_idle: usize) -> Pool { let pool = Pool::new( super::Config { - enabled: true, - keep_alive_timeout: Some(Duration::from_millis(100)), + idle_timeout: Some(Duration::from_millis(100)), max_idle_per_host: max_idle, }, &Exec::Default, @@ -900,8 +904,7 @@ mod tests { let pool = Pool::new( super::Config { - enabled: true, - keep_alive_timeout: Some(Duration::from_millis(10)), + idle_timeout: Some(Duration::from_millis(10)), max_idle_per_host: std::usize::MAX, }, &Exec::Default, diff --git a/tests/client.rs b/tests/client.rs index 719003fdf8..0c815ba053 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -1282,13 +1282,9 @@ mod dispatch_impl { let _ = rx2.recv(); }); - let client = - Client::builder() - .keep_alive(false) - .build(DebugConnector::with_http_and_closes( - HttpConnector::new(), - closes_tx, - )); + let client = Client::builder().pool_max_idle_per_host(0).build( + DebugConnector::with_http_and_closes(HttpConnector::new(), closes_tx), + ); let req = Request::builder() .uri(&*format!("http://{}/a", addr)) diff --git a/tests/support/mod.rs b/tests/support/mod.rs index a0c02e03fc..fd089bc561 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -4,7 +4,6 @@ use std::sync::{ atomic::{AtomicUsize, Ordering}, Arc, Mutex, }; -use std::time::Duration; use hyper::client::HttpConnector; use hyper::service::{make_service_fn, service_fn}; @@ -326,7 +325,6 @@ async fn async_test(cfg: __TestConfig) { let connector = HttpConnector::new(); let client = Client::builder() - .keep_alive_timeout(Duration::from_secs(10)) .http2_only(cfg.client_version == 2) .build::<_, Body>(connector); @@ -450,7 +448,6 @@ struct ProxyConfig { fn naive_proxy(cfg: ProxyConfig) -> (SocketAddr, impl Future) { let client = Client::builder() - .keep_alive_timeout(Duration::from_secs(10)) .http2_only(cfg.version == 2) .build_http::();