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

proposal: net: cache DNS responses #24796

Closed
iangudger opened this issue Apr 10, 2018 · 15 comments

Comments

@iangudger
Copy link
Contributor

@iangudger iangudger commented Apr 10, 2018

The new DNS client is significantly more efficient than the old one, but it hasn't changed overall performance much as the most expensive part by far is the network. If a user dials the same domain repeatedly, we shouldn't need to ask the DNS server each time in most situations.

I propose that we cache the parsed result along with the expiration time of the minimum TTL from the RRs used to create said result. This allows us to keep the benefits of incremental parsing and should minimize the performance impact of caching by minimizing what needs to be copied into and out of the cache. The caching logic would go at the call sites of net.(*Resolver).loopup.

We may want/need to rotate the order of the responses returned from the cache.

The simplest eviction strategy is to only evict expired results from the cache. If we want to limit maximum size, we could do LRU + expired.

https://www.ietf.org/rfc/rfc1034.txt
https://www.ietf.org/rfc/rfc1035.txt
https://tools.ietf.org/html/rfc2181#section-7 (SOA TTLs)
https://tools.ietf.org/html/rfc2181#section-8
https://tools.ietf.org/html/rfc1123#section-6.1.2.1
https://00f.net/2011/11/17/how-long-does-a-dns-ttl-last/ (article on the behavior of various caching DNS servers)

/cc @mikioh @bradfitz @mdempsky

@gopherbot gopherbot added this to the Proposal milestone Apr 10, 2018
@gopherbot gopherbot added the Proposal label Apr 10, 2018
@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented Apr 10, 2018

Ideally, operating systems should provide this & do local caching. (This was one of the things I worked on on Android, FWIW. Previously all apps did their own DNS lookups+caching, not sharing the cache between apps.)

What about making this a package that users can import, for people who want this functionality in a self-contained Go binary with a minimal/no OS environment?

Then that package can register itself as the net.DefaultResolver, or the user can.

@iangudger

This comment has been minimized.

Copy link
Contributor Author

@iangudger iangudger commented Apr 10, 2018

I have a caching resolver that I haven't yet sent out for review that goes with golang.org/cl/51631 and implements dnsresolver.Resolver. Is something like that what you had in mind?

The problem with caching is that it needs to be aware of stuff which is not currently exposed via the net package's current API (e.g. TTLs). Generally this would operate at the message/RR level, but that typically requires parsing the whole message (something that we have been explicitly avoiding). In order to integrate something like that into the net package as you describe, it might require exporting some DNS message types in the net package (another thing that we have been explicitly avoiding).

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented Apr 10, 2018

@iangudger, I haven't looked at it, and I probably won't have time. But I like that it's not in std. If others can just import it and use it, assigning it to net.DefaultResolver somehow, that's great.

@iangudger

This comment has been minimized.

Copy link
Contributor Author

@iangudger iangudger commented Apr 10, 2018

@bradfitz, are you saying that you don't think caching is something we should support in the standard library and instead should be an optional addon from another package?

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented Apr 10, 2018

I think caching is important, but it's not obvious to me (yet?) that it's our job. I think the operating system does/should provide it. I'd like to see numbers before deciding to take on that much new code in the standard library.

Numbers might be: how percentage of systems don't have a local caching resolver by default? How often would this save users DNS lookups?

What do Macs do by default?
What do popular Linux distos do?
What does Windows do by default?
What does libc do?

@iangudger

This comment has been minimized.

Copy link
Contributor Author

@iangudger iangudger commented Apr 10, 2018

glibc does seem to contain a cache.

Both Windows and macOS seem to have have caches. I don't think we support the native Go DNS client on Windows. It is not clear to me what needs to be done to integrate with the macOS DNS resolver.

As of a few years ago, it was not common for Linux distros to use a cache. There is nscd, but it was not commonly used. systemd now contains a DNS cache, but it is not clear to me if it is commonly used. systemd contains a lot of stuff which is not commonly used. The systemd cache (if enabled) can be used either with D-Bus or through glibc.

On top of this, applications which make lots of DNS lookups (e.g. Firefox) contain their own DNS cache.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Apr 16, 2018

It sounds like:

  • On Linux we use /etc/resolv.conf's nameserver, which on newer systems is set to localhost, which runs a cache.
  • On OS X we use the system C library, which I think has its own cache.
  • On Windows we use DnsQuery.

So it doesn't sound like a cache is needed in the standard library too. That would just be redundant with the system caches, no?

Maybe if a cache is needed in custom contexts, an out-of-standard-library cache would be fine.

@iangudger

This comment has been minimized.

Copy link
Contributor Author

@iangudger iangudger commented Apr 16, 2018

On Linux we use /etc/resolv.conf's nameserver, which on newer systems is set to localhost, which runs a cache.

Where did you find this? I don't think this is true.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 16, 2018

It seems to be true on my system. In /etc/resolv.conf I see nameserver 127.0.0.1. Listening on 127.0.0.1:53 is a program called dnsmasq. The man page for dnsmasq says that it provides a small local DNS cache that forwards to a real, recursive, DNS server.

@iangudger

This comment has been minimized.

Copy link
Contributor Author

@iangudger iangudger commented Apr 16, 2018

Is this your work system? If so, that is a non-standard config.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 17, 2018

Yes, that is my work system, and it may well be non-standard. Another system I have (Fedora GNU/Linux) seems to up the default DNS resolver from DHCP.

Personally I don't think the default should be to have an application-specific cache. It makes sense to offer that as a package, but it doesn't seem like the right choice for most programs.

@thanm

This comment has been minimized.

Copy link
Member

@thanm thanm commented Apr 17, 2018

My home linux machine (unmodified/stock Ubuntu 16.04) also has the same setup (nameserver -> localhost with dnsmasq enabled).

@mikioh

This comment has been minimized.

Copy link
Contributor

@mikioh mikioh commented Apr 18, 2018

I'm not keen on putting DNS RR cache stuff into the net package of the standard library because once it exists it may call surround stuff including Doh (DNS-over-HTTPS) and we perhaps suffer complicated troubleshooting on various circumstances (nowadays it's not uncommon that an IP node has multiple network interfaces, connectivities, and connectivity-dependent DNS recursive servers) and circular dependencies. At the same time, we need to deal with fancy signals carried by DHCP or RA such as PvD (provisioning domain) for keeping RR cache accurate and reliable.

I'm fine with putting a hook point into Dialer for fancy DNS stuff injection.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Apr 23, 2018

Per #24796 (comment) and discussion since then, it sounds like:

  1. Caching at the host level is the right approach anyway (not per-process).
  2. Go on many systems already goes through the host cache.
  3. If a per-application cache is needed by custom applications, they can already hook one into the standard library, with a net.DefaultResolver that intercepts Dials to DNS servers.

Given that, I think we can avoid any other changes to the standard library. Closing as not accepted.

@rsc rsc closed this Apr 23, 2018
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 26, 2018

Change https://golang.org/cl/145197 mentions this issue: net: remove TODO for DNS cache

gopherbot pushed a commit that referenced this issue Oct 27, 2018
The proposal to add a DNS cache was rejected, so there is no longer a
need for the associated TODO.

Updates #24796

Change-Id: Ifcedcff72c75a70b2143de0bd3f7bf85ac3528f6
Reviewed-on: https://go-review.googlesource.com/c/145197
Run-TryBot: Ian Gudger <igudger@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Mikio Hara <mikioh.public.networking@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Oct 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
7 participants
You can’t perform that action at this time.