szetszwo commented on a change in pull request #188:
URL: https://github.com/apache/incubator-ratis/pull/188#discussion_r493415470
##########
File path:
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -358,6 +360,10 @@ public boolean isLeader() {
return role.isLeader();
}
+ public boolean isActive() {
+ return !lifeCycle.getCurrentState().isPausingOrPaused();
+ }
Review comment:
When the state is CLOSED, isActive() returns true. It does not sound
right. Let's rename this method to isPausingOrPaused?
##########
File path: ratis-server/src/test/java/org/apache/ratis/RaftBasicTests.java
##########
@@ -337,6 +337,22 @@ public void testWithLoad() throws Exception {
runWithNewCluster(NUM_SERVERS, cluster -> testWithLoad(10, 300, false,
cluster, LOG));
}
+ @Test
+ public void testPauseFollowers() throws Exception {
+ runWithNewCluster(NUM_SERVERS, this::runTestPauseFollowers);
+ }
+
+ void runTestPauseFollowers(CLUSTER cluster) throws Exception {
Review comment:
The test is too simple for the pause() feature. The test should keep
sending requests. Then call pause() and make sure the request after paused
will not be executed.
##########
File path:
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -1171,6 +1181,23 @@ public InstallSnapshotReplyProto
installSnapshot(InstallSnapshotRequestProto req
return reply;
}
+ public boolean pause() throws IOException {
Review comment:
pause() and resume() should be implemented together. Otherwise, the
feature is incomplete.
----------------------------------------------------------------
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:
[email protected]
