RUST-774 Allow tests to specify backgroundThreadIntervalMS to fix a race condition. (#396)

Author: abr-egn

src/cmap/options.rs

@@ -1,6 +1,10 @@
+#[cfg(test)]
+use std::cmp::Ordering;
 use std::{sync::Arc, time::Duration};
 
 use derivative::Derivative;
+#[cfg(test)]
+use serde::de::{Deserializer, Error};
 use serde::Deserialize;
 use typed_builder::TypedBuilder;
 
@@ -40,10 +44,10 @@ pub(crate) struct ConnectionPoolOptions {
     #[serde(skip)]
     pub(crate) event_handler: Option<Arc<dyn CmapEventHandler>>,
 
-    /// How often the background thread performs its maintenance (e.g. ensure minPoolSize).
+    /// Interval between background thread maintenance runs (e.g. ensure minPoolSize).
     #[cfg(test)]
-    #[serde(skip)]
-    pub(crate) maintenance_frequency: Option<Duration>,
+    #[serde(rename = "backgroundThreadIntervalMS")]
+    pub(crate) background_thread_interval: Option<BackgroundThreadInterval>,
 
     /// Connections that have been ready for usage in the pool for longer than `max_idle_time` will
     /// not be used.
@@ -101,7 +105,7 @@ impl ConnectionPoolOptions {
             credential: options.credential.clone(),
             event_handler: options.cmap_event_handler.clone(),
             #[cfg(test)]
-            maintenance_frequency: None,
+            background_thread_interval: None,
             #[cfg(test)]
             ready: None,
         }
@@ -116,6 +120,30 @@ impl ConnectionPoolOptions {
     }
 }
 
+#[cfg(test)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub(crate) enum BackgroundThreadInterval {
+    Never,
+    Every(Duration),
+}
+
+#[cfg(test)]
+impl<'de> Deserialize<'de> for BackgroundThreadInterval {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let millis = i64::deserialize(deserializer)?;
+        Ok(match millis.cmp(&0) {
+            Ordering::Less => BackgroundThreadInterval::Never,
+            Ordering::Equal => return Err(D::Error::custom("zero is not allowed")),
+            Ordering::Greater => {
+                BackgroundThreadInterval::Every(Duration::from_millis(millis as u64))
+            }
+        })
+    }
+}
+
 /// Options used for constructing a `Connection`.
 #[derive(Derivative)]
 #[derivative(Debug)]

src/cmap/test/mod.rs

@@ -150,17 +150,13 @@ impl Executor {
     async fn execute_test(self) {
         let mut subscriber = self.state.handler.subscribe();
 
-        // CMAP spec requires setting this to 50ms.
-        let mut options = self.pool_options;
-        options.maintenance_frequency = Some(Duration::from_millis(50));
-
         let (update_sender, mut update_receiver) = ServerUpdateSender::channel();
 
         let pool = ConnectionPool::new(
             CLIENT_OPTIONS.hosts[0].clone(),
             Default::default(),
             update_sender,
-            Some(options),
+            Some(self.pool_options),
         );
 
         // Mock a monitoring task responding to errors reported by the pool.

src/cmap/worker.rs

@@ -1,5 +1,7 @@
 use derivative::Derivative;
 
+#[cfg(test)]
+use super::options::BackgroundThreadInterval;
 use super::{
     conn::PendingConnection,
     connection_requester,
@@ -177,7 +179,13 @@ impl ConnectionPoolWorker {
         #[cfg(test)]
         let maintenance_frequency = options
             .as_ref()
-            .and_then(|opts| opts.maintenance_frequency)
+            .and_then(|opts| opts.background_thread_interval)
+            .map(|i| match i {
+                // One year is long enough to count as never for tests, but not so long that it
+                // will overflow interval math.
+                BackgroundThreadInterval::Never => Duration::from_secs(31_556_952),
+                BackgroundThreadInterval::Every(d) => d,
+            })
             .unwrap_or(MAINTENACE_FREQUENCY);
 
         #[cfg(not(test))]

src/test/spec/json/connection-monitoring-and-pooling/README.rst

@@ -36,7 +36,20 @@ Unit Test Format:
 
 All Unit Tests have some of the following fields:
 
-- ``poolOptions``: if present, connection pool options to use when creating a pool
+- ``poolOptions``: If present, connection pool options to use when creating a pool;
+  both `standard ConnectionPoolOptions <https://github.com/mongodb/specifications/blob/master/source/connection-monitoring-and-pooling/connection-monitoring-and-pooling.rst#connection-pool-options-1>`__
+  and the following test-specific options are allowed:
+
+  - ``backgroundThreadIntervalMS``: A time interval between the end of a
+    `Background Thread Run <https://github.com/mongodb/specifications/blob/master/source/connection-monitoring-and-pooling/connection-monitoring-and-pooling.rst#background-thread>`__
+    and the beginning of the next Run. If a Connection Pool does not implement a Background Thread, the Test Runner MUST ignore the option.
+    If the option is not specified, an implementation is free to use any value it finds reasonable.
+
+    Possible values (0 is not allowed):
+
+    - A negative value: never begin a Run.
+    - A positive value: the interval between Runs in milliseconds.
+
 - ``operations``: A list of operations to perform. All operations support the following fields:
 
   - ``name``: A string describing which operation to issue.
@@ -155,8 +168,6 @@ For each YAML file with ``style: unit``:
 
   - If ``poolOptions`` is specified, use those options to initialize both pools
   - The returned pool must have an ``address`` set as a string value.
-  - If the pool uses a background thread to satisfy ``minPoolSize``, ensure it
-    attempts to create a new connection every 50ms.
 
 - Process each ``operation`` in ``operations`` (on the main thread)
 

src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-idle.json

@@ -3,7 +3,8 @@
   "style": "unit",
   "description": "must destroy and must not check out an idle connection if found while iterating available connections",
   "poolOptions": {
-    "maxIdleTimeMS": 10
+    "maxIdleTimeMS": 10,
+    "backgroundThreadIntervalMS": -1
   },
   "operations": [
     {
@@ -23,6 +24,11 @@
     },
     {
       "name": "checkOut"
+    },
+    {
+      "name": "waitForEvent",
+      "event": "ConnectionCheckedOut",
+      "count": 2
     }
   ],
   "events": [

src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-idle.yml

@@ -3,6 +3,7 @@ style: unit
 description: must destroy and must not check out an idle connection if found while iterating available connections
 poolOptions:
   maxIdleTimeMS: 10
+  backgroundThreadIntervalMS: -1
 operations:
   - name: ready
   - name: checkOut
@@ -12,6 +13,9 @@ operations:
   - name: wait
     ms: 50
   - name: checkOut
+  - name: waitForEvent
+    event: ConnectionCheckedOut
+    count: 2
 events:
   - type: ConnectionPoolCreated
     address: 42

src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-stale.json

@@ -2,6 +2,9 @@
   "version": 1,
   "style": "unit",
   "description": "must destroy and must not check out a stale connection if found while iterating available connections",
+  "poolOptions": {
+    "backgroundThreadIntervalMS": -1
+  },
   "operations": [
     {
       "name": "ready"
@@ -22,6 +25,11 @@
     },
     {
       "name": "checkOut"
+    },
+    {
+      "name": "waitForEvent",
+      "event": "ConnectionCheckedOut",
+      "count": 2
     }
   ],
   "events": [

src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-stale.yml

@@ -1,6 +1,8 @@
 version: 1
 style: unit
 description: must destroy and must not check out a stale connection if found while iterating available connections
+poolOptions:
+  backgroundThreadIntervalMS: -1
 operations:
   - name: ready
   - name: checkOut
@@ -10,6 +12,9 @@ operations:
   - name: clear
   - name: ready
   - name: checkOut
+  - name: waitForEvent
+    event: ConnectionCheckedOut
+    count: 2
 events:
   - type: ConnectionPoolCreated
     address: 42

src/test/spec/json/connection-monitoring-and-pooling/pool-clear-min-size.json

@@ -3,7 +3,8 @@
   "style": "unit",
   "description": "pool clear halts background minPoolSize establishments",
   "poolOptions": {
-    "minPoolSize": 1
+    "minPoolSize": 1,
+    "backgroundThreadIntervalMS": 50
   },
   "operations": [
     {

src/test/spec/json/connection-monitoring-and-pooling/pool-clear-min-size.yml

@@ -3,6 +3,7 @@ style: unit
 description: pool clear halts background minPoolSize establishments
 poolOptions:
   minPoolSize: 1
+  backgroundThreadIntervalMS: 50
 operations:
   - name: ready
   - name: waitForEvent