szetszwo commented on a change in pull request #383:
URL: https://github.com/apache/incubator-ratis/pull/383#discussion_r551806316
##########
File path:
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -104,6 +104,20 @@ public RaftPeerRole getCurrentRole() {
return getRole().getCurrentRole();
}
+ @Override
+ public boolean isLeader() {
Review comment:
Assume that the leader won't step down if it has lost the leader lease.
If the assumption is correct, then it is better to change isLeaderRead()
instead of isLeader(). The client will see LeaderNotReadyException and retry
with the same leader.
If we change isLeader(), the client will see NotLeaderException and retry
with a different peer.
##########
File path:
ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
##########
@@ -905,6 +906,47 @@ private void yieldLeaderToHigherPriorityPeer() {
}
}
+ boolean isLeaderLeaseValid() {
+ if (checkLeaderLease()) {
+ return true;
+ }
+
+ updateLeaderLease();
+ return checkLeaderLease();
+ }
+
+ private boolean checkLeaderLease() {
+ return lastLeaderTimestamp.get().elapsedTimeMs() <
server.properties().leaderLeaseTimeoutMs();
+ }
+
+ private boolean updateLeaderLease() {
+ Timestamp startLease = Timestamp.currentTime();
+
+ List<RaftPeerId> activePeers = new ArrayList<>();
+ List<FollowerInfo> followerInfos = getFollowerInfos();
+ for (final FollowerInfo info : followerInfos) {
+ if (info.getLastRpcSendTimeWithResponse().elapsedTimeMs() <=
server.properties().leaderLeaseTimeoutMs()) {
+ if (startLease.compareTo(info.getLastRpcSendTimeWithResponse()) > 0) {
+ startLease = info.getLastRpcSendTimeWithResponse();
+ }
+ activePeers.add(info.getPeer().getId());
+ }
+ }
Review comment:
The for-loop will choose all info with condition
lastRpcSendTimeWithResponse < leaderLeaseTimeoutMs. Suppose there are 5
servers (1 leader + 4 followers) and all 4 followers satisfy condition. The
lastRpcSendTimeWithResponse of the followers are 100, 200, 300, 400. The
for-loop will update startLease to the min, i.e. 100. However, it should
update to 300 since 300, 400 and self have the majority.
----------------------------------------------------------------
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]
