Skip to content

Poll for the duplicate reply in its test instead of waiting two seconds - #3306

Merged
DL6ER merged 1 commit into
pi-hole:developmentfrom
010011110:duplicate-test-no-sleep
Oct 26, 2026
Merged

DL6ER merged 1 commit into
pi-hole:developmentfrom
010011110:duplicate-test-no-sleep

Conversation

@010011110

@010011110 010011110 commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

What does this implement/fix?

as suggested in the review of #3296: its test waited two seconds for the queries to move into the in-memory database. since #3294 the query log reads them from shared memory, so the wait is only needed for the reply to the second query, which dnsmasq logs, and runs the FTL hook for, only after it has answered that client. the test now polls pihole.log for that line, up to five seconds, instead of sleeping

How to test the change during review

the test in test_final.bats, "The reply to a duplicate of a query in flight is not logged as an unknown reply"

Additional information

Related issue or feature (if applicable): #3296

Pull request in docs with documentation (if applicable): N/A


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)
  6. My change does not modify src/dnsmasq/. That tree is a verbatim copy of upstream dnsmasq and we do not carry anything in it that deviates from upstream. Fixes have to go through the dnsmasq-discuss mailing list first, we merge them once they are in dnsmasq master.

Checklist:

  • The code change is tested and works locally.
  • I based my code and PRs against the repository's development branch.
  • I signed off all commits. Pi-hole enforces the DCO for all contributions
  • I signed all my commits. Pi-hole requires signatures to verify authorship
  • I have read the above and my PR is ready for review.

@010011110
010011110 requested a review from a team as a code owner October 26, 2026 19:36

@DL6ER DL6ER left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Required before approval: dnsmasq logs the duplicate line, and runs the FTL hook, only after send_from() to the second client, so the logs can be read too early. #3220 writes pihole.log from a separate thread, which widens this. Please poll for the line (up to a few seconds) instead of dropping the wait.

The test waited two seconds, as queries moved into the in-memory
database once per second. Since pi-hole#3294 the query log reads them from
shared memory, so a wait is only needed for the reply to the second
query: dnsmasq answers that client first and only then runs the FTL
hook and logs "reply query is duplicate". `_log_query()` calls
`FTL_hook()` before it writes the line, so once the line is in
pihole.log the hook has run as well.

The test now polls pihole.log for that line, up to five seconds, with
`pihole-FTL wait-for` from the size of the log before the queries.

Signed-off-by: 010011110 <duckenheim@posteo.de>
@010011110
010011110 force-pushed the duplicate-test-no-sleep branch from 8961110 to d1160aa Compare October 26, 2026 20:56
@010011110 010011110 changed the title Drop the wait from the duplicate reply test Poll for the duplicate reply in its test instead of waiting two seconds Oct 26, 2026
@010011110

Copy link
Copy Markdown
Contributor Author

done, squashed into one commit on top of current development: the test polls pihole.log for the duplicate line with wait-for, up to five seconds. _log_query() calls FTL_hook() before it writes the line, so the hook has run once the line is there, also with the writer thread of #3220

@DL6ER
DL6ER merged commit 9075c18 into pi-hole:development Oct 26, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants