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 optional UUID column to showServers() #7191

Merged
merged 2 commits into from
Nov 15, 2018

Conversation

chbruyand
Copy link
Member

Short description

This adds an optional UUID column to the showServers() output. This PR also tries to clarify what parameters the chashed distribution is based on.

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)

Copy link
Member

@rgacogne rgacogne left a comment

Choose a reason for hiding this comment

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

Looks good! It looks like the update to the completion code is missing, though.

@@ -48,7 +48,7 @@ The current hash algorithm is based on the qname of the query.

``chashed`` is a consistent hashing distribution policy. Identical questions with identical hashes will be distributed to the same servers. But unlike the ``whashed`` policy, this distribution will keep consistent over time. Adding or removing servers will only remap a small part of the queries.

You can also set the hash perturbation value, see :func:`setWHashedPertubation`.
You can also set the hash perturbation value, see :func:`setWHashedPertubation`. To achieve consistent distribution over :program:`dnsdist` restarts, you will also need to explicitly set the backend's UUIDs with the ``id`` option of :func:`newServer`. You can get the current UUIDs of your backends by calling :func:`showServers`.
Copy link
Member

Choose a reason for hiding this comment

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

We might want to hint at the showUUIDs option here?

@@ -321,6 +321,7 @@ Servers

newServer({
address="IP:PORT", -- IP and PORT of the backend server (mandatory)
id=STRING -- Use a pre-defined UUID instead of a random one
Copy link
Member

Choose a reason for hiding this comment

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

A comma is missing

@chbruyand
Copy link
Member Author

Last commit should have addressed your comments. Thanks for the review!

@rgacogne rgacogne merged commit 6c50850 into PowerDNS:master Nov 15, 2018
@chbruyand chbruyand deleted the dnsdist-backend-uuid branch November 15, 2018 14:07
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

2 participants