szetszwo commented on code in PR #658:
URL: https://github.com/apache/ratis/pull/658#discussion_r908800855
##########
ratis-client/src/main/java/org/apache/ratis/client/api/AdminApi.java:
##########
@@ -31,24 +32,108 @@
* such as setting raft configuration and transferring leadership.
*/
public interface AdminApi {
+
+ class SetConfigurationArguments {
Review Comment:
Let's move it to SetConfigurationRequest so that it can be used everywhere.
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -1109,10 +1110,21 @@ public CompletableFuture<RaftClientReply>
setConfigurationAsync(SetConfiguration
return pending.getFuture();
}
- getRaftServer().addRaftPeers(peersInNewConf);
- // add staging state into the leaderState
- pending = leaderState.startSetConfiguration(request);
+ if(request.getMode() == SetConfigurationRequest.Mode.ADD) {
+ List<RaftPeer> peers = Stream.of(peersInNewConf, new
ArrayList<>(current.getAllPeers()))
+ .flatMap(List :: stream).distinct().collect(Collectors.toList());
Review Comment:
@codings-dan , But we use `distinct()` so that we are not sure whether the
existing or the new server is used.
BTW, RaftConfigurationImpl#hasNoChange does not check listeners at all. It
is a bug.
--
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]
