Skip to content

Commit c3cf9cc

Browse files
committed
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.
1 parent 75c51d9 commit c3cf9cc

File tree

3 files changed

+34
-9
lines changed

3 files changed

+34
-9
lines changed

src/conn/pool/metrics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ impl MetricsHistogram {
6868
#[cfg(feature = "hdrhistogram")]
6969
impl Default for MetricsHistogram {
7070
fn default() -> Self {
71-
let hdr = hdrhistogram::Histogram::new_with_bounds(1, 30 * 1_000_000, 2).unwrap();
71+
let hdr = hdrhistogram::Histogram::new(2).unwrap();
7272
Self(std::sync::Mutex::new(hdr))
7373
}
7474
}

src/conn/pool/mod.rs

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,8 @@ impl Pool {
359359
.connection_idle_duration
360360
.lock()
361361
.unwrap()
362-
.saturating_record(since.elapsed().as_micros() as u64);
362+
.record(since.elapsed().as_micros() as u64)
363+
.ok();
363364
#[cfg(feature = "hdrhistogram")]
364365
let metrics = self.metrics();
365366
conn.inner.active_since = Instant::now();
@@ -371,9 +372,8 @@ impl Pool {
371372
.check_duration
372373
.lock()
373374
.unwrap()
374-
.saturating_record(
375-
conn.inner.active_since.elapsed().as_micros() as u64
376-
);
375+
.record(conn.inner.active_since.elapsed().as_micros() as u64)
376+
.ok();
377377
Ok(conn)
378378
}
379379
.boxed(),
@@ -412,9 +412,8 @@ impl Pool {
412412
.connect_duration
413413
.lock()
414414
.unwrap()
415-
.saturating_record(
416-
conn.inner.active_since.elapsed().as_micros() as u64
417-
);
415+
.record(conn.inner.active_since.elapsed().as_micros() as u64)
416+
.ok();
418417
}
419418
conn
420419
}
@@ -1244,6 +1243,31 @@ mod test {
12441243
Ok(())
12451244
}
12461245

1246+
#[cfg(feature = "hdrhistogram")]
1247+
#[tokio::test]
1248+
async fn metrics() -> super::Result<()> {
1249+
let pool = pool_with_one_connection();
1250+
1251+
let metrics = pool.metrics();
1252+
let conn = pool.get_conn().await.unwrap();
1253+
tokio::time::sleep(Duration::from_millis(100)).await;
1254+
drop(conn);
1255+
pool.get_conn().await.unwrap();
1256+
1257+
let metrics = serde_json::to_value(&*metrics).unwrap();
1258+
let connection_active_duration = metrics
1259+
.get("connection_active_duration")
1260+
.unwrap()
1261+
.get("max")
1262+
.unwrap()
1263+
.as_u64()
1264+
.unwrap();
1265+
// We slept for 100 miliseconds holding a conneciton.
1266+
assert!(connection_active_duration > 100_000);
1267+
1268+
Ok(())
1269+
}
1270+
12471271
#[cfg(feature = "nightly")]
12481272
mod bench {
12491273
use futures_util::future::{FutureExt, TryFutureExt};

src/conn/pool/recycler.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ impl Future for Recycler {
8686
.connection_active_duration
8787
.lock()
8888
.unwrap()
89-
.saturating_record($conn.inner.active_since.elapsed().as_micros() as u64);
89+
.record($conn.inner.active_since.elapsed().as_micros() as u64)
90+
.ok();
9091
exchange.available.push_back($conn.into());
9192
$self
9293
.inner

0 commit comments

Comments
 (0)