don't sample lost PONGs

3 files changed, 25 insertions, 31 deletions

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<usize>,
     /// 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<f64>,
     /// 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,19 +187,26 @@ 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))
         );