szetszwo commented on code in PR #1250:
URL: https://github.com/apache/ratis/pull/1250#discussion_r2054507808
##########
ratis-server-api/src/main/java/org/apache/ratis/server/leader/FollowerInfo.java:
##########
@@ -112,4 +112,10 @@ public interface FollowerInfo {
/** Update lastRpcResponseTime and LastRespondedAppendEntriesSendTime */
void updateLastRespondedAppendEntriesSendTime(Timestamp sendTime);
+
+ /** Set the caughtUp flag to true. */
+ void catchUp();
+
+ /** @return true if this follower is caught up. */
+ boolean isCaughtUp();
Review Comment:
Let's update the `isFollowerBootstrapping` implementation instead.
BTW, we should also update `isBootStrappingPeer`.
```diff
diff --git
a/ratis-server-api/src/main/java/org/apache/ratis/server/leader/LogAppender.java
b/ratis-server-api/src/main/java/org/apache/ratis/server/leader/LogAppender.java
index 78f61300b8..a333b8393a 100644
---
a/ratis-server-api/src/main/java/org/apache/ratis/server/leader/LogAppender.java
+++
b/ratis-server-api/src/main/java/org/apache/ratis/server/leader/LogAppender.java
@@ -145,7 +145,7 @@ public interface LogAppender {
// we should install snapshot if the follower needs to catch up and:
// 1. there is no local log entry but there is snapshot
// 2. or the follower's next index is smaller than the log start index
- // 3. or the follower is bootstrapping and has not installed any
snapshot yet
+ // 3. or the follower is bootstrapping (i.e. not yet caught up) and has
not installed any snapshot yet
final FollowerInfo follower = getFollower();
final boolean isFollowerBootstrapping =
getLeaderState().isFollowerBootstrapping(follower);
final SnapshotInfo snapshot =
getServer().getStateMachine().getLatestSnapshot();
diff --git
a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
index 93b5ae4d12..e58bb61bab 100644
---
a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
+++
b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
@@ -820,7 +820,9 @@ class LeaderStateImpl implements LeaderState {
@Override
public boolean isFollowerBootstrapping(FollowerInfo follower) {
- return isBootStrappingPeer(follower.getId());
+ // It is better to check caught up than staging state
+ // since a follower may have already caught up but still in the staging
state.
+ return isCaughtUp(follower);
}
private void checkStaging() {
@@ -852,7 +854,12 @@ class LeaderStateImpl implements LeaderState {
}
boolean isBootStrappingPeer(RaftPeerId peerId) {
- return Optional.ofNullable(stagingState).map(s ->
s.contains(peerId)).orElse(false);
+ final Optional<LogAppender> info = getLogAppender(peerId);
+ if (info.isPresent()) {
+ return isCaughtUp(info.get().getFollower());
+ }
+ final ConfigurationStagingState staging = stagingState;
+ return staging != null && staging.contains(peerId);
}
void submitUpdateCommitEvent() {
```
--
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]
