From c9b0f259729f087784b5f178fbbe14e18a215541 Mon Sep 17 00:00:00 2001 From: tv Date: Wed, 28 Oct 2026 20:26:39 +0100 Subject: libp2p: ask for lost relay slot again --- src/transport/libp2p.rs | 106 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 87 insertions(+), 19 deletions(-) diff --git a/src/transport/libp2p.rs b/src/transport/libp2p.rs index d621c8e..060f614 100644 --- a/src/transport/libp2p.rs +++ b/src/transport/libp2p.rs @@ -113,9 +113,11 @@ const RELAY_RETRY: Duration = Duration::from_secs(30); /// needs them. const AUTO_RELAYS: usize = 2; const RELAY_CHECK: Duration = Duration::from_secs(30); -/// A relay that lost the slot before granting it (it doesn't answer, or another peer -/// answers at its address: a relay restarted without its libp2p key) isn't picked again -/// for this long. +/// A relay that refused a slot (`relay_refused`: none left, not allowed, not a relay, +/// another peer at its address: a relay restarted without its libp2p key) isn't picked +/// again for this long. One that didn't grant it for another reason (it doesn't answer, +/// the connection closed) isn't for `RELAY_RETRY`; a slot lost is reserved again at +/// once. const RELAY_PAUSE: Duration = Duration::from_secs(600); /// A peer nobody mentioned for this long (the member list, identify, mDNS, a /// connection) is forgotten, unless connected: mostly old peer IDs of nodes restarted @@ -248,7 +250,7 @@ struct Book { /// With `Libp2pConfig::public`: other peers that relay (identify says so), and /// where. public_relays: HashMap, - /// Relays picked that never granted a slot, and when (`RELAY_PAUSE`). + /// Relays not to be picked, until when (`RELAY_PAUSE`, `RELAY_RETRY`). failed_relays: HashMap, /// Relays that granted us a slot we hold. slots: std::collections::HashSet, @@ -347,6 +349,22 @@ fn relay_failure_text(debug: &str, innermost: &str) -> String { innermost.to_owned() } +/// Whether a slot ended, or wasn't granted, because the relay refused it (its error's +/// debug form and innermost cause, as for `relay_failure_text`), rather than because it +/// didn't answer or the connection closed: only then is it worth not asking again for a +/// while. +fn relay_refused(debug: &str, innermost: &str) -> bool { + let refusals = [ + "NoAddressesInReservation", + "ResourceLimitExceeded", + "PermissionDenied", + "connection gated", + "UnsupportedProtocol", + "WrongPeerId", + ]; + refusals.iter().any(|pattern| debug.contains(pattern)) || innermost == "Failed" +} + /// The innermost of `error`'s causes, each distinct one once: those libp2p lists in a /// "Multiple dial errors occurred:" text (one per line, ` - : ...`), /// or the last of the source chain. @@ -459,7 +477,7 @@ impl Book { paused: self .failed_relays .values() - .filter(|at| at.elapsed() < RELAY_PAUSE) + .filter(|until| tokio::time::Instant::now() < **until) .count(), granted: self.counts.slots_granted, lost: self.counts.slots_lost, @@ -1080,8 +1098,8 @@ fn pick_relays( } let (direct, configured, listed, members, public) = { let mut book = lock(book); - book.failed_relays - .retain(|_, at| at.elapsed() < RELAY_PAUSE); + let now = tokio::time::Instant::now(); + book.failed_relays.retain(|_, until| now < *until); let failed = book.failed_relays.clone(); let usable = |offers: &HashMap| -> HashMap { offers @@ -1363,31 +1381,50 @@ async fn run( // A relay picked: the next check picks again, another one if this one // never granted the slot. if let Some(relay) = picked.remove(&listener_id) { - let lost = { + let refused = match &reason { + Ok(()) => false, + Err(error) => relay_refused(&format!("{error:?}"), &innermost_causes(error)), + }; + let (lost, pause) = { let mut book = lock(&book); let lost = book.slots.remove(&relay); + let pause = match (refused, lost) { + (true, _) => Some(RELAY_PAUSE), + (false, true) => None, + (false, false) => Some(RELAY_RETRY), + }; if lost { book.counts.slots_lost += 1; } else { book.counts.slots_not_granted += 1; - book.failed_relays.insert(relay, tokio::time::Instant::now()); // Named without its peer ID: maybe another one now. if let Some(address) = book.find_again(relay) { let _ = again.send(Command::FindRelay { address }); } } - lost + if let Some(pause) = pause { + book.failed_relays.insert(relay, tokio::time::Instant::now() + pause); + } + (lost, pause) }; + // Again at once, or others meanwhile. + let _ = again.send(Command::CheckRelays); let reason = relay_failure(&reason); - if lost { - debug!(relay = %Addressed(SCHEME, &relay), reason = %reason, "Lost the slot on a relay"); - } else { - info!( - relay = %Addressed(SCHEME, &relay), - pause = ?RELAY_PAUSE, - reason = %reason, - "A relay didn't grant a slot; picking others for a while" - ); + match (lost, pause) { + (true, None) => { + info!(relay = %Addressed(SCHEME, &relay), reason = %reason, "Lost the slot on a relay; reserving again"); + } + (true, Some(pause)) => { + info!(relay = %Addressed(SCHEME, &relay), pause = ?pause, reason = %reason, "Lost the slot on a relay; picking others for a while"); + } + (false, pause) => { + info!( + relay = %Addressed(SCHEME, &relay), + pause = ?pause.unwrap_or_default(), + reason = %reason, + "A relay didn't grant a slot; picking others for a while" + ); + } } } } @@ -2308,6 +2345,37 @@ mod tests { assert_eq!(relay_failure(&Ok(())), "closed"); } + /// Only refusals pause a relay; a slot that ended otherwise is asked for again. + #[test] + fn only_refusals_pause_relays() { + let refused = [ + ("Reservation(Protocol(NoAddressesInReservation))", "x"), + ("Reservation(Status(ResourceLimitExceeded))", "x"), + ("Reservation(Status(PermissionDenied))", "x"), + ("Denied { cause: \"connection gated\" }", "x"), + ("Upgrade(UnsupportedProtocol)", "x"), + ("Dial(WrongPeerId { obtained: … })", "x"), + ( + "Codec(Error(Custom { kind: Other, error: Failed }))", + "Failed", + ), + ]; + for (debug, innermost) in refused { + assert!(relay_refused(debug, innermost), "{debug}"); + } + let not = [ + ("Io(Custom { kind: Other, error: Timeout })", "Timeout"), + ( + "Io(Custom { kind: Other, error: Right(Closed) })", + "connection is closed", + ), + ("Io(…)", "connection reset"), + ]; + for (debug, innermost) in not { + assert!(!relay_refused(debug, innermost), "{debug}"); + } + } + /// libp2p's nested dial errors come out as each address's cause, once. #[test] fn dial_errors_are_short() { -- cgit v1.3.1