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 2021/01/07 14:55:21 UTC

[GitHub] [incubator-ratis] szetszwo opened a new pull request #392: RATIS-1283. Refactor the code for preVote and vote in RaftServerImpl.

szetszwo opened a new pull request #392:
URL: https://github.com/apache/incubator-ratis/pull/392


   See https://issues.apache.org/jira/browse/RATIS-1283


----------------------------------------------------------------
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 #392: RATIS-1283. Refactor the code for preVote and vote in RaftServerImpl.

Posted by GitBox <gi...@apache.org>.
runzhiwang closed pull request #392:
URL: https://github.com/apache/incubator-ratis/pull/392


   


----------------------------------------------------------------
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 #392: RATIS-1283. Refactor the code for preVote and vote in RaftServerImpl.

Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #392:
URL: https://github.com/apache/incubator-ratis/pull/392#discussion_r553686287



##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -1118,46 +1037,34 @@ private RequestVoteReplyProto requestVote(
     synchronized (this) {
       // Check life cycle state again to avoid the PAUSING/PAUSED state.
       assertLifeCycleState(LifeCycle.States.RUNNING);
-      FollowerState fs = role.getFollowerState().orElse(null);
-      if (shouldWithholdVotes(candidateTerm)) {
-        LOG.info("{}-{}: Withhold vote from candidate {} with term {}. State: leader={}, term={}, lastRpcElapsed={}",
-            getMemberId(), role, candidateId, candidateTerm, state.getLeaderId(), state.getCurrentTerm(),
-            fs != null? fs.getLastRpcTime().elapsedTimeMs() + "ms": null);
-      } else if (state.recognizeCandidate(candidateId, candidateTerm)) {
-        final boolean termUpdated = changeToFollower(candidateTerm, true, "recognizeCandidate:" + candidateId);
-        // if current server is leader, before changeToFollower FollowerState should be null,
-        // so after leader changeToFollower we should get FollowerState again.
-        fs = role.getFollowerState().orElse(null);
-
-        // see Section 5.4.1 Election restriction
-        RaftPeer candidate = getRaftConf().getPeer(candidateId);
-        if (fs != null && candidate != null) {
-          int compare = ServerState.compareLog(state.getLastEntry(), candidateLastEntry);
-          int priority = getRaftConf().getPeer(getId()).getPriority();
-          LOG.info("{} priority:{} candidate:{} candidatePriority:{} compare:{}",
-              this, priority, candidate, candidate.getPriority(), compare);
-          // vote for candidate if:
-          // 1. log lags behind candidate
-          // 2. log equals candidate's, and priority less or equal candidate's
-          if (compare < 0 || (compare == 0 && priority <= candidate.getPriority())) {
-            state.grantVote(candidateId);
-            voteGranted = true;
+
+      final VoteContext context = new VoteContext(this, phase, candidateId);
+      final RaftPeer candidate = context.recognizeCandidate(candidateTerm);
+      LOG.info("{}: recognizeCandidate {}? {}", getMemberId(), candidateId, candidate != null);
+      if (candidate != null) {

Review comment:
       That's a good idea.  Will update the code.  Thanks.




----------------------------------------------------------------
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 a change in pull request #392: RATIS-1283. Refactor the code for preVote and vote in RaftServerImpl.

Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #392:
URL: https://github.com/apache/incubator-ratis/pull/392#discussion_r553671826



##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -1118,46 +1037,34 @@ private RequestVoteReplyProto requestVote(
     synchronized (this) {
       // Check life cycle state again to avoid the PAUSING/PAUSED state.
       assertLifeCycleState(LifeCycle.States.RUNNING);
-      FollowerState fs = role.getFollowerState().orElse(null);
-      if (shouldWithholdVotes(candidateTerm)) {
-        LOG.info("{}-{}: Withhold vote from candidate {} with term {}. State: leader={}, term={}, lastRpcElapsed={}",
-            getMemberId(), role, candidateId, candidateTerm, state.getLeaderId(), state.getCurrentTerm(),
-            fs != null? fs.getLastRpcTime().elapsedTimeMs() + "ms": null);
-      } else if (state.recognizeCandidate(candidateId, candidateTerm)) {
-        final boolean termUpdated = changeToFollower(candidateTerm, true, "recognizeCandidate:" + candidateId);
-        // if current server is leader, before changeToFollower FollowerState should be null,
-        // so after leader changeToFollower we should get FollowerState again.
-        fs = role.getFollowerState().orElse(null);
-
-        // see Section 5.4.1 Election restriction
-        RaftPeer candidate = getRaftConf().getPeer(candidateId);
-        if (fs != null && candidate != null) {
-          int compare = ServerState.compareLog(state.getLastEntry(), candidateLastEntry);
-          int priority = getRaftConf().getPeer(getId()).getPriority();
-          LOG.info("{} priority:{} candidate:{} candidatePriority:{} compare:{}",
-              this, priority, candidate, candidate.getPriority(), compare);
-          // vote for candidate if:
-          // 1. log lags behind candidate
-          // 2. log equals candidate's, and priority less or equal candidate's
-          if (compare < 0 || (compare == 0 && priority <= candidate.getPriority())) {
-            state.grantVote(candidateId);
-            voteGranted = true;
+
+      final VoteContext context = new VoteContext(this, phase, candidateId);
+      final RaftPeer candidate = context.recognizeCandidate(candidateTerm);
+      LOG.info("{}: recognizeCandidate {}? {}", getMemberId(), candidateId, candidate != null);
+      if (candidate != null) {

Review comment:
       Can we put  `voteGranted = context.shouldVote(candidate, candidateLastEntry);` here ? Because Both vote and pre-vote need this check.




----------------------------------------------------------------
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 #392: RATIS-1283. Refactor the code for preVote and vote in RaftServerImpl.

Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #392:
URL: https://github.com/apache/incubator-ratis/pull/392#issuecomment-756515315


   @szetszwo Thanks the patch. I have merged 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 merged pull request #392: RATIS-1283. Refactor the code for preVote and vote in RaftServerImpl.

Posted by GitBox <gi...@apache.org>.
runzhiwang merged pull request #392:
URL: https://github.com/apache/incubator-ratis/pull/392


   


----------------------------------------------------------------
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 #392: RATIS-1283. Refactor the code for preVote and vote in RaftServerImpl.

Posted by GitBox <gi...@apache.org>.
runzhiwang closed pull request #392:
URL: https://github.com/apache/incubator-ratis/pull/392


   


----------------------------------------------------------------
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