Skip to content

Commit

Permalink
feat(iroh-net)!: improve dns behaviour by staggering requests (#2313)
Browse files Browse the repository at this point in the history
## Description

Improves dns behaviour by staggering concurrent queries. Other changes
include:
- Remove unnecessary trait bounds from `ResolverExt` functions.
- Unify dns capabilities under `ResolverExt`.
- Add helper fn `stagger_call` with a minimal test.
- Add staggering versions of the `ResolverExt` functions.
- Use staggered options and turn on tests

## Breaking Changes

- `iroh_net::dns::node_info::lookup_by_domain` moved to
`iroh_net::ResolverExt::lookup_by_domain`

## Notes & open questions

Whether test are less prone to fail randomly is something I can't test
locally (no windows) so I'm trusting ci here.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
- [x] All breaking changes documented.
  • Loading branch information
divagant-martian authored May 22, 2024
1 parent 6d1a6dd commit d813089
Show file tree
Hide file tree
Showing 7 changed files with 269 additions and 47 deletions.
2 changes: 1 addition & 1 deletion iroh-dns-server/examples/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ async fn main() -> anyhow::Result<()> {
TxtAttrs::<String>::lookup_by_id(&resolver, &node_id, origin).await?
}
Command::Domain { domain } => {
TxtAttrs::<String>::lookup_by_domain(&resolver, &domain).await?
TxtAttrs::<String>::lookup_by_name(&resolver, &domain).await?
}
};
println!("resolved node {}", resolved.node_id());
Expand Down
9 changes: 3 additions & 6 deletions iroh-dns-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ mod tests {
};
use iroh_net::{
discovery::pkarr_publish::PkarrRelayClient,
dns::{
node_info::{lookup_by_id, NodeInfo},
DnsResolver,
},
dns::{node_info::NodeInfo, DnsResolver, ResolverExt},
key::SecretKey,
};
use pkarr::{PkarrClient, SignedPacket};
Expand Down Expand Up @@ -168,7 +165,7 @@ mod tests {
pkarr.publish(&signed_packet).await?;

let resolver = test_resolver(nameserver);
let res = lookup_by_id(&resolver, &node_id, origin).await?;
let res = resolver.lookup_by_id(&node_id, origin).await?;

assert_eq!(res.node_id, node_id);
assert_eq!(res.info.relay_url.map(Url::from), Some(relay_url));
Expand Down Expand Up @@ -204,7 +201,7 @@ mod tests {

// resolve via DNS from our server, which will lookup from our DHT
let resolver = test_resolver(nameserver);
let res = lookup_by_id(&resolver, &node_id, origin).await?;
let res = resolver.lookup_by_id(&node_id, origin).await?;

assert_eq!(res.node_id, node_id);
assert_eq!(res.info.relay_url.map(Url::from), Some(relay_url));
Expand Down
6 changes: 3 additions & 3 deletions iroh-net/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ mod test_dns_pkarr {

use crate::{
discovery::pkarr_publish::PkarrPublisher,
dns::node_info::{lookup_by_id, NodeInfo},
dns::{node_info::NodeInfo, ResolverExt},
relay::{RelayMap, RelayMode},
test_utils::{
dns_server::{create_dns_resolver, run_dns_server},
Expand Down Expand Up @@ -590,7 +590,7 @@ mod test_dns_pkarr {
state.upsert(signed_packet)?;

let resolver = create_dns_resolver(nameserver)?;
let resolved = lookup_by_id(&resolver, &node_info.node_id, &origin).await?;
let resolved = resolver.lookup_by_id(&node_info.node_id, &origin).await?;

assert_eq!(resolved, node_info.into());

Expand Down Expand Up @@ -620,7 +620,7 @@ mod test_dns_pkarr {
publisher.update_addr_info(&addr_info);
// wait until our shared state received the update from pkarr publishing
dns_pkarr_server.on_node(&node_id, timeout).await?;
let resolved = lookup_by_id(&resolver, &node_id, &origin).await?;
let resolved = resolver.lookup_by_id(&node_id, &origin).await?;

let expected = NodeAddr {
info: addr_info,
Expand Down
9 changes: 6 additions & 3 deletions iroh-net/src/discovery/dns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ use futures_lite::stream::Boxed as BoxStream;

use crate::{
discovery::{Discovery, DiscoveryItem},
dns, Endpoint, NodeId,
dns::ResolverExt,
Endpoint, NodeId,
};

/// The n0 testing DNS node origin
pub const N0_DNS_NODE_ORIGIN: &str = "dns.iroh.link";
const DNS_STAGGERING_MS: &[u64] = &[200, 300];

/// DNS node discovery
///
Expand Down Expand Up @@ -53,8 +55,9 @@ impl Discovery for DnsDiscovery {
let resolver = ep.dns_resolver().clone();
let origin_domain = self.origin_domain.clone();
let fut = async move {
let node_addr =
dns::node_info::lookup_by_id(&resolver, &node_id, &origin_domain).await?;
let node_addr = resolver
.lookup_by_id_staggered(&node_id, &origin_domain, DNS_STAGGERING_MS)
.await?;
Ok(DiscoveryItem {
provenance: "dns",
last_updated: None,
Expand Down
Loading

0 comments on commit d813089

Please sign in to comment.