Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(net): Fix off-by-one error in DNS seed peer retries, and clarify logs #6460

Merged
merged 2 commits into from
Apr 4, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 30 additions & 8 deletions zebra-network/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//! Configuration for Zebra's network communication.

use std::{
collections::HashSet,
net::{IpAddr, SocketAddr},
Expand All @@ -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)]
Expand Down Expand Up @@ -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::<futures::stream::FuturesUnordered<_>>()
.concat()
.await;
Expand All @@ -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<SocketAddr> {
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",
);
}
}
teor2345 marked this conversation as resolved.
Show resolved Hide resolved

HashSet::new()
Expand Down