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

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

Good catch on the bug!
- Let's reuse the same code in LogAppender, i.e.
{code}
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/LogAppender.java
@@ -430,19 +430,7 @@ public class LogAppender extends Daemon {
     if (reply != null) {
       switch (reply.getResult()) {
         case SUCCESS:
-          final long oldNextIndex = follower.getNextIndex();
-          final long nextIndex = reply.getNextIndex();
-          if (nextIndex < oldNextIndex) {
-            throw new IllegalStateException("nextIndex=" + nextIndex
-                + " < oldNextIndex=" + oldNextIndex
-                + ", reply=" + ProtoUtils.toString(reply));
-          }
-
-          if (nextIndex > oldNextIndex) {
-            follower.updateMatchIndex(nextIndex - 1);
-            follower.updateNextIndex(nextIndex);
-            submitEventOnSuccessAppend();
-          }
+          onSuccess(reply);
           break;
         case NOT_LEADER:
           // check if should step down
@@ -459,6 +447,22 @@ public class LogAppender extends Daemon {
     }
   }
 
+  protected void onSuccess(AppendEntriesReplyProto reply){
+    final long oldNextIndex = follower.getNextIndex();
+    final long nextIndex = reply.getNextIndex();
+    if (nextIndex < oldNextIndex) {
+      throw new IllegalStateException("nextIndex=" + nextIndex
+          + " < oldNextIndex=" + oldNextIndex
+          + ", reply=" + ProtoUtils.toString(reply));
+    }
+
+    if (nextIndex > oldNextIndex) {
+      follower.updateMatchIndex(nextIndex - 1);
+      follower.updateNextIndex(nextIndex);
+      submitEventOnSuccessAppend();
+    }
+  }
+
{code}
{code}
+++ b/ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GRpcLogAppender.java
@@ -277,7 +277,7 @@ public class GRpcLogAppender extends LogAppender {
     follower.decreaseNextIndex(newNextIndex);
   }
 
-  private void onSuccess(AppendEntriesReplyProto reply) {
+  protected synchronized void onSuccess(AppendEntriesReplyProto reply) {
     AppendEntriesRequestProto request = 
pendingRequests.remove(reply.getServerReply().getCallId());
     if (request == null) {
       // If reply comes after timeout, the reply is ignored.
@@ -303,9 +303,8 @@ public class GRpcLogAppender extends LogAppender {
       Preconditions.assertTrue(replyNextIndex == lastEntryIndex + 1,
           "reply's next index is %s, request's last entry index is %s",
           replyNextIndex, lastEntryIndex);
-      follower.updateMatchIndex(lastEntryIndex);
-      submitEventOnSuccessAppend();
     }
+    super.onSuccess(reply);
   }
{code}
- Could you see if you can add a test?


> 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.001.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