-
Notifications
You must be signed in to change notification settings - Fork 37
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
Limit cache-max-negative-ttl in resolving #1292
Conversation
Please check #495 before implementing this change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like explicitly setting the cache-max-negative-ttl
even though normally there would not be a change in behavior. Only if settings.CACHE_TTL
would be set to something higher than 1 hour roughly.
checks/tasks/__init__.py
Outdated
self._ub_ctx.set_option("cache-max-negative-ttl:", str(settings.CACHE_TTL * 0.9)) | ||
# Some (unknown) tests probably depend on consistent ordering in unbound responses | ||
# https://github.com/internetstandards/Internet.nl/pull/613#discussion_r892196819 | ||
self._ub_ctx.set_option("rrset-roundrobin:", "no") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change normally would have no effect. cache-max-ttl
applies to all records in the cache. cache-max-negative-ttl
applies after that only for negative records (NXDOMAIN, NOERROR) as an exception for those records. Since cache-max-ttl
would normally be a value of less than 1 hour (default cache-max-negative-ttl
) for internet.nl the change is moot. However, explicitly setting it as the same value would guarantee homogeneous behavior across different setups and configurations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm ok, I will apply it then - original reason was a report that the test was not picking up new DNS records, then I looked in unbound config to find no cache-max-ttl at all, then realised it would be in the unbound context. So the original problem is likely not related to this, but it's still a good consistency fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If caching was a problem for new DNS records (especially in DNSSEC signed zones) then #495 may still be related. I changed its title because it was not only DNSSEC related.
Also in batch mode, all the pyunbounds forward to a central Unbound IIRC. That one also needs the proper TTL max values in its configuration.
checks/tasks/__init__.py
Outdated
# Some (unknown) tests probably depend on consistent ordering in unbound responses | ||
# https://github.com/internetstandards/Internet.nl/pull/613#discussion_r892196819 | ||
self._ub_ctx.set_option("rrset-roundrobin:", "no") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, the comment about rrset-roundrobin is for tests that get back a DNS answer but only care about the first X records from the results like the mailtest where only the first 8(?) mailservers are checked. Because this call to Unbound could be done from different places, turning off roundrobin guarantees that the same 8 in the mailtest for example will be used in different points of the code.
See comments in #1292. Prevents stale responses.
(in addition to cache-max-ttl)
No description provided.