Option to disable cache or refresh it asynchronously

[thibmeu]

This resolver provides a cache for IP addresses and TXT records.
Depending on the TTL set for the DNS record, this can lead to a significant delay before the resolver returns an up to date DNS record.

If this is done for latency concerns, updating the cache in the background via a goroutine would make sure the cache is refreshed regularly.
If the concern is for performance, being able to set a maximum TTL of records in the cache (0 for disabling it) would be great.

[marten-seemann]

If this is done for latency concerns, updating the cache in the background via a goroutine would make sure the cache is refreshed regularly.

How would cache entries ever expire and be garbage collected then?

[thibmeu]

The code I see would be something like

result, ok := r.getCachedTXT(domain)
if ok {
  go lookupAndUpdateCache(domain)
  return result
}
return lookupAndUpdateCache(domain)

Similar to how gc performed today. If the domain does not resolve, the cache is cleared.

If there needs to be GC, this can be build independently of this feature, as a new issue.

[marten-seemann]

If the domain does not resolve, the cache is cleared.

Domain resolution failures will be very rare. This means that the cache will just keep growing unboundedly as lookups for new domains are performed. This approach seems infeasible to me, unless you build something like an LRU cache.

Depending on the TTL set for the DNS record, this can lead to a significant delay before the resolver returns an up to date DNS record.

Taking a step back, isn't this exactly what the TTL value in the DNS record is there for in the first place?

[thibmeu]

This means that the cache will just keep growing unboundedly as lookups for new domains are performed

I may misunderstand, but I think this is how the current implementation is designed. I agree it's not ideal, but I think it is beyond the scope of this issue to decide on the best way to purge expired cache entry.

Taking a step back, isn't this exactly what the TTL value in the DNS record is there for in the first place?

Not really. TTL is set by the domain owner. Then it's up to implementations to decide if this TTL suits their needs. Using a custom DoH for instance, the domain records can be updated faster than the set TTL. Quoting RFC2181 Section 8

Implementations are always free to place an upper bound on any TTL received, and treat any larger values as if they were that upper bound. The TTL specifies a maximum time to live, not a mandatory time to live.

[marten-seemann]

I may misunderstand, but I think this is how the current implementation is designed. I agree it's not ideal, but I think it is beyond the scope of this issue to decide on the best way to purge expired cache entry.

I see. Deletion happens only when that record is requested. That's not ideal. There should probably be a GC go routine to clean up expired entries.

TTL is set by the domain owner. Then it's up to implementations to decide if this TTL suits their needs. Using a custom DoH for instance, the domain records can be updated faster than the set TTL. Quoting RFC2181 Section 8

Thanks for referencing the RFC. Your second proposal makes sense to me:

being able to set a maximum TTL of records in the cache (0 for disabling it) would be great.

@vyzo, since you wrote this resolver, what's your opinion on this? Should we add a WithMaximumTTL(time.Duration) option to the constructor?

[vyzo]

fine by me, the option sounds good.

…

On Wed, Oct 6, 2021, 17:43 Marten Seemann ***@***.***> wrote: I may misunderstand, but I think this is how the current implementation is designed. I agree it's not ideal, but I think it is beyond the scope of this issue to decide on the best way to purge expired cache entry. I see. Deletion happens only when that record is requested. That's not ideal. There should probably be a GC go routine to clean up expired entries. TTL is set by the domain owner. Then it's up to implementations to decide if this TTL suits their needs. Using a custom DoH for instance, the domain records can be updated faster than the set TTL. Quoting RFC2181 Section 8 Thanks for referencing the RFC. Your second proposal makes sense to me: being able to set a maximum TTL of records in the cache (0 for disabling it) would be great. @vyzo <https://github.com/vyzo>, since you wrote this resolver, what's your opinion on this? Should we add a WithMaximumTTL(time.Duration) option to the constructor? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAI4SWKEZN3DX32EVFWUCDUFROADANCNFSM5FOSBQRA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[aschmahmann]

@thibmeu are you going to be putting up a PR for this?