Skip to content

Conversation

@miodvallat
Copy link
Contributor

Short description

The current state of the health check thread is to check for statuses checks to launch, every lua-health-checks-interval seconds.

This design has two issues:

This PR attempts to address this in two commits:

  • the checker thread was always being started (if not running already) before adding checks to its work list, creating a race where the thread could run, notice its list of checks is empty, and go to sleep, after which its check job is immediately added.
  • because further check requests can occur at any time, let it wake up every second to check for more work to do.

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 Oct 29, 2025

Pull Request Test Coverage Report for Build 19332892815

Details

  • 0 of 13 (0.0%) changed or added relevant lines in 1 file are covered.
  • 175 unchanged lines in 14 files lost coverage.
  • Overall coverage decreased (-0.04%) to 73.029%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pdns/lua-record.cc 0 13 0.0%
Files with Coverage Reduction New Missed Lines %
modules/geoipbackend/geoipbackend.cc 1 60.22%
pdns/iputils.hh 1 76.11%
modules/gsqlite3backend/gsqlite3backend.cc 2 94.55%
modules/pipebackend/pipebackend.cc 2 60.79%
pdns/recursordist/sortlist.cc 2 72.94%
pdns/recursordist/test-rec-tcounters_cc.cc 2 95.59%
pdns/dnsdistdist/dnsdist-carbon.cc 3 62.01%
pdns/iputils.cc 3 56.99%
pdns/lua-record.cc 3 0.18%
pdns/dnsdistdist/dnsdist-tcp.cc 4 77.05%
Totals Coverage Status
Change from base Build 19332602989: -0.04%
Covered Lines: 128003
Relevant Lines: 164594

💛 - Coveralls

Habbie
Habbie previously approved these changes Oct 30, 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.

approved, but please think about the comment i posted

@miodvallat
Copy link
Contributor Author

Logic slightly updated to try and make sure the checker thread, unless awaken earlier to handle new requests, sleeps the same time as before and not a full interval (i.e. takes into account the time it spent processing its queue). This yields a bit more deterministic timings, which the Lua tests are relying upon.

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