szetszwo commented on a change in pull request #602:
URL: https://github.com/apache/ratis/pull/602#discussion_r808028770
##########
File path:
ratis-client/src/main/java/org/apache/ratis/client/impl/ClientProtoUtils.java
##########
@@ -519,22 +520,38 @@ static SetConfigurationRequestProto
toSetConfigurationRequestProto(
static TransferLeadershipRequest toTransferLeadershipRequest(
TransferLeadershipRequestProto p) {
final RaftRpcRequestProto m = p.getRpcRequest();
- final RaftPeer newLeader = ProtoUtils.toRaftPeer(p.getNewLeader());
- return new TransferLeadershipRequest(
- ClientId.valueOf(m.getRequestorId()),
- RaftPeerId.valueOf(m.getReplyId()),
- ProtoUtils.toRaftGroupId(m.getRaftGroupId()),
- p.getRpcRequest().getCallId(),
- newLeader.getId(),
- m.getTimeoutMs());
+ String address = p.getNewLeaderOrBuilder().getAddress();
+ if (!Objects.equals(address, "")) {
Review comment:
We should use p.hasNewLeader():
```
static TransferLeadershipRequest toTransferLeadershipRequest(
TransferLeadershipRequestProto p) {
final RaftRpcRequestProto m = p.getRpcRequest();
- final RaftPeer newLeader = ProtoUtils.toRaftPeer(p.getNewLeader());
+ final RaftPeerId newLeader = p.hasNewLeader()?
ProtoUtils.toRaftPeer(p.getNewLeader()).getId(): null;
return new TransferLeadershipRequest(
ClientId.valueOf(m.getRequestorId()),
RaftPeerId.valueOf(m.getReplyId()),
ProtoUtils.toRaftGroupId(m.getRaftGroupId()),
p.getRpcRequest().getCallId(),
- newLeader.getId(),
+ newLeader,
m.getTimeoutMs());
}
```
##########
File path:
ratis-client/src/main/java/org/apache/ratis/client/impl/ClientProtoUtils.java
##########
@@ -519,22 +520,38 @@ static SetConfigurationRequestProto
toSetConfigurationRequestProto(
static TransferLeadershipRequest toTransferLeadershipRequest(
TransferLeadershipRequestProto p) {
final RaftRpcRequestProto m = p.getRpcRequest();
- final RaftPeer newLeader = ProtoUtils.toRaftPeer(p.getNewLeader());
- return new TransferLeadershipRequest(
- ClientId.valueOf(m.getRequestorId()),
- RaftPeerId.valueOf(m.getReplyId()),
- ProtoUtils.toRaftGroupId(m.getRaftGroupId()),
- p.getRpcRequest().getCallId(),
- newLeader.getId(),
- m.getTimeoutMs());
+ String address = p.getNewLeaderOrBuilder().getAddress();
+ if (!Objects.equals(address, "")) {
+ return new TransferLeadershipRequest(
+ ClientId.valueOf(m.getRequestorId()),
+ RaftPeerId.valueOf(m.getReplyId()),
+ ProtoUtils.toRaftGroupId(m.getRaftGroupId()),
+ p.getRpcRequest().getCallId(),
+ ProtoUtils.toRaftPeer(p.getNewLeader()).getId(),
+ m.getTimeoutMs());
+ } else {
+ return new TransferLeadershipRequest(
+ ClientId.valueOf(m.getRequestorId()),
+ RaftPeerId.valueOf(m.getReplyId()),
+ ProtoUtils.toRaftGroupId(m.getRaftGroupId()),
+ p.getRpcRequest().getCallId(),
+ null,
+ m.getTimeoutMs());
+ }
}
static TransferLeadershipRequestProto toTransferLeadershipRequestProto(
TransferLeadershipRequest request) {
- return TransferLeadershipRequestProto.newBuilder()
- .setRpcRequest(toRaftRpcRequestProtoBuilder(request))
-
.setNewLeader(RaftPeer.newBuilder().setId(request.getNewLeader()).build().getRaftPeerProto())
- .build();
+ if (request.getNewLeader() != null) {
+ return TransferLeadershipRequestProto.newBuilder()
+ .setRpcRequest(toRaftRpcRequestProtoBuilder(request))
+
.setNewLeader(RaftPeer.newBuilder().setId(request.getNewLeader()).build().getRaftPeerProto())
+ .build();
+ } else {
+ return TransferLeadershipRequestProto.newBuilder()
+ .setRpcRequest(toRaftRpcRequestProtoBuilder(request))
+ .build();
+ }
Review comment:
Let's combine the common code:
```
static TransferLeadershipRequestProto toTransferLeadershipRequestProto(
TransferLeadershipRequest request) {
final TransferLeadershipRequestProto.Builder b =
TransferLeadershipRequestProto.newBuilder()
.setRpcRequest(toRaftRpcRequestProtoBuilder(request));
Optional.ofNullable(request.getNewLeader())
.map(l -> RaftPeer.newBuilder().setId(l).build())
.map(RaftPeer::getRaftPeerProto)
.ifPresent(b::setNewLeader);
return b.build();
}
```
##########
File path:
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -967,6 +967,10 @@ void finishTransferLeadership() {
transferLeadership.finish(state.getLeaderId(), false);
}
+ public RaftClientReply stepDownLeader(TransferLeadershipRequest request)
throws IOException {
+ return waitForReply(request, stepDownLeaderAsync(request));
+ }
+
public CompletableFuture<RaftClientReply>
transferLeadershipAsync(TransferLeadershipRequest request)
Review comment:
Let's change transferLeadershipAsync. Then, we don't have to change
RaftServerProxy at all. BTW, could you also remove "public" from
transferLeadership(..) and transferLeadershipAsync(..)?
```
+++
b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
@@ -948,7 +948,7 @@ class RaftServerImpl implements RaftServer.Division,
}
}
- public RaftClientReply transferLeadership(TransferLeadershipRequest
request) throws IOException {
+ RaftClientReply transferLeadership(TransferLeadershipRequest request)
throws IOException {
return waitForReply(request, transferLeadershipAsync(request));
}
@@ -967,8 +967,12 @@ class RaftServerImpl implements RaftServer.Division,
transferLeadership.finish(state.getLeaderId(), false);
}
- public CompletableFuture<RaftClientReply>
transferLeadershipAsync(TransferLeadershipRequest request)
+ CompletableFuture<RaftClientReply>
transferLeadershipAsync(TransferLeadershipRequest request)
throws IOException {
+ if (request.getNewLeader() == null) {
+ return stepDownLeaderAsync(request);
+ }
+
LOG.info("{}: receive transferLeadership {}", getMemberId(), request);
assertLifeCycleState(LifeCycle.States.RUNNING);
assertGroup(request.getRequestorId(), request.getRaftGroupId());
```
--
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]
