peterxcli opened a new pull request, #1567:
URL: https://github.com/apache/ratis/pull/1567
## What changes were proposed in this pull request?
RATIS-2487 added a clause to `LogAppender.shouldInstallSnapshot` that
triggers an install snapshot when the leader cannot provide the previous entry
for appendEntries:
```java
return followerNextIndex == leaderStartIndex &&
followerNextIndex > RaftLog.LEAST_VALID_LOG_INDEX &&
getPrevious(followerNextIndex) == null;
```
However, `LogAppenderBase.newAppendEntriesRequest` (and `assertProtos`)
explicitly allow sending appendEntries **without** the previous entry when
`followerNextIndex == follower.getSnapshotIndex() + 1`. Since the appender run
loop consults `shouldNotifyToInstallSnapshot()` before building an append
request, that exemption is unreachable and the leader gets stuck in an infinite
loop:
1. The follower's `nextIndex` equals the leader's log start index (the
previous entry is purged), and the leader state machine's latest snapshot index
does not exactly equal `leaderStartIndex - 1`, so `getPrevious` returns null.
This is the normal situation for DB-backed state machines (e.g. Ozone OM/SCM)
whose `getLatestSnapshot()` advances with the applied index.
2. The leader sends an InstallSnapshotNotification for the first available
log entry.
3. The follower replies `ALREADY_INSTALLED` with its snapshot index `==
leaderStartIndex - 1` (`SnapshotInstallationHandler`: `snapshotIndex + 1 >=
firstAvailableLogIndex`).
4. The leader records `follower.snapshotIndex`, sets `nextIndex =
leaderStartIndex`, and goes back to step 2.
The loop is tight (observed ~100 notification RPCs per second). The
follower's matchIndex never advances, so `TransferLeadership` always times out;
in a 3-node group with one node down, the leader can never commit its
placeholder entry, i.e. the group loses write availability.
This PR adds the same exemption to `shouldInstallSnapshot`: when
`followerNextIndex == follower.getSnapshotIndex() + 1`, do not install a
snapshot — appendEntries can proceed without the previous entry. This restores
the recovery path that worked before RATIS-2487 (verified against Ratis 3.2.1
behavior, where the `ALREADY_INSTALLED` reply leads to a successful append).
This was found while upgrading Apache Ozone to Ratis 3.3.0
([HDDS-16196](https://issues.apache.org/jira/browse/HDDS-16196),
apache/ozone#11032): `TestOzoneManagerHAWithStoppedNodes` wedges consistently
with the leader looping `notifyInstallSnapshot` / `ALREADY_INSTALLED` while
`TransferLeadership` reports `NOT_UP_TO_DATE(followerMatchIndex = 158 <
leaderLastEntry.getIndex() = 167)`.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-2500
## How was this patch tested?
New unit tests in `TestLogAppenderDefault`;
`noInstallSnapshotWhenFollowerSnapshotCoversPreviousEntry` fails without the
fix and passes with it. Existing tests pass. The failure scenario was
reproduced with Ozone's `TestOzoneManagerHAWithStoppedNodes` on Ratis 3.3.0.
Generated-by: Claude Code (claude-fable-5)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
