szetszwo commented on code in PR #658:
URL: https://github.com/apache/ratis/pull/658#discussion_r905310482
##########
ratis-client/src/main/java/org/apache/ratis/client/api/AdminApi.java:
##########
@@ -31,24 +32,88 @@
* such as setting raft configuration and transferring leadership.
*/
public interface AdminApi {
+
+ class SetConfigurationArguments {
+ private List<RaftPeer> serversInNewConf;
+ private List<RaftPeer> listenersInNewConf;
+ private SetConfigurationRequest.Mode mode;
+
+ private SetConfigurationArguments(List<RaftPeer> serversInNewConf,
+ List<RaftPeer> listenersInNewConf, SetConfigurationRequest.Mode mode) {
+ this.serversInNewConf = serversInNewConf;
+ this.listenersInNewConf = listenersInNewConf;
+ this.mode = mode;
+ }
+
+ public List<RaftPeer> getServersInNewConf() {
+ return serversInNewConf;
+ }
+
+ public List<RaftPeer> getListenersInNewConf() {
+ return listenersInNewConf;
+ }
+
+ public SetConfigurationRequest.Mode getMode() {
+ return mode;
+ }
+
+ public static Builder newBuilder() {
+ return new Builder();
+ }
+
+ public static class Builder {
+ private List<RaftPeer> serversInNewConf;
+ private List<RaftPeer> listenersInNewConf = Collections.emptyList();
+ private SetConfigurationRequest.Mode mode =
SetConfigurationRequest.Mode.SET_UNCONDITIONALLY;
+
+ public Builder setServersInNewConf(List<RaftPeer> serversInNewConf) {
+ this.serversInNewConf = serversInNewConf;
+ return this;
+ }
+
+ public Builder setListenersInNewConf(List<RaftPeer> listenersInNewConf) {
+ this.listenersInNewConf = listenersInNewConf;
+ return this;
+ }
+
+ public Builder setServersInNewConfArray(RaftPeer[]
serversInNewConfArray) {
+ this.serversInNewConf = Arrays.asList(serversInNewConfArray);
+ return this;
+ }
+
+ public Builder setListenersInNewConfArray(RaftPeer[]
listenersInNewConfArray) {
+ this.listenersInNewConf = Arrays.asList(listenersInNewConfArray);
+ return this;
+ }
+
+ public Builder setMode(SetConfigurationRequest.Mode mode) {
+ this.mode = mode;
+ return this;
+ }
+
+ public SetConfigurationArguments build() {
+ return new SetConfigurationArguments(serversInNewConf,
listenersInNewConf, mode);
+ }
+ }
+ }
+
+ RaftClientReply setConfiguration(SetConfigurationArguments arguments)
+ throws IOException;
+
/** The same as setConfiguration(serversInNewConf, Collections.emptyList()).
*/
default RaftClientReply setConfiguration(List<RaftPeer> serversInNewConf)
throws IOException {
- return setConfiguration(serversInNewConf, Collections.emptyList());
+ return setConfiguration(SetConfigurationArguments
+ .newBuilder()
+ .setServersInNewConf(serversInNewConf)
+ .build());
}
/** The same as setConfiguration(Arrays.asList(serversInNewConf)). */
default RaftClientReply setConfiguration(RaftPeer[] serversInNewConf) throws
IOException {
- return setConfiguration(Arrays.asList(serversInNewConf));
- }
-
- /** Set the configuration request to the raft service. */
- RaftClientReply setConfiguration(List<RaftPeer> serversInNewConf,
List<RaftPeer> listenersInNewConf)
- throws IOException;
-
- /** The same as setConfiguration(Arrays.asList(serversInNewConf),
Arrays.asList(listenersInNewConf)). */
- default RaftClientReply setConfiguration(RaftPeer[] serversInNewConf,
RaftPeer[] listenersInNewConf)
- throws IOException {
Review Comment:
We cannot remove these methods. Otherwise, it will become an incompatible
change.
##########
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:
But the new peer may have a different priority. Should we allow it?
##########
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 {
+ SET_UNCONDITIONALLY,
+ ADD
+ }
private final List<RaftPeer> peers;
private final List<RaftPeer> listeners;
+ private Mode mode;
Review Comment:
Let's make it final in order to keep it immutable.
--
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]
