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

dnsdist: Slightly reduce contention around a pool's servers #11852

Merged
merged 2 commits into from
Aug 16, 2022

Conversation

rgacogne
Copy link
Member

Short description

We only need to take the lock to get the shared pointer, as the actual content is guaranteed not to change, so we do not need to hold the lock while we iterate over the servers list to check whether they are up, or what their current outstanding count is.

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)

We only need to take the lock to get the shared pointer, as the
actual content is guaranteed not to change, so we do not need to
hold the lock while we iterate over the servers list to check
whether they are up, or what their current outstanding count is.
@rgacogne rgacogne added this to the dnsdist-1.8.0 milestone Aug 12, 2022
@omoerbeek omoerbeek self-assigned this Aug 16, 2022
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.

You might consider start using std::shared_ptr<const ServerPolicy::NumberedServerVector> From a quick look, this will need a few changes here and there, but not too much. The big benefit is it will make clear that the vector does not change. It might even prevent mistakes...
But OK anyway.

@rgacogne
Copy link
Member Author

Good idea, thanks, I'll give it a try!

@rgacogne
Copy link
Member Author

Done!

@omoerbeek omoerbeek removed their assignment Aug 16, 2022
@rgacogne rgacogne merged commit f523281 into PowerDNS:master Aug 16, 2022
@rgacogne rgacogne deleted the ddist-less-contention-servers branch August 16, 2022 12:44
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.

2 participants