substitutes held longer

2 files changed, 28 insertions, 17 deletions

diff --git a/docs/peers.md b/docs/peers.md
index 79f2abb..85d65a9 100644
--- a/docs/peers.md
+++ b/docs/peers.md
@@ -365,8 +365,9 @@ its registry entry ([`swarm/working_links.rs`](../src/swarm/working_links.rs), `
 its neighbours by the graph it doesn't reach (`unreached`), once down for four keepalive
 intervals, and its neighbours it reaches that the graph doesn't link it to (`reached`:
 a ring going round a member it doesn't reach, a member reaching out from behind NAT),
-once up for one; each comes out again after the
-other hold; a link the other end records as unreached already isn't recorded again.
+once up for four too (a ring goes round a member only while it is away, so a short
+absence writes nothing); a link back comes out after one keepalive, a substitute gone
+after four; a link the other end records as unreached already isn't recorded again.
 On a stable swarm the entry has none. A member that went away is recorded
 as unreached by its neighbours (they can't tell it from a link that broke), and taken
 out once they reach it again: so a node first asks up to three
diff --git a/src/swarm/working_links.rs b/src/swarm/working_links.rs
index 13337b9..d0ea41a 100644
--- a/src/swarm/working_links.rs
+++ b/src/swarm/working_links.rs
@@ -8,9 +8,11 @@
 //! records its neighbours by the graph that it doesn't reach (`unreached`), and its
 //! neighbours it reaches that the graph doesn't link it to (`reached`: a ring that goes
 //! round a member it doesn't reach, a member behind NAT that reaches out to one the
-//! graph doesn't give it), each only once it has stayed so for a while: going down for
-//! `DOWN_FOR` keepalive intervals, up for `UP_FOR`, so a link that drops for a
-//! keepalive isn't written down, and one back is soon again. A link either end records
+//! graph doesn't give it), each only once it has stayed so for a while: `DOWN_FOR`
+//! keepalive intervals for a link down, or one the graph doesn't give (a ring goes
+//! round a member only while it is away, so a short absence writes nothing), `UP_FOR`
+//! for a link up again, so a link that drops for a keepalive isn't written down, and
+//! one back is soon again. A link either end records
 //! as unreached is one that doesn't work; one either records as reached, one that does;
 //! so a link the other end records as unreached already isn't recorded by this one.
 //! Only neighbours (`topology::our_neighbours`, those we sync with) count as reached:
@@ -43,10 +45,11 @@ use crate::swarm::{reach, topology};
 use crate::transport::{ParsedMessage, ProbeReply, ProbeRequest};
 
 /// Keepalive intervals a link stays down before it is recorded so (`unreached`, or out
-/// of `reached`): as long as a silent peer takes to be held dormant (`dormant_after`).
+/// of `reached`), and a link the graph doesn't give stays up before it is recorded
+/// (into `reached`): as long as a silent peer takes to be held dormant
+/// (`dormant_after`).
 const DOWN_FOR: u32 = 4;
-/// Keepalive intervals a link stays up before it is recorded so (out of `unreached`,
-/// or into `reached`).
+/// Keepalive intervals a link stays up before it is recorded so (out of `unreached`).
 const UP_FOR: u32 = 1;
 
 /// Members asked about a member at once at most: of its
@@ -355,9 +358,10 @@ fn via(
 }
 
 /// The record to keep: `recorded`, but where a member's link has differed from it as
-/// `now_links` says since long enough before `now` (`pending`): down for `DOWN_FOR`
-/// keepalive intervals (newly unreached, or no longer reached), up for `UP_FOR` (reached
-/// again, or newly reached, or no longer our neighbour by the graph). A link newly
+/// `now_links` says since long enough before `now` (`pending`): `DOWN_FOR` keepalive
+/// intervals (newly unreached, no longer reached, or newly reached: a substitute
+/// neighbour while one is away), `UP_FOR` otherwise (reached again, or no longer our
+/// neighbour by the graph). A link newly
 /// unreached goes in only if `confirmed` has it (all do without): those held long
 /// enough that it hasn't are returned, to ask others about.
 fn decide(
@@ -387,8 +391,8 @@ fn decide(
             pending.since.remove(member);
             continue;
         }
-        let down = (unreached && !was_unreached) || (was_reached && !reached);
-        let hold = keepalive * if down { DOWN_FOR } else { UP_FOR };
+        let long = (unreached && !was_unreached) || (was_reached != reached);
+        let hold = keepalive * if long { DOWN_FOR } else { UP_FOR };
         let since = *pending.since.entry(member.clone()).or_insert(now);
         if now.saturating_duration_since(since) < hold {
             continue;
@@ -431,7 +435,8 @@ mod tests {
     }
 
     /// A link goes into the record once down for `DOWN_FOR` keepalives, not for one; out
-    /// once up for `UP_FOR`; one reached the graph doesn't give, once up for `UP_FOR`.
+    /// once up for `UP_FOR`; one reached the graph doesn't give, in and out after
+    /// `DOWN_FOR`.
     #[test]
     fn links_are_recorded_after_their_hold() {
         let keepalive = Duration::from_secs(5);
@@ -510,7 +515,8 @@ mod tests {
             none
         );
 
-        // Reached beyond the graph: in after `UP_FOR`; gone: out after `DOWN_FOR`.
+        // Reached beyond the graph: in after `DOWN_FOR`, not `UP_FOR`; gone: out after
+        // `DOWN_FOR`.
         let beyond = links(&[], &["c"]);
         assert_eq!(
             decide(&mut pending, &none, &beyond, at(50), keepalive, None).0,
@@ -518,14 +524,18 @@ mod tests {
         );
         assert_eq!(
             decide(&mut pending, &none, &beyond, at(55), keepalive, None).0,
+            none
+        );
+        assert_eq!(
+            decide(&mut pending, &none, &beyond, at(70), keepalive, None).0,
             beyond
         );
         assert_eq!(
-            decide(&mut pending, &beyond, &none, at(60), keepalive, None).0,
+            decide(&mut pending, &beyond, &none, at(75), keepalive, None).0,
             beyond
         );
         assert_eq!(
-            decide(&mut pending, &beyond, &none, at(80), keepalive, None).0,
+            decide(&mut pending, &beyond, &none, at(95), keepalive, None).0,
             none
         );
     }