RATIS-2487. Trigger installSnapshot if leader cannot get previous entry#1420
RATIS-2487. Trigger installSnapshot if leader cannot get previous entry#1420szetszwo merged 8 commits intoapache:masterfrom
Conversation
…extIndex == startIndex
| @@ -238,6 +217,15 @@ public AppendEntriesRequestProto newAppendEntriesRequest(long callId, boolean he | |||
| final long snapshotIndex = follower.getSnapshotIndex(); | |||
| final long leaderNext = getRaftLog().getNextIndex(); | |||
| final long followerNext = follower.getNextIndex(); | |||
There was a problem hiding this comment.
Seems getNextIndex is called twice in this method, we might need to call it only once per method.
szetszwo
left a comment
There was a problem hiding this comment.
@ivandika3 , thanks for working on this!
The change looks good. The current shouldInstallSnapshot code is distributed in three difference places, LogAppender, LogAppenderBase and GrpcLogAppender. How about we refactor the code and move them together? See https://issues.apache.org/jira/secure/attachment/13081541/1420_review.patch
|
@szetszwo Thanks for the review and the suggestion. Updated. Edit: TestRaftAsyncWithNetty has been flaky for a while. |
szetszwo
left a comment
There was a problem hiding this comment.
+1 the change looks good.
You are right that TestRaftAsyncWithNetty became flaky recently. Cc @slfan1989
What changes were proposed in this pull request?
See https://issues.apache.org/jira/browse/RATIS-2487
The current approach is to make
newAppendEntriesRequestreturns null which will skip the current iteration and change theshouldInstallSnapshot(if installSnapshot is enabled) andshouldNotifyToInstallSnapshot(if installSnapshot is disabled) so that subsequent LogAppender run will trigger snapshot since the leader cannot get the previous log (otherwise the logAppender is not going to send any RPC to the follower).What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-2487
How was this patch tested?
UT.