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: Add setTCPUseSinglePipe() to use a single TCP waiting queue #4817

Merged
merged 2 commits into from Jan 30, 2017

Conversation

rgacogne
Copy link
Member

Short description

By default, every TCP worker thread has its own queue, and incoming TCP connections are dispatched to TCP workers on a round-robin basis.
This might cause issues if some connections are taking a very long time, since incoming ones will be waiting until the TCP worker they have been assigned to has finished handling its current query, while other TCP workers might be available.
This experimental setTCPUseSinglePipe(true) directive can be used so that all the incoming TCP connections are put into a single queue and handled by thefirst TCP worker available.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code

Copy link
Contributor

@RobinGeuze RobinGeuze left a comment

Choose a reason for hiding this comment

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

Some small comments, but overal looks good. I'm curious how well a single pipe works when reading from it from multiple threads.

public:

TCPClientCollection(size_t maxThreads)
TCPClientCollection(size_t maxThreads, bool useSinglePipe=false): d_maxthreads(maxThreads), d_useSinglePipe(useSinglePipe)
{
d_maxthreads = maxThreads;
Copy link
Contributor

Choose a reason for hiding this comment

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

Bit pointless to leave this here with the constructor initialisation list in place.

}
}
else {
d_singlePipe[0] = -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be more consistent to preinitialize d_singlePipe to -1 for both, in that case you can do away with this else.

@rgacogne rgacogne force-pushed the dnsdist-tcp-single-pipe branch 3 times, most recently from 9262b65 to 2ec61e4 Compare January 19, 2017 19:51
By default, every TCP worker thread has its own queue, and incoming TCP
connections are dispatched to TCP workers on a round-robin basis. This might
cause issues if some connections are taking a very long time, since incoming
ones will be waiting until the TCP worker they have been assigned to has finished
handling its current query, while other TCP workers might be available.
This experimental `setTCPUseSinglePipe(true)` directive can be used so that all the
incoming TCP connections are put into a single queue and handled by the
first TCP worker available.
@pieterlexis pieterlexis merged commit 06fef27 into PowerDNS:master Jan 30, 2017
@rgacogne rgacogne deleted the dnsdist-tcp-single-pipe branch January 30, 2017 16:03
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.

None yet

3 participants