Add reap_all_idle_connections option, fixes djc#123

Adds a new boolean option to Builder named `reap_all_idle_connections`. If true (the default value), the current behavior of the pool is maintained. However, if false, the reaper will not close any idle connections if it results in there being fewer than `min_idle` connections, even if the connections have been idle for longer than `idle_timeout`.

This option would best be disabled if connections are expensive to establish and/or are generally stable over time.

bb8/src/api.rs

@@ -87,6 +87,9 @@ pub struct Builder<M: ManageConnection> {
     pub(crate) max_lifetime: Option<Duration>,
     /// The duration, if any, after which idle_connections in excess of `min_idle` are closed.
     pub(crate) idle_timeout: Option<Duration>,
+    /// Whether or not to reap all connections idle for longer than
+    /// `idle_timeout`, rather than just those in excess of `min_idle`.
+    pub(crate) reap_all_idle_connections: bool,
     /// The duration to wait to start a connection before giving up.
     pub(crate) connection_timeout: Duration,
     /// The error sink.
@@ -106,6 +109,7 @@ impl<M: ManageConnection> Default for Builder<M> {
             test_on_check_out: true,
             max_lifetime: Some(Duration::from_secs(30 * 60)),
             idle_timeout: Some(Duration::from_secs(10 * 60)),
+            reap_all_idle_connections: true,
             connection_timeout: Duration::from_secs(30),
             error_sink: Box::new(NopErrorSink),
             reaper_rate: Duration::from_secs(30),
@@ -185,6 +189,20 @@ impl<M: ManageConnection> Builder<M> {
         self
     }
 
+    /// If true, all connections that have been idle for longer than
+    /// `idle_timeout` will be closed at the next reaping, rather than just
+    /// those in excess of `min_idle`.
+    ///
+    /// If connections are expensive to establish and/or are generally stable
+    /// over time, it may be preferable to disable this so that connections are
+    /// not excessively closed and reopened.
+    ///
+    /// Defaults to true.
+    pub fn reap_all_idle_connections(mut self, reap_all_idle_connections: bool) -> Builder<M> {
+        self.reap_all_idle_connections = reap_all_idle_connections;
+        self
+    }
+
     /// Sets the connection timeout used by the pool.
     ///
     /// Futures returned by `Pool::get` will wait this long before giving up and

bb8/src/internals.rs

@@ -7,6 +7,7 @@ use parking_lot::Mutex;
 
 use crate::api::{Builder, ManageConnection};
 use std::collections::VecDeque;
+use std::convert::TryFrom;
 
 /// The guts of a `Pool`.
 #[allow(missing_debug_implementations)]
@@ -130,19 +131,32 @@ where
     pub(crate) fn reap(&mut self, config: &Builder<M>) -> ApprovalIter {
         let now = Instant::now();
         let before = self.conns.len();
+        let mut after = before;
+        // If `config.min_idle` does not fit in a usize, then saturate to the maximum value of a
+        // usize so that `after <= min_idle` below is always true.
+        let min_idle = config
+            .min_idle
+            .and_then(|min_idle| usize::try_from(min_idle).ok())
+            .unwrap_or(usize::MAX);
 
         self.conns.retain(|conn| {
             let mut keep = true;
-            if let Some(timeout) = config.idle_timeout {
-                keep &= now - conn.idle_start < timeout;
+            if config.reap_all_idle_connections || after > min_idle {
+                if let Some(timeout) = config.idle_timeout {
+                    keep &= now - conn.idle_start < timeout;
+                }
             }
             if let Some(lifetime) = config.max_lifetime {
                 keep &= now - conn.conn.birth < lifetime;
             }
+
+            if !keep {
+                after -= 1;
+            }
             keep
         });
 
-        self.dropped((before - self.conns.len()) as u32, config)
+        self.dropped((before - after) as u32, config)
     }
 
     pub(crate) fn state(&self) -> State {

bb8/tests/test.rs

@@ -507,6 +507,34 @@ async fn test_min_idle() {
     assert_eq!(5, state.connections);
 }
 
+#[tokio::test]
+async fn test_reap_all_idle_connections_off() {
+    static CREATED: AtomicUsize = AtomicUsize::new(0);
+
+    struct Connection;
+
+    impl Default for Connection {
+        fn default() -> Self {
+            CREATED.fetch_add(1, Ordering::SeqCst);
+            Self
+        }
+    }
+
+    let manager = OkManager::<Connection>::new();
+    let _pool = Pool::builder()
+        .min_idle(Some(1))
+        .idle_timeout(Some(Duration::from_millis(10)))
+        .reap_all_idle_connections(false)
+        .reaper_rate(Duration::from_millis(20))
+        .build(manager)
+        .await
+        .unwrap();
+
+    tokio::time::sleep(Duration::from_millis(200)).await;
+
+    assert!(CREATED.load(Ordering::SeqCst) == 1)
+}
+
 #[tokio::test]
 async fn test_conns_drop_on_pool_drop() {
     static DROPPED: AtomicUsize = AtomicUsize::new(0);