-
Notifications
You must be signed in to change notification settings - Fork 904
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
rec: Add hashed indexes to the caches, for faster retrieval #6505
Conversation
pdns/recursor_cache.cc
Outdated
ret+= sizeof(*j); // XXX WRONG we don't know the stored size! j->size(); | ||
ret+=(unsigned int)i.d_qname.toString().length(); | ||
for(const auto& record : i.d_records) | ||
ret+= sizeof(record); // XXX WRONG we don't know the stored size! j->size(); |
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.
- Is this comment still relevant?
j->size()
has no meaning here
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.
given #6447, maybe just remove the comment here
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.
The comment is still relevant (agreed, j
doesn't make sense anymore) until #6447 is merged so I'd rather keep it. I'll remove the j->size();
part.
@@ -51,7 +51,7 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, N | |||
ni->d_qtype == qtnull) { | |||
// We have something | |||
if ((uint32_t)now.tv_sec < ni->d_ttd) { | |||
ne = *ni; | |||
*ne = &(*ni); |
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.
damn knights
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.
looks good. Straightforward and I think we can stand the 8% insertion slowdown and memory consequences.
Short description
Most of the lookups in the query (positive) and negative caches are done on the exact name only, and doing so using a hash is much more faster than using canonical ordering.
In both cases we still need canonical ordering to be able to wipe a domain and its subtree from the cache efficiently, but regular retrieval (cachecache, get()) doesn't.
Pros:
Cons:
Checklist
I have: