You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ratis.apache.org by GitBox <gi...@apache.org> on 2020/08/27 03:33:49 UTC
[GitHub] [incubator-ratis] runzhiwang opened a new pull request #182: RATIS-1033. Change leader election to add priority consideration
runzhiwang opened a new pull request #182:
URL: https://github.com/apache/incubator-ratis/pull/182
## What changes were proposed in this pull request?
Change leader election to add priority consideration
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1033
## How was this patch tested?
add ut to test it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] runzhiwang commented on pull request #182: RATIS-1033. Change leader election to add priority consideration
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #182:
URL: https://github.com/apache/incubator-ratis/pull/182#issuecomment-681419059
@szetszwo Thanks for review. I have updated the PR.
> Please also add a test for the timeout case.
I add test at: https://github.com/apache/incubator-ratis/blob/9bc000a89e2b03f56ca46d45e1776a40e1abc49f/ratis-server/src/test/java/org/apache/ratis/server/impl/GroupManagementBaseTest.java#L127
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] runzhiwang closed pull request #182: RATIS-1033. Change leader election to add priority consideration
Posted by GitBox <gi...@apache.org>.
runzhiwang closed pull request #182:
URL: https://github.com/apache/incubator-ratis/pull/182
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] szetszwo commented on a change in pull request #182: RATIS-1033. Change leader election to add priority consideration
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #182:
URL: https://github.com/apache/incubator-ratis/pull/182#discussion_r477537179
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java
##########
@@ -269,13 +271,30 @@ private int submitRequests(final long electionTerm, final TermIndex lastEntry,
return submitted;
}
+ private Set<RaftPeerId> getHigherPriorityPeers(RaftConfiguration conf) {
+ Set<RaftPeerId> higherPriorityPeers = new HashSet<>();
+
+ int currPriority = conf.getPeer(server.getId()).getPriority();
+ Collection<RaftPeer> peers = conf.getPeers();
+
+ for (RaftPeer peer : peers) {
+ if (peer.getPriority() > currPriority) {
+ higherPriorityPeers.add(peer.getId());
+ }
+ }
+
+ return higherPriorityPeers;
+ }
+
private ResultAndTerm waitForResults(final long electionTerm, final int submitted,
RaftConfiguration conf, Executor voteExecutor) throws InterruptedException {
final Timestamp timeout = Timestamp.currentTime().addTimeMs(server.getRandomTimeoutMs());
final Map<RaftPeerId, RequestVoteReplyProto> responses = new HashMap<>();
final List<Exception> exceptions = new ArrayList<>();
int waitForNum = submitted;
Collection<RaftPeerId> votedPeers = new ArrayList<>();
+ Set<RaftPeerId> higherPriorityPeers = getHigherPriorityPeers(conf);
+
while (waitForNum > 0 && shouldRun(electionTerm)) {
final TimeDuration waitTime = timeout.elapsedTime().apply(n -> -n);
if (waitTime.isNonPositive()) {
Review comment:
We need to change timeout behavior -- when timeout, if the server can get majority, the election is passed even if higherPriorityPeers is non-empty.
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java
##########
@@ -318,19 +318,24 @@ boolean recognizeCandidate(RaftPeerId candidateId, long candidateTerm) {
return false;
}
- boolean isLogUpToDate(TermIndex candidateLastEntry) {
+ int compareLog(TermIndex candidateLastEntry) {
TermIndex local = log.getLastEntryTermIndex();
// need to take into account snapshot
SnapshotInfo snapshot = server.getStateMachine().getLatestSnapshot();
- if (local == null && snapshot == null) {
- return true;
+ if (local == null && snapshot == null) {
+ // If the lastEntry of candidate is null, the proto will transfer an empty TermIndexProto,
+ // then term and index of candidateLastEntry will both be 0
+ if (candidateLastEntry.getTerm() == 0 && candidateLastEntry.getIndex() == 0) {
+ return 0;
+ }
+ return -1;
} else if (candidateLastEntry == null) {
Review comment:
The case candidateLastEntry == null is never happen so that we should remove it. (Or, if we want to support candidateLastEntry == null, we have to check null in line 328.)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] runzhiwang commented on pull request #182: RATIS-967. Change leader election to add priority consideration
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #182:
URL: https://github.com/apache/incubator-ratis/pull/182#issuecomment-680410528
@szetszwo Could you help review this patch ? Thank you very much.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] szetszwo merged pull request #182: RATIS-1033. Change leader election to add priority consideration
Posted by GitBox <gi...@apache.org>.
szetszwo merged pull request #182:
URL: https://github.com/apache/incubator-ratis/pull/182
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] runzhiwang edited a comment on pull request #182: RATIS-1033. Change leader election to add priority consideration
Posted by GitBox <gi...@apache.org>.
runzhiwang edited a comment on pull request #182:
URL: https://github.com/apache/incubator-ratis/pull/182#issuecomment-681419059
@szetszwo Thanks for review. I have updated the PR.
> Please also add a test for the timeout case.
I add test at: https://github.com/apache/incubator-ratis/blob/9bc000a89e2b03f56ca46d45e1776a40e1abc49f/ratis-server/src/test/java/org/apache/ratis/server/impl/GroupManagementBaseTest.java#L127
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org