Skip to content
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

Modify the checking logic of cname cache, #14918

Closed
wants to merge 3 commits into from

Conversation

zjs604381586
Copy link
Contributor

Short description

Modify the cname cache check logic so that if the final cname has a record cache of the parsed type qtype, it will be directly cached and returned without qname-minimization. It does not make sense to do qname-minimization. If the final cname does not have a record cache of qtype, continue to do qname-minimization.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
  • checked that this code was merged to master

@rgacogne rgacogne requested a review from omoerbeek December 3, 2024 10:04
@rgacogne rgacogne added the rec label Dec 3, 2024
@coveralls
Copy link

coveralls commented Dec 3, 2024

Pull Request Test Coverage Report for Build 12178384969

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 16 of 16 (100.0%) changed or added relevant lines in 1 file are covered.
  • 87 unchanged lines in 16 files lost coverage.
  • Overall coverage decreased (-0.02%) to 64.676%

Files with Coverage Reduction New Missed Lines %
pdns/packethandler.cc 1 72.4%
pdns/dnsdistdist/dnsdist.cc 2 68.38%
pdns/misc.hh 3 87.62%
pdns/backends/gsql/gsqlbackend.hh 3 96.57%
pdns/opensslsigners.cc 3 61.2%
pdns/recursordist/recpacketcache.hh 3 89.55%
modules/gpgsqlbackend/spgsql.cc 3 68.42%
modules/godbcbackend/sodbc.cc 4 70.8%
pdns/recursordist/rec-tcp.cc 4 64.68%
pdns/signingpipe.cc 5 83.38%
Totals Coverage Status
Change from base Build 12136705085: -0.02%
Covered Lines: 125831
Relevant Lines: 163659

💛 - Coveralls

@omoerbeek
Copy link
Member

Thanks for the PR!
I like the idea and with first study it looks good. I have the feeling the mechanism could be more simple, I will look into that. A test would also be nice., will work on that as well.

@omoerbeek omoerbeek self-assigned this Dec 6, 2024
@omoerbeek omoerbeek added this to the rec-5.3.0 milestone Dec 16, 2024
omoerbeek added a commit to omoerbeek/pdns that referenced this pull request Dec 16, 2024
@omoerbeek
Copy link
Member

Alternative approach from #14973 was merged.

@omoerbeek omoerbeek closed this Dec 17, 2024
@zjs604381586 zjs604381586 deleted the find-cname-cache branch December 18, 2024 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants