looking at paths doesn't change them

1 files changed, 83 insertions, 28 deletions

diff --git a/src/transport/paths.rs b/src/transport/paths.rs
index b9dfc96..2e7ef76 100644
--- a/src/transport/paths.rs
+++ b/src/transport/paths.rs
@@ -147,19 +147,28 @@ struct Table {
 
 impl Table {
     /// `usable`, with each one's place in the configured order, best first (as
-    /// `Paths::rank` says), noting the choice.
-    fn rank(&mut self, mut usable: Vec<(String, usize)>, now: Instant) -> Vec<String> {
+    /// `Paths::rank` says). With `sending`, for a message about to go: they count as
+    /// used (`ACTIVE_FOR`), and the one in use may change for a faster one. Without, a
+    /// look at the choice as it stands, changing nothing (`/peers`, figures, LINKS).
+    fn rank(
+        &mut self,
+        mut usable: Vec<(String, usize)>,
+        now: Instant,
+        sending: bool,
+    ) -> Vec<String> {
         usable.sort_by_key(|(_, preference)| *preference);
         usable.dedup_by(|a, b| a.0 == b.0);
         if usable.len() < 2 {
             return usable.into_iter().map(|(address, _)| address).collect();
         }
-        if self.paths.len() >= MAX_PATHS {
-            self.forget_unused(now);
-        }
-        for (address, _) in &usable {
-            if self.paths.len() < MAX_PATHS || self.paths.contains_key(address) {
-                self.paths.entry(address.clone()).or_default().used = Some(now);
+        if sending {
+            if self.paths.len() >= MAX_PATHS {
+                self.forget_unused(now);
+            }
+            for (address, _) in &usable {
+                if self.paths.len() < MAX_PATHS || self.paths.contains_key(address) {
+                    self.paths.entry(address.clone()).or_default().used = Some(now);
+                }
             }
         }
         let rtt = |table: &Self, address: &str| table.paths.get(address).and_then(|path| path.rtt);
@@ -190,6 +199,7 @@ impl Table {
                 .min_by_key(|(_, rtt)| *rtt);
             if let (Some((fastest, fastest_rtt)), Some(current_rtt)) =
                 (fastest, rtt(self, &current))
+                && sending
                 && *fastest != current
                 && current_rtt.saturating_sub(fastest_rtt)
                     >= SWITCH_MIN.max(current_rtt.mul_f64(SWITCH_GAIN))
@@ -204,7 +214,7 @@ impl Table {
                 self.counts.switches += 1;
                 current = fastest.clone();
             }
-            if self.chosen.len() < MAX_PATHS || self.chosen.contains_key(&key) {
+            if sending && (self.chosen.len() < MAX_PATHS || self.chosen.contains_key(&key)) {
                 self.chosen.insert(key, current.clone());
             }
             // The rest: measured ones by round-trip time, then unmeasured ones in the
@@ -456,14 +466,18 @@ impl Paths {
 
     /// The usable ones of `addresses` (as paths keep them), best first: paths that are
     /// up before those down, the one in use first, then by round-trip time, then in
-    /// the configured order (module docs). Several addresses are paths to measure.
-    pub(crate) fn rank(&self, addresses: &[String]) -> Vec<String> {
+    /// the configured order (module docs). For a send (`sending`): several addresses
+    /// are paths to measure, and the one in use may change; else only a look
+    /// (`Table::rank`).
+    pub(crate) fn rank(&self, addresses: &[String], sending: bool) -> Vec<String> {
         let usable: Vec<(String, usize)> = addresses
             .iter()
             .filter(|address| self.usable(address))
             .map(|address| (address.clone(), self.preference(address)))
             .collect();
-        self.table.lock_or_recover().rank(usable, Instant::now())
+        self.table
+            .lock_or_recover()
+            .rank(usable, Instant::now(), sending)
     }
 
     /// The smoothed round-trip time of the path to `address`, if measured.
@@ -1006,27 +1020,39 @@ mod tests {
         let mut table = Table::default();
         let mut now = Instant::now();
         let set = paths(&["libp2p:x", "mycelium:x"]);
-        assert_eq!(table.rank(set.clone(), now), ["libp2p:x", "mycelium:x"]);
+        assert_eq!(
+            table.rank(set.clone(), now, true),
+            ["libp2p:x", "mycelium:x"]
+        );
         // libp2p a bit slower: not enough to switch.
         for _ in 0..8 {
             now = answer(&mut table, now, &[("libp2p:x", 110), ("mycelium:x", 100)]);
         }
-        assert_eq!(table.rank(set.clone(), now), ["libp2p:x", "mycelium:x"]);
+        assert_eq!(
+            table.rank(set.clone(), now, true),
+            ["libp2p:x", "mycelium:x"]
+        );
         assert_eq!(table.counts.switches, 0);
         // Much slower: switched, and it stays switched while that holds.
         for _ in 0..8 {
             now = answer(&mut table, now, &[("libp2p:x", 300), ("mycelium:x", 100)]);
         }
-        assert_eq!(table.rank(set.clone(), now), ["mycelium:x", "libp2p:x"]);
-        assert_eq!(table.rank(set.clone(), now), ["mycelium:x", "libp2p:x"]);
+        assert_eq!(
+            table.rank(set.clone(), now, true),
+            ["mycelium:x", "libp2p:x"]
+        );
+        assert_eq!(
+            table.rank(set.clone(), now, true),
+            ["mycelium:x", "libp2p:x"]
+        );
         assert_eq!(table.counts.switches, 1);
         // Small differences under SWITCH_MIN never switch.
         let lan = paths(&["libp2p:y", "mycelium:y"]);
-        table.rank(lan.clone(), now);
+        table.rank(lan.clone(), now, true);
         for _ in 0..8 {
             now = answer(&mut table, now, &[("libp2p:y", 4), ("mycelium:y", 1)]);
         }
-        assert_eq!(table.rank(lan, now), ["libp2p:y", "mycelium:y"]);
+        assert_eq!(table.rank(lan, now, true), ["libp2p:y", "mycelium:y"]);
     }
 
     #[test]
@@ -1034,14 +1060,17 @@ mod tests {
         let mut table = Table::default();
         let mut now = Instant::now();
         let set = paths(&["libp2p:x", "mycelium:x"]);
-        table.rank(set.clone(), now);
+        table.rank(set.clone(), now, true);
         // libp2p stops answering: down after UNANSWERED_DOWN PINGs, then last.
         for _ in 0..=UNANSWERED_DOWN {
             now = answer(&mut table, now, &[("mycelium:x", 100)]);
-            table.rank(set.clone(), now);
+            table.rank(set.clone(), now, true);
         }
         assert!(table.info("libp2p:x", now).down);
-        assert_eq!(table.rank(set.clone(), now), ["mycelium:x", "libp2p:x"]);
+        assert_eq!(
+            table.rank(set.clone(), now, true),
+            ["mycelium:x", "libp2p:x"]
+        );
         // It answers again: up, and measured.
         now = answer(&mut table, now, &[("libp2p:x", 50), ("mycelium:x", 100)]);
         assert!(!table.info("libp2p:x", now).down);
@@ -1054,6 +1083,32 @@ mod tests {
         assert!(!table.info("mycelium:x", now).down);
     }
 
+    /// A look at the choice (`/peers`, figures) changes nothing: no path counts as
+    /// used, and no faster one is switched to.
+    #[test]
+    fn a_look_changes_nothing() {
+        let mut table = Table::default();
+        let mut now = Instant::now();
+        let set = paths(&["libp2p:x", "mycelium:x"]);
+        assert_eq!(
+            table.rank(set.clone(), now, false),
+            ["libp2p:x", "mycelium:x"]
+        );
+        assert!(table.paths.is_empty() && table.chosen.is_empty());
+        table.rank(set.clone(), now, true);
+        for _ in 0..4 {
+            now = answer(&mut table, now, &[("libp2p:x", 500), ("mycelium:x", 10)]);
+        }
+        // Much faster, but only looked at: still the one in use.
+        assert_eq!(
+            table.rank(set.clone(), now, false),
+            ["libp2p:x", "mycelium:x"]
+        );
+        assert_eq!(table.counts.switches, 0);
+        assert_eq!(table.rank(set, now, true), ["mycelium:x", "libp2p:x"]);
+        assert_eq!(table.counts.switches, 1);
+    }
+
     #[test]
     fn only_paths_in_use_are_pinged() {
         let mut table = Table::default();
@@ -1064,9 +1119,9 @@ mod tests {
             nonce
         };
         // One address: nothing to choose, nothing to measure.
-        table.rank(paths(&["libp2p:z"]), now);
+        table.rank(paths(&["libp2p:z"]), now, true);
         assert!(table.due(now, &mut next).is_empty());
-        table.rank(paths(&["libp2p:x", "mycelium:x"]), now);
+        table.rank(paths(&["libp2p:x", "mycelium:x"]), now, true);
         assert_eq!(table.due(now, &mut next).len(), 2);
         // Not again within PROBE_EVERY; a stale answer is ignored.
         assert!(table.due(now + PROBE_TIMEOUT, &mut next).is_empty());
@@ -1099,7 +1154,7 @@ mod tests {
     fn round_trips_count_from_the_send() {
         let mut table = Table::default();
         let now = Instant::now();
-        table.rank(paths(&["libp2p:x", "mycelium:x"]), now);
+        table.rank(paths(&["libp2p:x", "mycelium:x"]), now, true);
         let due = table.due(now, || 7);
         assert_eq!(due.len(), 2);
         // Sent a second late (after another path's slow send).
@@ -1117,7 +1172,7 @@ mod tests {
     fn a_path_that_goes_elsewhere_is_measured_anew() {
         let mut table = Table::default();
         let now = Instant::now();
-        table.rank(paths(&["libp2p:x", "mycelium:x"]), now);
+        table.rank(paths(&["libp2p:x", "mycelium:x"]), now, true);
         let ping = |table: &mut Table, nonce: u64, at: Instant, ms: u64| {
             table.pinged("libp2p:x", nonce, at);
             table.pong("libp2p:x", nonce, at + Duration::from_millis(ms));
@@ -1143,7 +1198,7 @@ mod tests {
             nonce += 1;
             nonce
         };
-        table.rank(paths(&["libp2p:x", "mycelium:x"]), now);
+        table.rank(paths(&["libp2p:x", "mycelium:x"]), now, true);
         table.pinged("libp2p:x", 7, now);
         let due = table.due(now, &mut next);
         assert_eq!(due.len(), 1);
@@ -1312,13 +1367,13 @@ mod tests {
         tokio::spawn(async move { b.start_consumer(handler).await });
 
         let addresses = vec!["sim:b".to_owned(), "broken:b".to_owned()];
-        let ranked = a.rank(&addresses);
+        let ranked = a.rank(&addresses, true);
         assert_eq!(ranked, ["broken:b", "sim:b"]); // The configured order.
         let mut encoded = a.encode(&ranked[0], &some_status()).unwrap();
         encoded.peer = "node-b".to_owned();
         assert_eq!(a.send_first(&encoded, &ranked).await.unwrap(), "sim:b");
         assert_eq!(received.recv().await.unwrap(), "sim:a");
-        assert_eq!(a.rank(&addresses), ["sim:b", "broken:b"]);
+        assert_eq!(a.rank(&addresses, true), ["sim:b", "broken:b"]);
 
         // Without the message kept, nothing else can be tried: the transport's error.
         encoded.message = None;