From 351a2a6b5812f9ff4fb02f077df96e2793d06394 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 16 Jul 2021 10:59:59 -0400 Subject: [PATCH 1/5] allow tests to specify backgroundThreadIntervalMS --- src/cmap/options.rs | 39 ++++++++++++++++++++++++++++++++++----- src/cmap/test/mod.rs | 6 +----- src/cmap/worker.rs | 8 +++++++- 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/cmap/options.rs b/src/cmap/options.rs index cb43ebdf5..191bf656b 100644 --- a/src/cmap/options.rs +++ b/src/cmap/options.rs @@ -1,7 +1,11 @@ use std::{sync::Arc, time::Duration}; +#[cfg(test)] +use std::cmp::Ordering; use derivative::Derivative; -use serde::Deserialize; +use serde::{Deserialize}; +#[cfg(test)] +use serde::de::{Deserializer, Error}; use typed_builder::TypedBuilder; use crate::{ @@ -40,10 +44,10 @@ pub(crate) struct ConnectionPoolOptions { #[serde(skip)] pub(crate) event_handler: Option>, - /// 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, + #[serde(default, rename = "backgroundThreadIntervalMS", deserialize_with = "BackgroundThreadInterval::deserialize_from_i64_millis")] + pub(crate) background_thread_interval: Option, /// 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,31 @@ impl ConnectionPoolOptions { } } +#[cfg(test)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub(crate) enum BackgroundThreadInterval { + Never, + Every(Duration), +} + +#[cfg(test)] +impl BackgroundThreadInterval { + pub(crate) fn deserialize_from_i64_millis<'de, D>( + deserializer: D, + ) -> std::result::Result, D::Error> + where + D: Deserializer<'de>, + { + let millis = Option::::deserialize(deserializer)?; + let millis = if let Some(m) = millis { m } else { return Ok(None) }; + Ok(Some(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)] diff --git a/src/cmap/test/mod.rs b/src/cmap/test/mod.rs index 3dbce6bb5..26bb3f86d 100644 --- a/src/cmap/test/mod.rs +++ b/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. diff --git a/src/cmap/worker.rs b/src/cmap/worker.rs index f18f604a1..748444c15 100644 --- a/src/cmap/worker.rs +++ b/src/cmap/worker.rs @@ -18,6 +18,8 @@ use super::{ Connection, DEFAULT_MAX_POOL_SIZE, }; +#[cfg(test)] +use super::options::BackgroundThreadInterval; use crate::{ error::{Error, ErrorKind, Result}, event::cmap::{ @@ -177,7 +179,11 @@ 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 { + BackgroundThreadInterval::Never => Duration::MAX, + BackgroundThreadInterval::Every(d) => d, + }) .unwrap_or(MAINTENACE_FREQUENCY); #[cfg(not(test))] From 3267f1e7930924d36ea1afa6dd1f75d3f04470ee Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 16 Jul 2021 11:11:25 -0400 Subject: [PATCH 2/5] update spec tests --- .../README.rst | 17 ++++++++++++++--- .../pool-checkout-no-idle.json | 8 +++++++- .../pool-checkout-no-idle.yml | 4 ++++ .../pool-checkout-no-stale.json | 8 ++++++++ .../pool-checkout-no-stale.yml | 5 +++++ .../pool-clear-min-size.json | 3 ++- .../pool-clear-min-size.yml | 1 + 7 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/test/spec/json/connection-monitoring-and-pooling/README.rst b/src/test/spec/json/connection-monitoring-and-pooling/README.rst index 4c80405e9..9e5d7bebc 100644 --- a/src/test/spec/json/connection-monitoring-and-pooling/README.rst +++ b/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 `__ + and the following test-specific options are allowed: + + - ``backgroundThreadIntervalMS``: A time interval between the end of a + `Background Thread Run `__ + 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) diff --git a/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-idle.json b/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-idle.json index 7e6563228..0b0fe572f 100644 --- a/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-idle.json +++ b/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": [ diff --git a/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-idle.yml b/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-idle.yml index 7aa0bca83..4df351211 100644 --- a/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-idle.yml +++ b/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 diff --git a/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-stale.json b/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-stale.json index fcf20621e..ec76f4e9c 100644 --- a/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-stale.json +++ b/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": [ diff --git a/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-stale.yml b/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-stale.yml index 2d82d1bb0..02d827b77 100644 --- a/src/test/spec/json/connection-monitoring-and-pooling/pool-checkout-no-stale.yml +++ b/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 diff --git a/src/test/spec/json/connection-monitoring-and-pooling/pool-clear-min-size.json b/src/test/spec/json/connection-monitoring-and-pooling/pool-clear-min-size.json index 00c477c62..239df871b 100644 --- a/src/test/spec/json/connection-monitoring-and-pooling/pool-clear-min-size.json +++ b/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": [ { diff --git a/src/test/spec/json/connection-monitoring-and-pooling/pool-clear-min-size.yml b/src/test/spec/json/connection-monitoring-and-pooling/pool-clear-min-size.yml index cb8a9a18d..959c6ccaf 100644 --- a/src/test/spec/json/connection-monitoring-and-pooling/pool-clear-min-size.yml +++ b/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 From 96dcf3dde00e1698c5ab19e6d40a9bab7bbe3bf4 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 16 Jul 2021 12:02:27 -0400 Subject: [PATCH 3/5] use a year rather than Duration::MAX --- src/cmap/worker.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cmap/worker.rs b/src/cmap/worker.rs index 748444c15..27ca72ece 100644 --- a/src/cmap/worker.rs +++ b/src/cmap/worker.rs @@ -181,7 +181,9 @@ impl ConnectionPoolWorker { .as_ref() .and_then(|opts| opts.background_thread_interval) .map(|i| match i { - BackgroundThreadInterval::Never => Duration::MAX, + // 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); From 828391a9243412e39b41263ac1e3a7fb358a60e5 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 16 Jul 2021 12:03:42 -0400 Subject: [PATCH 4/5] rustfmt --- src/cmap/options.rs | 20 +++++++++++++++----- src/cmap/worker.rs | 4 ++-- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/cmap/options.rs b/src/cmap/options.rs index 191bf656b..f070b093e 100644 --- a/src/cmap/options.rs +++ b/src/cmap/options.rs @@ -1,11 +1,11 @@ -use std::{sync::Arc, time::Duration}; #[cfg(test)] use std::cmp::Ordering; +use std::{sync::Arc, time::Duration}; use derivative::Derivative; -use serde::{Deserialize}; #[cfg(test)] use serde::de::{Deserializer, Error}; +use serde::Deserialize; use typed_builder::TypedBuilder; use crate::{ @@ -46,7 +46,11 @@ pub(crate) struct ConnectionPoolOptions { /// Interval between background thread maintenance runs (e.g. ensure minPoolSize). #[cfg(test)] - #[serde(default, rename = "backgroundThreadIntervalMS", deserialize_with = "BackgroundThreadInterval::deserialize_from_i64_millis")] + #[serde( + default, + rename = "backgroundThreadIntervalMS", + deserialize_with = "BackgroundThreadInterval::deserialize_from_i64_millis" + )] pub(crate) background_thread_interval: Option, /// Connections that have been ready for usage in the pool for longer than `max_idle_time` will @@ -136,11 +140,17 @@ impl BackgroundThreadInterval { D: Deserializer<'de>, { let millis = Option::::deserialize(deserializer)?; - let millis = if let Some(m) = millis { m } else { return Ok(None) }; + let millis = if let Some(m) = millis { + m + } else { + return Ok(None); + }; Ok(Some(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)), + Ordering::Greater => { + BackgroundThreadInterval::Every(Duration::from_millis(millis as u64)) + } })) } } diff --git a/src/cmap/worker.rs b/src/cmap/worker.rs index 27ca72ece..18dcee204 100644 --- a/src/cmap/worker.rs +++ b/src/cmap/worker.rs @@ -1,5 +1,7 @@ use derivative::Derivative; +#[cfg(test)] +use super::options::BackgroundThreadInterval; use super::{ conn::PendingConnection, connection_requester, @@ -18,8 +20,6 @@ use super::{ Connection, DEFAULT_MAX_POOL_SIZE, }; -#[cfg(test)] -use super::options::BackgroundThreadInterval; use crate::{ error::{Error, ErrorKind, Result}, event::cmap::{ From 053ad1be30b7f8913ee0ce852b2e36e80a566cc0 Mon Sep 17 00:00:00 2001 From: Abraham Egnor Date: Fri, 16 Jul 2021 12:12:32 -0400 Subject: [PATCH 5/5] directly implement Deserialize --- src/cmap/options.rs | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/src/cmap/options.rs b/src/cmap/options.rs index f070b093e..141c50ac2 100644 --- a/src/cmap/options.rs +++ b/src/cmap/options.rs @@ -46,11 +46,7 @@ pub(crate) struct ConnectionPoolOptions { /// Interval between background thread maintenance runs (e.g. ensure minPoolSize). #[cfg(test)] - #[serde( - default, - rename = "backgroundThreadIntervalMS", - deserialize_with = "BackgroundThreadInterval::deserialize_from_i64_millis" - )] + #[serde(rename = "backgroundThreadIntervalMS")] pub(crate) background_thread_interval: Option, /// Connections that have been ready for usage in the pool for longer than `max_idle_time` will @@ -132,26 +128,19 @@ pub(crate) enum BackgroundThreadInterval { } #[cfg(test)] -impl BackgroundThreadInterval { - pub(crate) fn deserialize_from_i64_millis<'de, D>( - deserializer: D, - ) -> std::result::Result, D::Error> +impl<'de> Deserialize<'de> for BackgroundThreadInterval { + fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, { - let millis = Option::::deserialize(deserializer)?; - let millis = if let Some(m) = millis { - m - } else { - return Ok(None); - }; - Ok(Some(match millis.cmp(&0) { + 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)) } - })) + }) } }