Pre-vote campaign #474

SimonRichardson · 2021-09-20T10:09:25Z

The following attempts to implement a pre-voting campaign optimization.

One downside of Raft’s leader election algorithm is that a server that has been partitioned from the
cluster is likely to cause a disruption when it regains connectivity. When a server is partitioned, it
will not receive heartbeats. It will soon increment its term to start an election, although it won’t
be able to collect enough votes to become leader. When the server regains connectivity sometime
later, its larger term number will propagate to the rest of the cluster (either through the server’s
RequestVote requests or through its AppendEntries response). This will force the cluster leader to
step down, and a new election will have to take place to select a new leader. Fortunately, such events
are likely to be rare, and each will only cause one leader to step down.

rbhitchcock · 2021-10-07T15:08:40Z

Appears to resolve #31

hermanbanken · 2021-10-23T15:09:31Z

Nice work! Hopefully PreVote solves the issue - when running in Kubernetes - that when StatefulSet pods are restarting they will trigger an election because their DNS entry is not immediately available and they will start sending RequestVote's which will trigger a read-only-phase/election.

Any idea how this relates to issues remaining without CheckQuorum? I found the mention of this great article in #31:
https://decentralizedthoughts.github.io/2020-12-12-raft-liveness-full-omission/

The situation explained there can happen easily on a Kubernetes StatefulSet too. However, maybe CheckQuorum is already implemented in this implementation due to the gRPC transport? Do you know this?

SimonRichardson · 2021-11-08T20:13:49Z

@hermanbanken I don't believe checkQuorum is done in hashicorp/raft. I do have a rough idea of how to implement it, but I need to find time to be able to do it (as is life).

stale · 2022-01-09T01:34:47Z

Hey there,
We wanted to check in on this request since it has been inactive for at least 90 days.
Have you reviewed the latest godocs?
If you think this is still an important issue in the latest version of the Raft library or
its documentation please feel let us know and we'll keep it open for investigation.
If there is still no activity on this request in 30 days, we will go ahead and close it.
Thank you!

hermanbanken · 2022-01-10T06:21:49Z

Still relevant

markan · 2022-03-03T22:18:42Z

raft.go

 	// Create a response channel
 	respCh := make(chan *voteResult, len(r.configurations.latest.Servers))

+	// PreVote campaigns are sent on the next term.
+	term := r.getCurrentTerm() + 1
+	if preVote {


I'm confused about this part. We increment the current term by two if preVote is true; I would have thought we would leave it unchanged.

Part of my confusion I'm sure is because I've not found a very detailed description of Pre-Vote in the literature.

In Ongaro's thesis Pre-Vote is described as 'In the Pre-Vote algorithm, a candidate only increments its term if it first learns from a majority of the cluster that they would be willing to grant the candidate their votes' (9.6, pg 137 in my copy). Elsewhere (4.2.3 disruptive servers) "Only if the candidate believed it could get votes from a majority of the cluster would it increment its term and start a normal election."

Looking at the algorithm description in "4 modifications for Raft consensus" I think the intention is to increment by one, but they also talk about the need to avoid 'term inflation' and how this is avoided by Pre-Vote, which I freely admit I don't fully understand how to reconcile.

I'm still looking at other implementations for insight.

Part of my confusion I'm sure is because I've not found a very detailed description of Pre-Vote in the literature.

This took some effort to actually originally page this in as not to disrupt the original implementation, I'll have to look over my old notes about this.

Sorry, I can't be much more helpful atm.

Hey @SimonRichardson, @markan,
my understanding based on the same literature you are reading @markan is that a node should not increment it's own term for pre-vote to avoid term inflation but still need to send a pre-vote request with a term equal to what it would send if asking for a normal vote.
Based on this, I think the correct way of dealing with this in here is to only setCurrentTerm if preVote is false and only increment the term once. That way the node will send the exact same term for pre-vote and for normal vote if it decide to start an election after the pre-vote.
pre-vote should not increment the node term.

Based on this, I think the correct way of dealing with this in here is to only setCurrentTerm if preVote is false and only increment the term once. That way the node will send the exact same term for pre-vote and for normal vote if it decide to start an election after the pre-vote.

It seems like this is correct. I've spent some time reading back through this. Originally my notes say that it should be sent on the next term, but reading back through the papers we should send this on the same term as a normal vote.

hashicorp-cla · 2022-03-12T16:37:14Z

All committers have signed the CLA.

SimonRichardson · 2022-03-15T17:23:18Z

@markan When I get a chance, I'll rebase against main and update the term update.

This is a very rough draft on adding pre-voting (see 9.6 of the raft thesis) to raft. The commit is just familiarising the internal code base before ensuring the correct implementation. In the Pre-Vote algorithm, a candidate only increments its term if it first learns from a majority of the cluster that they would be willing to grant the candidate their votes. Thus saving any potential disruption when a server attempts to rejoin.

To ensure we haven't regressed any existing tests, we run the full test suite with PreVoting campaign off and on. Although not ideal, any new and old tests are run with both PreVoting on and off. It would be better to have some sort of test suite for this kind of thing, yet we can replicate that same behaviour here.

Ensuring we send prevotes on the same term, prevents term inflation. Although not bad in general, it prevents issues in large scale deployments where terms can be exhausted over long periods of time.

SimonRichardson · 2022-04-07T14:19:30Z

@markan this was rebased and fixed to just increment the term by 1 for all queries to prevent hyperinflation of terms.

danmilon · 2022-04-28T08:31:05Z

Thanks a lot for working on this @SimonRichardson. We're running a lot of vault clusters and are getting bitten by this very often. I hope @markan gets a chance to complete the review soon.

manadart · 2022-06-17T12:26:17Z

Any movement on this one? Love to see it landed.

stale · 2022-09-21T04:45:27Z

Hey there,
We wanted to check in on this request since it has been inactive for at least 90 days.
Have you reviewed the latest godocs?
If you think this is still an important issue in the latest version of the Raft library or
its documentation please feel let us know and we'll keep it open for investigation.
If there is still no activity on this request in 30 days, we will go ahead and close it.
Thank you!

hermanbanken · 2022-09-21T06:28:40Z

Still relevant!

banks

This optimization is something I've wanted for a long time. Thanks @SimonRichardson for contributing.

I might be missunderstanding (end of a long day) but I had a few questions inline. It seems like this doesn't actually implement the PreVote optimization as I understood it and I'm not clear that it actually prevents the problems it's designed to.

I'll look again soon, but if you can point out anything I've missed that would be great!

banks · 2022-09-29T17:22:13Z

raft.go

 	// Create a response channel
 	respCh := make(chan *voteResult, len(r.configurations.latest.Servers))

 	// Increment the term
+	// PreVote campaigns are sent on the same term.


I'm confused, the comment points out that PreVote shouldn't increment the term (true) but the code still does increment the term even if preVote == true.

Am I reading this wrong? If we are still incrementing the term then I don't think this is a working implementation of PreVote because this candidate will still increment it's term as long as it's disconnected and then will still disrupt the cluster with a newer term when it is reconnected right?

Or did I missunderstand?

ncabatoff requested a review from markan September 27, 2021 19:13

ducan-ne approved these changes Oct 19, 2021

View reviewed changes

stale bot added the waiting-reply label Jan 9, 2022

stale bot removed the waiting-reply label Jan 10, 2022

markan reviewed Mar 3, 2022

View reviewed changes

SimonRichardson added 3 commits March 15, 2022 17:36

Ensure we send PreVotes on the same term

00112c5

Ensuring we send prevotes on the same term, prevents term inflation. Although not bad in general, it prevents issues in large scale deployments where terms can be exhausted over long periods of time.

SimonRichardson force-pushed the pre-vote-campaign branch from 0be5ab7 to 00112c5 Compare March 15, 2022 17:38

dhiaayachi mentioned this pull request Sep 14, 2022

check if server is in configuration when receiving a voteRequest #526

Merged

stale bot added the waiting-reply label Sep 21, 2022

banks reviewed Sep 29, 2022

View reviewed changes

stale bot removed the waiting-reply label Sep 29, 2022

SimonRichardson closed this Aug 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pre-vote campaign #474

Pre-vote campaign #474

SimonRichardson commented Sep 20, 2021

rbhitchcock commented Oct 7, 2021

hermanbanken commented Oct 23, 2021 •

edited

Loading

SimonRichardson commented Nov 8, 2021

stale bot commented Jan 9, 2022

hermanbanken commented Jan 10, 2022

markan Mar 3, 2022

SimonRichardson Mar 4, 2022

dhiaayachi Mar 15, 2022

SimonRichardson Mar 15, 2022

hashicorp-cla commented Mar 12, 2022 •

edited

Loading

SimonRichardson commented Mar 15, 2022

SimonRichardson commented Apr 7, 2022

danmilon commented Apr 28, 2022

manadart commented Jun 17, 2022

stale bot commented Sep 21, 2022

hermanbanken commented Sep 21, 2022

banks left a comment

banks Sep 29, 2022

Pre-vote campaign #474

Pre-vote campaign #474

Conversation

SimonRichardson commented Sep 20, 2021

rbhitchcock commented Oct 7, 2021

hermanbanken commented Oct 23, 2021 • edited Loading

SimonRichardson commented Nov 8, 2021

stale bot commented Jan 9, 2022

hermanbanken commented Jan 10, 2022

markan Mar 3, 2022

Choose a reason for hiding this comment

SimonRichardson Mar 4, 2022

Choose a reason for hiding this comment

dhiaayachi Mar 15, 2022

Choose a reason for hiding this comment

SimonRichardson Mar 15, 2022

Choose a reason for hiding this comment

hashicorp-cla commented Mar 12, 2022 • edited Loading

SimonRichardson commented Mar 15, 2022

SimonRichardson commented Apr 7, 2022

danmilon commented Apr 28, 2022

manadart commented Jun 17, 2022

stale bot commented Sep 21, 2022

hermanbanken commented Sep 21, 2022

banks left a comment

Choose a reason for hiding this comment

banks Sep 29, 2022

Choose a reason for hiding this comment

hermanbanken commented Oct 23, 2021 •

edited

Loading

hashicorp-cla commented Mar 12, 2022 •

edited

Loading