szetszwo commented on a change in pull request #360:
URL: https://github.com/apache/incubator-ratis/pull/360#discussion_r544909361
##########
File path:
ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
##########
@@ -568,6 +572,13 @@ private TermIndex shouldNotifyToInstallSnapshot() {
// should be notified to install the latest snapshot through its
// State Machine.
return getRaftLog().getTermIndex(leaderStartIndex);
+ } else if (leaderStartIndex == RaftLog.INVALID_LOG_INDEX) {
+ // Leader has no logs to check from.
+ TermIndex snapshotTermIndex =
+ stateMachine.getLatestSnapshot().getTermIndex();
+ if (getFollower().getNextIndex() <= snapshotTermIndex.getIndex()) {
+ return snapshotTermIndex;
+ }
Review comment:
Since this is **Notify**ToInstallSnapshot, it does not matter if the
leader has a snapshot. Just return current term and next index.
```
} else if (leaderStartIndex == RaftLog.INVALID_LOG_INDEX) {
return TermIndex.valueOf(getServer().getInfo().getCurrentTerm(),
getRaftLog().getNextIndex());
}
```
##########
File path:
ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
##########
@@ -64,6 +66,7 @@
private volatile StreamObserver<AppendEntriesRequestProto>
appendLogRequestObserver;
private final GrpcServerMetrics grpcServerMetrics;
+ private final StateMachine stateMachine;
Review comment:
We may use getServer().getStateMachine(). Indeed StateMachine is not
needed; see the other comment.
----------------------------------------------------------------
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]
