You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2021/02/05 21:41:12 UTC
[GitHub] [incubator-ratis] errose28 opened a new pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
errose28 opened a new pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420
## What changes were proposed in this pull request?
After RATIS-1241, a leader with no logs will send install snapshot notifications to followers on every heartbeat, regardless of whether or not they are behind. Fix this so that followers only get install snapshot notifications when they are actually behind.
## What is the link to the Apache JIRA
RATIS-1305
## How was this patch tested?
Integration test added. The test still has an issue that is marked with a TODO. Leaving this PR as a draft while I fix this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] errose28 commented on pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420#issuecomment-776942638
New integration test is passing. Ready for review. Current CI failure looks unrelated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] szetszwo closed pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
szetszwo closed pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] runzhiwang commented on a change in pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420#discussion_r571376272
##########
File path: ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
##########
@@ -562,17 +562,23 @@ private void installSnapshot(TermIndex firstAvailableLogTermIndex) {
* @return the first available log's start term index
*/
private TermIndex shouldNotifyToInstallSnapshot() {
- final long leaderStartIndex = getRaftLog().getStartIndex();
Review comment:
change to:
```
final long followerNextIndex = getFollower().getNextIndex();
final long leaderNextIndex = getRaftLog().getNextIndex();
if (followerNextIndex >= leaderNextIndex) {
return null;
}
final long leaderStartIndex = getRaftLog().getStartIndex();
if (followerNextIndex < leaderStartIndex) {
// The Leader does not have the logs from the Follower's last log
// index onwards. And install snapshot is disabled. So the Follower
// should be notified to install the latest snapshot through its
// State Machine.
return getRaftLog().getTermIndex(leaderStartIndex);
} else if (leaderStartIndex == RaftLog.INVALID_LOG_INDEX) {
// Leader has no logs to check from, hence return next index.
return TermIndex.valueOf(getServer().getInfo().getCurrentTerm(),
leaderNextIndex);
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] szetszwo commented on pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420#issuecomment-777228657
Just have reopened this for triggering a new build. Will wait for it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] errose28 commented on a change in pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420#discussion_r571641628
##########
File path: ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
##########
@@ -562,17 +562,23 @@ private void installSnapshot(TermIndex firstAvailableLogTermIndex) {
* @return the first available log's start term index
*/
private TermIndex shouldNotifyToInstallSnapshot() {
- final long leaderStartIndex = getRaftLog().getStartIndex();
Review comment:
Changed code to the proposed suggestion.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] runzhiwang commented on a change in pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420#discussion_r571376272
##########
File path: ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
##########
@@ -562,17 +562,23 @@ private void installSnapshot(TermIndex firstAvailableLogTermIndex) {
* @return the first available log's start term index
*/
private TermIndex shouldNotifyToInstallSnapshot() {
- final long leaderStartIndex = getRaftLog().getStartIndex();
Review comment:
change to:
```
final long followerNextIndex = getFollower().getNextIndex();
final long leaderNextIndex = getRaftLog().getNextIndex();
if (followerNextIndex >= leaderNextIndex) {
return null;
}
final long leaderStartIndex = getRaftLog().getStartIndex();
if (followerNextIndex < leaderStartIndex) {
// The Leader does not have the logs from the Follower's last log
// index onwards. And install snapshot is disabled. So the Follower
// should be notified to install the latest snapshot through its
// State Machine.
return getRaftLog().getTermIndex(leaderStartIndex);
} else if (leaderStartIndex == RaftLog.INVALID_LOG_INDEX) {
// Leader has no logs to check from, hence return next index.
return TermIndex.valueOf(getServer().getInfo().getCurrentTerm(),
leaderNextIndex);
}
return null;
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] errose28 commented on pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420#issuecomment-774340107
cc @szetszwo @runzhiwang for review and any ideas on why the leader enters a closed state and adding peers to the configuration fails in the integration test.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] szetszwo closed pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
szetszwo closed pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] runzhiwang commented on a change in pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420#discussion_r571376272
##########
File path: ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
##########
@@ -562,17 +562,23 @@ private void installSnapshot(TermIndex firstAvailableLogTermIndex) {
* @return the first available log's start term index
*/
private TermIndex shouldNotifyToInstallSnapshot() {
- final long leaderStartIndex = getRaftLog().getStartIndex();
Review comment:
change to:
```
private TermIndex shouldNotifyToInstallSnapshot() {
final long leaderStartIndex = getRaftLog().getStartIndex();
if (getFollower().getNextIndex() >= leaderStartIndex) {
return null;
}
// The Leader does not have the logs from the Follower's last log
// index onwards. And install snapshot is disabled. So the Follower
// should be notified to install the latest snapshot through its
// State Machine.
return getRaftLog().getTermIndex(leaderStartIndex);
} else if (leaderStartIndex == RaftLog.INVALID_LOG_INDEX) {
// Leader has no logs to check from, hence return next index.
return TermIndex.valueOf(getServer().getInfo().getCurrentTerm(),
getRaftLog().getNextIndex());
final long followerNextIndex = getFollower().getNextIndex();
final long leaderNextIndex = getRaftLog().getNextIndex();
if (followerNextIndex < leaderNextIndex) {
final long leaderStartIndex = getRaftLog().getStartIndex();
if (followerNextIndex < leaderStartIndex) {
// The Leader does not have the logs from the Follower's last log
// index onwards. And install snapshot is disabled. So the Follower
// should be notified to install the latest snapshot through its
// State Machine.
return getRaftLog().getTermIndex(leaderStartIndex);
} else if (leaderStartIndex == RaftLog.INVALID_LOG_INDEX) {
// Leader has no logs to check from, hence return next index.
return TermIndex.valueOf(getServer().getInfo().getCurrentTerm(),
leaderNextIndex);
}
return null;
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] errose28 commented on pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420#issuecomment-774850829
The added integration test `InstallSnapshotNotificationTests#testInstallSnapshotNotificationCount` shows two log lines from the leader (s2 in this example):
```log
[s2@group-DF2C3007074F-SegmentedRaftLogWorker] INFO RaftServer$Division (ServerState.java:close(405)) - s2@group-DF2C3007074F: closes. applyIndex: 20
[s2@group-DF2C3007074F-SegmentedRaftLogWorker] INFO segmented.SegmentedRaftLogWorker (SegmentedRaftLogWorker.java:close(236)) - s2@group-DF2C3007074F-SegmentedRaftLogWorker close()
```
When it tries to add two new nodes to the cluster later in the test, `RaftServerTestUtil#waitAndCheckNewConf` fails because s2 thinks it is the leader, but it is closed so the rest of the cluster has elected a new leader, and the check deems the configuration unstable. It looks like s2 could have closed because of the `java.lang.IllegalStateException: lastWrittenIndex == 19, entry == term: 2 index: 21` which occurred on s2 when trying to push the new configuration. This is probably due to how the logs are being truncated and purged, but I have not found the exact cause.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] szetszwo closed pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
szetszwo closed pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] szetszwo merged pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
szetszwo merged pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] szetszwo commented on pull request #420: RATIS-1305. Leader stuck in infinite install snapshot cycle when logs have been purged
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #420:
URL: https://github.com/apache/incubator-ratis/pull/420#issuecomment-776590929
@errose28 , I just have tried the test. The two new servers, s3 and s4, have failed to start due to "Unexpected gap in segments" as shown below. It seems the changes might have some bug.
```
2021-02-10 15:32:49,699 [s4@group-9F91CFE6147C-StateMachineUpdater] ERROR impl.StateMachineUpdater (StateMachineUpdater.java:run(189)) - s4@group-9F91CFE6147C-StateMachineUpdater caught a Throwable.
java.lang.IllegalStateException: Unexpected gap in segments: binarySearch(20) returns -1, segments=[log-21_21]
at org.apache.ratis.server.raftlog.segmented.SegmentedRaftLogCache$LogSegmentList.purge(SegmentedRaftLogCache.java:340)
at org.apache.ratis.server.raftlog.segmented.SegmentedRaftLogCache.purge(SegmentedRaftLogCache.java:559)
at org.apache.ratis.server.raftlog.segmented.SegmentedRaftLog.purgeImpl(SegmentedRaftLog.java:362)
at org.apache.ratis.server.raftlog.RaftLogBase.purge(RaftLogBase.java:307)
at org.apache.ratis.server.impl.StateMachineUpdater.takeSnapshot(StateMachineUpdater.java:292)
at org.apache.ratis.server.impl.StateMachineUpdater.checkAndTakeSnapshot(StateMachineUpdater.java:255)
at org.apache.ratis.server.impl.StateMachineUpdater.run(StateMachineUpdater.java:178)
at java.lang.Thread.run(Thread.java:748)
2021-02-10 15:32:49,699 [s3@group-9F91CFE6147C-StateMachineUpdater] ERROR impl.StateMachineUpdater (StateMachineUpdater.java:run(189)) - s3@group-9F91CFE6147C-StateMachineUpdater caught a Throwable.
java.lang.IllegalStateException: Unexpected gap in segments: binarySearch(20) returns -1, segments=[log-21_21]
at org.apache.ratis.server.raftlog.segmented.SegmentedRaftLogCache$LogSegmentList.purge(SegmentedRaftLogCache.java:340)
at org.apache.ratis.server.raftlog.segmented.SegmentedRaftLogCache.purge(SegmentedRaftLogCache.java:559)
at org.apache.ratis.server.raftlog.segmented.SegmentedRaftLog.purgeImpl(SegmentedRaftLog.java:362)
at org.apache.ratis.server.raftlog.RaftLogBase.purge(RaftLogBase.java:307)
at org.apache.ratis.server.impl.StateMachineUpdater.takeSnapshot(StateMachineUpdater.java:292)
at org.apache.ratis.server.impl.StateMachineUpdater.checkAndTakeSnapshot(StateMachineUpdater.java:255)
at org.apache.ratis.server.impl.StateMachineUpdater.run(StateMachineUpdater.java:178)
at java.lang.Thread.run(Thread.java:748)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org