cache-min-ttl

[csousougen]

Admins are using using very low DNS TTLs, making caching inefficient.
Could you please add the option to set a minimum TTL value and override the one provided?

Thank you.

[ameshkov]

Thank you for the feature request! Let's see how many upvotes it gets

[280blocker]

This is the most voted request in open issues.
I think this feature will reduce the load on AdGuardHome.

[csousougen]

CoreDNS supports this feature as much as serve_stale and prefetch.
While CoreDNS is overkill for a home system, these feature would make it lighter and save data for mobile users.

[ameshkov]

Assigned to v0.102

[adworacz]

Since I configure this on dnsmasq or DNSCrypt-proxy currently, I'd like to request the following:

• cache_size (entries - easier than thinking about bytes)
• cache_size = 1024
• cache_min_ttl = 2400
• cache_max_ttl = 86400
• cache_neg_min_ttl = 60
• cache_neg_max_ttl = 600

Those config directives and values are taken from DNSCrypt-proxy: https://github.com/DNSCrypt/dnscrypt-proxy/wiki/Caching

If I had to pick, I'd say that cache_min_ttl and cache_max_ttl are the highest priority (for me), followed by cache_size (entries), and then the *neg* versions of things.

[adworacz]

Hmm, looks like (unless I'm way off base) the primary ttl/caching logic happens here:
https://github.com/AdguardTeam/dnsproxy/blob/d4772f979c7711d93fa67a1de5bb3f45974226fb/proxy/cache.go#L126-L152

I don't know go much, but I might take a crack at implementing cache_min/max_ttl here. We'll see how free I am this weekend.

[ameshkov]

Yeah, but I am not sure how safe would be to change that method.

Maybe it'd be better to change packResponse which is used before putting DNS queries to the cache:
https://github.com/AdguardTeam/dnsproxy/blob/d4772f979c7711d93fa67a1de5bb3f45974226fb/proxy/cache.go#L197

[adworacz]

Ahh okay, I'll take a look at what it will take to update packResponse.

I should be able to find some time in the next few days.

[adworacz]

Got a quick version put together, pushed here: https://github.com/adworacz/dnsproxy/tree/minMaxTTL

However, it's still a WIP, as tests still need to be updated, and likely one or two other things (like the README).

[ameshkov]

Cool, the only thing that I don't like are the default values. I suppose by default dnsproxy should not modify what it gets from the DNS server.

[adworacz]

Cool, the only thing that I don't like are the default values. I suppose by default dnsproxy should not modify what it gets from the DNS server.

Fair enough - I was on the fence about this when I was working on it, so I'll support "unset" options for the min/max ttl, which should fix the existing tests in the process. I'll write a custom test specifically around the ttl overriding.

[adworacz]

Got unset ttl working, iterating on the tests now.

All existing tests pass, but I wrote a custom one for ttl overrides, and that’s failing currently. I’m looking into the cause (while I learn Go at the same time).

[adworacz]

Okay got tests working!

My branch is updated (linked above).

I want to update the README to reflect the new CLI options, then I’ll submit a pull request to dnsproxy.

Do y’all prefer squashed commits or are multiple commits in a PR okay?

[ameshkov]

@adworacz cool, thank you! I'd prefer squashed

[adworacz]

Pull request for dnsproxy is up: AdguardTeam/dnsproxy#84

For anyone else reading along - this does not mean Adguard Home supports setting TTL's yet. The front end interface needs to be updated, now that the core DNS library has support for setting TTLs.

[szolin]

Found in dnsmasq's man page:

--neg-ttl=<time>
    Negative replies from upstream servers normally contain time-to-live information in SOA records which dnsmasq uses for caching. If the replies from upstream servers omit this information, dnsmasq does not cache the reply. This option gives a default value for time-to-live (in seconds) which dnsmasq uses to cache negative replies even in the absence of an SOA record. 
--max-ttl=<time>
    Set a maximum TTL value that will be handed out to clients. The specified maximum TTL will be given to clients instead of the true TTL value if it is lower. The true TTL value is however kept in the cache to avoid flooding the upstream DNS servers. 
--max-cache-ttl=<time>
    Set a maximum TTL value for entries in the cache. 
--min-cache-ttl=<time>
    Extend short TTL values to the time given when caching them. Note that artificially extending TTL values is in general a bad idea, do not do it unless you have a good reason, and understand what you are doing. Dnsmasq limits the value of this option to one hour, unless recompiled. 

Interesting that dnsmasq warns users when they use min-cache-ttl as something may break; also they limit this value to one hour - should we do the same?

[ameshkov]

Yes, I think limiting it is reasonable

[adworacz]

Hmm okay, I can do that. Where’s the best place to store the constant representing the max?

And do we want a simple min() call, passing in the two values, or more official validation?

[szolin]

As this is just one Go package - you can simply add a new constant to the const section at the top in proxy.go.

I don't think we should warn user if he uses a value larger than our limit, so a simple min() will suffice.

[ameshkov]

We should warn in the help text, though

[adworacz]

Roger roger - I'll work on these updates (still without a proper go install on this laptop, will be on a better laptop soon).

Update: I've added the cap, and associated help text. There's a few open questions on the pull request. Once those have been answered, I'll post a new revision.

[lordraiden]

@adworacz @ameshkov
By default the values are for ttl min and max are 0. What is the behabiour in this case? In my log I see different ttl values for each domain, how are these defined?

Do you plan to add a place in the interface to edit this? Something like the advaced settings of pfsense

[ameshkov]

@lordraiden there's a task for that: #1587

[lhzw]

Er, I don't like the limit of min-cache-ttl, we should believe our users can do the right thing, just like linux.