You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2022/11/28 01:46:37 UTC

[GitHub] [ratis] SzyWilliam opened a new pull request, #792: RATIS-1754. Set leader to null when stepping down

SzyWilliam opened a new pull request, #792:
URL: https://github.com/apache/ratis/pull/792

   ## What changes were proposed in this pull request?
   ServerState.getLeaderId() still returns itself when a leader just steps down. This PR proposes to update leader to null when stepping down.
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/RATIS-1754
   


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

[GitHub] [ratis] szetszwo commented on pull request #792: RATIS-1754. Set leader to null when stepping down

Posted by GitBox <gi...@apache.org>.
szetszwo commented on PR #792:
URL: https://github.com/apache/ratis/pull/792#issuecomment-1332665132

   @SzyWilliam , since this set leaderId to null, it becomes easier to get NPE; see #794.


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

[GitHub] [ratis] SzyWilliam commented on pull request #792: RATIS-1754. Set leader to null when stepping down

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #792:
URL: https://github.com/apache/ratis/pull/792#issuecomment-1340560258

   @szetszwo I've rebased the master. The CI is now passed. Thanks again!


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

[GitHub] [ratis] SzyWilliam commented on pull request #792: RATIS-1754. Set leader to null when stepping down

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #792:
URL: https://github.com/apache/ratis/pull/792#issuecomment-1331751994

   @szetszwo Thanks for the reviews! Applied the review patch.


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

[GitHub] [ratis] szetszwo merged pull request #792: RATIS-1754. Set leader to null when stepping down

Posted by GitBox <gi...@apache.org>.
szetszwo merged PR #792:
URL: https://github.com/apache/ratis/pull/792


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

[GitHub] [ratis] szetszwo commented on a diff in pull request #792: RATIS-1754. Set leader to null when stepping down

Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #792:
URL: https://github.com/apache/ratis/pull/792#discussion_r1034147470


##########
ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java:
##########
@@ -568,6 +568,7 @@ void submitStepDownEvent(long term, StepDownReason reason) {
   private void stepDown(long term, StepDownReason reason) {
     try {
       server.changeToFollowerAndPersistMetadata(term, false, reason);
+      server.getState().setLeader(null, "stepDown");

Review Comment:
   Let's set it in `changeToFollower` as below
   ```java
   +++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
   @@ -534,6 +534,7 @@ class RaftServerImpl implements RaftServer.Division,
          setRole(RaftPeerRole.FOLLOWER, reason);
          if (old == RaftPeerRole.LEADER) {
            role.shutdownLeaderState(false);
   +        state.setLeader(null, reason);
          } else if (old == RaftPeerRole.CANDIDATE) {
            role.shutdownLeaderElection();
          } else if (old == RaftPeerRole.FOLLOWER) {
   ```



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

[GitHub] [ratis] szetszwo commented on pull request #792: RATIS-1754. Set leader to null when stepping down

Posted by GitBox <gi...@apache.org>.
szetszwo commented on PR #792:
URL: https://github.com/apache/ratis/pull/792#issuecomment-1340339278

   @SzyWilliam ,  `ReadOnlyRequestTests.testLinearizableReadTimeout` still fails with NPE, could you rebase this with the master branch?


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