szetszwo commented on code in PR #895:
URL: https://github.com/apache/ratis/pull/895#discussion_r1276632473


##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -597,10 +597,11 @@ synchronized void changeToLeader() {
     Preconditions.assertTrue(getInfo().isCandidate());
     role.shutdownLeaderElection();
     setRole(RaftPeerRole.LEADER, "changeToLeader");
+    role.updateLeaderState(this);

Review Comment:
   Let's store the LeaderStateImpl and then use it later.
   ```java
   @@ -603,10 +603,11 @@ class RaftServerImpl implements RaftServer.Division,
        Preconditions.assertTrue(getInfo().isCandidate());
        role.shutdownLeaderElection();
        setRole(RaftPeerRole.LEADER, "changeToLeader");
   +    final LeaderStateImpl leader = role.updateLeaderState(this);
        state.becomeLeader();
    
        // start sending AppendEntries RPC to followers
   -    final LogEntryProto e = role.startLeaderState(this);
   +    final LogEntryProto e = leader.start();
        getState().setRaftConf(e);
      }
   ```



##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RoleInfo.java:
##########
@@ -79,8 +79,12 @@ LeaderStateImpl getLeaderStateNonNull() {
     return Objects.requireNonNull(leaderState.get(), "leaderState is null");
   }
 
-  LogEntryProto startLeaderState(RaftServerImpl server) {
-    return updateAndGet(leaderState, new LeaderStateImpl(server)).start();
+  void updateLeaderState(RaftServerImpl server) {
+    updateAndGet(leaderState, new LeaderStateImpl(server));
+  }
+
+  LogEntryProto startLeaderState() {
+    return leaderState.get().start();

Review Comment:
   Then, we don't need the new `startLeaderState()` method.
   ```java
   +++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/RoleInfo.java
   @@ -79,8 +79,8 @@ class RoleInfo {
        return Objects.requireNonNull(leaderState.get(), "leaderState is null");
      }
    
   -  LogEntryProto startLeaderState(RaftServerImpl server) {
   -    return updateAndGet(leaderState, new LeaderStateImpl(server)).start();
   +  LeaderStateImpl updateLeaderState(RaftServerImpl server) {
   +    return updateAndGet(leaderState, new LeaderStateImpl(server));
      }
    
   ```j



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to