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

Set MinQuorum variable in Autopilot #6654

Merged
merged 8 commits into from
Oct 29, 2019
Merged

Set MinQuorum variable in Autopilot #6654

merged 8 commits into from
Oct 29, 2019

Conversation

schristoff
Copy link
Contributor

@schristoff schristoff commented Oct 18, 2019

Fixes #5922
MinQuorum allows users to pass in the number of servers needed in a cluster before pruning of dead servers in autopilot can occur.

@schristoff schristoff requested review from a team October 18, 2019 23:04
agent/config/builder.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mikemorris mikemorris left a comment

Choose a reason for hiding this comment

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

One question on possible test flappiness but overall looks great!

agent/consul/autopilot_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@freddygv freddygv left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -782,6 +782,9 @@ default will automatically work with some tooling.
the maximum number of log entries that a server can trail the leader by before being considered unhealthy. Defaults
to 250.

* <a name="min_quorum"></a><a href="#min_quorum">`min_quorum`</a> - Sets the minimum number of servers allowed in a cluster
Copy link
Contributor

Choose a reason for hiding this comment

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

Allowed doesn't quite fit, maybe again use "needed" or "necessary". Is cluster the right term? Also is there a default? If there isn't a default, then you can say so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sound better?

Sets the minimum number of servers necessary in a cluster
      before autopilot can prune dead servers. There is no default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know the proper way to say there isn't a default, but I appreciate the "necessary" wording suggestion. It sounds way more clear then "allowed"

schristoff and others added 5 commits October 28, 2019 14:43
Co-Authored-By: kaitlincarter-hc <43049322+kaitlincarter-hc@users.noreply.github.com>
Co-Authored-By: kaitlincarter-hc <43049322+kaitlincarter-hc@users.noreply.github.com>
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.

Auto Pilot breaks Quorum Invariant
5 participants