From ac0882aa0be779cd14eb87aef57916e7335794d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20=C3=81vila=20de=20Esp=C3=ADndola?= Date: Wed, 23 Jul 2025 17:30:37 +0000 Subject: [PATCH] Don't use an arbitrary bound in histograms I hit this trying to figure out how an mostly idle server was reporting that no connection was idle for more than 30s. --- src/conn/pool/metrics.rs | 2 +- src/conn/pool/mod.rs | 31 ++++++++++++++++++++++++------- src/conn/pool/recycler.rs | 3 ++- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/conn/pool/metrics.rs b/src/conn/pool/metrics.rs index c5c6a08..91cd1fd 100644 --- a/src/conn/pool/metrics.rs +++ b/src/conn/pool/metrics.rs @@ -68,7 +68,7 @@ impl MetricsHistogram { #[cfg(feature = "hdrhistogram")] impl Default for MetricsHistogram { fn default() -> Self { - let hdr = hdrhistogram::Histogram::new_with_bounds(1, 30 * 1_000_000, 2).unwrap(); + let hdr = hdrhistogram::Histogram::new(2).unwrap(); Self(std::sync::Mutex::new(hdr)) } } diff --git a/src/conn/pool/mod.rs b/src/conn/pool/mod.rs index 53688d9..38acf3e 100644 --- a/src/conn/pool/mod.rs +++ b/src/conn/pool/mod.rs @@ -359,7 +359,8 @@ impl Pool { .connection_idle_duration .lock() .unwrap() - .saturating_record(since.elapsed().as_micros() as u64); + .record(since.elapsed().as_micros() as u64) + .ok(); #[cfg(feature = "hdrhistogram")] let metrics = self.metrics(); conn.inner.active_since = Instant::now(); @@ -371,9 +372,8 @@ impl Pool { .check_duration .lock() .unwrap() - .saturating_record( - conn.inner.active_since.elapsed().as_micros() as u64 - ); + .record(conn.inner.active_since.elapsed().as_micros() as u64) + .ok(); Ok(conn) } .boxed(), @@ -412,9 +412,8 @@ impl Pool { .connect_duration .lock() .unwrap() - .saturating_record( - conn.inner.active_since.elapsed().as_micros() as u64 - ); + .record(conn.inner.active_since.elapsed().as_micros() as u64) + .ok(); } conn } @@ -1244,6 +1243,24 @@ mod test { Ok(()) } + #[cfg(feature = "hdrhistogram")] + #[tokio::test] + async fn metrics() -> super::Result<()> { + let pool = pool_with_one_connection(); + + let metrics = pool.metrics(); + let conn = pool.get_conn().await.unwrap(); + tokio::time::sleep(Duration::from_millis(100)).await; + drop(conn); + pool.get_conn().await.unwrap(); + + let max = metrics.connection_active_duration.lock().unwrap().max(); + // We slept for 100 miliseconds holding a conneciton. + assert!(max > 100_000); + + Ok(()) + } + #[cfg(feature = "nightly")] mod bench { use futures_util::future::{FutureExt, TryFutureExt}; diff --git a/src/conn/pool/recycler.rs b/src/conn/pool/recycler.rs index 2809dc0..ac886fa 100644 --- a/src/conn/pool/recycler.rs +++ b/src/conn/pool/recycler.rs @@ -86,7 +86,8 @@ impl Future for Recycler { .connection_active_duration .lock() .unwrap() - .saturating_record($conn.inner.active_since.elapsed().as_micros() as u64); + .record($conn.inner.active_since.elapsed().as_micros() as u64) + .ok(); exchange.available.push_back($conn.into()); $self .inner