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

Deprecate Staging suffrage and Promote command #486

Merged
merged 2 commits into from
Jan 24, 2022

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Jan 17, 2022

AddStaging and Promote were never used. raft-autopilot handles this promotion without the need for the extra suffrage type.

This PR deprecates these constants in a backwards compatible way. It also updates the godoc to remove misleading suggestions of code that does not exist. The old TODO is also removes since we no longer plan to do it.

Replace it with AddVoter, which is how this has worked for the last 5 years.

This change should be backwards compatible. The old commands still work, but are now an
alias for AddVoter.
Comment on lines +182 to +191
// AddVoter: was missing.
{Configuration{}, AddVoter, 1, "{[{Voter id1 addr1}]}"},
{singleServer, AddVoter, 2, "{[{Voter id1 addr1x} {Voter id2 addr2}]}"},
// AddVoter: was Voter.
{singleServer, AddVoter, 1, "{[{Voter id1 addr1}]}"},
// AddVoter: was Staging.
{oneOfEach, AddVoter, 2, "{[{Voter id1 addr1x} {Voter id2 addr2} {Nonvoter id3 addr3x}]}"},
// AddVoter: was Nonvoter.
{oneOfEach, AddVoter, 3, "{[{Voter id1 addr1x} {Staging id2 addr2x} {Voter id3 addr3}]}"},

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 is just copy/paste of the AddStaging test cases above, with the second field change. I left the original test cases to show backwards compat.

r.logger.Warn("clearing log suffix",
"from", entry.Index,
"to", lastLogIdx)
r.logger.Warn("clearing log suffix", "from", entry.Index, "to", lastLogIdx)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated cleanup I had done while reading the code. This log line fits pretty easily on one line.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've seen this done stylistically to better show the pairing of variadic args (which I think reads better but this one is short enough not to be confusing)

Comment on lines +102 to +104
// AddStaging makes a server a Voter.
// Deprecated: AddStaging was actually AddVoter. Use AddVoter instead.
AddStaging = 0 // explicit 0 to preserve the old value.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By keeping the constant value 0 it should be safe to upgrade raft. Because any existing clients will send a 0 in the RPC message, and new clients will also send a 0 using the new AddVoter constant.

@dnephin dnephin added the docs label Jan 17, 2022
@dnephin dnephin merged commit ba08869 into main Jan 24, 2022
@dnephin dnephin deleted the dnephin/deprecate-staging branch January 24, 2022 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants