RATIS-993. Support pre vote - #161

Merged
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify
Jan 6, 2021
Merged

RATIS-993. Support pre vote#161
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify

Conversation

@runzhiwang

@runzhiwangrunzhiwang commented Aug 2, 2020

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  1. Implement Preventing disruptions when a server rejoins the cluster according to raft paper section 9.6.

  2. In ozone, rejoin happen easily because the unstable network, for example sometimes one server's network is slow, it will try to trigger election, but which is unnecessary, this can be prevented by this PR.

  3. In this PR, before follower trigger leader election, it send pre-vote request to the other servers, if other server allow the follower to trigger leader election, it reply PASSED to the pre-vote request. If majority of the other servers allow the follower trigger leader election, the follower will trigger leader election.

  4. The other server allow the follower to trigger leader election only when 1. the voters have not received heartbeats from a valid leader for at least a baseline election timeout. and 2. the follower's log is newer than voter's, or the follower's log is equal to voter's and follower's priority is not less than voter's

  5. If the above condition can not satisfied, even though the follower trigger leader election, it can not be elected as a leader but it can step down the leader by it's bigger term, so we do not allow it trigger leader election.

  6. If the follower is not in the conf, we will shutdown the follower.

Details of the paper:

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.
If desired, Raft’s basic leader election algorithm can be extended with an additional phase to
prevent such disruptions, forming the Pre-Vote algorithm. 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 (if the candidate’s log is sufficiently up-to-date, and the voters
have not received heartbeats from a valid leader for at least a baseline election timeout). This was
inspired by ZooKeeper’s algorithm [42], in which a server must receive a majority of votes before
it calculates a new epoch and sends NewEpoch messages (however, in ZooKeeper servers do not
solicit votes, other servers offer them).
The Pre-Vote algorithm solves the issue of a partitioned server disrupting the cluster when it
rejoins. While a server is partitioned, it won’t be able to increment its term, since it can’t receive
permission from a majority of the cluster. Then, when it rejoins the cluster, it still won’t be able
to increment its term, since the other servers will have been receiving regular heartbeats from the
leader. Once the server receives a heartbeat from the leader itself, it will return to the follower state
(in the same term).
We recommend the Pre-Vote extension in deployments that would benefit from additional robustness.
We also tested it in various leader election scenarios in AvailSim, and it does not appear
to significantly harm election performance.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-993

How was this patch tested?

add ut: testPreVote

@runzhiwangrunzhiwang reopened this Aug 2, 2020
@runzhiwangrunzhiwang reopened this Aug 4, 2020
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo@bshashikant@lokeshj1703 @GlenGeng Could you help review this patch ? Thank you very much.

@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang thanks for working on this.

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo

Copy link
Copy Markdown
Contributor
  1. the follower is not in the conf.

I cannot see this requirement in the paper. Could you give me a pointer?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo Thanks for review. You are right, I did not implement it in this PR because I want to make this PR simpler to review, and I will implement it after this PR. Even though without checkout timeout, it will not introduce new bug, because in this case pre-vote passed and trigger a new leader election. I can also implement it in this PR, it's ok to me. Which one do you prefer ?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

the follower is not in the conf.
I cannot see this requirement in the paper. Could you give me a pointer?

@szetszwo You are right, it does not exist in the paper. I implement it to adapt current ratis implementation. Because in current ratis, when leader election, leader will send shutdown to follower if follower, such as follower1, is not in leader's conf at shouldSendShutdown(candidateId, candidateLastEntry). So I have two choice to make follower1 shutdown: 1.pass the pre-vote in this case, so that follower1 can shutdown in real leader election; 2.pre-vote in charge of shutdown follower1, but this will make pre-vote complicated. So I choose the first option. What do you think?

@GlenGeng-awxGlenGeng-awx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the work!

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang Let's just implement exactly raft paper section 9.6.

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@runzhiwang

runzhiwang commented Aug 6, 2020

Copy link
Copy Markdown
ContributorAuthor

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@szetszwo Thanks for explanation. Do you mean shutdown the follower not in the conf when pre-vote ?

@runzhiwangrunzhiwang reopened this Aug 7, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

Yes, we should shutdown the follower for that group.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Thanks for review. I have updated the patch. Could you help review it again ?

@runzhiwangrunzhiwang changed the title RATIS-993. Preventing disruptions when a server rejoins the clusterRATIS-993. Support pre voteJan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo I have updated this pre-vote patch. Could you help review it ?

@runzhiwangrunzhiwang reopened this Jan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

will fix the failed ut.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for working on this.

  • Terminology: Let's use "Pre-Vote" or "pre-vote" as in https://github.com/ongardie/dissertation
  • There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

Some other comments inlined.

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

@szetszwo Thanks for review. I will refactor the code in next PR.

Besides, when s1 transferLeaderShip to s2, s2 should skip askForPreVote, and askForVote directly. Because when s2 askForPreVote, s1 and s3 will reject it because current leader is still valid. So I add a flag: LeaderElection#force which means we should skip askForPreVote and askForVote directly, this flag will be true when s2 receive StartLeaderElectionRequest.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 the change looks good.

@szetszwo
szetszwo merged commit 35f17fa into apache:masterJan 6, 2021
@szetszwo

Copy link
Copy Markdown
Contributor

... I will refactor the code in next PR.

@runzhiwang , thanks a lot. Since you are working on RATIS-1273, let me do the refactoring.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Sure, thanks a lot.

symious pushed a commit to symious/ratis that referenced this pull request Feb 19, 2024
Sign up for freeto 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.

3 participants

@runzhiwang@szetszwo@GlenGeng-awx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

RATIS-993. Support pre vote - #161

Merged
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify
Jan 6, 2021
Merged

RATIS-993. Support pre vote#161
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify

Conversation

@runzhiwang

@runzhiwangrunzhiwang commented Aug 2, 2020

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  1. Implement Preventing disruptions when a server rejoins the cluster according to raft paper section 9.6.

  2. In ozone, rejoin happen easily because the unstable network, for example sometimes one server's network is slow, it will try to trigger election, but which is unnecessary, this can be prevented by this PR.

  3. In this PR, before follower trigger leader election, it send pre-vote request to the other servers, if other server allow the follower to trigger leader election, it reply PASSED to the pre-vote request. If majority of the other servers allow the follower trigger leader election, the follower will trigger leader election.

  4. The other server allow the follower to trigger leader election only when 1. the voters have not received heartbeats from a valid leader for at least a baseline election timeout. and 2. the follower's log is newer than voter's, or the follower's log is equal to voter's and follower's priority is not less than voter's

  5. If the above condition can not satisfied, even though the follower trigger leader election, it can not be elected as a leader but it can step down the leader by it's bigger term, so we do not allow it trigger leader election.

  6. If the follower is not in the conf, we will shutdown the follower.

Details of the paper:

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.
If desired, Raft’s basic leader election algorithm can be extended with an additional phase to
prevent such disruptions, forming the Pre-Vote algorithm. 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 (if the candidate’s log is sufficiently up-to-date, and the voters
have not received heartbeats from a valid leader for at least a baseline election timeout). This was
inspired by ZooKeeper’s algorithm [42], in which a server must receive a majority of votes before
it calculates a new epoch and sends NewEpoch messages (however, in ZooKeeper servers do not
solicit votes, other servers offer them).
The Pre-Vote algorithm solves the issue of a partitioned server disrupting the cluster when it
rejoins. While a server is partitioned, it won’t be able to increment its term, since it can’t receive
permission from a majority of the cluster. Then, when it rejoins the cluster, it still won’t be able
to increment its term, since the other servers will have been receiving regular heartbeats from the
leader. Once the server receives a heartbeat from the leader itself, it will return to the follower state
(in the same term).
We recommend the Pre-Vote extension in deployments that would benefit from additional robustness.
We also tested it in various leader election scenarios in AvailSim, and it does not appear
to significantly harm election performance.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-993

How was this patch tested?

add ut: testPreVote

@runzhiwangrunzhiwang reopened this Aug 2, 2020
@runzhiwangrunzhiwang reopened this Aug 4, 2020
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo@bshashikant@lokeshj1703 @GlenGeng Could you help review this patch ? Thank you very much.

@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang thanks for working on this.

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo

Copy link
Copy Markdown
Contributor
  1. the follower is not in the conf.

I cannot see this requirement in the paper. Could you give me a pointer?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo Thanks for review. You are right, I did not implement it in this PR because I want to make this PR simpler to review, and I will implement it after this PR. Even though without checkout timeout, it will not introduce new bug, because in this case pre-vote passed and trigger a new leader election. I can also implement it in this PR, it's ok to me. Which one do you prefer ?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

the follower is not in the conf.
I cannot see this requirement in the paper. Could you give me a pointer?

@szetszwo You are right, it does not exist in the paper. I implement it to adapt current ratis implementation. Because in current ratis, when leader election, leader will send shutdown to follower if follower, such as follower1, is not in leader's conf at shouldSendShutdown(candidateId, candidateLastEntry). So I have two choice to make follower1 shutdown: 1.pass the pre-vote in this case, so that follower1 can shutdown in real leader election; 2.pre-vote in charge of shutdown follower1, but this will make pre-vote complicated. So I choose the first option. What do you think?

@GlenGeng-awxGlenGeng-awx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the work!

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang Let's just implement exactly raft paper section 9.6.

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@runzhiwang

runzhiwang commented Aug 6, 2020

Copy link
Copy Markdown
ContributorAuthor

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@szetszwo Thanks for explanation. Do you mean shutdown the follower not in the conf when pre-vote ?

@runzhiwangrunzhiwang reopened this Aug 7, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

Yes, we should shutdown the follower for that group.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Thanks for review. I have updated the patch. Could you help review it again ?

@runzhiwangrunzhiwang changed the title RATIS-993. Preventing disruptions when a server rejoins the clusterRATIS-993. Support pre voteJan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo I have updated this pre-vote patch. Could you help review it ?

@runzhiwangrunzhiwang reopened this Jan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

will fix the failed ut.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for working on this.

  • Terminology: Let's use "Pre-Vote" or "pre-vote" as in https://github.com/ongardie/dissertation
  • There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

Some other comments inlined.

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

@szetszwo Thanks for review. I will refactor the code in next PR.

Besides, when s1 transferLeaderShip to s2, s2 should skip askForPreVote, and askForVote directly. Because when s2 askForPreVote, s1 and s3 will reject it because current leader is still valid. So I add a flag: LeaderElection#force which means we should skip askForPreVote and askForVote directly, this flag will be true when s2 receive StartLeaderElectionRequest.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 the change looks good.

@szetszwo
szetszwo merged commit 35f17fa into apache:masterJan 6, 2021
@szetszwo

Copy link
Copy Markdown
Contributor

... I will refactor the code in next PR.

@runzhiwang , thanks a lot. Since you are working on RATIS-1273, let me do the refactoring.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Sure, thanks a lot.

symious pushed a commit to symious/ratis that referenced this pull request Feb 19, 2024
Sign up for freeto 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.

3 participants

@runzhiwang@szetszwo@GlenGeng-awx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

RATIS-993. Support pre vote - #161

Merged
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify
Jan 6, 2021
Merged

RATIS-993. Support pre vote#161
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify

Conversation

@runzhiwang

@runzhiwangrunzhiwang commented Aug 2, 2020

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  1. Implement Preventing disruptions when a server rejoins the cluster according to raft paper section 9.6.

  2. In ozone, rejoin happen easily because the unstable network, for example sometimes one server's network is slow, it will try to trigger election, but which is unnecessary, this can be prevented by this PR.

  3. In this PR, before follower trigger leader election, it send pre-vote request to the other servers, if other server allow the follower to trigger leader election, it reply PASSED to the pre-vote request. If majority of the other servers allow the follower trigger leader election, the follower will trigger leader election.

  4. The other server allow the follower to trigger leader election only when 1. the voters have not received heartbeats from a valid leader for at least a baseline election timeout. and 2. the follower's log is newer than voter's, or the follower's log is equal to voter's and follower's priority is not less than voter's

  5. If the above condition can not satisfied, even though the follower trigger leader election, it can not be elected as a leader but it can step down the leader by it's bigger term, so we do not allow it trigger leader election.

  6. If the follower is not in the conf, we will shutdown the follower.

Details of the paper:

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.
If desired, Raft’s basic leader election algorithm can be extended with an additional phase to
prevent such disruptions, forming the Pre-Vote algorithm. 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 (if the candidate’s log is sufficiently up-to-date, and the voters
have not received heartbeats from a valid leader for at least a baseline election timeout). This was
inspired by ZooKeeper’s algorithm [42], in which a server must receive a majority of votes before
it calculates a new epoch and sends NewEpoch messages (however, in ZooKeeper servers do not
solicit votes, other servers offer them).
The Pre-Vote algorithm solves the issue of a partitioned server disrupting the cluster when it
rejoins. While a server is partitioned, it won’t be able to increment its term, since it can’t receive
permission from a majority of the cluster. Then, when it rejoins the cluster, it still won’t be able
to increment its term, since the other servers will have been receiving regular heartbeats from the
leader. Once the server receives a heartbeat from the leader itself, it will return to the follower state
(in the same term).
We recommend the Pre-Vote extension in deployments that would benefit from additional robustness.
We also tested it in various leader election scenarios in AvailSim, and it does not appear
to significantly harm election performance.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-993

How was this patch tested?

add ut: testPreVote

@runzhiwangrunzhiwang reopened this Aug 2, 2020
@runzhiwangrunzhiwang reopened this Aug 4, 2020
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo@bshashikant@lokeshj1703 @GlenGeng Could you help review this patch ? Thank you very much.

@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang thanks for working on this.

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo

Copy link
Copy Markdown
Contributor
  1. the follower is not in the conf.

I cannot see this requirement in the paper. Could you give me a pointer?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo Thanks for review. You are right, I did not implement it in this PR because I want to make this PR simpler to review, and I will implement it after this PR. Even though without checkout timeout, it will not introduce new bug, because in this case pre-vote passed and trigger a new leader election. I can also implement it in this PR, it's ok to me. Which one do you prefer ?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

the follower is not in the conf.
I cannot see this requirement in the paper. Could you give me a pointer?

@szetszwo You are right, it does not exist in the paper. I implement it to adapt current ratis implementation. Because in current ratis, when leader election, leader will send shutdown to follower if follower, such as follower1, is not in leader's conf at shouldSendShutdown(candidateId, candidateLastEntry). So I have two choice to make follower1 shutdown: 1.pass the pre-vote in this case, so that follower1 can shutdown in real leader election; 2.pre-vote in charge of shutdown follower1, but this will make pre-vote complicated. So I choose the first option. What do you think?

@GlenGeng-awxGlenGeng-awx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the work!

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang Let's just implement exactly raft paper section 9.6.

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@runzhiwang

runzhiwang commented Aug 6, 2020

Copy link
Copy Markdown
ContributorAuthor

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@szetszwo Thanks for explanation. Do you mean shutdown the follower not in the conf when pre-vote ?

@runzhiwangrunzhiwang reopened this Aug 7, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

Yes, we should shutdown the follower for that group.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Thanks for review. I have updated the patch. Could you help review it again ?

@runzhiwangrunzhiwang changed the title RATIS-993. Preventing disruptions when a server rejoins the clusterRATIS-993. Support pre voteJan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo I have updated this pre-vote patch. Could you help review it ?

@runzhiwangrunzhiwang reopened this Jan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

will fix the failed ut.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for working on this.

  • Terminology: Let's use "Pre-Vote" or "pre-vote" as in https://github.com/ongardie/dissertation
  • There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

Some other comments inlined.

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

@szetszwo Thanks for review. I will refactor the code in next PR.

Besides, when s1 transferLeaderShip to s2, s2 should skip askForPreVote, and askForVote directly. Because when s2 askForPreVote, s1 and s3 will reject it because current leader is still valid. So I add a flag: LeaderElection#force which means we should skip askForPreVote and askForVote directly, this flag will be true when s2 receive StartLeaderElectionRequest.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 the change looks good.

@szetszwo
szetszwo merged commit 35f17fa into apache:masterJan 6, 2021
@szetszwo

Copy link
Copy Markdown
Contributor

... I will refactor the code in next PR.

@runzhiwang , thanks a lot. Since you are working on RATIS-1273, let me do the refactoring.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Sure, thanks a lot.

symious pushed a commit to symious/ratis that referenced this pull request Feb 19, 2024
Sign up for freeto 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.

3 participants

@runzhiwang@szetszwo@GlenGeng-awx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

RATIS-993. Support pre vote - #161

Merged
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify
Jan 6, 2021
Merged

RATIS-993. Support pre vote#161
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify

Conversation

@runzhiwang

@runzhiwangrunzhiwang commented Aug 2, 2020

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  1. Implement Preventing disruptions when a server rejoins the cluster according to raft paper section 9.6.

  2. In ozone, rejoin happen easily because the unstable network, for example sometimes one server's network is slow, it will try to trigger election, but which is unnecessary, this can be prevented by this PR.

  3. In this PR, before follower trigger leader election, it send pre-vote request to the other servers, if other server allow the follower to trigger leader election, it reply PASSED to the pre-vote request. If majority of the other servers allow the follower trigger leader election, the follower will trigger leader election.

  4. The other server allow the follower to trigger leader election only when 1. the voters have not received heartbeats from a valid leader for at least a baseline election timeout. and 2. the follower's log is newer than voter's, or the follower's log is equal to voter's and follower's priority is not less than voter's

  5. If the above condition can not satisfied, even though the follower trigger leader election, it can not be elected as a leader but it can step down the leader by it's bigger term, so we do not allow it trigger leader election.

  6. If the follower is not in the conf, we will shutdown the follower.

Details of the paper:

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.
If desired, Raft’s basic leader election algorithm can be extended with an additional phase to
prevent such disruptions, forming the Pre-Vote algorithm. 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 (if the candidate’s log is sufficiently up-to-date, and the voters
have not received heartbeats from a valid leader for at least a baseline election timeout). This was
inspired by ZooKeeper’s algorithm [42], in which a server must receive a majority of votes before
it calculates a new epoch and sends NewEpoch messages (however, in ZooKeeper servers do not
solicit votes, other servers offer them).
The Pre-Vote algorithm solves the issue of a partitioned server disrupting the cluster when it
rejoins. While a server is partitioned, it won’t be able to increment its term, since it can’t receive
permission from a majority of the cluster. Then, when it rejoins the cluster, it still won’t be able
to increment its term, since the other servers will have been receiving regular heartbeats from the
leader. Once the server receives a heartbeat from the leader itself, it will return to the follower state
(in the same term).
We recommend the Pre-Vote extension in deployments that would benefit from additional robustness.
We also tested it in various leader election scenarios in AvailSim, and it does not appear
to significantly harm election performance.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-993

How was this patch tested?

add ut: testPreVote

@runzhiwangrunzhiwang reopened this Aug 2, 2020
@runzhiwangrunzhiwang reopened this Aug 4, 2020
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo@bshashikant@lokeshj1703 @GlenGeng Could you help review this patch ? Thank you very much.

@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang thanks for working on this.

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo

Copy link
Copy Markdown
Contributor
  1. the follower is not in the conf.

I cannot see this requirement in the paper. Could you give me a pointer?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo Thanks for review. You are right, I did not implement it in this PR because I want to make this PR simpler to review, and I will implement it after this PR. Even though without checkout timeout, it will not introduce new bug, because in this case pre-vote passed and trigger a new leader election. I can also implement it in this PR, it's ok to me. Which one do you prefer ?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

the follower is not in the conf.
I cannot see this requirement in the paper. Could you give me a pointer?

@szetszwo You are right, it does not exist in the paper. I implement it to adapt current ratis implementation. Because in current ratis, when leader election, leader will send shutdown to follower if follower, such as follower1, is not in leader's conf at shouldSendShutdown(candidateId, candidateLastEntry). So I have two choice to make follower1 shutdown: 1.pass the pre-vote in this case, so that follower1 can shutdown in real leader election; 2.pre-vote in charge of shutdown follower1, but this will make pre-vote complicated. So I choose the first option. What do you think?

@GlenGeng-awxGlenGeng-awx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the work!

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang Let's just implement exactly raft paper section 9.6.

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@runzhiwang

runzhiwang commented Aug 6, 2020

Copy link
Copy Markdown
ContributorAuthor

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@szetszwo Thanks for explanation. Do you mean shutdown the follower not in the conf when pre-vote ?

@runzhiwangrunzhiwang reopened this Aug 7, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

Yes, we should shutdown the follower for that group.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Thanks for review. I have updated the patch. Could you help review it again ?

@runzhiwangrunzhiwang changed the title RATIS-993. Preventing disruptions when a server rejoins the clusterRATIS-993. Support pre voteJan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo I have updated this pre-vote patch. Could you help review it ?

@runzhiwangrunzhiwang reopened this Jan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

will fix the failed ut.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for working on this.

  • Terminology: Let's use "Pre-Vote" or "pre-vote" as in https://github.com/ongardie/dissertation
  • There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

Some other comments inlined.

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

@szetszwo Thanks for review. I will refactor the code in next PR.

Besides, when s1 transferLeaderShip to s2, s2 should skip askForPreVote, and askForVote directly. Because when s2 askForPreVote, s1 and s3 will reject it because current leader is still valid. So I add a flag: LeaderElection#force which means we should skip askForPreVote and askForVote directly, this flag will be true when s2 receive StartLeaderElectionRequest.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 the change looks good.

@szetszwo
szetszwo merged commit 35f17fa into apache:masterJan 6, 2021
@szetszwo

Copy link
Copy Markdown
Contributor

... I will refactor the code in next PR.

@runzhiwang , thanks a lot. Since you are working on RATIS-1273, let me do the refactoring.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Sure, thanks a lot.

symious pushed a commit to symious/ratis that referenced this pull request Feb 19, 2024
Sign up for freeto 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.

3 participants

@runzhiwang@szetszwo@GlenGeng-awx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

RATIS-993. Support pre vote - #161

Merged
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify
Jan 6, 2021
Merged

RATIS-993. Support pre vote#161
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify

Conversation

@runzhiwang

@runzhiwangrunzhiwang commented Aug 2, 2020

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  1. Implement Preventing disruptions when a server rejoins the cluster according to raft paper section 9.6.

  2. In ozone, rejoin happen easily because the unstable network, for example sometimes one server's network is slow, it will try to trigger election, but which is unnecessary, this can be prevented by this PR.

  3. In this PR, before follower trigger leader election, it send pre-vote request to the other servers, if other server allow the follower to trigger leader election, it reply PASSED to the pre-vote request. If majority of the other servers allow the follower trigger leader election, the follower will trigger leader election.

  4. The other server allow the follower to trigger leader election only when 1. the voters have not received heartbeats from a valid leader for at least a baseline election timeout. and 2. the follower's log is newer than voter's, or the follower's log is equal to voter's and follower's priority is not less than voter's

  5. If the above condition can not satisfied, even though the follower trigger leader election, it can not be elected as a leader but it can step down the leader by it's bigger term, so we do not allow it trigger leader election.

  6. If the follower is not in the conf, we will shutdown the follower.

Details of the paper:

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.
If desired, Raft’s basic leader election algorithm can be extended with an additional phase to
prevent such disruptions, forming the Pre-Vote algorithm. 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 (if the candidate’s log is sufficiently up-to-date, and the voters
have not received heartbeats from a valid leader for at least a baseline election timeout). This was
inspired by ZooKeeper’s algorithm [42], in which a server must receive a majority of votes before
it calculates a new epoch and sends NewEpoch messages (however, in ZooKeeper servers do not
solicit votes, other servers offer them).
The Pre-Vote algorithm solves the issue of a partitioned server disrupting the cluster when it
rejoins. While a server is partitioned, it won’t be able to increment its term, since it can’t receive
permission from a majority of the cluster. Then, when it rejoins the cluster, it still won’t be able
to increment its term, since the other servers will have been receiving regular heartbeats from the
leader. Once the server receives a heartbeat from the leader itself, it will return to the follower state
(in the same term).
We recommend the Pre-Vote extension in deployments that would benefit from additional robustness.
We also tested it in various leader election scenarios in AvailSim, and it does not appear
to significantly harm election performance.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-993

How was this patch tested?

add ut: testPreVote

@runzhiwangrunzhiwang reopened this Aug 2, 2020
@runzhiwangrunzhiwang reopened this Aug 4, 2020
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo@bshashikant@lokeshj1703 @GlenGeng Could you help review this patch ? Thank you very much.

@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang thanks for working on this.

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo

Copy link
Copy Markdown
Contributor
  1. the follower is not in the conf.

I cannot see this requirement in the paper. Could you give me a pointer?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo Thanks for review. You are right, I did not implement it in this PR because I want to make this PR simpler to review, and I will implement it after this PR. Even though without checkout timeout, it will not introduce new bug, because in this case pre-vote passed and trigger a new leader election. I can also implement it in this PR, it's ok to me. Which one do you prefer ?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

the follower is not in the conf.
I cannot see this requirement in the paper. Could you give me a pointer?

@szetszwo You are right, it does not exist in the paper. I implement it to adapt current ratis implementation. Because in current ratis, when leader election, leader will send shutdown to follower if follower, such as follower1, is not in leader's conf at shouldSendShutdown(candidateId, candidateLastEntry). So I have two choice to make follower1 shutdown: 1.pass the pre-vote in this case, so that follower1 can shutdown in real leader election; 2.pre-vote in charge of shutdown follower1, but this will make pre-vote complicated. So I choose the first option. What do you think?

@GlenGeng-awxGlenGeng-awx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the work!

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang Let's just implement exactly raft paper section 9.6.

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@runzhiwang

runzhiwang commented Aug 6, 2020

Copy link
Copy Markdown
ContributorAuthor

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@szetszwo Thanks for explanation. Do you mean shutdown the follower not in the conf when pre-vote ?

@runzhiwangrunzhiwang reopened this Aug 7, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

Yes, we should shutdown the follower for that group.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Thanks for review. I have updated the patch. Could you help review it again ?

@runzhiwangrunzhiwang changed the title RATIS-993. Preventing disruptions when a server rejoins the clusterRATIS-993. Support pre voteJan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo I have updated this pre-vote patch. Could you help review it ?

@runzhiwangrunzhiwang reopened this Jan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

will fix the failed ut.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for working on this.

  • Terminology: Let's use "Pre-Vote" or "pre-vote" as in https://github.com/ongardie/dissertation
  • There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

Some other comments inlined.

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

@szetszwo Thanks for review. I will refactor the code in next PR.

Besides, when s1 transferLeaderShip to s2, s2 should skip askForPreVote, and askForVote directly. Because when s2 askForPreVote, s1 and s3 will reject it because current leader is still valid. So I add a flag: LeaderElection#force which means we should skip askForPreVote and askForVote directly, this flag will be true when s2 receive StartLeaderElectionRequest.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 the change looks good.

@szetszwo
szetszwo merged commit 35f17fa into apache:masterJan 6, 2021
@szetszwo

Copy link
Copy Markdown
Contributor

... I will refactor the code in next PR.

@runzhiwang , thanks a lot. Since you are working on RATIS-1273, let me do the refactoring.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Sure, thanks a lot.

symious pushed a commit to symious/ratis that referenced this pull request Feb 19, 2024
Sign up for freeto 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.

3 participants

@runzhiwang@szetszwo@GlenGeng-awx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

RATIS-993. Support pre vote - #161

Merged
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify
Jan 6, 2021
Merged

RATIS-993. Support pre vote#161
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify

Conversation

@runzhiwang

@runzhiwangrunzhiwang commented Aug 2, 2020

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  1. Implement Preventing disruptions when a server rejoins the cluster according to raft paper section 9.6.

  2. In ozone, rejoin happen easily because the unstable network, for example sometimes one server's network is slow, it will try to trigger election, but which is unnecessary, this can be prevented by this PR.

  3. In this PR, before follower trigger leader election, it send pre-vote request to the other servers, if other server allow the follower to trigger leader election, it reply PASSED to the pre-vote request. If majority of the other servers allow the follower trigger leader election, the follower will trigger leader election.

  4. The other server allow the follower to trigger leader election only when 1. the voters have not received heartbeats from a valid leader for at least a baseline election timeout. and 2. the follower's log is newer than voter's, or the follower's log is equal to voter's and follower's priority is not less than voter's

  5. If the above condition can not satisfied, even though the follower trigger leader election, it can not be elected as a leader but it can step down the leader by it's bigger term, so we do not allow it trigger leader election.

  6. If the follower is not in the conf, we will shutdown the follower.

Details of the paper:

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.
If desired, Raft’s basic leader election algorithm can be extended with an additional phase to
prevent such disruptions, forming the Pre-Vote algorithm. 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 (if the candidate’s log is sufficiently up-to-date, and the voters
have not received heartbeats from a valid leader for at least a baseline election timeout). This was
inspired by ZooKeeper’s algorithm [42], in which a server must receive a majority of votes before
it calculates a new epoch and sends NewEpoch messages (however, in ZooKeeper servers do not
solicit votes, other servers offer them).
The Pre-Vote algorithm solves the issue of a partitioned server disrupting the cluster when it
rejoins. While a server is partitioned, it won’t be able to increment its term, since it can’t receive
permission from a majority of the cluster. Then, when it rejoins the cluster, it still won’t be able
to increment its term, since the other servers will have been receiving regular heartbeats from the
leader. Once the server receives a heartbeat from the leader itself, it will return to the follower state
(in the same term).
We recommend the Pre-Vote extension in deployments that would benefit from additional robustness.
We also tested it in various leader election scenarios in AvailSim, and it does not appear
to significantly harm election performance.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-993

How was this patch tested?

add ut: testPreVote

@runzhiwangrunzhiwang reopened this Aug 2, 2020
@runzhiwangrunzhiwang reopened this Aug 4, 2020
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo@bshashikant@lokeshj1703 @GlenGeng Could you help review this patch ? Thank you very much.

@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang thanks for working on this.

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo

Copy link
Copy Markdown
Contributor
  1. the follower is not in the conf.

I cannot see this requirement in the paper. Could you give me a pointer?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo Thanks for review. You are right, I did not implement it in this PR because I want to make this PR simpler to review, and I will implement it after this PR. Even though without checkout timeout, it will not introduce new bug, because in this case pre-vote passed and trigger a new leader election. I can also implement it in this PR, it's ok to me. Which one do you prefer ?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

the follower is not in the conf.
I cannot see this requirement in the paper. Could you give me a pointer?

@szetszwo You are right, it does not exist in the paper. I implement it to adapt current ratis implementation. Because in current ratis, when leader election, leader will send shutdown to follower if follower, such as follower1, is not in leader's conf at shouldSendShutdown(candidateId, candidateLastEntry). So I have two choice to make follower1 shutdown: 1.pass the pre-vote in this case, so that follower1 can shutdown in real leader election; 2.pre-vote in charge of shutdown follower1, but this will make pre-vote complicated. So I choose the first option. What do you think?

@GlenGeng-awxGlenGeng-awx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the work!

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang Let's just implement exactly raft paper section 9.6.

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@runzhiwang

runzhiwang commented Aug 6, 2020

Copy link
Copy Markdown
ContributorAuthor

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@szetszwo Thanks for explanation. Do you mean shutdown the follower not in the conf when pre-vote ?

@runzhiwangrunzhiwang reopened this Aug 7, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

Yes, we should shutdown the follower for that group.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Thanks for review. I have updated the patch. Could you help review it again ?

@runzhiwangrunzhiwang changed the title RATIS-993. Preventing disruptions when a server rejoins the clusterRATIS-993. Support pre voteJan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo I have updated this pre-vote patch. Could you help review it ?

@runzhiwangrunzhiwang reopened this Jan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

will fix the failed ut.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for working on this.

  • Terminology: Let's use "Pre-Vote" or "pre-vote" as in https://github.com/ongardie/dissertation
  • There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

Some other comments inlined.

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

@szetszwo Thanks for review. I will refactor the code in next PR.

Besides, when s1 transferLeaderShip to s2, s2 should skip askForPreVote, and askForVote directly. Because when s2 askForPreVote, s1 and s3 will reject it because current leader is still valid. So I add a flag: LeaderElection#force which means we should skip askForPreVote and askForVote directly, this flag will be true when s2 receive StartLeaderElectionRequest.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 the change looks good.

@szetszwo
szetszwo merged commit 35f17fa into apache:masterJan 6, 2021
@szetszwo

Copy link
Copy Markdown
Contributor

... I will refactor the code in next PR.

@runzhiwang , thanks a lot. Since you are working on RATIS-1273, let me do the refactoring.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Sure, thanks a lot.

symious pushed a commit to symious/ratis that referenced this pull request Feb 19, 2024
Sign up for freeto 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.

3 participants

@runzhiwang@szetszwo@GlenGeng-awx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

RATIS-993. Support pre vote - #161

Merged
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify
Jan 6, 2021
Merged

RATIS-993. Support pre vote#161
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify

Conversation

@runzhiwang

@runzhiwangrunzhiwang commented Aug 2, 2020

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  1. Implement Preventing disruptions when a server rejoins the cluster according to raft paper section 9.6.

  2. In ozone, rejoin happen easily because the unstable network, for example sometimes one server's network is slow, it will try to trigger election, but which is unnecessary, this can be prevented by this PR.

  3. In this PR, before follower trigger leader election, it send pre-vote request to the other servers, if other server allow the follower to trigger leader election, it reply PASSED to the pre-vote request. If majority of the other servers allow the follower trigger leader election, the follower will trigger leader election.

  4. The other server allow the follower to trigger leader election only when 1. the voters have not received heartbeats from a valid leader for at least a baseline election timeout. and 2. the follower's log is newer than voter's, or the follower's log is equal to voter's and follower's priority is not less than voter's

  5. If the above condition can not satisfied, even though the follower trigger leader election, it can not be elected as a leader but it can step down the leader by it's bigger term, so we do not allow it trigger leader election.

  6. If the follower is not in the conf, we will shutdown the follower.

Details of the paper:

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.
If desired, Raft’s basic leader election algorithm can be extended with an additional phase to
prevent such disruptions, forming the Pre-Vote algorithm. 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 (if the candidate’s log is sufficiently up-to-date, and the voters
have not received heartbeats from a valid leader for at least a baseline election timeout). This was
inspired by ZooKeeper’s algorithm [42], in which a server must receive a majority of votes before
it calculates a new epoch and sends NewEpoch messages (however, in ZooKeeper servers do not
solicit votes, other servers offer them).
The Pre-Vote algorithm solves the issue of a partitioned server disrupting the cluster when it
rejoins. While a server is partitioned, it won’t be able to increment its term, since it can’t receive
permission from a majority of the cluster. Then, when it rejoins the cluster, it still won’t be able
to increment its term, since the other servers will have been receiving regular heartbeats from the
leader. Once the server receives a heartbeat from the leader itself, it will return to the follower state
(in the same term).
We recommend the Pre-Vote extension in deployments that would benefit from additional robustness.
We also tested it in various leader election scenarios in AvailSim, and it does not appear
to significantly harm election performance.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-993

How was this patch tested?

add ut: testPreVote

@runzhiwangrunzhiwang reopened this Aug 2, 2020
@runzhiwangrunzhiwang reopened this Aug 4, 2020
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo@bshashikant@lokeshj1703 @GlenGeng Could you help review this patch ? Thank you very much.

@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang thanks for working on this.

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo

Copy link
Copy Markdown
Contributor
  1. the follower is not in the conf.

I cannot see this requirement in the paper. Could you give me a pointer?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo Thanks for review. You are right, I did not implement it in this PR because I want to make this PR simpler to review, and I will implement it after this PR. Even though without checkout timeout, it will not introduce new bug, because in this case pre-vote passed and trigger a new leader election. I can also implement it in this PR, it's ok to me. Which one do you prefer ?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

the follower is not in the conf.
I cannot see this requirement in the paper. Could you give me a pointer?

@szetszwo You are right, it does not exist in the paper. I implement it to adapt current ratis implementation. Because in current ratis, when leader election, leader will send shutdown to follower if follower, such as follower1, is not in leader's conf at shouldSendShutdown(candidateId, candidateLastEntry). So I have two choice to make follower1 shutdown: 1.pass the pre-vote in this case, so that follower1 can shutdown in real leader election; 2.pre-vote in charge of shutdown follower1, but this will make pre-vote complicated. So I choose the first option. What do you think?

@GlenGeng-awxGlenGeng-awx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the work!

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang Let's just implement exactly raft paper section 9.6.

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@runzhiwang

runzhiwang commented Aug 6, 2020

Copy link
Copy Markdown
ContributorAuthor

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@szetszwo Thanks for explanation. Do you mean shutdown the follower not in the conf when pre-vote ?

@runzhiwangrunzhiwang reopened this Aug 7, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

Yes, we should shutdown the follower for that group.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Thanks for review. I have updated the patch. Could you help review it again ?

@runzhiwangrunzhiwang changed the title RATIS-993. Preventing disruptions when a server rejoins the clusterRATIS-993. Support pre voteJan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo I have updated this pre-vote patch. Could you help review it ?

@runzhiwangrunzhiwang reopened this Jan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

will fix the failed ut.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for working on this.

  • Terminology: Let's use "Pre-Vote" or "pre-vote" as in https://github.com/ongardie/dissertation
  • There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

Some other comments inlined.

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

@szetszwo Thanks for review. I will refactor the code in next PR.

Besides, when s1 transferLeaderShip to s2, s2 should skip askForPreVote, and askForVote directly. Because when s2 askForPreVote, s1 and s3 will reject it because current leader is still valid. So I add a flag: LeaderElection#force which means we should skip askForPreVote and askForVote directly, this flag will be true when s2 receive StartLeaderElectionRequest.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 the change looks good.

@szetszwo
szetszwo merged commit 35f17fa into apache:masterJan 6, 2021
@szetszwo

Copy link
Copy Markdown
Contributor

... I will refactor the code in next PR.

@runzhiwang , thanks a lot. Since you are working on RATIS-1273, let me do the refactoring.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Sure, thanks a lot.

symious pushed a commit to symious/ratis that referenced this pull request Feb 19, 2024
Sign up for freeto 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.

3 participants

@runzhiwang@szetszwo@GlenGeng-awx
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

RATIS-993. Support pre vote - #161

Merged
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify
Jan 6, 2021
Merged

RATIS-993. Support pre vote#161
szetszwo merged 2 commits into
apache:masterfrom
runzhiwang:pre-vote-2-simplify

Conversation

@runzhiwang

@runzhiwangrunzhiwang commented Aug 2, 2020

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  1. Implement Preventing disruptions when a server rejoins the cluster according to raft paper section 9.6.

  2. In ozone, rejoin happen easily because the unstable network, for example sometimes one server's network is slow, it will try to trigger election, but which is unnecessary, this can be prevented by this PR.

  3. In this PR, before follower trigger leader election, it send pre-vote request to the other servers, if other server allow the follower to trigger leader election, it reply PASSED to the pre-vote request. If majority of the other servers allow the follower trigger leader election, the follower will trigger leader election.

  4. The other server allow the follower to trigger leader election only when 1. the voters have not received heartbeats from a valid leader for at least a baseline election timeout. and 2. the follower's log is newer than voter's, or the follower's log is equal to voter's and follower's priority is not less than voter's

  5. If the above condition can not satisfied, even though the follower trigger leader election, it can not be elected as a leader but it can step down the leader by it's bigger term, so we do not allow it trigger leader election.

  6. If the follower is not in the conf, we will shutdown the follower.

Details of the paper:

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.
If desired, Raft’s basic leader election algorithm can be extended with an additional phase to
prevent such disruptions, forming the Pre-Vote algorithm. 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 (if the candidate’s log is sufficiently up-to-date, and the voters
have not received heartbeats from a valid leader for at least a baseline election timeout). This was
inspired by ZooKeeper’s algorithm [42], in which a server must receive a majority of votes before
it calculates a new epoch and sends NewEpoch messages (however, in ZooKeeper servers do not
solicit votes, other servers offer them).
The Pre-Vote algorithm solves the issue of a partitioned server disrupting the cluster when it
rejoins. While a server is partitioned, it won’t be able to increment its term, since it can’t receive
permission from a majority of the cluster. Then, when it rejoins the cluster, it still won’t be able
to increment its term, since the other servers will have been receiving regular heartbeats from the
leader. Once the server receives a heartbeat from the leader itself, it will return to the follower state
(in the same term).
We recommend the Pre-Vote extension in deployments that would benefit from additional robustness.
We also tested it in various leader election scenarios in AvailSim, and it does not appear
to significantly harm election performance.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-993

How was this patch tested?

add ut: testPreVote

@runzhiwangrunzhiwang reopened this Aug 2, 2020
@runzhiwangrunzhiwang reopened this Aug 4, 2020
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo@bshashikant@lokeshj1703 @GlenGeng Could you help review this patch ? Thank you very much.

@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang thanks for working on this.

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo

Copy link
Copy Markdown
Contributor
  1. the follower is not in the conf.

I cannot see this requirement in the paper. Could you give me a pointer?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

... the voters have not received heartbeats from a valid leader for at least a baseline election timeout ...

This requirement seems not yet implemented in the current change.

@szetszwo Thanks for review. You are right, I did not implement it in this PR because I want to make this PR simpler to review, and I will implement it after this PR. Even though without checkout timeout, it will not introduce new bug, because in this case pre-vote passed and trigger a new leader election. I can also implement it in this PR, it's ok to me. Which one do you prefer ?

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

the follower is not in the conf.
I cannot see this requirement in the paper. Could you give me a pointer?

@szetszwo You are right, it does not exist in the paper. I implement it to adapt current ratis implementation. Because in current ratis, when leader election, leader will send shutdown to follower if follower, such as follower1, is not in leader's conf at shouldSendShutdown(candidateId, candidateLastEntry). So I have two choice to make follower1 shutdown: 1.pass the pre-vote in this case, so that follower1 can shutdown in real leader election; 2.pre-vote in charge of shutdown follower1, but this will make pre-vote complicated. So I choose the first option. What do you think?

@GlenGeng-awxGlenGeng-awx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the work!

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@runzhiwangrunzhiwang reopened this Aug 6, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

@runzhiwang Let's just implement exactly raft paper section 9.6.

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@runzhiwang

runzhiwang commented Aug 6, 2020

Copy link
Copy Markdown
ContributorAuthor

When the follower is not in the conf, it should not allowed to join the group. Otherwise, any followers could join any groups.

@szetszwo Thanks for explanation. Do you mean shutdown the follower not in the conf when pre-vote ?

@runzhiwangrunzhiwang reopened this Aug 7, 2020
@szetszwo

Copy link
Copy Markdown
Contributor

Yes, we should shutdown the follower for that group.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Thanks for review. I have updated the patch. Could you help review it again ?

@runzhiwangrunzhiwang changed the title RATIS-993. Preventing disruptions when a server rejoins the clusterRATIS-993. Support pre voteJan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo I have updated this pre-vote patch. Could you help review it ?

@runzhiwangrunzhiwang reopened this Jan 6, 2021
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

will fix the failed ut.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for working on this.

  • Terminology: Let's use "Pre-Vote" or "pre-vote" as in https://github.com/ongardie/dissertation
  • There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

Some other comments inlined.

Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java Outdated
Comment threadratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java Outdated
@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

There are quite many code duplications between pre-vote and vote. We may refactor the code here or in a separated JIRA.

@szetszwo Thanks for review. I will refactor the code in next PR.

Besides, when s1 transferLeaderShip to s2, s2 should skip askForPreVote, and askForVote directly. Because when s2 askForPreVote, s1 and s3 will reject it because current leader is still valid. So I add a flag: LeaderElection#force which means we should skip askForPreVote and askForVote directly, this flag will be true when s2 receive StartLeaderElectionRequest.

@szetszwoszetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 the change looks good.

@szetszwo
szetszwo merged commit 35f17fa into apache:masterJan 6, 2021
@szetszwo

Copy link
Copy Markdown
Contributor

... I will refactor the code in next PR.

@runzhiwang , thanks a lot. Since you are working on RATIS-1273, let me do the refactoring.

@runzhiwang

Copy link
Copy Markdown
ContributorAuthor

@szetszwo Sure, thanks a lot.

symious pushed a commit to symious/ratis that referenced this pull request Feb 19, 2024
Sign up for freeto 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.

3 participants

@runzhiwang@szetszwo@GlenGeng-awx