[GitHub] [incubator-ratis] lokeshj1703 commented on a change in pull request #289: RATIS-1161. Updating commit index of follower does not require term check

[GitBox <gi...@apache.org>]

lokeshj1703 commented on a change in pull request #289:
URL: https://github.com/apache/incubator-ratis/pull/289#discussion_r527668633



##########
File path: ratis-server/src/main/java/org/apache/ratis/server/raftlog/RaftLog.java
##########
@@ -123,15 +123,20 @@ public boolean isOpened() {
    * Update the last committed index.
    * @param majorityIndex the index that has achieved majority.
    * @param currentTerm the current term.
+   * @param isLeader Is this server the leader?
    * @return true if update is applied; otherwise, return false, i.e. no update required.
    */
-  public boolean updateLastCommitted(long majorityIndex, long currentTerm) {
+  public boolean updateLastCommitted(long majorityIndex, long currentTerm, boolean isLeader) {
     try(AutoCloseableLock writeLock = writeLock()) {
       final long oldCommittedIndex = getLastCommittedIndex();
       final long newCommitIndex = Math.min(majorityIndex, getFlushIndex());
       if (oldCommittedIndex < newCommitIndex) {
-        // Only update last committed index for current term. See §5.4.2 in
-        // paper for details.
+        if (!isLeader) {

Review comment:
       @szetszwo Minor comment: instead of sending a truncated leaderCommit, should we use minimum of leader commit and entry size in appendEntriesAsync itself?
   
   
         private CompletableFuture<AppendEntriesReplyProto> appendEntriesAsync(
         RaftPeerId leaderId, long leaderTerm, TermIndex previous, long leaderCommit, long callId, boolean initializing,
         List<CommitInfoProto> commitInfos, LogEntryProto... entries) throws IOException {
         ...
           long commitIndex = min(leaderCommit, previous.getIndex() + entries.length);
           state.updateCommitIndex(commitIndex, currentTerm, false);
         ...
   




----------------------------------------------------------------
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:
users@infra.apache.org