szetszwo commented on code in PR #658:
URL: https://github.com/apache/ratis/pull/658#discussion_r901960332
##########
ratis-client/src/main/java/org/apache/ratis/client/api/AdminApi.java:
##########
@@ -41,10 +42,16 @@ default RaftClientReply setConfiguration(RaftPeer[]
serversInNewConf) throws IOE
return setConfiguration(Arrays.asList(serversInNewConf));
}
+ RaftClientReply setConfiguration(List<RaftPeer> serversInNewConf,
SetConfigurationRequest.Mode mode)
+ throws IOException;
+
/** Set the configuration request to the raft service. */
RaftClientReply setConfiguration(List<RaftPeer> serversInNewConf,
List<RaftPeer> listenersInNewConf)
throws IOException;
+ RaftClientReply setConfiguration(List<RaftPeer> serversInNewConf,
List<RaftPeer> listenersInNewConf,
+ SetConfigurationRequest.Mode mode) throws IOException;
+
Review Comment:
With ADD and CAS modes, the combination of arguments becomes complicated.
Let's add a SetConfigurationArguments class and a Builder.
##########
ratis-proto/src/main/proto/Raft.proto:
##########
@@ -409,6 +409,12 @@ message SetConfigurationRequestProto {
RaftRpcRequestProto rpcRequest = 1;
repeated RaftPeerProto peers = 2;
repeated RaftPeerProto listeners = 3;
+ enum RaftConfigurationMode {
Review Comment:
Similar to SetConfigurationRequest, let's simply call it "Mode". Also,
please move the enum to the top.
##########
ratis-common/src/main/java/org/apache/ratis/protocol/SetConfigurationRequest.java:
##########
@@ -23,19 +23,26 @@
import java.util.List;
public class SetConfigurationRequest extends RaftClientRequest {
+
+ public enum Mode {
+ NORMAL,
Review Comment:
Let's call it SET_UNCONDITIONALLY.
##########
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:
What should happen in ADD mode when a new peer already exists in the current
peer list? Should the request fail?
--
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]
