RATIS-2378. fix listener role transition

[JoeCqupt]

What changes were proposed in this pull request?

fix https://issues.apache.org/jira/browse/RATIS-2378

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-2378

How was this patch tested?

unit tests

[szetszwo]

@JoeCqupt , thanks a lot for working on this! Please see the comments inlined and below

+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
@@ -578,6 +578,14 @@ class RaftServerImpl implements RaftServer.Division,
     }
   }
 
+  private boolean shouldSetFollower(RaftPeerRole old, boolean force) {
+    if (old == RaftPeerRole.LISTENER) {
+      final RaftConfigurationImpl conf = state.getRaftConf();
+      return conf.isStable() && conf.containsInConf(getId());
+    }
+    return old != RaftPeerRole.FOLLOWER || force;
+  }
+
   /**
    * Change the server state to Follower if this server is in a different role or force is true.
    * @param newTerm The new term.
@@ -591,7 +599,7 @@ class RaftServerImpl implements RaftServer.Division,
       throw new IllegalStateException("Unexpected role " + old);
     }
     CompletableFuture<Void> future = CompletableFuture.completedFuture(null);
-    if ((old != RaftPeerRole.FOLLOWER || force) && old != RaftPeerRole.LISTENER) {
+    if (shouldSetFollower(old, force)) {
       setRole(RaftPeerRole.FOLLOWER, reason);
       if (old == RaftPeerRole.LEADER) {
         future = role.shutdownLeaderState(false)
@@ -607,7 +615,7 @@ class RaftServerImpl implements RaftServer.Division,
         state.setLeader(null, reason);
       } else if (old == RaftPeerRole.CANDIDATE) {
         future = role.shutdownLeaderElection();
-      } else if (old == RaftPeerRole.FOLLOWER) {
+      } else if (old == RaftPeerRole.FOLLOWER || old == RaftPeerRole.LISTENER) {
         future = role.shutdownFollowerState();
       }
 

[JoeCqupt]

I have actually considered moving the logic to changeToFollower(..). but it has one drawback：

in appendEntriesAsync(...) method changeToFollowerAndPersistMetadata(..) run before state.updateConfiguration(..)

when NewConf RaftConfiguration log Entry come in. the listener do not change to follower immediately. the listener will change to follower at next time heartbeat or log entry append.

Never mind, it works correctly now. let the logic in changeToFollower(..)

[JoeCqupt]

now unit tests fail because the listener do not change to follower immediately after received NewConf RaftConfiguration

https://github.com/apache/ratis/actions/runs/20610847852/job/59198634271#step:11:1020

[JoeCqupt]

i add logic to ServerState#setRaftConf(...).
let the listener change to follower immediately after added NewConf RaftConfiguration

@szetszwo
unit tests passed

[szetszwo]

@JoeCqupt , thanks for the update! Please see the comment inlined and also https://issues.apache.org/jira/secure/attachment/13080065/1331_review.patch

[szetszwo]

This setRaftConf(..) method is used by many cases including initRaftLog and leader appendConfiguration. It is better to check listener in the updateConfiguration(..) method.

BTW, there is another bug that we should also change SnapshotInstallationHandler to call updateConfiguration(..) instead of setRaftConf(..). Otherwise, the conf change will not be handled correctly.