[
https://issues.apache.org/jira/browse/RATIS-1838?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17716767#comment-17716767
]
Tsz-wo Sze commented on RATIS-1838:
-----------------------------------
[~William Song], all three ideas are good; see the comments below. Could you
test them?
{quote}The assertions should compare snapshot last included with commit index
rather that next index.
{quote}
You are right. It should compare the commit index.
{quote}... can we just simply reply ALREADY_INSTALLED?
{quote}
Yes, this is a good idea.
{quote}onError only happens when grpc connections has something wrong ...
{quote}
The code supports follower to reply error on a request. The follower can return
an exception with a call id; see
[https://github.com/apache/ratis/blob/60587b63a4401cc6160907d33fb5cd89dbbdc724/ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java#L419]
We could try not decreasing the next index when request == null.
{code:java}
+++ b/ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
@@ -123,8 +123,8 @@ public class GrpcLogAppender extends LogAppenderBase {
.map(AppendEntriesRequest::getPreviousLog)
.map(TermIndex::getIndex)
.orElseGet(f::getMatchIndex);
- if (onError && f.getMatchIndex() == 0 && request == null) {
- LOG.warn("{}: Follower failed when matchIndex == 0, " +
+ if (onError && request == null) {
+ LOG.warn("{}: Follower failed and request == null, " +
" keep nextIndex ({}) unchanged and retry.", this, f.getNextIndex());
return;
}
{code}
> java.lang.IllegalStateException thrown when nextIndex in follower is
> inconsistent with leader's snapshot
> --------------------------------------------------------------------------------------------------------
>
> Key: RATIS-1838
> URL: https://issues.apache.org/jira/browse/RATIS-1838
> Project: Ratis
> Issue Type: Bug
> Components: snapshot
> Affects Versions: 2.5.0
> Reporter: Song Ziyang
> Priority: Major
> Attachments: image-2023-04-25-22-10-01-209.png
>
>
> h3. 1. Error Stack
> !image-2023-04-25-22-10-01-209.png!
>
> The reason: The leader sets follower's nextIndex to 0 in
> [https://github.com/apache/ratis/blob/60587b63a4401cc6160907d33fb5cd89dbbdc724/ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java#L134]
> and then sends a snapshot to the follower.
> The SnapshotInstallationHandler in follower asserts it's a stale snapshot and
> throws a unrecoverable IllegalStatementException.
>
> h3. ISSUE 1
> The assertions should compare snapshot last included with *commit index*
> rather that next index.
> [https://github.com/apache/ratis/blob/60587b63a4401cc6160907d33fb5cd89dbbdc724/ratis-server/src/main/java/org/apache/ratis/server/impl/SnapshotInstallationHandler.java#L177-L179]
>
>
> h3. ISSUE 2
> When a follower receives a stale snapshot from leader, rather than throwing a
> IlleaglStatementException in assert, can we just simply reply
> ALREADY_INSTALLED?
>
> h3. ISSUE 3
> When appendEntriesHandler onError is called, shall we remain the nextIndex
> unchanged?
> [https://github.com/apache/ratis/blob/60587b63a4401cc6160907d33fb5cd89dbbdc724/ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java#L420]
>
> onError only happens when grpc connections has something wrong (channel
> disconnected, timeout, etc) and the leader receives no reply. I think we can
> keep nextIndex unchanged and retry. When the communications are restored, let
> onNext decide what is the correct nextIndex (may experience INCONSISTENCY).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
