From f0115783a2e70e7610fed1ec061baa8ed6969a2a Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 4 Apr 2023 08:09:50 +1000 Subject: [PATCH 1/2] Fix off-by-one error in DNS seed peer retries, and clarify logs --- zebra-network/src/config.rs | 38 +++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/zebra-network/src/config.rs b/zebra-network/src/config.rs index b422d32514c..d9779855de1 100644 --- a/zebra-network/src/config.rs +++ b/zebra-network/src/config.rs @@ -1,3 +1,5 @@ +//! Configuration for Zebra's network communication. + use std::{ collections::HashSet, net::{IpAddr, SocketAddr}, @@ -24,7 +26,14 @@ mod tests; /// The number of times Zebra will retry each initial peer's DNS resolution, /// before checking if any other initial peers have returned addresses. -const MAX_SINGLE_PEER_RETRIES: usize = 1; +/// +/// After doing this number of retries of a failed single peer, Zebra will +/// check if it has enough peer addresses from other seed peers. If it has +/// enough addresses, it won't retry this peer again. +/// +/// If the number of tries is `0`, other peers are checked after every successful +/// or failed DNS attempt. +const MAX_SINGLE_SEED_PEER_DNS_RETRIES: usize = 0; /// Configuration for networking code. #[derive(Clone, Debug, Serialize)] @@ -164,7 +173,7 @@ impl Config { // address peers. Individual retries avoid this issue. let peer_addresses = peers .iter() - .map(|s| Config::resolve_host(s, MAX_SINGLE_PEER_RETRIES)) + .map(|s| Config::resolve_host(s, MAX_SINGLE_SEED_PEER_DNS_RETRIES)) .collect::>() .concat() .await; @@ -188,12 +197,25 @@ impl Config { /// /// If DNS continues to fail, returns an empty list of addresses. async fn resolve_host(host: &str, max_retries: usize) -> HashSet { - for retry_count in 1..=max_retries { - match Config::resolve_host_once(host).await { - Ok(addresses) => return addresses, - Err(_) => tracing::info!(?host, ?retry_count, "Retrying peer DNS resolution"), - }; - tokio::time::sleep(DNS_LOOKUP_TIMEOUT).await; + for attempts in 0..=max_retries { + if let Ok(addresses) = Config::resolve_host_once(host).await { + return addresses; + } + + if attempts < max_retries { + tracing::info!( + ?host, + previous_attempts = ?(attempts + 1), + "Waiting {DNS_LOOKUP_TIMEOUT:?} to retry seed peer DNS resolution", + ); + tokio::time::sleep(DNS_LOOKUP_TIMEOUT).await; + } else { + tracing::info!( + ?host, + attempts = ?(attempts + 1), + "Seed peer DNS resolution failed, checking for addresses from other seed peers", + ); + } } HashSet::new() From 8b40495c337464dc7bdf4f52e26b819444b5eb82 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 4 Apr 2023 08:15:58 +1000 Subject: [PATCH 2/2] Fix confusing variable names --- zebra-network/src/config.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/zebra-network/src/config.rs b/zebra-network/src/config.rs index d9779855de1..b0374e041cc 100644 --- a/zebra-network/src/config.rs +++ b/zebra-network/src/config.rs @@ -31,7 +31,7 @@ mod tests; /// check if it has enough peer addresses from other seed peers. If it has /// enough addresses, it won't retry this peer again. /// -/// If the number of tries is `0`, other peers are checked after every successful +/// If the number of retries is `0`, other peers are checked after every successful /// or failed DNS attempt. const MAX_SINGLE_SEED_PEER_DNS_RETRIES: usize = 0; @@ -197,22 +197,22 @@ impl Config { /// /// If DNS continues to fail, returns an empty list of addresses. async fn resolve_host(host: &str, max_retries: usize) -> HashSet { - for attempts in 0..=max_retries { + for retries in 0..=max_retries { if let Ok(addresses) = Config::resolve_host_once(host).await { return addresses; } - if attempts < max_retries { + if retries < max_retries { tracing::info!( ?host, - previous_attempts = ?(attempts + 1), + previous_attempts = ?(retries + 1), "Waiting {DNS_LOOKUP_TIMEOUT:?} to retry seed peer DNS resolution", ); tokio::time::sleep(DNS_LOOKUP_TIMEOUT).await; } else { tracing::info!( ?host, - attempts = ?(attempts + 1), + attempts = ?(retries + 1), "Seed peer DNS resolution failed, checking for addresses from other seed peers", ); }