[
https://issues.apache.org/jira/browse/RATIS-748?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16976527#comment-16976527
]
Lokesh Jain commented on RATIS-748:
-----------------------------------
[~szetszwo] Here is the scenario from one of the clusters.
One of the followers has the following metadata -
flushIndex - 1371
commitIndex - 269
leaderCommitIndex - 1618
endIndex(open segment) - 1371
The follower is receiving a leader commit of 1618. Therefore the majority index
argument in the function posted in description gets a value of 1618.
updateLastCommitted(long majorityIndex, long currentTerm)
This function makes a call to get the term index for the index 1618.
final TermIndex entry = getTermIndex(majorityIndex);
if (entry != null && entry.getTerm() == currentTerm) {
.... // update commit index
}
But since the follower has the endIndex at 1371 the updateLastCommit call is
ignored as entry is null.
All the future appendEntries call would have a leader commit >= 1618 and hence
majorityCommit >=1618. Therefore the follower commit will never be updated
before the entry corresponding to index 1618 is received. The new
followerCommit would be >= 1618. This leads to commit being updated in bursts.
Ideally the commitIndex should be around the log flushIndex for the follower
i.e. 1371 but it is currently at 269.
If the newCommitIndex is from a previous term then also the follower's commit
index will not be updated. But I was thinking that the check should not be
required. If the follower recognises a leader and accepts append entries from
it then it can just update its commit index without making a check for the
newCommitIndex's term? The argument is that the follower log is already
consistent with the leader and leader makes a term check before updating its
own commit index.
> Follower might not update its commit index
> ------------------------------------------
>
> Key: RATIS-748
> URL: https://issues.apache.org/jira/browse/RATIS-748
> Project: Ratis
> Issue Type: Bug
> Reporter: Lokesh Jain
> Assignee: Lokesh Jain
> Priority: Critical
> Attachments: RATIS-748.001.patch, RATIS-748.002.patch
>
>
> While updating the commit index, the follower checks whether majority index
> is present in the raft log. There can be cases where leader is ahead of the
> follower and follower does not have the entry corresponding to the
> majorityIndex. In such cases the follower commit index is not updated. Below
> is the corresponding code snippet.
> {code:java}
> public boolean updateLastCommitted(long majorityIndex, long currentTerm) {
> try(AutoCloseableLock writeLock = writeLock()) {
> final long oldCommittedIndex = getLastCommittedIndex();
> if (oldCommittedIndex < majorityIndex) {
> // Only update last committed index for current term. See §5.4.2 in
> // paper for details.
> final TermIndex entry = getTermIndex(majorityIndex);
> if (entry != null && entry.getTerm() == currentTerm) {
> final long newCommitIndex = Math.min(majorityIndex, getFlushIndex());
> if (newCommitIndex > oldCommittedIndex) {
> commitIndex.updateIncreasingly(newCommitIndex, traceIndexChange);
> }
> return true;
> }
> }
> }
> return false;
> }{code}
> This function RaftLog#updateLastCommitted is also used by follower to update
> its commit index. The follower does not require the check of entry.getTerm()
> == currentTerm and its commitIndex can be updated to min(majorityIndex,
> getFlushIndex()). It has already verified the entries in the
> appendEntriesAsync call.
> This can lead to the follower commit being updated in bursts and can lead to
> failure of watch requests.
> cc [~shashikant] [~szetszwo]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
