szetszwo commented on a change in pull request #373:
URL: https://github.com/apache/incubator-ratis/pull/373#discussion_r548461680
##########
File path:
ratis-common/src/main/java/org/apache/ratis/protocol/TransferLeadershipRequest.java
##########
@@ -19,14 +19,21 @@
public class TransferLeadershipRequest extends RaftClientRequest {
private final RaftPeerId newLeader;
+ private final long timeoutInMs;
Review comment:
Let's add it to RaftClientRequest. It would be useful for the other
requests.
Also, let's just name it "timeoutMs".
##########
File path: ratis-proto/src/main/proto/Raft.proto
##########
@@ -390,6 +390,7 @@ message SetConfigurationRequestProto {
message TransferLeadershipRequestProto {
RaftRpcRequestProto rpcRequest = 1;
RaftPeerProto newLeader = 2;
+ uint64 timeoutInMs = 3;
Review comment:
Similar to TransferLeadershipRequest, let's move it to
RaftRpcRequestProto and name it "timeoutMs".
BTW, let's also move routingTable from RaftClientRequestProto to
RaftRpcRequestProto. The routingTable in RaftRpcRequestProto can be used for
appendEntries later. See if you want to do it here, or we may do it in a
separated JIRA.
##########
File path:
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -619,7 +619,7 @@ RaftClientReply newExceptionReply(RaftClientRequest
request, RaftException excep
}
if (isWrite && isSteppingDown()) {
- final LeaderSteppingDownException lsde = new
LeaderSteppingDownException(getMemberId());
+ final LeaderSteppingDownException lsde = new
LeaderSteppingDownException(getMemberId() + " is in steppingDown");
Review comment:
Change " is in steppingDown" to " is stepping down."
##########
File path:
ratis-server/src/main/java/org/apache/ratis/server/impl/TransferLeadership.java
##########
@@ -59,8 +60,9 @@ void complete(RaftPeerId currentLeader, boolean timeout) {
if (currentLeader != null &&
currentLeader.equals(request.getNewLeader())) {
replyFuture.complete(server.newSuccessReply(request));
} else if (timeout) {
- final TransferLeadershipException tle = new
TransferLeadershipException(server.getMemberId(),
- "Failed to transfer leadership to " + request.getNewLeader() + ":
current leader is " + currentLeader);
+ final TransferLeadershipException tle = new
TransferLeadershipException(server.getMemberId() +
+ " Timeout and Failed to transfer leadership to " +
request.getNewLeader() + ": current leader is " +
Review comment:
Let's add the timeout value to the messages as below.
```
final TransferLeadershipException tle = new
TransferLeadershipException(server.getMemberId()
+ ": Failed to transfer leadership to " + request.getNewLeader()
+ " (timed out " + request.getTimeoutMs() + "ms): current leader
is " + currentLeader);
```
----------------------------------------------------------------
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]
