From 47e331119cd1c3c0bbc2f5946943f6d5af9bd3a7 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 4 Apr 2023 12:28:58 +1000 Subject: [PATCH] fix(net): Fix off-by-one error in DNS seed peer retries, and clarify logs (#6460) * Fix off-by-one error in DNS seed peer retries, and clarify logs * Fix confusing variable names --- 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..b0374e041cc 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 retries 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 retries in 0..=max_retries { + if let Ok(addresses) = Config::resolve_host_once(host).await { + return addresses; + } + + if retries < max_retries { + tracing::info!( + ?host, + 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 = ?(retries + 1), + "Seed peer DNS resolution failed, checking for addresses from other seed peers", + ); + } } HashSet::new()