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

Fix recursor not responsive after Lua config reload #11850

Conversation

fredmorcos
Copy link
Contributor

@fredmorcos fredmorcos commented Aug 11, 2022

Short description

  • Avoid reloading Lua config if it hasn't changed
  • Asynchronously destroys dnstap connections from the old config in case of reload

Fixes #11795

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)

@fredmorcos fredmorcos added the rec label Aug 11, 2022
@fredmorcos fredmorcos self-assigned this Aug 11, 2022
@fredmorcos fredmorcos marked this pull request as ready for review August 11, 2022 22:03
Copy link
Member

@omoerbeek omoerbeek left a comment

Choose a reason for hiding this comment

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

It does compile with--disable-dnstap. But:
FrameStreamExportConfig and its methods can be moved into rec-main.cc
struct FrameStreamServersInfo can be moved into rec-main.hh inside the existing HAVE_FSTRM block.
and then check again if it compile both with --disable-dnstap and ``--enable-dnstap`

pdns/rec-lua-conf.cc Outdated Show resolved Hide resolved
pdns/recursordist/rec-main.cc Show resolved Hide resolved
pdns/recursordist/rec-main.cc Outdated Show resolved Hide resolved
@omoerbeek omoerbeek added this to the rec-4.8.0 milestone Aug 12, 2022
This also groups together 1) the list of frame stream servers, 2) the config from which
the list was created and 3) the config's generation into a single struct called
FrameStreamServersInfo. The struct is used to compare the old and new configuration to
decide whether to destroy the old config object or not.

Part of PowerDNS#11795
@fredmorcos fredmorcos force-pushed the fred/11795-fix-recursor-not-responsive-after-config-reload branch from 2ec9780 to 2e0757d Compare August 12, 2022 11:00
@omoerbeek omoerbeek self-requested a review August 12, 2022 13:04
@omoerbeek omoerbeek merged commit 7b246dd into PowerDNS:master Aug 16, 2022
@fredmorcos fredmorcos deleted the fred/11795-fix-recursor-not-responsive-after-config-reload branch August 23, 2022 07:32
omoerbeek added a commit that referenced this pull request Aug 24, 2022
Backport #11850 (Fix recursor not responsive after Lua config reload) to rec 4.7.x
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.

Recursor stops responding after rec_control reload-lua-config
2 participants