[ 
https://issues.apache.org/jira/browse/RATIS-225?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16437013#comment-16437013
 ] 

Tsz Wo Nicholas Sze commented on RATIS-225:
-------------------------------------------

Thanks.  Some comments on the patch:
- Let's do some refactoring in onSuccess(..).
{code}
@@ -291,20 +291,26 @@ public class GRpcLogAppender extends LogAppender {
     Objects.requireNonNull(request,
         () -> "Got reply with next index " + replyNextIndex
             + " but the pending queue is empty");
+    final long lastIndex = replyNextIndex - 1;
+    final boolean updateMatchIndex;
 
     if (request.getEntriesCount() == 0) {
-      Preconditions.assertTrue(!request.hasPreviousLog() ||
-              replyNextIndex - 1 == request.getPreviousLog().getIndex(),
+      Preconditions.assertTrue(!request.hasPreviousLog() || lastIndex == 
request.getPreviousLog().getIndex(),
           "reply's next index is %s, request's previous is %s",
           replyNextIndex, request.getPreviousLog());
+      updateMatchIndex = request.hasPreviousLog() && follower.getMatchIndex() 
< lastIndex;
     } else {
       // check if the reply and the pending request is consistent
       final long lastEntryIndex = request
           .getEntries(request.getEntriesCount() - 1).getIndex();
-      Preconditions.assertTrue(replyNextIndex == lastEntryIndex + 1,
+      Preconditions.assertTrue(lastIndex == lastEntryIndex,
           "reply's next index is %s, request's last entry index is %s",
           replyNextIndex, lastEntryIndex);
-      follower.updateMatchIndex(lastEntryIndex);
+      updateMatchIndex = true;
+    }
+
+    if (updateMatchIndex) {
+      follower.updateMatchIndex(lastIndex);
       submitEventOnSuccessAppend();
     }
   }
{code}
- In testUpdateViaHeartbeat, let's assert the entire log in follower match the 
log in the leader instead of just checking the nextIndex.
-* getNextIndex should be happened after checking the log.  (it may change if 
we get it too early.  I see it once.)
{code}
+      long index = cluster.getLeader().getState().getLog().getNextIndex();
{code}
- Just add getLogAppenders to RaftServerTestUtil.


> Grpc LogAppender should update next index on heartbeat reply
> ------------------------------------------------------------
>
>                 Key: RATIS-225
>                 URL: https://issues.apache.org/jira/browse/RATIS-225
>             Project: Ratis
>          Issue Type: Improvement
>            Reporter: Lokesh Jain
>            Assignee: Lokesh Jain
>            Priority: Major
>         Attachments: RATIS-225.002.patch
>
>
> Grpc LogAppender currently does not update next index in heartbeat reply. In 
> case the previous appendEntry times out the next index may not be updated in 
> the leader for a long time. Therefore we need to update the next index on a 
> heartbeat success.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to