From d70bb6d06cbf934070e78d9abdd9f57237cc9025 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 7 Jun 2018 17:16:07 -0700 Subject: [PATCH] proxy: More DNS cleanup (#1052) Depends on #1032. This branch makes some additional changes to the proxy's DNS code. In particular, since we no longer need to clone the resolver on every lookup, it removes some `clone()` calls in `DestinationSet::reset_dns_query`. I've also changed the DNS futures to use the new contextual logging code on master. Signed-off-by: Eliza Weisman --- proxy/src/dns.rs | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/proxy/src/dns.rs b/proxy/src/dns.rs index df39d7d33..85faaed93 100644 --- a/proxy/src/dns.rs +++ b/proxy/src/dns.rs @@ -45,6 +45,22 @@ pub type IpAddrListFuture = Box + Send /// valid certificate. pub type Name = tls::DnsName; +struct ResolveAllCtx(Name); + +impl fmt::Display for ResolveAllCtx { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + write!(f, "resolve_all_ips={}", self.0) + } +} + +struct ResolveOneCtx(Name); + +impl fmt::Display for ResolveOneCtx { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + write!(f, "resolve_one_ip={}", self.0) + } +} + impl Resolver { /// Construct a new `Resolver` from the system configuration and Conduit's @@ -85,8 +101,9 @@ impl Resolver { pub fn resolve_one_ip(&self, host: &transport::Host) -> IpAddrFuture { match *host { transport::Host::DnsName(ref name) => { - trace!("resolve_one_ip {}", name); - IpAddrFuture::DNS(Box::new(self.clone().lookup_ip(name))) + let ctx = ResolveOneCtx(name.clone()); + let f = ::logging::context_future(ctx, self.lookup_ip(name)); + IpAddrFuture::DNS(Box::new(f)) } transport::Host::Ip(addr) => IpAddrFuture::Fixed(addr), } @@ -94,16 +111,14 @@ impl Resolver { pub fn resolve_all_ips(&self, deadline: Instant, host: &Name) -> IpAddrListFuture { let name = host.clone(); - let name_clone = name.clone(); - trace!("resolve_all_ips {}", &name); - let resolver = self.clone(); + let lookup = self.lookup_ip(&name); let f = Delay::new(deadline) .then(move |_| { - trace!("resolve_all_ips {} after delay", &name); - resolver.lookup_ip(&name) + trace!("after delay"); + lookup }) .then(move |result| { - trace!("resolve_all_ips {}: completed with {:?}", name_clone, &result); + trace!("completed with {:?}", &result); match result { Ok(ips) => Ok(Response::Exists(ips)), Err(e) => { @@ -115,12 +130,10 @@ impl Resolver { } } }); - Box::new(f) + Box::new(::logging::context_future(ResolveAllCtx(name), f)) } - // `ResolverFuture` can only be used for one lookup, so we have to clone all - // the state during each resolution. - fn lookup_ip(self, name: &Name) + fn lookup_ip(&self, name: &Name) -> impl Future { self.resolver.lookup_ip(name.as_ref()) @@ -144,7 +157,10 @@ impl Future for IpAddrFuture { fn poll(&mut self) -> Poll { match *self { IpAddrFuture::DNS(ref mut inner) => match inner.poll() { - Ok(Async::NotReady) => Ok(Async::NotReady), + Ok(Async::NotReady) => { + trace!("dns not ready"); + Ok(Async::NotReady) + } , Ok(Async::Ready(ips)) => { match ips.iter().next() { Some(ip) => {