-
Notifications
You must be signed in to change notification settings - Fork 184
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(iroh-net): race ipv4 and ipv6 dns resolution #2026
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a reasonable tradeoff and a quick improvement.
I guess the right thing is to rework reportgen so that it can start probe sets as soon as a result is returned and return a stream of results. But let's do the simple thing first.
iroh-net/src/dns.rs
Outdated
|
||
let resolver = AsyncResolver::tokio(config, options); | ||
Ok(resolver) | ||
} | ||
|
||
/// Resolve IPv4 and IPv6 in parallel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably elaborate a little why we don't use the Ipv4AndIpv6
strategy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added some details
iroh-net/src/dns.rs
Outdated
.collect(); | ||
Ok(res) | ||
} | ||
(Ok(Ok(ipv4)), Err(_err)) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Ok(Ok(ipv4)), Err(_err)) => { | |
(Ok(Ok(ipv4)), Err(_timeout)) => { |
is perhaps a little bit more readable?
(in all places were a timeout err is matched)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
4f26bae
to
870bce3
Compare
This now races the resolution, with a fixed timeout on each, returning as many results as found during that time. Ref n0-computer#2006
This now races the resolution, with a fixed timeout on each, returning as many results as found during that time.
Ref #2006