-
Notifications
You must be signed in to change notification settings - Fork 33
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
Fixes a bug in Redis global cache triggered when TTL is 0 #1327
Conversation
Why don’t we skip caching altogether if the TTL is 0? |
That is an alternative to work-around the bug, but that will decrease caching in some cases compared to "local cache". With the discovered example, there is more than one test case querying for the CDS record, and with "local cache" that query only goes on the wire once. The global caching should be long enough to survive a test, in the normal case. Another answer is that I started my investigation by setting the minimum cache value to 1 to see if that solved the issue, and that works as well. |
You probably should skip inserting the key in Redis if the TTL is 0, the Redis cache still has a "local cache" so the answer will still be cached for the duration of the test anyway, same as the previous cache system.
|
Thanks! I will create an update. I did not realize that the local cache also was used. |
Purpose
This PR fixes the bug described in #1326 by setting the lowest TTL value to 1.
This should be seen as a temporary solution. There should probably be a shortest time that messages are cached in Redis to make sure that in the normal case the same query should not be sent more than once when running a test. In issue #1326 we can see that the TTL of "iis.se. CDS" is 0, and a query for that would be sent several times if not cached.
That shortest time should be configurable in the profile.
Context
Fixes #1326.
How to test this PR
Find a zone that returns TTL=0 for some record that Zonemaster queries for a verify that there is no fatal error of the type describe in #1326.