Skip to content

Conversation

@miodvallat
Copy link
Contributor

Short description

This apparently simple PR reworks the behaviour of the handle internal object of UeberBackend to:

  • factor the code performing backend selection and lookup call.
  • make the current backend pointer a private field to guarantee that the above factored logic is the only thing which decides which backend to use.

It is not supposed to introduce any behaviour change. But it will help make further (possibly breaking) changes in UeberBackend with confidence.

P.S.: do we really need to keep the instances atomic counter? It doesn't seem to be used by anything and its value is not retrievable either.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • read and accepted the Developer Certificate of Origin document, including the AI Policy, and added a "Signed-off-by" to my commits
  • 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)

@coveralls
Copy link

coveralls commented Nov 24, 2025

Pull Request Test Coverage Report for Build 19757600681

Details

  • 32 of 32 (100.0%) changed or added relevant lines in 2 files are covered.
  • 35 unchanged lines in 9 files lost coverage.
  • Overall coverage increased (+0.04%) to 73.146%

Files with Coverage Reduction New Missed Lines %
pdns/pollmplexer.cc 1 84.97%
modules/godbcbackend/sodbc.cc 2 71.43%
pdns/recursordist/rec-tcpout.cc 2 79.53%
pdns/axfr-retriever.cc 3 66.94%
pdns/recursordist/test-syncres_cc2.cc 3 89.12%
pdns/tsigverifier.cc 3 77.22%
pdns/recursordist/syncres.cc 6 81.28%
pdns/recursordist/test-syncres_cc1.cc 7 90.23%
pdns/packethandler.cc 8 72.44%
Totals Coverage Status
Change from base Build 19741851688: 0.04%
Covered Lines: 128173
Relevant Lines: 164538

💛 - Coveralls

@Habbie
Copy link
Member

Habbie commented Nov 25, 2025

P.S.: do we really need to keep the instances atomic counter? It doesn't seem to be used by anything and its value is not retrievable either.

the only thing using it appears to be a // commented out log message. I think it can go.

Habbie
Habbie previously approved these changes Nov 25, 2025
Copy link
Member

@Habbie Habbie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice cleanup

{
DLOG(g_log << "Ueber get() was called for a " << qtype << " record" << endl);
bool isMore = false;
while (d_hinterBackend != nullptr && !(isMore = d_hinterBackend->get(record))) { // this backend out of answers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

late note: this comment is missing a word

This allows making the current backend pointer private and makes the
behaviour more explicit:
- all backends will be queried in sequence.
- as soon as one backend answers the given (qname, qtype) query, none of
  the remaining backends will be queried until the next UeberBackend::lookup
  call (which starts a fresh new query).

Signed-off-by: Miod Vallat <[email protected]>
Habbie
Habbie previously approved these changes Nov 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants