szetszwo commented on code in PR #1597:
URL: https://github.com/apache/ratis/pull/1597#discussion_r4049093001
##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java:
##########
@@ -788,6 +788,9 @@ public void onNext(InstallSnapshotReplyProto reply) {
case SNAPSHOT_EXPIRED:
LOG.warn("{}: Follower failed since the request expired, {}",
name, ServerStringUtils.toInstallSnapshotReplyString(reply));
+ getFollower().setAttemptedToInstallSnapshot();
+ removePending(reply);
+ break;
default:
break;
Review Comment:
Let's handle SNAPSHOT_EXPIRED the same way as SNAPSHOT_UNAVAILABLE:
```diff
diff --git
a/ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
b/ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
index e6870b9b8..60d7ee2b2 100644
---
a/ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
+++
b/ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
@@ -775,24 +775,26 @@ public class GrpcLogAppender extends LogAppenderBase {
removePending(reply);
break;
case SNAPSHOT_UNAVAILABLE:
- BatchLogger.print(BatchLogKey.SNAPSHOT_UNAVAILABLE, name,
- suffix -> LOG.info("{}: Follower failed since the snapshot is
unavailable {}", this, suffix));
- getFollower().setAttemptedToInstallSnapshot();
-
notifyInstallSnapshotFinished(InstallSnapshotResult.SNAPSHOT_UNAVAILABLE,
RaftLog.INVALID_LOG_INDEX);
- removePending(reply);
- break;
- case UNRECOGNIZED:
- LOG.error("{}: Reply result {}, {}",
- name, reply.getResult(),
ServerStringUtils.toInstallSnapshotReplyString(reply));
+ handleFailureReply(reply, BatchLogKey.SNAPSHOT_UNAVAILABLE);
break;
case SNAPSHOT_EXPIRED:
- LOG.warn("{}: Follower failed since the request expired, {}",
- name, ServerStringUtils.toInstallSnapshotReplyString(reply));
+ handleFailureReply(reply, BatchLogKey.SNAPSHOT_UNAVAILABLE);
+ break;
default:
+ LOG.error("{}: Unexpected InstallSnapshotReply result {} in {}",
+ name, reply.getResult(),
ServerStringUtils.toInstallSnapshotReplyString(reply));
break;
}
}
+ private void handleFailureReply(InstallSnapshotReplyProto reply,
BatchLogKey logKey) {
+ BatchLogger.print(logKey, name,
+ suffix -> LOG.info("{}: Follower failed due to {}: {}{}", this,
reply.getResult(), reply, suffix));
+ getFollower().setAttemptedToInstallSnapshot();
+ notifyInstallSnapshotFinished(reply.getResult(),
RaftLog.INVALID_LOG_INDEX);
+ removePending(reply);
+ }
+
@Override
public void onError(Throwable t) {
if (!isRunning()) {
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
