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/06 10:30:46 UTC

[GitHub] [incubator-ratis] szetszwo commented on a change in pull request #161: RATIS-993. Support pre vote

szetszwo commented on a change in pull request #161:
URL: https://github.com/apache/incubator-ratis/pull/161#discussion_r552490729



##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java
##########
@@ -64,10 +64,14 @@
 
   private ResultAndTerm logAndReturn(Result result,
       Map<RaftPeerId, RequestVoteReplyProto> responses,
-      List<Exception> exceptions, long newTerm) {
-    LOG.info(this + ": Election " + result + "; received " + responses.size() + " response(s) "
-        + responses.values().stream().map(ServerStringUtils::toRequestVoteReplyString).collect(Collectors.toList())
-        + " and " + exceptions.size() + " exception(s); " + server.getState());
+      List<Exception> exceptions, long newTerm, boolean preVote) {
+    LOG.info("{}: {} {} received {} response(s):{}",
+        this,
+        preVote ? "Pre vote " : "Election ",

Review comment:
       Let's add a hyphen, i.e. "Pre-vote".

##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -1026,10 +1026,84 @@ private boolean shouldSendShutdown(RaftPeerId candidateId,
   public RequestVoteReplyProto requestVote(RequestVoteRequestProto r)
       throws IOException {
     final RaftRpcRequestProto request = r.getServerRequest();
-    return requestVote(RaftPeerId.valueOf(request.getRequestorId()),
-        ProtoUtils.toRaftGroupId(request.getRaftGroupId()),
-        r.getCandidateTerm(),
-        TermIndex.valueOf(r.getCandidateLastEntry()));
+    if (r.getPreVote()) {
+      return requestPreVote(RaftPeerId.valueOf(request.getRequestorId()),
+          ProtoUtils.toRaftGroupId(request.getRaftGroupId()),
+          r.getCandidateTerm(),
+          TermIndex.valueOf(r.getCandidateLastEntry()));
+    } else {
+      return requestVote(RaftPeerId.valueOf(request.getRequestorId()),
+          ProtoUtils.toRaftGroupId(request.getRaftGroupId()),
+          r.getCandidateTerm(),
+          TermIndex.valueOf(r.getCandidateLastEntry()));
+    }
+  }
+
+  private boolean isCurrentLeaderValid() {
+    if (getInfo().isLeader()) {
+      return role.getLeaderState().map(LeaderStateImpl::checkLeadership).orElse(false);
+    } else if (getInfo().isFollower()) {
+      return state.hasLeader()
+          && role.getFollowerState().map(FollowerState::isCurrentLeaderValid).orElse(false);
+    } else if (getInfo().isCandidate()) {
+      return false;
+    }
+
+    return false;
+  }
+
+  private RequestVoteReplyProto requestPreVote(
+      RaftPeerId candidateId, RaftGroupId candidateGroupId,
+      long candidateTerm, TermIndex candidateLastEntry) throws IOException {
+    CodeInjectionForTesting.execute(REQUEST_VOTE, getId(),
+        candidateId, candidateTerm, candidateLastEntry);
+    LOG.info("{}: receive preVote({}, {}, {}, {})",
+        getMemberId(), candidateId, candidateGroupId, candidateTerm, candidateLastEntry);
+    assertLifeCycleState(LifeCycle.States.RUNNING);
+    assertGroup(candidateId, candidateGroupId);
+
+    boolean preVoteGranted = false;
+    boolean shouldShutdown = false;
+    final RequestVoteReplyProto reply;
+    synchronized (this) {
+      boolean isCurrentLeaderValid = isCurrentLeaderValid();
+      RaftPeer candidate = getRaftConf().getPeer(candidateId);
+
+      if (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. current leader is not valid
+        // 2. log lags behind candidate or
+        //    log equals candidate's, and priority less or equal candidate's
+        if (!isCurrentLeaderValid &&

Review comment:
       The condition `!isCurrentLeaderValid` should be moved to the previous if-statement.

##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -1026,10 +1026,84 @@ private boolean shouldSendShutdown(RaftPeerId candidateId,
   public RequestVoteReplyProto requestVote(RequestVoteRequestProto r)
       throws IOException {
     final RaftRpcRequestProto request = r.getServerRequest();
-    return requestVote(RaftPeerId.valueOf(request.getRequestorId()),
-        ProtoUtils.toRaftGroupId(request.getRaftGroupId()),
-        r.getCandidateTerm(),
-        TermIndex.valueOf(r.getCandidateLastEntry()));
+    if (r.getPreVote()) {
+      return requestPreVote(RaftPeerId.valueOf(request.getRequestorId()),
+          ProtoUtils.toRaftGroupId(request.getRaftGroupId()),
+          r.getCandidateTerm(),
+          TermIndex.valueOf(r.getCandidateLastEntry()));
+    } else {
+      return requestVote(RaftPeerId.valueOf(request.getRequestorId()),
+          ProtoUtils.toRaftGroupId(request.getRaftGroupId()),
+          r.getCandidateTerm(),
+          TermIndex.valueOf(r.getCandidateLastEntry()));
+    }
+  }
+
+  private boolean isCurrentLeaderValid() {
+    if (getInfo().isLeader()) {
+      return role.getLeaderState().map(LeaderStateImpl::checkLeadership).orElse(false);
+    } else if (getInfo().isFollower()) {
+      return state.hasLeader()
+          && role.getFollowerState().map(FollowerState::isCurrentLeaderValid).orElse(false);
+    } else if (getInfo().isCandidate()) {
+      return false;
+    }
+
+    return false;
+  }
+
+  private RequestVoteReplyProto requestPreVote(
+      RaftPeerId candidateId, RaftGroupId candidateGroupId,
+      long candidateTerm, TermIndex candidateLastEntry) throws IOException {
+    CodeInjectionForTesting.execute(REQUEST_VOTE, getId(),
+        candidateId, candidateTerm, candidateLastEntry);
+    LOG.info("{}: receive preVote({}, {}, {}, {})",
+        getMemberId(), candidateId, candidateGroupId, candidateTerm, candidateLastEntry);
+    assertLifeCycleState(LifeCycle.States.RUNNING);
+    assertGroup(candidateId, candidateGroupId);
+
+    boolean preVoteGranted = false;
+    boolean shouldShutdown = false;
+    final RequestVoteReplyProto reply;
+    synchronized (this) {
+      boolean isCurrentLeaderValid = isCurrentLeaderValid();
+      RaftPeer candidate = getRaftConf().getPeer(candidateId);
+
+      if (candidate != null) {

Review comment:
       It should check also isCurrentLeaderValid, i.e.
   ```
         if (!isCurrentLeaderValid && candidate != null) {
   ```
   




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