From 614e57396d11700721660d54f3d90a4f01cfae7d Mon Sep 17 00:00:00 2001 From: tv Date: Wed, 28 Oct 2026 03:13:35 +0100 Subject: don't sample lost PONGs --- docs/openapi.yaml | 2 +- src/api/peers.rs | 2 +- src/api/stats/prometheus.rs | 2 +- src/swarm/rtt.rs | 52 ++++++++++++++++++++------------------------- 4 files changed, 26 insertions(+), 32 deletions(-) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 8919ca6..15e9974 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -1186,7 +1186,7 @@ components: description: We keep it alive and sync with it on our own; other peers we only answer. rttMs: type: [number, 'null'] - description: Effective round-trip time (smoothed, raised by unanswered requests); null without a recent sample. + description: Effective round-trip time (smoothed, raised by an unanswered HELLO); null without a recent sample. traffic: $ref: '#/components/schemas/Traffic' incidents: diff --git a/src/api/peers.rs b/src/api/peers.rs index ac73296..f2281c2 100644 --- a/src/api/peers.rs +++ b/src/api/peers.rs @@ -69,7 +69,7 @@ pub(crate) struct PeerStatus { heads: Option, /// We keep it alive and sync with it on our own; other peers we only answer. neighbour: bool, - /// Effective round-trip time in milliseconds (smoothed, raised by requests still + /// Effective round-trip time in milliseconds (smoothed, raised by a HELLO still /// unanswered); absent without a recent sample. rtt_ms: Option, /// Messages and payload bytes exchanged with it, by kind, since we started (or diff --git a/src/api/stats/prometheus.rs b/src/api/stats/prometheus.rs index 9a4f162..911228c 100644 --- a/src/api/stats/prometheus.rs +++ b/src/api/stats/prometheus.rs @@ -808,7 +808,7 @@ pub(super) async fn render(state: &AppState, stats: &StatsResponse) -> String { out.family( "peer_rtt_seconds", "gauge", - "Effective round-trip time (smoothed, raised by unanswered requests).", + "Effective round-trip time (smoothed, raised by an unanswered HELLO).", ); for peer in &peers { if let Some(rtt) = peer.rtt_secs { diff --git a/src/swarm/rtt.rs b/src/swarm/rtt.rs index 466fc98..04d7c8d 100644 --- a/src/swarm/rtt.rs +++ b/src/swarm/rtt.rs @@ -8,10 +8,12 @@ //! (exponential moving average, weight 1/4) to ride out jitter. //! //! What we use is the *effective* round-trip time: the smoothed value, raised to the age -//! of the oldest request still unanswered (the round trip takes at least that long), so -//! a peer that stops answering drifts away by itself. Without a recent sample (nodes -//! move; an old value says little) it is unknown. Silence alone doesn't count: we don't -//! ask non-neighbours anything, so their quiet is ours. +//! of a HELLO still unanswered (the round trip takes at least that long), so a peer that +//! stops answering its handshake drifts away by itself. An unanswered PING isn't: a lost +//! PING or PONG is no sample, not a slow one (the next keepalive measures again, and a +//! peer that is gone goes dormant). Without a recent sample (nodes move; an old value +//! says little) it is unknown. Silence alone doesn't count: we don't ask non-neighbours +//! anything, so their quiet is ours. use std::collections::HashMap; use std::time::Duration; @@ -30,8 +32,6 @@ pub(crate) struct RttTable { struct Ping { nonce: u64, sent: Instant, - /// When the oldest PING still unanswered went out (an earlier one, replaced). - waiting_since: Instant, } #[derive(Debug, Clone, Copy)] @@ -47,19 +47,10 @@ struct Outstanding { } impl RttTable { + /// A PING to `peer`: it replaces one still unanswered. pub(crate) fn ping_sent(&mut self, peer: &str, nonce: u64, now: Instant) { - let waiting_since = self - .pings - .get(peer) - .map_or(now, |earlier| earlier.waiting_since); - self.pings.insert( - peer.to_owned(), - Ping { - nonce, - sent: now, - waiting_since, - }, - ); + self.pings + .insert(peer.to_owned(), Ping { nonce, sent: now }); } pub(crate) fn pong_received(&mut self, peer: &str, nonce: u64, now: Instant) { @@ -92,15 +83,11 @@ impl RttTable { .smoothed .get(peer) .filter(|sample| now.saturating_duration_since(sample.at) < fresh_for)?; - let waiting = [ - self.pings.get(peer).map(|ping| ping.waiting_since), - self.hellos.get(peer).map(|outstanding| outstanding.sent), - ] - .into_iter() - .flatten() - .map(|sent| now.saturating_duration_since(sent)) - .max() - .unwrap_or_default(); + let waiting = self + .hellos + .get(peer) + .map(|outstanding| now.saturating_duration_since(outstanding.sent)) + .unwrap_or_default(); Some(sample.rtt.max(waiting)) } @@ -200,17 +187,24 @@ mod tests { } #[test] - fn unanswered_requests_raise_it_and_old_samples_expire() { + fn unanswered_hellos_raise_it_and_old_samples_expire() { let mut table = RttTable::default(); let start = Instant::now(); table.ping_sent("a", 1, start); table.pong_received("a", 1, start + Duration::from_millis(50)); - // PINGs unanswered for 40 s: at least 40 s, from the oldest. + // PINGs unanswered (lost): no sample, nothing raised. let sent = start + Duration::from_secs(60); table.ping_sent("a", 2, sent); table.ping_sent("a", 3, sent + Duration::from_secs(30)); let later = sent + Duration::from_secs(40); + assert_eq!( + table.effective("a", later, FRESH), + Some(Duration::from_millis(50)) + ); + + // A HELLO unanswered for 40 s: at least 40 s. + table.hello_sent("a", sent); assert_eq!( table.effective("a", later, FRESH), Some(Duration::from_secs(40)) -- cgit v1.3.1