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

@@ -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,38 +872,69 @@ 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<D>(&mut self, val: D) -> &mut Self
+    where
+        D: Into<Option<Duration>>,
+    {
+        self.pool_idle_timeout(val)
     }
 
     /// Set an optional timeout for idle sockets being kept-alive.
     ///
     /// Pass `None` to disable timeout.
     ///
     /// Default is 90 seconds.
-    #[inline]
-    pub fn keep_alive_timeout<D>(&mut self, val: D) -> &mut Self
+    pub fn pool_idle_timeout<D>(&mut self, val: D) -> &mut Self
     where
         D: Into<Option<Duration>>,
     {
-        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)
     }

src/client/pool.rs

@@ -88,14 +88,19 @@ struct WeakOpt<T>(Option<Weak<T>>);
 
 #[derive(Clone, Copy, Debug)]
 pub(super) struct Config {
-    pub(super) enabled: bool,
-    pub(super) keep_alive_timeout: Option<Duration>,
+    pub(super) idle_timeout: Option<Duration>,
     pub(super) max_idle_per_host: usize,
 }
 
+impl Config {
+    pub(super) fn is_enabled(&self) -> bool {
+        self.max_idle_per_host > 0
+    }
+}
+
 impl<T> Pool<T> {
     pub fn new(config: Config, __exec: &Exec) -> Pool<T> {
-        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<T> Pool<T> {
                 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<T>(max_idle: usize) -> Pool<T> {
         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,

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))

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<Output = ()>) {
     let client = Client::builder()
-        .keep_alive_timeout(Duration::from_secs(10))
         .http2_only(cfg.version == 2)
         .build_http::<Body>();