[GitHub] [ratis] szetszwo commented on pull request #560: RATIS-1446. Avoid leader election for invalid conf

[GitBox <gi...@apache.org>]

szetszwo commented on pull request #560:
URL: https://github.com/apache/ratis/pull/560#issuecomment-989779930


   > ..., I am not sure whether we should make a specific check for each getPriority() use. 
   
   That's a good point.  We should check.  I just have walked through the code.  There are two other cases needed to be fixed:
   ```
   diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
   index 03850114..d1413040 100644
   --- a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
   +++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
   @@ -916,13 +916,13 @@ class LeaderStateImpl implements LeaderState {
    
        for (LogAppender logAppender : senders.getSenders()) {
          FollowerInfo followerInfo = logAppender.getFollower();
   -      RaftPeerId followerID = followerInfo.getPeer().getId();
   -      int followerPriority = conf.getPeer(followerID).getPriority();
   -
   +      final RaftPeer follower = followerInfo.getPeer();
   +      final int followerPriority = follower.getPriority();
          if (followerPriority <= leaderPriority) {
            continue;
          }
    
   +      final RaftPeerId followerID = follower.getId();
          final TermIndex leaderLastEntry = server.getState().getLastEntry();
          if (leaderLastEntry == null) {
            LOG.info("{} send StartLeaderElectionRequest to follower:{} on term:{} because follower's priority:{} " +
   diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/VoteContext.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/VoteContext.java
   index 1ef721aa..fde198fe 100644
   --- a/ratis-server/src/main/java/org/apache/ratis/server/impl/VoteContext.java
   +++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/VoteContext.java
   @@ -144,7 +144,11 @@ class VoteContext {
        }
    
        // Check priority
   -    final int priority = impl.getRaftConf().getPeer(impl.getId()).getPriority();
   +    final RaftPeer peer = conf.getPeer(impl.getId());
   +    if (peer == null) {
   +      return reject("our server " + impl.getId() + " is not in the conf");
   +    }
   +    final int priority = peer.getPriority();
        if (priority <= candidate.getPriority()) {
          return log(true, "our priority " + priority + " <= candidate's priority " + candidate.getPriority());
        } else {
   ```


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

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org